From: Sean Hunt Date: Tue, 11 Oct 2011 04:55:36 +0000 (+0000) Subject: Consolidate copy constructor deletion into ShouldDeleteSpecialMember. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c32d684d6c787b332a64c9013598d5ef668c4b45;p=clang Consolidate copy constructor deletion into ShouldDeleteSpecialMember. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@141645 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index fec14c5826..809b529b31 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -2834,10 +2834,6 @@ public: /// definition when it is defaulted. bool ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM); - /// \brief Determine if a defaulted copy constructor ought to be - /// deleted. - bool ShouldDeleteCopyConstructor(CXXConstructorDecl *CD); - /// \brief Determine if a defaulted copy assignment operator ought to be /// deleted. bool ShouldDeleteCopyAssignmentOperator(CXXMethodDecl *MD); diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index a7370cfcad..8b9877ac7c 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -3812,7 +3812,7 @@ void Sema::CheckExplicitlyDefaultedCopyConstructor(CXXConstructorDecl *CD) { return; } - if (ShouldDeleteCopyConstructor(CD)) { + if (ShouldDeleteSpecialMember(CD, CXXCopyConstructor)) { if (First) { CD->setDeletedAsWritten(); } else { @@ -4101,6 +4101,7 @@ void Sema::CheckExplicitlyDefaultedDestructor(CXXDestructorDecl *DD) { /// This function implements the following C++0x paragraphs: /// - [class.ctor]/5 +/// - [class.copy]/11 bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM) { assert(!MD->isInvalidDecl()); CXXRecordDecl *RD = MD->getParent(); @@ -4119,13 +4120,17 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM) { case CXXDefaultConstructor: IsConstructor = true; break; + case CXXCopyConstructor: + IsConstructor = true; + ConstArg = MD->getParamDecl(0)->getType().isConstQualified(); + break; default: llvm_unreachable("function only currently implemented for default ctors"); } SourceLocation Loc = MD->getLocation(); - // Do access control from the constructor + // Do access control from the special member function ContextRAII MethodContext(*this, MD); bool AllConst = true; @@ -4159,7 +4164,8 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM) { } // Finding the corresponding member in the base should lead to a - // unique, accessible, non-deleted function. + // unique, accessible, non-deleted function. If we are doing + // a destructor, we have already checked this case. if (CSM != CXXDestructor) { SpecialMemberOverloadResult *SMOR = LookupSpecialMember(BaseDecl, CSM, ConstArg, false, IsMove, false, @@ -4228,20 +4234,14 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM) { if (IsUnion && !FieldType.isConstQualified()) AllConst = false; + // For a copy constructor, data members must not be of rvalue reference + // type. + } else if (CSM == CXXCopyConstructor) { + if (FieldType->isRValueReferenceType()) + return true; } if (FieldRecord) { - // Unless we're doing assignment, the field's destructor must be - // accessible and not deleted. - if (!IsAssignment) { - CXXDestructorDecl *FieldDtor = LookupDestructor(FieldRecord); - if (FieldDtor->isDeleted()) - return true; - if (CheckDestructorAccess(Loc, FieldDtor, PDiag()) != - AR_accessible) - return true; - } - // For a default constructor, a const member must have a user-provided // default constructor or else be explicitly initialized. if (CSM == CXXDefaultConstructor && FieldType.isConstQualified() && @@ -4249,9 +4249,8 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM) { !FieldRecord->hasUserProvidedDefaultConstructor()) return true; - // For a default constructor, additional restrictions exist on the - // variant members. - if (CSM == CXXDefaultConstructor && !IsUnion && FieldRecord->isUnion() && + // Some additional restrictions exist on the variant members. + if (!IsUnion && FieldRecord->isUnion() && FieldRecord->isAnonymousStructOrUnion()) { // We're okay to reuse AllConst here since we only care about the // value otherwise if we're in a union. @@ -4267,12 +4266,49 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM) { if (!UnionFieldType.isConstQualified()) AllConst = false; - if (UnionFieldRecord && - !UnionFieldRecord->hasTrivialDefaultConstructor()) - return true; + 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 = LookupDestructor(UnionFieldRecord); + if (FieldDtor->isDeleted()) + return true; + if (CheckDestructorAccess(Loc, FieldDtor, PDiag()) != + AR_accessible) + return true; + if (!FieldDtor->isTrivial()) + return true; + } + + if (CSM != CXXDestructor) { + SpecialMemberOverloadResult *SMOR = + LookupSpecialMember(UnionFieldRecord, CSM, ConstArg, false, + IsMove, false, false); + // 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 + // constructor. + if (!FieldMember->isTrivial()) + return true; + + if (IsConstructor) { + CXXConstructorDecl *FieldCtor = cast(FieldMember); + if (CheckConstructorAccess(Loc, FieldCtor, FieldCtor->getAccess(), + PDiag()) != AR_accessible) + return true; + } + } + } } - if (AllConst) + // At least one member in each anonymous union must be non-const + if (CSM == CXXDefaultConstructor && AllConst) return true; // Don't try to initialize the anonymous union @@ -4280,6 +4316,17 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM) { continue; } + // Unless we're doing assignment, the field's destructor must be + // accessible and not deleted. + if (!IsAssignment) { + CXXDestructorDecl *FieldDtor = LookupDestructor(FieldRecord); + if (FieldDtor->isDeleted()) + return true; + if (CheckDestructorAccess(Loc, FieldDtor, PDiag()) != + AR_accessible) + return true; + } + // 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. @@ -4322,165 +4369,6 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM) { return false; } -bool Sema::ShouldDeleteCopyConstructor(CXXConstructorDecl *CD) { - CXXRecordDecl *RD = CD->getParent(); - assert(!RD->isDependentType() && "do deletion after instantiation"); - if (!LangOpts.CPlusPlus0x || RD->isInvalidDecl()) - return false; - - SourceLocation Loc = CD->getLocation(); - - // Do access control from the constructor - ContextRAII CtorContext(*this, CD); - - bool Union = RD->isUnion(); - - assert(!CD->getParamDecl(0)->getType()->getPointeeType().isNull() && - "copy assignment arg has no pointee type"); - unsigned ArgQuals = - CD->getParamDecl(0)->getType()->getPointeeType().isConstQualified() ? - Qualifiers::Const : 0; - - // We do this because we should never actually use an anonymous - // union's constructor. - if (Union && RD->isAnonymousStructOrUnion()) - return false; - - // FIXME: We should put some diagnostic logic right into this function. - - // C++0x [class.copy]/11 - // A defaulted [copy] constructor for class X is defined as delete if X has: - - for (CXXRecordDecl::base_class_iterator BI = RD->bases_begin(), - BE = RD->bases_end(); - BI != BE; ++BI) { - // We'll handle this one later - if (BI->isVirtual()) - continue; - - QualType BaseType = BI->getType(); - CXXRecordDecl *BaseDecl = BaseType->getAsCXXRecordDecl(); - assert(BaseDecl && "base isn't a CXXRecordDecl"); - - // -- any [direct base class] of a type with a destructor that is deleted or - // inaccessible from the defaulted constructor - CXXDestructorDecl *BaseDtor = LookupDestructor(BaseDecl); - if (BaseDtor->isDeleted()) - return true; - if (CheckDestructorAccess(Loc, BaseDtor, PDiag()) != - AR_accessible) - return true; - - // -- a [direct base class] B that cannot be [copied] because overload - // resolution, as applied to B's [copy] constructor, results in an - // ambiguity or a function that is deleted or inaccessible from the - // defaulted constructor - CXXConstructorDecl *BaseCtor = LookupCopyingConstructor(BaseDecl, ArgQuals); - if (!BaseCtor || BaseCtor->isDeleted()) - return true; - if (CheckConstructorAccess(Loc, BaseCtor, BaseCtor->getAccess(), PDiag()) != - AR_accessible) - return true; - } - - for (CXXRecordDecl::base_class_iterator BI = RD->vbases_begin(), - BE = RD->vbases_end(); - BI != BE; ++BI) { - QualType BaseType = BI->getType(); - CXXRecordDecl *BaseDecl = BaseType->getAsCXXRecordDecl(); - assert(BaseDecl && "base isn't a CXXRecordDecl"); - - // -- any [virtual base class] of a type with a destructor that is deleted or - // inaccessible from the defaulted constructor - CXXDestructorDecl *BaseDtor = LookupDestructor(BaseDecl); - if (BaseDtor->isDeleted()) - return true; - if (CheckDestructorAccess(Loc, BaseDtor, PDiag()) != - AR_accessible) - return true; - - // -- a [virtual base class] B that cannot be [copied] because overload - // resolution, as applied to B's [copy] constructor, results in an - // ambiguity or a function that is deleted or inaccessible from the - // defaulted constructor - CXXConstructorDecl *BaseCtor = LookupCopyingConstructor(BaseDecl, ArgQuals); - if (!BaseCtor || BaseCtor->isDeleted()) - return true; - if (CheckConstructorAccess(Loc, BaseCtor, BaseCtor->getAccess(), PDiag()) != - AR_accessible) - return true; - } - - for (CXXRecordDecl::field_iterator FI = RD->field_begin(), - FE = RD->field_end(); - FI != FE; ++FI) { - if (FI->isUnnamedBitfield()) - continue; - - QualType FieldType = Context.getBaseElementType(FI->getType()); - - // -- for a copy constructor, a non-static data member of rvalue reference - // type - if (FieldType->isRValueReferenceType()) - return true; - - CXXRecordDecl *FieldRecord = FieldType->getAsCXXRecordDecl(); - - if (FieldRecord) { - // This is an anonymous union - if (FieldRecord->isUnion() && FieldRecord->isAnonymousStructOrUnion()) { - // Anonymous unions inside unions do not variant members create - if (!Union) { - for (CXXRecordDecl::field_iterator UI = FieldRecord->field_begin(), - UE = FieldRecord->field_end(); - UI != UE; ++UI) { - QualType UnionFieldType = Context.getBaseElementType(UI->getType()); - CXXRecordDecl *UnionFieldRecord = - UnionFieldType->getAsCXXRecordDecl(); - - // -- a variant member with a non-trivial [copy] constructor and X - // is a union-like class - if (UnionFieldRecord && - !UnionFieldRecord->hasTrivialCopyConstructor()) - return true; - } - } - - // Don't try to initalize an anonymous union - continue; - } else { - // -- a variant member with a non-trivial [copy] constructor and X is a - // union-like class - if (Union && !FieldRecord->hasTrivialCopyConstructor()) - return true; - - // -- any [non-static data member] of a type with a destructor that is - // deleted or inaccessible from the defaulted constructor - CXXDestructorDecl *FieldDtor = LookupDestructor(FieldRecord); - if (FieldDtor->isDeleted()) - return true; - if (CheckDestructorAccess(Loc, FieldDtor, PDiag()) != - AR_accessible) - return true; - } - - // -- a [non-static data member of class type (or array thereof)] B that - // cannot be [copied] because overload resolution, as applied to B's - // [copy] constructor, results in an ambiguity or a function that is - // deleted or inaccessible from the defaulted constructor - CXXConstructorDecl *FieldCtor = LookupCopyingConstructor(FieldRecord, - ArgQuals); - if (!FieldCtor || FieldCtor->isDeleted()) - return true; - if (CheckConstructorAccess(Loc, FieldCtor, FieldCtor->getAccess(), - PDiag()) != AR_accessible) - return true; - } - } - - return false; -} - bool Sema::ShouldDeleteCopyAssignmentOperator(CXXMethodDecl *MD) { CXXRecordDecl *RD = MD->getParent(); assert(!RD->isDependentType() && "do deletion after instantiation"); @@ -8735,7 +8623,7 @@ CXXConstructorDecl *Sema::DeclareImplicitCopyConstructor( // deleted; ... if (ClassDecl->hasUserDeclaredMoveConstructor() || ClassDecl->hasUserDeclaredMoveAssignment() || - ShouldDeleteCopyConstructor(CopyConstructor)) + ShouldDeleteSpecialMember(CopyConstructor, CXXCopyConstructor)) CopyConstructor->setDeletedAsWritten(); return CopyConstructor; diff --git a/test/CXX/special/class.copy/p11.0x.copy.cpp b/test/CXX/special/class.copy/p11.0x.copy.cpp new file mode 100644 index 0000000000..6024ffb431 --- /dev/null +++ b/test/CXX/special/class.copy/p11.0x.copy.cpp @@ -0,0 +1,90 @@ +// RUN: %clang_cc1 -std=c++0x -fsyntax-only -verify %s + +struct NonTrivial { + NonTrivial(const NonTrivial&); +}; + +union DeletedNTVariant { // expected-note{{here}} + NonTrivial NT; + DeletedNTVariant(); +}; +DeletedNTVariant DVa; +DeletedNTVariant DVb(DVa); // expected-error{{call to deleted constructor}} + +struct DeletedNTVariant2 { // expected-note{{here}} + union { + NonTrivial NT; + }; + DeletedNTVariant2(); +}; +DeletedNTVariant2 DV2a; +DeletedNTVariant2 DV2b(DV2a); // expected-error{{call to deleted constructor}} + +struct NoAccess { + NoAccess() = default; +private: + NoAccess(const NoAccess&); + + friend struct HasAccess; +}; + +struct HasNoAccess { // expected-note{{here}} + NoAccess NA; +}; +HasNoAccess HNAa; +HasNoAccess HNAb(HNAa); // expected-error{{call to deleted constructor}} + +struct HasAccess { + NoAccess NA; +}; + +HasAccess HAa; +HasAccess HAb(HAa); + +struct NonConst { + NonConst(NonConst&); +}; +struct Ambiguity { + Ambiguity(const Ambiguity&); + Ambiguity(volatile Ambiguity&); +}; + +struct IsAmbiguous { // expected-note{{here}} + NonConst NC; + Ambiguity A; + IsAmbiguous(); +}; +IsAmbiguous IAa; +IsAmbiguous IAb(IAa); // expected-error{{call to deleted constructor}} + +struct Deleted { // expected-note{{here}} + IsAmbiguous IA; +}; +Deleted Da; +Deleted Db(Da); // expected-error{{call to deleted constructor}} + +struct NoAccessDtor { +private: + ~NoAccessDtor(); + friend struct HasAccessDtor; +}; + +struct HasNoAccessDtor { // expected-note{{here}} + NoAccessDtor NAD; + HasNoAccessDtor(); + ~HasNoAccessDtor(); +}; +HasNoAccessDtor HNADa; +HasNoAccessDtor HNADb(HNADa); // expected-error{{call to deleted constructor}} + +struct HasAccessDtor { + NoAccessDtor NAD; +}; +HasAccessDtor HADa; +HasAccessDtor HADb(HADa); + +struct RValue { // expected-note{{here}} + int && ri = 1; +}; +RValue RVa; +RValue RVb(RVa); // expected-error{{call to deleted constructor}}