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

Eli Friedman eli.friedman at gmail.com
Tue Jun 3 18:49:26 PDT 2008

On Tue, Jun 3, 2008 at 6:06 PM, Argiris Kirtzidis <akyrtzi at gmail.com> wrote:
> Hi,
> The attached patch contains these changes:
> -New Decl subclasses:
>   CXXField (derived from Field)  - instance fields
>   CXXRecord (derived from Record)  - Records for C++
>   CXXMethod (derived from Function)  - static and instance methods
>   CXXClassVar (derived from Var)  - static data members
> -Because there are two "types" of tags (Struct/CXXStruct etc.), to avoid
> constantly checking for both (i.e. when you need to know whether the Record
> is a 'struct' or 'union'),
> I've added a TagKind enum in TagDecl and a TagDecl::getTagKind method.
> Plus, RecordDecl/CXXRecordDecl::Create methods receive a TagDecl::TagKind
> enum instead of the general Decl::Kind enum.
> -DeclContext gets a "ScopedDecl *DeclChain" member, which is used to chain
> decls that belong to the same DeclContext.

>From my reading, there's nothing obviously wrong with this patch,
except that it's too big, which makes reviewing it a lot more
difficult.  Please put the DeclBase code movement into a separate
patch (moving the DeclBase code is fine without review), put the
DeclChain changes into a separate patch, put the Decl::Struct ->
TagDecl::TK_struct changes into a separate patch, and split out
anything else that's obviously independent.

Minor issue I spotted: in CXXMethodDecl::getThisType, there's no point
to making "this" const; it isn't an lvalue, so the const modifier
doesn't do anything.


More information about the cfe-dev mailing list