[cfe-dev] [PATCH]: Preparing AST for C++ declarations

[Argiris Kirtzidis]

Eli Friedman wrote:
> Most important issue: you're adding an Access member to TypeDecl, a
> class which isn't C++-specific.  This will increase the size of all
> TypeDecls for C code by 4 bytes.  Adding new members to data
> structures requires additional scrutiny, because it affects memory
> usage.  In this case, it doesn't appear to be worthwhile to bloat the
> memory usage for C code.  There are a couple of possible ways to fix
> this: the simplest is to add a CXXEnumDecl and CXXTypedefDecl, and
> shuffle the Access member down to the C++-specific classes.  This
> isn't exactly pretty, but it's straightforward to implement.
>   

I've suggested this in a previous patch, but it seemed ugly wasn't well 
received:
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2008-May/001780.html

> This will increase the size of all TypeDecls for C code by 4 bytes
Two suggestions about this:
1) "Swizzle" the access specifier bits into the "TypeForDecl" member 
pointer.
2) Or move the access specifier bits to NamedDecl, thus they get packed 
with the bitfields from Decl

I'm more fond of (2), since it also simplifies things by letting the 
access specifier be shared by all C++ decls.

> Have you considered what will happen to RecordDecls once inheritance
> is implemented?  I'm not sure that a RecordDecl will continue to
> properly make sense, at least in terms of the internal data structure.
>  The Nth member of a RecordDecl might not really have any
> correspondence to anything useful, although I guess most code
> iterating over members needs to be aware of C++ anyway.  Member name
> lookup will also need to be completely different.  I guess it isn't
> really an issue at this point, but something to think about.
>   

This is from the previous patch I posted:
> The instance fields of CXXRecord are stored in the members array of 
> RecordDecl, thus the data layout of CXXRecord is calculated through 
> the Record.
> All the other members (including the static fields), are ScopedDecls 
> with the CXXRecord as declaration context, so they can be iterated 
> through a general DeclContext member iterator (not implemented yet).
> Name lookup for class members will be efficient through the use of the 
> IdentifierResolver.

The Nth member (as "getMember(N)") will be the Nth instance field of a 
specific Record. It seems to me that it makes sense even when 
inheritance comes into the picture, do you disagree ?

> A CXXInstanceMethodType and CXXStaticMethodType are going to be
> necessary at some point; also not something that needs to be done
> here, but something to consider.
>   
Ok.
> I notice you didn't include the serialization implementation; it's
> okay to leave it out for now, but it will need to be done.
>   
Yes, of course.
> Overall, the patch seems to be well thought out.  I'm not completely
> sure everything is going to remain the same once some of this stuff
> actually has parser/sema support, but it's hard to tell without
> implementing it.
>   

Well, I've come to the point where simple stuff like this gets passed 
through -fsyntax-only:

class S {
public:
  int f1(int p) {
    A z = 6;
    return p + x + this->y + z;
  }

  typedef int A;

private:
  int x,y;
};


-Argiris