This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [C++ PATCH] Enforce extern "Java" aggregates to be heap allocated (PR c++/30293, c++/30294)
- From: Mark Mitchell <mark at codesourcery dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Jason Merrill <jason at redhat dot com>, Tom Tromey <tromey at redhat dot com>, Andrew Haley <aph at redhat dot com>, gcc-patches at gcc dot gnu dot org
- Date: Fri, 23 Nov 2007 10:49:04 -0800
- Subject: Re: [C++ PATCH] Enforce extern "Java" aggregates to be heap allocated (PR c++/30293, c++/30294)
- References: <20071122125107.GX5451@devserv.devel.redhat.com>
Jakub Jelinek wrote:
> All of the listed attributes are also true of C++, though C++ has
> extra features (for example in C++ objects may be allocated not just
> on the heap, but also statically or in a local stack frame).
This looks OK, but I've picked some nits. I'd also like one of the Java
maintainers to sign off on the concept, just to be sure that we don't
break Java CNI code in the field.
> + if (TYPE_FOR_JAVA (type) && IS_AGGR_TYPE (type))
> + {
> + tree jclass;
> + /* Allow libjava/prims.cc define primitive classes. */
> + if (init != NULL_TREE
> + || (jclass
> + = IDENTIFIER_GLOBAL_VALUE (get_identifier ("jclass")))
> + == NULL_TREE
Please move the assignment to jclass outside the conditional. If you're
trying to avoid the get_identifier in the !init case, then (a) this is
not fast-path code as there are few declarations of extern "Java"
classes, and (b) you could just put "init &&" on the first line above,
if I'm reading correctly.
> + || TYPE_MAIN_VARIANT (type)
> + != TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (jclass))))
You should parenthesize this last clause: (TYPE_MAIN .. != TYPE_MAIN
...) so that using Emacs auto-indent doesn't reformat the line. Also,
would this be clearer if you used same_type_ignoring_top_level_qualifiers?
> + error ("Java object %qD must be heap allocated", decl);
To be grammatical, that should be "heap-allocated". (Use a hyphen, not
a space.) But, "heap-allocated" is jargon; we should use either C++ or
Java terminology. So, I would suggest:
Java object %qD not allocated with %<new%>
(I'm not sure if %<...%> is the right syntax for quotes, but I know we
have one.)
> + error ("field %qD has Java class type", decl);
C++ doesn't have fields; it has non-static data members. I suggest:
non-static data member %qD has Java class type
> + error ("parameter %qD invalidly declared Java class type", decl);
Just:
parameter %qD has Java class type
The parameter may be invalid; the class type is not.
> + error ("returning Java type %q#T by value", return_type);
return type has Java class type %q#T
> + else if (TYPE_FOR_JAVA (elt_type))
> + {
> + error ("Java class %q#T object allocated using placement new", elt_type);
> + return error_mark_node;
> + }
I don't think this one is safe. We can't know that placement new
doesn't do heap allocation.
--
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713