[PATCH]: Move FIELD_DECL into it's own structure

[Mark Mitchell]

Daniel Berlin wrote:
> This moves FIELD_DECL into it's own structure, as well as unsharing
> previously unshared fields shared between FIELD_DECL and other *_DECL's.
> 
> FIELD_DECL's now have their own struct, struct tree_field_decl, derived
> from struct tree_decl_common.
> 
> tree_decl_common is now the common base for _DECL nodes, and will
> decrease in size as more things get split out and unshared (IE i don't
> expect it to stay the size listed below forever, as some things don't
> need even those fields).  This means that if we decide one of the DECL's
> (CONST_DECL maybe) can't have attributes, we'll make a substructure
> shared by everything but CONST_DECL that contains the attributes tree).
> 
> I gave up trying to split out DECL_CONTEXT for the moment, the C++ FE
> and Java FE use it all over the place on FIELD_DECL's, and i stopped
> after changing about 20 users :)
> 
> All things not in common with FIELD_DECL and all other DECL's was moved
> to tree_decl_extra.  This is just a temporary storage place as
> structures get moved around and split. 
> 
> 
> I've moved the FIELD_DECL macros to right before the structure, in line
> with the placement of the rest of the file macros.

I like the idea of this patch, but I'm concerned about a few issues.

One is that if we go a little way down this path, but not all the way, 
we could end up truly tangled.  I don't know exactly what to do about 
that, except ask you to promise to keep going for a while.  In other 
words, I'm a little nervous about this being a weekend project that then 
gets dropped after a weekend or two with no clear plan.  In fact, it 
would be nice if there were a Wiki page with objectives and methodology 
explaining what you're trying to do so that other people could help. 
I'd include things like how you plan to emulate inheritance in C, and 
how the macros and such will have to work for that.  In the end, this is 
documentation that we'll want anyhow for future development.

I'm not trying to throw up roadblocks, nor asking you to write a book. 
But, this is a major change to the way GCC's IR works, and nobody is 
happy with the current lack of design and documentation.  I think you 
need to lay the appropriate foundation, and get buy-in, before we start 
hacking.

> All things not in common with FIELD_DECL and all other DECL's was moved
> to tree_decl_extra. This is just a temporary storage place as
> structures get moved around and split. 

In the way of temporary changes, this will likely live forever.  And, as 
you say, you want to split things out of CONST_DECL, etc., later, 
leading to additional shared structure types.  So, tree_decl_extra isn't 
a good name.  For now, tree_non_field_decl_common is probably the best 
we can do.

> A tree_class_except macro was added to be able to check that something
> is any member of a tree class *except* some member.  This is used to
> make sure nothing is still trying to access the moved fields in a
> FIELD_DECL.

I'm not sure this is a very good idea.  You've made the logic rather 
more complex by adding negation.  And, a front-end that inherits from 
FIELD_DECL would now not work.  I'd rather we stay with positive-only 
checks.

It doesn't make sense to have DECL_FIELD_CONTEXT.  All DECLs have a 
scope and always will.  DECL_CONTEXT should work on all DECLs.

And, even if you were going to invent DECL_FIELD_CONTEXT this:

+  if (TREE_CODE (olddecl) != FIELD_DECL)
+    {
+      unsigned olddecl_uid = DECL_UID (olddecl);
+      tree olddecl_context = DECL_CONTEXT (olddecl);
+
+      memcpy ((char *) olddecl + sizeof (struct tree_common),
+	      (char *) newdecl + sizeof (struct tree_common),
+	      sizeof (struct tree_decl_common) - sizeof (struct tree_common));
+      memcpy ((char *) olddecl + sizeof (struct tree_decl_common),
+	      (char *) newdecl + sizeof (struct tree_decl_common),
+	      sizeof (struct tree_decl_extra) - sizeof (struct tree_decl_common));
+      DECL_UID (olddecl) = olddecl_uid;
+      DECL_CONTEXT (olddecl) = olddecl_context;
+    }
+  else
+    {
+      unsigned olddecl_uid = DECL_UID (olddecl);
+      tree olddecl_context = DECL_FIELD_CONTEXT (olddecl);
+
+      memcpy ((char *) olddecl + sizeof (struct tree_common),
+	      (char *) newdecl + sizeof (struct tree_common),
+	      sizeof (struct tree_decl_common) - sizeof (struct tree_common));
+      memcpy ((char *) olddecl + sizeof (struct tree_decl_common),
+	      (char *) newdecl + sizeof (struct tree_decl_common),
+	      sizeof (struct tree_field_decl) - sizeof (struct tree_decl_common));
+      DECL_UID (olddecl) = olddecl_uid;
+      DECL_FIELD_CONTEXT (olddecl) = olddecl_context;

would be mostly unnecessary duplication.

This:

+    gcc_assert ((TREE_CODE (decl) == FIELD_DECL)  || 
!DECL_ASSEMBLER_NAME_SET_P (decl));

seems ugly.  We're going to be forever updating such conditions as 
additional things (CONST_DECL?) lose their DECL_ASSEMBLER_NAME slot. 
Maybe it would be better to have DECL_ASSEMBLER_NAME_SET_P return false 
for thse cases.  Or to introduce CAN_HAVE_DECL_ASSEMBLER_NAME_P?

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304