From 9a561d539158a30b68fc258b81a994f3fac10212 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Sun, 26 Feb 2012 09:11:52 +0000 Subject: Ensure that we delete destructors in the right cases. Specifically: - variant members with nontrivial destructors make the containing class's destructor deleted - check for a virtual destructor after checking for overridden methods in the base class(es) - check for an inaccessible operator delete for a class with a virtual destructor. Do not try to call an anonymous union field's destructor from the destructor of the containing class. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@151483 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Sema/SemaDeclCXX.cpp | 187 ++++++++++++++++++++++------------------------- 1 file changed, 86 insertions(+), 101 deletions(-) (limited to 'lib/Sema/SemaDeclCXX.cpp') diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index ea678688bb..4f3035466e 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -3293,6 +3293,9 @@ Sema::MarkBaseAndMemberDestructorsReferenced(SourceLocation Location, continue; if (FieldClassDecl->hasIrrelevantDestructor()) continue; + // The destructor for an implicit anonymous union member is never invoked. + if (FieldClassDecl->isUnion() && FieldClassDecl->isAnonymousStructOrUnion()) + continue; CXXDestructorDecl *Dtor = LookupDestructor(FieldClassDecl); assert(Dtor && "No dtor found for FieldClassDecl!"); @@ -4372,31 +4375,33 @@ struct SpecialMemberDeletionInfo { TQ & Qualifiers::Volatile); } + bool shouldDeleteForClassSubobject(CXXRecordDecl *Class, FieldDecl *Field); + bool shouldDeleteForBase(CXXRecordDecl *BaseDecl, bool IsVirtualBase); bool shouldDeleteForField(FieldDecl *FD); bool shouldDeleteForAllConstMembers(); }; } -/// Check whether we should delete a special member function due to the class -/// having a particular direct or virtual base class. -bool SpecialMemberDeletionInfo::shouldDeleteForBase(CXXRecordDecl *BaseDecl, - bool IsVirtualBase) { - // C++11 [class.copy]p23: - // -- for the move assignment operator, any direct or indirect virtual - // base class. - if (CSM == Sema::CXXMoveAssignment && IsVirtualBase) - return true; - +/// Check whether we should delete a special member function due to having a +/// direct or virtual base class or static data member of class type M. +bool SpecialMemberDeletionInfo::shouldDeleteForClassSubobject( + CXXRecordDecl *Class, FieldDecl *Field) { // C++11 [class.ctor]p5, C++11 [class.copy]p11, C++11 [class.dtor]p5: // -- any direct or virtual base class [...] has a type with a destructor // that is deleted or inaccessible if (!IsAssignment) { - CXXDestructorDecl *BaseDtor = S.LookupDestructor(BaseDecl); - if (BaseDtor->isDeleted()) + CXXDestructorDecl *Dtor = S.LookupDestructor(Class); + if (Dtor->isDeleted()) return true; - if (S.CheckDestructorAccess(Loc, BaseDtor, S.PDiag()) - != Sema::AR_accessible) + if (S.CheckDestructorAccess(Loc, Dtor, S.PDiag()) != Sema::AR_accessible) + return true; + + // C++11 [class.dtor]p5: + // -- X is a union-like class that has a variant member with a non-trivial + // destructor + if (CSM == Sema::CXXDestructor && Field && Field->getParent()->isUnion() && + !Dtor->isTrivial()) return true; } @@ -4410,50 +4415,58 @@ bool SpecialMemberDeletionInfo::shouldDeleteForBase(CXXRecordDecl *BaseDecl, // overload resolution, as applied to B's corresponding special member, // results in an ambiguity or a function that is deleted or inaccessible // from the defaulted special member + // FIXME: in-class initializers should be handled here if (CSM != Sema::CXXDestructor) { - Sema::SpecialMemberOverloadResult *SMOR = lookupIn(BaseDecl); + Sema::SpecialMemberOverloadResult *SMOR = lookupIn(Class); if (!SMOR->hasSuccess()) return true; - CXXMethodDecl *BaseMember = SMOR->getMethod(); + CXXMethodDecl *Member = SMOR->getMethod(); + // A member of a union must have a trivial corresponding special member. + if (Field && Field->getParent()->isUnion() && !Member->isTrivial()) + return true; + if (IsConstructor) { - CXXConstructorDecl *BaseCtor = cast(BaseMember); - if (S.CheckConstructorAccess(Loc, BaseCtor, BaseCtor->getAccess(), - S.PDiag()) != Sema::AR_accessible) + CXXConstructorDecl *Ctor = cast(Member); + if (S.CheckConstructorAccess(Loc, Ctor, Ctor->getAccess(), S.PDiag()) + != Sema::AR_accessible) return true; // -- for the move constructor, a [...] direct or virtual base class with // a type that does not have a move constructor and is not trivially // copyable. - if (IsMove && !BaseCtor->isMoveConstructor() && - !BaseDecl->isTriviallyCopyable()) + if (IsMove && !Ctor->isMoveConstructor() && !Class->isTriviallyCopyable()) return true; } else { assert(IsAssignment && "unexpected kind of special member"); - if (S.CheckDirectMemberAccess(Loc, BaseMember, S.PDiag()) + if (S.CheckDirectMemberAccess(Loc, Member, S.PDiag()) != Sema::AR_accessible) return true; // -- for the move assignment operator, a direct base class with a type // that does not have a move assignment operator and is not trivially // copyable. - if (IsMove && !BaseMember->isMoveAssignmentOperator() && - !BaseDecl->isTriviallyCopyable()) + if (IsMove && !Member->isMoveAssignmentOperator() && + !Class->isTriviallyCopyable()) return true; } } - // C++11 [class.dtor]p5: - // -- for a virtual destructor, lookup of the non-array deallocation function - // results in an ambiguity or in a function that is deleted or inaccessible - if (CSM == Sema::CXXDestructor && MD->isVirtual()) { - FunctionDecl *OperatorDelete = 0; - DeclarationName Name = - S.Context.DeclarationNames.getCXXOperatorName(OO_Delete); - if (S.FindDeallocationFunction(Loc, MD->getParent(), Name, - OperatorDelete, false)) - return true; - } + return false; +} + +/// Check whether we should delete a special member function due to the class +/// having a particular direct or virtual base class. +bool SpecialMemberDeletionInfo::shouldDeleteForBase(CXXRecordDecl *BaseDecl, + bool IsVirtualBase) { + // C++11 [class.copy]p23: + // -- for the move assignment operator, any direct or indirect virtual + // base class. + if (CSM == Sema::CXXMoveAssignment && IsVirtualBase) + return true; + + if (shouldDeleteForClassSubobject(BaseDecl, 0)) + return true; return false; } @@ -4500,58 +4513,14 @@ bool SpecialMemberDeletionInfo::shouldDeleteForField(FieldDecl *FD) { UE = FieldRecord->field_end(); UI != UE; ++UI) { QualType UnionFieldType = S.Context.getBaseElementType(UI->getType()); - CXXRecordDecl *UnionFieldRecord = - UnionFieldType->getAsCXXRecordDecl(); if (!UnionFieldType.isConstQualified()) AllVariantFieldsAreConst = false; - if (UnionFieldRecord) { - // FIXME: Checking for accessibility and validity of this - // destructor is technically going beyond the - // standard, but this is believed to be a defect. - if (!IsAssignment) { - CXXDestructorDecl *FieldDtor = S.LookupDestructor(UnionFieldRecord); - if (FieldDtor->isDeleted()) - return true; - if (S.CheckDestructorAccess(Loc, FieldDtor, S.PDiag()) != - Sema::AR_accessible) - return true; - if (!FieldDtor->isTrivial()) - return true; - } - - // FIXME: in-class initializers should be handled here - if (CSM != Sema::CXXDestructor) { - Sema::SpecialMemberOverloadResult *SMOR = - lookupIn(UnionFieldRecord); - // FIXME: Checking for accessibility and validity of this - // corresponding member is technically going beyond the - // standard, but this is believed to be a defect. - if (!SMOR->hasSuccess()) - return true; - - CXXMethodDecl *FieldMember = SMOR->getMethod(); - // A member of a union must have a trivial corresponding - // special member. - if (!FieldMember->isTrivial()) - return true; - - if (IsConstructor) { - CXXConstructorDecl *FieldCtor = - cast(FieldMember); - if (S.CheckConstructorAccess(Loc, FieldCtor, - FieldCtor->getAccess(), - S.PDiag()) != Sema::AR_accessible) - return true; - } else { - assert(IsAssignment && "unexpected kind of special member"); - if (S.CheckDirectMemberAccess(Loc, FieldMember, S.PDiag()) - != Sema::AR_accessible) - return true; - } - } - } + CXXRecordDecl *UnionFieldRecord = UnionFieldType->getAsCXXRecordDecl(); + if (UnionFieldRecord && + shouldDeleteForClassSubobject(UnionFieldRecord, *UI)) + return true; } // At least one member in each anonymous union must be non-const @@ -4564,9 +4533,9 @@ bool SpecialMemberDeletionInfo::shouldDeleteForField(FieldDecl *FD) { return false; } - // Unless we're doing assignment, the field's destructor must be - // accessible and not deleted. - if (!IsAssignment) { + // When checking a constructor, the field's destructor must be accessible + // and not deleted. + if (IsConstructor) { CXXDestructorDecl *FieldDtor = S.LookupDestructor(FieldRecord); if (FieldDtor->isDeleted()) return true; @@ -4578,13 +4547,18 @@ bool SpecialMemberDeletionInfo::shouldDeleteForField(FieldDecl *FD) { // Check that the corresponding member of the field is accessible, // unique, and non-deleted. We don't do this if it has an explicit // initialization when default-constructing. - if (CSM != Sema::CXXDestructor && - !(CSM == Sema::CXXDefaultConstructor && FD->hasInClassInitializer())) { + if (!(CSM == Sema::CXXDefaultConstructor && FD->hasInClassInitializer())) { Sema::SpecialMemberOverloadResult *SMOR = lookupIn(FieldRecord); if (!SMOR->hasSuccess()) return true; CXXMethodDecl *FieldMember = SMOR->getMethod(); + + // We need the corresponding member of a union to be trivial so that + // we can safely process all members simultaneously. + if (inUnion() && !FieldMember->isTrivial()) + return true; + if (IsConstructor) { CXXConstructorDecl *FieldCtor = cast(FieldMember); if (S.CheckConstructorAccess(Loc, FieldCtor, FieldCtor->getAccess(), @@ -4597,6 +4571,13 @@ bool SpecialMemberDeletionInfo::shouldDeleteForField(FieldDecl *FD) { if (IsMove && !FieldCtor->isMoveConstructor() && !FieldRecord->isTriviallyCopyable()) return true; + } else if (CSM == Sema::CXXDestructor) { + CXXDestructorDecl *FieldDtor = S.LookupDestructor(FieldRecord); + if (FieldDtor->isDeleted()) + return true; + if (S.CheckDestructorAccess(Loc, FieldDtor, S.PDiag()) != + Sema::AR_accessible) + return true; } else { assert(IsAssignment && "unexpected kind of special member"); if (S.CheckDirectMemberAccess(Loc, FieldMember, S.PDiag()) @@ -4610,14 +4591,6 @@ bool SpecialMemberDeletionInfo::shouldDeleteForField(FieldDecl *FD) { !FieldRecord->isTriviallyCopyable()) return true; } - - // We need the corresponding member of a union to be trivial so that - // we can safely copy them all simultaneously. - // FIXME: Note that performing the check here (where we rely on the lack - // of an in-class initializer) is technically ill-formed. However, this - // seems most obviously to be a bug in the standard. - if (inUnion() && !FieldMember->isTrivial()) - return true; } } else if (CSM == Sema::CXXDefaultConstructor && !inUnion() && FieldType.isConstQualified() && !FD->hasInClassInitializer()) { @@ -4652,6 +4625,8 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM) { if (!LangOpts.CPlusPlus0x || RD->isInvalidDecl()) return false; + // FIXME: Provide the ability to diagnose why a special member was deleted. + // C++11 [expr.lambda.prim]p19: // The closure type associated with a lambda-expression has a // deleted (8.4.3) default constructor and a deleted copy @@ -4660,6 +4635,18 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM) { (CSM == CXXDefaultConstructor || CSM == CXXCopyAssignment)) return true; + // C++11 [class.dtor]p5: + // -- for a virtual destructor, lookup of the non-array deallocation function + // results in an ambiguity or in a function that is deleted or inaccessible + if (CSM == Sema::CXXDestructor && MD->isVirtual()) { + FunctionDecl *OperatorDelete = 0; + DeclarationName Name = + Context.DeclarationNames.getCXXOperatorName(OO_Delete); + if (FindDeallocationFunction(MD->getLocation(), MD->getParent(), Name, + OperatorDelete, false)) + return true; + } + // For an anonymous struct or union, the copy and assignment special members // will never be used, so skip the check. For an anonymous union declared at // namespace scope, the constructor and destructor are used. @@ -4672,8 +4659,6 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM) { SpecialMemberDeletionInfo SMI(*this, MD, CSM); - // FIXME: We should put some diagnostic logic right into this function. - for (CXXRecordDecl::base_class_iterator BI = RD->bases_begin(), BE = RD->bases_end(); BI != BE; ++BI) if (!BI->isVirtual() && @@ -7216,11 +7201,11 @@ CXXDestructorDecl *Sema::DeclareImplicitDestructor(CXXRecordDecl *ClassDecl) { // This could be uniqued if it ever proves significant. Destructor->setTypeSourceInfo(Context.getTrivialTypeSourceInfo(Ty)); + AddOverriddenMethods(ClassDecl, Destructor); + if (ShouldDeleteSpecialMember(Destructor, CXXDestructor)) Destructor->setDeletedAsWritten(); - - AddOverriddenMethods(ClassDecl, Destructor); - + return Destructor; } -- cgit v1.2.3