[cfe-commits] r74666 - in /cfe/trunk: include/clang/AST/DeclCXX.h lib/AST/DeclCXX.cpp lib/Sema/SemaDeclCXX.cpp

[Douglas Gregor]

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