[cfe-commits] r76663 - in /cfe/trunk: include/clang/AST/DeclCXX.h include/clang/Parse/Action.h lib/AST/DeclCXX.cpp lib/AST/DeclPrinter.cpp lib/Parse/ParseCXXInlineMethods.cpp lib/Parse/Parser.cpp lib/Sema/Sema.h lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclCXX.cpp

[Fariborz Jahanian]

On Jul 21, 2009, at 5:05 PM, Douglas Gregor wrote:

>
> On Jul 21, 2009, at 3:36 PM, Fariborz Jahanian wrote:
>
>> Author: fjahanian
>> Date: Tue Jul 21 17:36:06 2009
>> New Revision: 76663
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=76663&view=rev
>> Log:
>> Patch to accomodate Doug's comment on default
>> destruction of base/members for each destructor AST.
>
> Great, thanks! Some minor comments below.

Done in r76708

- fariborz

>
>
>> Modified: cfe/trunk/include/clang/AST/DeclCXX.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=76663&r1=76662&r2=76663&view=diff
>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)
>> +++ cfe/trunk/include/clang/AST/DeclCXX.h Tue Jul 21 17:36:06 2009
>> @@ -901,10 +901,7 @@
>>  /// Support for base and member destruction.
>>  /// BaseOrMemberDestructions - The arguments used to destruct the  
>> base
>>  /// or member.
>> -  // FIXME. May want to use a new class as  
>> CXXBaseOrMemberInitializer has
>> -  // more info. than is needed. At the least,  
>> CXXBaseOrMemberInitializer need
>> -  // be renamed to something neutral.
>> -  CXXBaseOrMemberInitializer **BaseOrMemberDestructions;
>> +  uintptr_t *BaseOrMemberDestructions;
>
> Please document what the uintptr_t values actually mean.
>
>> +  /// isVbaseToDestroy - returns true, if object is virtual base.
>> +  bool isVbaseToDestroy(uintptr_t Vbase) const {
>> +    return (Vbase & 0x1) != 0;
>> +  }
>> +  /// isDirectNonVBaseToDestroy - returns true, if object is  
>> direct non-virtual
>> +  /// base.
>> +  bool isDirectNonVBaseToDestroy(uintptr_t DrctNonVbase) const {
>> +    return (DrctNonVbase & 0x2) != 0;
>> +  }
>> +  /// isAnyBaseToDestroy - returns true, if object is any base  
>> (virtual or
>> +  /// direct non-virtual)
>> +  bool isAnyBaseToDestroy(uintptr_t AnyBase) const {
>> +    return (AnyBase & 0x3) != 0;
>> +  }
>> +  /// isMemberToDestroy - returns true if object is a non-static  
>> data member.
>> +  bool isMemberToDestroy(uintptr_t Member) const {
>> +    return (Member & 0x3)  == 0;
>> +  }
>> +  /// getAnyBaseClassToDestroy - Get the type for the given base  
>> class object.
>> +  Type *getAnyBaseClassToDestroy(uintptr_t Base) const {
>> +    if (isAnyBaseToDestroy(Base))
>> +      return reinterpret_cast<Type*>(Base  & ~0x03);
>> +    return 0;
>> +  }
>> +  /// getMemberToDestroy - Get the member for the given object.
>> +  FieldDecl *getMemberToDestroy(uintptr_t Member) {
>> +    if (isMemberToDestroy(Member))
>> +      return reinterpret_cast<FieldDecl *>(Member);
>> +    return 0;
>> +  }
>> +  /// getVbaseClassToDestroy - Get the virtual base.
>> +  Type *getVbaseClassToDestroy(uintptr_t Vbase) const {
>> +    if (isVbaseToDestroy(Vbase))
>> +      return reinterpret_cast<Type*>(Vbase  & ~0x01);
>> +    return 0;
>> +  }
>> +  /// getDirectNonVBaseClassToDestroy - Get the virtual base.
>> +  Type *getDirectNonVBaseClassToDestroy(uintptr_t Base) const {
>> +    if (isDirectNonVBaseToDestroy(Base))
>> +      return reinterpret_cast<Type*>(Base  & ~0x02);
>> +    return 0;
>> +  }
>
> These hardcoded 0x01, 0x02, and 0x03 constants can be tricky to  
> follow. Could you create an enum that gives useful names to these  
> values?
>
>> +  /// computeBaseOrMembersToDestroy - Compute information in current
>> +  /// destructor decl's AST of bases and non-static data members  
>> which will be
>> +  /// implicitly destroyed. We are storing the destruction in the  
>> order that
>> +  /// they should occur (which is the reverse of construction  
>> order).
>> +  void computeBaseOrMembersToDestroy(ASTContext &C);
>
> Thanks!
>
>> void
>> -CXXDestructorDecl::setBaseOrMemberDestructions(ASTContext &C) {
>> +CXXDestructorDecl::Destroy(ASTContext& C) {
>> +  C.Deallocate(BaseOrMemberDestructions);
>> +  CXXMethodDecl::Destroy(C);
>> +}
>
> Looks good!
>
>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=76663&r1=76662&r2=76663&view=diff
>>
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> =====================================================================
>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Jul 21 17:36:06 2009
>> @@ -3282,12 +3282,15 @@
>>  // Verify that that gotos and switch cases don't jump into scopes  
>> illegally.
>>  if (CurFunctionNeedsScopeChecking)
>>    DiagnoseInvalidJumps(Body);
>> -
>> -  // C++ constructors that have function-try-blocks can't have  
>> return statements
>> -  // in the handlers of that block. (C++ [except.handle]p14)  
>> Verify this.
>> +
>> +  // C++ constructors that have function-try-blocks can't have  
>> return
>> +  // statements in the handlers of that block. (C++  
>> [except.handle]p14)
>> +  // Verify this.
>>  if (isa<CXXConstructorDecl>(dcl) && isa<CXXTryStmt>(Body))
>>     
>> DiagnoseReturnInConstructorExceptionHandler(cast<CXXTryStmt>(Body));
>> -
>> +
>> +  if (CXXDestructorDecl *Destructor =  
>> dyn_cast<CXXDestructorDecl>(dcl))
>> +    Destructor->computeBaseOrMembersToDestroy(Context);
>>  return D;
>> }
>
> I really like this. Thanks!
>
>  - Doug