[cfe-dev] [PATCH]: C++ decl classes for the AST

[Argiris Kirtzidis]

Chris Lattner wrote:
>
>> -'ClassMember' serves as a base class for members of a CXXRecord. It 
>> provides the access specifier and the parent CXXRecord. Decls that 
>> inherit ClassMember:
>>   CXXField - for instance fields (inherits FieldDecl)
>>   CXXMethod - for static and instance methods (inherits FunctionDecl)
>>   NestedTypedef - for nested typedefs (inherits TypedefDecl)
>>   NestedRecordDecl - for nested struct/union/classes (inherits 
>> CXXRecordDecl)
>>   ClassVar - for static data members (inherits VarDecl)
>
> I'm less enthused about using multiple inheritance for this :).  Is 
> there more common between members than just access specifiers?  Would 
> it be horrible to just give each of those classes their own 
> AccessSpecifiers member?
>
> It doesn't seem very useful to pass around generic ClassMember*'s.  If 
> we need a generic "give me the access specifiers for this member", it 
> could be written as a top-level method on decl or something.
>
> Alternatively, maybe AccessSpecifiers should be pushed up the class 
> hierarchy, say to NamedDecl?  Though it would not be used by every 
> subclass, it would make things simpler that way.  This would eliminate 
> the need for classes like 'NestedTypedef' and 'NestedRecordDecl' which 
> is strange.  A typedef shouldn't be represented with a different class 
> based on where it is defined.

All the extra classes were due to the "keep to a decl only the 
absolutely necessary" philosophy so that a typedef in C doesn't carry a 
redundant member, but I actually agree with you and would prefer that 
things were simpler.
I'd also suggest to *not* use a CXXRecord. The only difference from a 
Record is that CXXRecord can be used as a DeclContext, and it adds this 
kind of complexity:

-create CXXRecord
-parse the struct definition
-determine that Record is sufficient
-create new Record with new Fields
-Replace CXXRecord with new Record in scope

But this is not so bad as this:

-struct Foo; // should create a Record #1
-Foo *x; #2
-struct Foo { typedef int Bar; } // should create a CXXRecord #3

Now merging #1 with #3 is not so simple anymore. x's type is of pointer 
of #1 but Bar belongs to #3. If there was only a Record class, we would 
easily say that Bar belongs to #1.

To sum up, I suggest:

-Not using a separate CXXRecord
-Set Tag as DeclContext so that both Enum and Record can serve as 
DeclContext
-Add AccessSpecifier at TypeDecl; Typedef, Record, Enum can use it (I 
had forgotten about Enum)
-Add AccessSpecifier at CXXMethod, CXXClassVar. (Could be in ValueDecl 
but EnumConstant can get it from Enum and ParmVar don't use it)
-Add 'isCXXClassMember()' and 'getCXXClassParent()' to NamedDecl.

>
> Some minor things: I don't think it makes sense for CXXMethod to 
> derive from FunctionDecl, a CXXMethod doesn't have the "isa" property 
> for FunctionDecl.

I'd regard a CXXMethod as a Function with an implicit 'this' parameter; 
everything from FunctionDecl can be reused.
What is the benefit of CXXMethodDecl as a different class from 
FunctionDecl ?

>
> Should ClassVarDecl -> CXXClassVarDecl? 

Ok.

> Does it make sense to distinguish these from CXXField in the class 
> hierarchy? 

-The logic is that Field/CXXFields are instance vars that contribute to 
the data layout of the Record. Everything that refers to the layout need 
only refer to the members array of the Record without having to check 
whether a member from the array is static or not; all C struct layout 
code can work on a C++ struct with no modifications.
-A static field have no use for Field's BitWidth, and as far as codegen 
is concerned, a static field has more in common with a VarDecl than with 
a FieldDecl.


-Argiris