This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: Integrated hashtables, take 3



    Neil> This is looking like it'll bootstrap and make check.
    Neil> Assuming it does, (kneels down) OK to commit?

:-)  You've been very good-spirited about the whole thing.

Almost -- but I think the only changes remaining are minor enough that
you don't even need to retest.

Intead of this:

  +/* Identifier part common to the C front ends.  */
  +struct c_common_identifier
  +{
  +  struct tree_identifier ident;
  +  char padding[(sizeof (cpp_hashnode) + sizeof (struct tree_common)
  +		- sizeof (struct tree_identifier))];
  +  ENUM_BITFIELD(rid) rid_code;
   };

how about:

  struct c_common_identifier
  {
    /* The `tree_common' plus the first bit of the `hashnode' are 
       layout compatible with a `tree_identifier'.  */
    struct tree_common common;
    struct cpp_hashnode hashnode;
    ENUM_BITFIELD(rid) rid_code;
  };

?

And then adjust the accessor macros if need be.

If that doesn't work for some reason, then the patch is OK as it is --
but you need a comment on that `padding' field for sure.

Also, these guys could have better names: I thought they were creating
something.

  +#define MAKE_GCC_IDENT(NODE) \
  +	((tree) ((char *) (NODE) - sizeof (struct tree_common)))
  +#define MAKE_HT_IDENT(NODE) \
  +	(&((struct tree_identifier *) (NODE))->id)

I suggest HT_IDENT_TO_GCC_IDENT and GCC_IDENT_TO_HT_IDENT.  A little
typing won't hurt.  Also, the latter needs a comment.

This is definite goodness.  Thank you for working it through to get a
clean design.

Yours,

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]