[cfe-commits] r151483 - in /cfe/trunk: lib/CodeGen/CGClass.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExprCXX.cpp test/CXX/special/class.dtor/p5-0x.cpp
Richard Smith
richard-llvm at metafoo.co.uk
Sun Feb 26 01:11:52 PST 2012
Author: rsmith
Date: Sun Feb 26 03:11:52 2012
New Revision: 151483
URL: http://llvm.org/viewvc/llvm-project?rev=151483&view=rev
Log:
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.
Added:
cfe/trunk/test/CXX/special/class.dtor/p5-0x.cpp
Modified:
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=151483&r1=151482&r2=151483&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Sun Feb 26 03:11:52 2012
@@ -1062,6 +1062,10 @@
QualType::DestructionKind dtorKind = type.isDestructedType();
if (!dtorKind) continue;
+ // Anonymous union members do not have their destructors called.
+ const RecordType *RT = type->getAsUnionType();
+ if (RT && RT->getDecl()->isAnonymousStructOrUnion()) continue;
+
CleanupKind cleanupKind = getCleanupKind(dtorKind);
EHStack.pushCleanup<DestroyField>(cleanupKind, field,
getDestroyer(dtorKind),
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=151483&r1=151482&r2=151483&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Sun Feb 26 03:11:52 2012
@@ -3293,6 +3293,9 @@
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 @@
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 @@
// 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<CXXConstructorDecl>(BaseMember);
- if (S.CheckConstructorAccess(Loc, BaseCtor, BaseCtor->getAccess(),
- S.PDiag()) != Sema::AR_accessible)
+ CXXConstructorDecl *Ctor = cast<CXXConstructorDecl>(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 @@
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<CXXConstructorDecl>(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 @@
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 @@
// 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<CXXConstructorDecl>(FieldMember);
if (S.CheckConstructorAccess(Loc, FieldCtor, FieldCtor->getAccess(),
@@ -4597,6 +4571,13 @@
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 @@
!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 @@
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 @@
(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 @@
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 @@
// 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;
}
Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=151483&r1=151482&r2=151483&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Sun Feb 26 03:11:52 2012
@@ -1666,9 +1666,13 @@
Args[i] = Result.takeAs<Expr>();
}
+
Operator = FnDecl;
- CheckAllocationAccess(StartLoc, Range, R.getNamingClass(), Best->FoundDecl,
- Diagnose);
+
+ if (CheckAllocationAccess(StartLoc, Range, R.getNamingClass(),
+ Best->FoundDecl, Diagnose) == AR_inaccessible)
+ return true;
+
return false;
}
@@ -1895,8 +1899,10 @@
return true;
}
- CheckAllocationAccess(StartLoc, SourceRange(), Found.getNamingClass(),
- Matches[0], Diagnose);
+ if (CheckAllocationAccess(StartLoc, SourceRange(), Found.getNamingClass(),
+ Matches[0], Diagnose) == AR_inaccessible)
+ return true;
+
return false;
// We found multiple suitable operators; complain about the ambiguity.
Added: cfe/trunk/test/CXX/special/class.dtor/p5-0x.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.dtor/p5-0x.cpp?rev=151483&view=auto
==============================================================================
--- cfe/trunk/test/CXX/special/class.dtor/p5-0x.cpp (added)
+++ cfe/trunk/test/CXX/special/class.dtor/p5-0x.cpp Sun Feb 26 03:11:52 2012
@@ -0,0 +1,103 @@
+// RUN: %clang_cc1 -verify -std=c++11 %s
+
+struct NonTrivDtor {
+ ~NonTrivDtor();
+};
+struct DeletedDtor {
+ ~DeletedDtor() = delete;
+};
+class InaccessibleDtor {
+ ~InaccessibleDtor() = default;
+};
+
+// A defaulted destructor for a class X is defined as deleted if:
+
+// -- X is a union-like class that has a variant member with a non-trivial
+// destructor.
+union A1 { // expected-note {{here}}
+ A1();
+ NonTrivDtor n;
+};
+A1 a1; // expected-error {{deleted function}}
+struct A2 { // expected-note {{here}}
+ A2();
+ union {
+ NonTrivDtor n;
+ };
+};
+A2 a2; // expected-error {{deleted function}}
+union A3 { // expected-note {{here}}
+ A3();
+ NonTrivDtor n[3];
+};
+A3 a3; // expected-error {{deleted function}}
+struct A4 { // expected-note {{here}}
+ A4();
+ union {
+ NonTrivDtor n[3];
+ };
+};
+A4 a4; // expected-error {{deleted function}}
+
+// -- any of the non-static data members has class type M (or array thereof) and
+// M has a deleted or inaccessible destructor.
+struct B1 { // expected-note {{here}}
+ B1();
+ DeletedDtor a;
+};
+B1 b1; // expected-error {{deleted function}}
+struct B2 { // expected-note {{here}}
+ B2();
+ InaccessibleDtor a;
+};
+B2 b2; // expected-error {{deleted function}}
+struct B3 { // expected-note {{here}}
+ B3();
+ DeletedDtor a[4];
+};
+B3 b3; // expected-error {{deleted function}}
+struct B4 { // expected-note {{here}}
+ B4();
+ InaccessibleDtor a[4];
+};
+B4 b4; // expected-error {{deleted function}}
+union B5 { // expected-note {{here}}
+ B5();
+ union {
+ DeletedDtor a;
+ };
+};
+B5 b5; // expected-error {{deleted function}}
+union B6 { // expected-note {{here}}
+ B6();
+ union {
+ InaccessibleDtor a;
+ };
+};
+B6 b6; // expected-error {{deleted function}}
+
+// -- any direct or virtual base class has a deleted or inaccessible destructor.
+struct C1 : DeletedDtor { C1(); } c1; // expected-error {{deleted function}} expected-note {{here}}
+struct C2 : InaccessibleDtor { C2(); } c2; // expected-error {{deleted function}} expected-note {{here}}
+struct C3 : virtual DeletedDtor { C3(); } c3; // expected-error {{deleted function}} expected-note {{here}}
+struct C4 : virtual InaccessibleDtor { C4(); } c4; // expected-error {{deleted function}} expected-note {{here}}
+
+// -- for a virtual destructor, lookup of the non-array deallocation function
+// results in an ambiguity or a function that is deleted or inaccessible.
+class D1 {
+ void operator delete(void*);
+public:
+ virtual ~D1() = default;
+} d1; // ok
+struct D2 : D1 { // expected-note {{deleted here}}
+ // implicitly-virtual destructor
+} d2; // expected-error {{deleted function}}
+struct D3 {
+ virtual ~D3() = default; // expected-note {{deleted here}}
+ void operator delete(void*, double = 0.0);
+ void operator delete(void*, char = 0);
+} d3; // expected-error {{deleted function}}
+struct D4 {
+ virtual ~D4() = default; // expected-note {{deleted here}}
+ void operator delete(void*) = delete;
+} d4; // expected-error {{deleted function}}
More information about the cfe-commits
mailing list