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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
