[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
Chad Rosier
mcrosier at apple.com
Mon Feb 27 09:47:18 PST 2012
Hi Richard,
We have a number of regression test failures on our MSVC buildbot and I suspect this commit or another related commit is causing the problem. Unfortunately, I don't have an easy way of reproducing the failures as I don't have a Windows machine. Would you mind taking a look?
The specific tests are:
********************
Failing Tests (13):
Clang :: CXX/dcl.dcl/dcl.spec/dcl.typedef/p2-0x.cpp
Clang :: CXX/dcl.decl/dcl.meaning/dcl.fct/p6-0x.cpp
Clang :: CXX/dcl.decl/p4-0x.cpp
Clang :: CXX/expr/expr.mptr.oper/p6-0x.cpp
Clang :: CXX/over/over.load/p2-0x.cpp
Clang :: CXX/over/over.match/over.match.funcs/p4-0x.cpp
Clang :: CXX/special/class.ctor/p4-0x.cpp
Clang :: CXX/special/class.dtor/p2-0x.cpp
Clang :: CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.type/p8-0x.cpp
Clang :: CodeGenCXX/mangle-ref-qualifiers.cpp
Clang :: SemaCXX/alias-template.cpp
Clang :: SemaCXX/cxx0x-cursory-default-delete.cpp
Clang :: SemaCXX/issue547.cpp
These tests began failing between r151468 and r151514.
Chad
On Feb 26, 2012, at 1:11 AM, Richard Smith <richard-llvm at metafoo.co.uk> wrote:
> 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}}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120227/ee13ecd1/attachment.html>
More information about the cfe-commits
mailing list