On Jul 1, 2009, at 4:35 PM, Fariborz Jahanian wrote: > Author: fjahanian > Date: Wed Jul 1 18:35:25 2009 > New Revision: 74666 > > URL: http://llvm.org/viewvc/llvm-project?rev=74666&view=rev > Log: > Use Destroy for member initializer list clean up. > Per Doug's comments. Doug please review. > > Modified: > cfe/trunk/include/clang/AST/DeclCXX.h > cfe/trunk/lib/AST/DeclCXX.cpp > cfe/trunk/lib/Sema/SemaDeclCXX.cpp > > Modified: cfe/trunk/include/clang/AST/DeclCXX.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=74666&r1=74665&r2=74666&view=diff > > = > = > = > = > = > = > = > = > ====================================================================== > --- cfe/trunk/include/clang/AST/DeclCXX.h (original) > +++ cfe/trunk/include/clang/AST/DeclCXX.h Wed Jul 1 18:35:25 2009 > @@ -710,8 +710,7 @@ > BaseOrMemberInitializers(0), NumBaseOrMemberInitializers(0) { > setImplicit(isImplicitlyDeclared); > } > - > - ~CXXConstructorDecl() { delete [] BaseOrMemberInitializers; } > + virtual void Destroy(ASTContext& C); > > public: > static CXXConstructorDecl *Create(ASTContext &C, CXXRecordDecl *RD, > @@ -742,23 +741,23 @@ > ImplicitlyDefined = ID; > } > > - /// arg_iterator - Iterates through the member/base initializer > list. > - typedef CXXBaseOrMemberInitializer **arg_iterator; > + /// init_iterator - Iterates through the member/base initializer > list. > + typedef CXXBaseOrMemberInitializer **init_iterator; > > - /// arg_const_iterator - Iterates through the memberbase > initializer list. > - typedef CXXBaseOrMemberInitializer * const * arg_const_iterator; > + /// init_const_iterator - Iterates through the memberbase > initializer list. > + typedef CXXBaseOrMemberInitializer * const * init_const_iterator; > > /// begin() - Retrieve an iterator to the first initializer. > - arg_iterator begin() { return > BaseOrMemberInitializers; } > + init_iterator begin() { return > BaseOrMemberInitializers; } > /// begin() - Retrieve an iterator to the first initializer. > - arg_const_iterator begin() const { return > BaseOrMemberInitializers; } > + init_const_iterator begin() const { return > BaseOrMemberInitializers; } > > /// end() - Retrieve an iterator past the last initializer. > - arg_iterator end() { > + init_iterator end() { > return BaseOrMemberInitializers + NumBaseOrMemberInitializers; > } > /// end() - Retrieve an iterator past the last initializer. > - arg_const_iterator end() const { > + init_const_iterator end() const { > return BaseOrMemberInitializers + NumBaseOrMemberInitializers; > }
The names init_begin/init_end would be more typical for Clang's AST, IMO. > @@ -768,7 +767,8 @@ > return NumBaseOrMemberInitializers; > } > > - void setBaseOrMemberInitializers(CXXBaseOrMemberInitializer > **Initializers, > + void setBaseOrMemberInitializers(ASTContext &C, > + CXXBaseOrMemberInitializer > **Initializers, > unsigned NumInitializers); > > /// isDefaultConstructor - Whether this constructor is a default > > Modified: cfe/trunk/lib/AST/DeclCXX.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=74666&r1=74665&r2=74666&view=diff > > = > = > = > = > = > = > = > = > ====================================================================== > --- cfe/trunk/lib/AST/DeclCXX.cpp (original) > +++ cfe/trunk/lib/AST/DeclCXX.cpp Wed Jul 1 18:35:25 2009 > @@ -409,17 +409,25 @@ > > void > CXXConstructorDecl::setBaseOrMemberInitializers( > + ASTContext &C, > CXXBaseOrMemberInitializer > **Initializers, > unsigned NumInitializers) { > if (NumInitializers > 0) { > NumBaseOrMemberInitializers = NumInitializers; > BaseOrMemberInitializers = > - new CXXBaseOrMemberInitializer*[NumInitializers]; > + new (C, 8) CXXBaseOrMemberInitializer*[NumInitializers]; new (C) should suffice; we don't actually need 8-byte alignment, do we? > for (unsigned Idx = 0; Idx < NumInitializers; ++Idx) > BaseOrMemberInitializers[Idx] = Initializers[Idx]; > } > } > > +void > +CXXConstructorDecl::Destroy(ASTContext& C) { > + C.Deallocate(BaseOrMemberInitializers); > + this->~CXXMethodDecl(); > + C.Deallocate((void *)this); > +} Rather than these last two lines, I suggest just: CXXMethodDecl::Destroy(C) which will let base classes destroy their members before deallocating the memory for this node. Everything else looks good! - Doug _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
