[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.
-Eli
More information about the cfe-dev
mailing list