On Apr 20, 2012, at 1:20 PM, Richard Smith <[email protected]> wrote:
> Without this, std::pair<std::unique_ptr<int>, int> fails to instantiate when > using libstdc++4.7. Integrate to branch requested. Approved. > On Fri, Apr 20, 2012 at 11:46 AM, Richard Smith <[email protected]> > wrote: > Author: rsmith > Date: Fri Apr 20 13:46:14 2012 > New Revision: 155218 > > URL: http://llvm.org/viewvc/llvm-project?rev=155218&view=rev > Log: > Fix bug where a class's (deleted) copy constructor would be implicitly given a > non-const reference parameter type if the class had any subobjects with > deleted > copy constructors. This causes a rejects-valid if the class's copy constructor > is explicitly defaulted (as happens for some implementations of std::pair > etc). > > Added: > cfe/trunk/test/CXX/special/class.copy/p8-cxx11.cpp > Modified: > cfe/trunk/include/clang/Sema/Sema.h > cfe/trunk/lib/Sema/SemaDeclCXX.cpp > cfe/trunk/lib/Sema/SemaLookup.cpp > cfe/trunk/test/CXX/special/class.copy/implicit-move.cpp > > Modified: cfe/trunk/include/clang/Sema/Sema.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=155218&r1=155217&r2=155218&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Sema/Sema.h (original) > +++ cfe/trunk/include/clang/Sema/Sema.h Fri Apr 20 13:46:14 2012 > @@ -665,23 +665,19 @@ > /// SpecialMemberOverloadResult - The overloading result for a special > member > /// function. > /// > - /// This is basically a wrapper around PointerIntPair. The lowest bit of > the > - /// integer is used to determine whether we have a parameter qualification > - /// match, the second-lowest is whether we had success in resolving the > - /// overload to a unique non-deleted function. > - /// > - /// The ConstParamMatch bit represents whether, when looking up a copy > - /// constructor or assignment operator, we found a potential copy > - /// constructor/assignment operator whose first parameter is > const-qualified. > - /// This is used for determining parameter types of other objects and is > - /// utterly meaningless on other types of special members. > + /// This is basically a wrapper around PointerIntPair. The lowest bits of > the > + /// integer are used to determine whether overload resolution succeeded, > and > + /// whether, when looking up a copy constructor or assignment operator, we > + /// found a potential copy constructor/assignment operator whose first > + /// parameter is const-qualified. This is used for determining parameter > types > + /// of other objects and is utterly meaningless on other types of special > + /// members. > class SpecialMemberOverloadResult : public llvm::FastFoldingSetNode { > public: > enum Kind { > NoMemberOrDeleted, > Ambiguous, > - SuccessNonConst, > - SuccessConst > + Success > }; > > private: > @@ -697,9 +693,6 @@ > > Kind getKind() const { return static_cast<Kind>(Pair.getInt()); } > void setKind(Kind K) { Pair.setInt(K); } > - > - bool hasSuccess() const { return getKind() >= SuccessNonConst; } > - bool hasConstParamMatch() const { return getKind() == SuccessConst; } > }; > > /// \brief A cache of special member function overload resolution results > @@ -1921,11 +1914,9 @@ > DeclContextLookupResult LookupConstructors(CXXRecordDecl *Class); > CXXConstructorDecl *LookupDefaultConstructor(CXXRecordDecl *Class); > CXXConstructorDecl *LookupCopyingConstructor(CXXRecordDecl *Class, > - unsigned Quals, > - bool *ConstParam = 0); > + unsigned Quals); > CXXMethodDecl *LookupCopyingAssignment(CXXRecordDecl *Class, unsigned Quals, > - bool RValueThis, unsigned ThisQuals, > - bool *ConstParam = 0); > + bool RValueThis, unsigned > ThisQuals); > CXXConstructorDecl *LookupMovingConstructor(CXXRecordDecl *Class); > CXXMethodDecl *LookupMovingAssignment(CXXRecordDecl *Class, bool RValueThis, > unsigned ThisQuals); > > Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=155218&r1=155217&r2=155218&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original) > +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Fri Apr 20 13:46:14 2012 > @@ -7577,8 +7577,9 @@ > assert(!Base->getType()->isDependentType() && > "Cannot generate implicit members for class with dependent > bases."); > CXXRecordDecl *BaseClassDecl = Base->getType()->getAsCXXRecordDecl(); > - LookupCopyingAssignment(BaseClassDecl, Qualifiers::Const, false, 0, > - &HasConstCopyAssignment); > + HasConstCopyAssignment &= > + (bool)LookupCopyingAssignment(BaseClassDecl, Qualifiers::Const, > + false, 0); > } > > // In C++11, the above citation has "or virtual" added > @@ -7589,8 +7590,9 @@ > assert(!Base->getType()->isDependentType() && > "Cannot generate implicit members for class with dependent > bases."); > CXXRecordDecl *BaseClassDecl = Base->getType()->getAsCXXRecordDecl(); > - LookupCopyingAssignment(BaseClassDecl, Qualifiers::Const, false, 0, > - &HasConstCopyAssignment); > + HasConstCopyAssignment &= > + (bool)LookupCopyingAssignment(BaseClassDecl, Qualifiers::Const, > + false, 0); > } > } > > @@ -7604,8 +7606,9 @@ > ++Field) { > QualType FieldType = Context.getBaseElementType((*Field)->getType()); > if (CXXRecordDecl *FieldClassDecl = FieldType->getAsCXXRecordDecl()) { > - LookupCopyingAssignment(FieldClassDecl, Qualifiers::Const, false, 0, > - &HasConstCopyAssignment); > + HasConstCopyAssignment &= > + (bool)LookupCopyingAssignment(FieldClassDecl, Qualifiers::Const, > + false, 0); > } > } > > @@ -8608,8 +8611,8 @@ > > CXXRecordDecl *BaseClassDecl > = cast<CXXRecordDecl>(Base->getType()->getAs<RecordType>()->getDecl()); > - LookupCopyingConstructor(BaseClassDecl, Qualifiers::Const, > - &HasConstCopyConstructor); > + HasConstCopyConstructor &= > + (bool)LookupCopyingConstructor(BaseClassDecl, Qualifiers::Const); > } > > for (CXXRecordDecl::base_class_iterator Base = ClassDecl->vbases_begin(), > @@ -8618,8 +8621,8 @@ > ++Base) { > CXXRecordDecl *BaseClassDecl > = cast<CXXRecordDecl>(Base->getType()->getAs<RecordType>()->getDecl()); > - LookupCopyingConstructor(BaseClassDecl, Qualifiers::Const, > - &HasConstCopyConstructor); > + HasConstCopyConstructor &= > + (bool)LookupCopyingConstructor(BaseClassDecl, Qualifiers::Const); > } > > // -- for all the nonstatic data members of X that are of a > @@ -8632,8 +8635,8 @@ > ++Field) { > QualType FieldType = Context.getBaseElementType((*Field)->getType()); > if (CXXRecordDecl *FieldClassDecl = FieldType->getAsCXXRecordDecl()) { > - LookupCopyingConstructor(FieldClassDecl, Qualifiers::Const, > - &HasConstCopyConstructor); > + HasConstCopyConstructor &= > + (bool)LookupCopyingConstructor(FieldClassDecl, Qualifiers::Const); > } > } > // Otherwise, the implicitly declared copy constructor will have > > Modified: cfe/trunk/lib/Sema/SemaLookup.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=155218&r1=155217&r2=155218&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/SemaLookup.cpp (original) > +++ cfe/trunk/lib/Sema/SemaLookup.cpp Fri Apr 20 13:46:14 2012 > @@ -2277,7 +2277,7 @@ > Result->setMethod(DD); > Result->setKind(DD->isDeleted() ? > SpecialMemberOverloadResult::NoMemberOrDeleted : > - SpecialMemberOverloadResult::SuccessNonConst); > + SpecialMemberOverloadResult::Success); > return Result; > } > > @@ -2288,6 +2288,9 @@ > Expr *Arg = 0; > unsigned NumArgs; > > + QualType ArgType = CanTy; > + ExprValueKind VK = VK_LValue; > + > if (SM == CXXDefaultConstructor) { > Name = Context.DeclarationNames.getCXXConstructorName(CanTy); > NumArgs = 0; > @@ -2308,7 +2311,6 @@ > DeclareImplicitMoveAssignment(RD); > } > > - QualType ArgType = CanTy; > if (ConstArg) > ArgType.addConst(); > if (VolatileArg) > @@ -2321,14 +2323,17 @@ > // Possibly an XValue is actually correct in the case of move, but > // there is no semantic difference for class types in this restricted > // case. > - ExprValueKind VK; > if (SM == CXXCopyConstructor || SM == CXXCopyAssignment) > VK = VK_LValue; > else > VK = VK_RValue; > + } > > + OpaqueValueExpr FakeArg(SourceLocation(), ArgType, VK); > + > + if (SM != CXXDefaultConstructor) { > NumArgs = 1; > - Arg = new (Context) OpaqueValueExpr(SourceLocation(), ArgType, VK); > + Arg = &FakeArg; > } > > // Create the object argument > @@ -2338,17 +2343,14 @@ > if (VolatileThis) > ThisTy.addVolatile(); > Expr::Classification Classification = > - (new (Context) OpaqueValueExpr(SourceLocation(), ThisTy, > - RValueThis ? VK_RValue : VK_LValue))-> > - Classify(Context); > + OpaqueValueExpr(SourceLocation(), ThisTy, > + RValueThis ? VK_RValue : VK_LValue).Classify(Context); > > // Now we perform lookup on the name we computed earlier and do overload > // resolution. Lookup is only performed directly into the class since there > // will always be a (possibly implicit) declaration to shadow any others. > OverloadCandidateSet OCS((SourceLocation())); > DeclContext::lookup_iterator I, E; > - SpecialMemberOverloadResult::Kind SuccessKind = > - SpecialMemberOverloadResult::SuccessNonConst; > > llvm::tie(I, E) = RD->lookup(Name); > assert((I != E) && > @@ -2378,17 +2380,6 @@ > else > AddOverloadCandidate(M, DeclAccessPair::make(M, AS_public), > llvm::makeArrayRef(&Arg, NumArgs), OCS, true); > - > - // Here we're looking for a const parameter to speed up creation of > - // implicit copy methods. > - if ((SM == CXXCopyAssignment && M->isCopyAssignmentOperator()) || > - (SM == CXXCopyConstructor && > - cast<CXXConstructorDecl>(M)->isCopyConstructor())) { > - QualType ArgType = > M->getType()->getAs<FunctionProtoType>()->getArgType(0); > - if (!ArgType->isReferenceType() || > - ArgType->getPointeeType().isConstQualified()) > - SuccessKind = SpecialMemberOverloadResult::SuccessConst; > - } > } else if (FunctionTemplateDecl *Tmpl = > dyn_cast<FunctionTemplateDecl>(Cand)) { > if (SM == CXXCopyAssignment || SM == CXXMoveAssignment) > @@ -2409,7 +2400,7 @@ > switch (OCS.BestViableFunction(*this, SourceLocation(), Best)) { > case OR_Success: > Result->setMethod(cast<CXXMethodDecl>(Best->Function)); > - Result->setKind(SuccessKind); > + Result->setKind(SpecialMemberOverloadResult::Success); > break; > > case OR_Deleted: > @@ -2442,17 +2433,13 @@ > > /// \brief Look up the copying constructor for the given class. > CXXConstructorDecl *Sema::LookupCopyingConstructor(CXXRecordDecl *Class, > - unsigned Quals, > - bool *ConstParamMatch) { > + unsigned Quals) { > assert(!(Quals & ~(Qualifiers::Const | Qualifiers::Volatile)) && > "non-const, non-volatile qualifiers for copy ctor arg"); > SpecialMemberOverloadResult *Result = > LookupSpecialMember(Class, CXXCopyConstructor, Quals & Qualifiers::Const, > Quals & Qualifiers::Volatile, false, false, false); > > - if (ConstParamMatch) > - *ConstParamMatch = Result->hasConstParamMatch(); > - > return cast_or_null<CXXConstructorDecl>(Result->getMethod()); > } > > @@ -2485,8 +2472,7 @@ > /// \brief Look up the copying assignment operator for the given class. > CXXMethodDecl *Sema::LookupCopyingAssignment(CXXRecordDecl *Class, > unsigned Quals, bool RValueThis, > - unsigned ThisQuals, > - bool *ConstParamMatch) { > + unsigned ThisQuals) { > assert(!(Quals & ~(Qualifiers::Const | Qualifiers::Volatile)) && > "non-const, non-volatile qualifiers for copy assignment arg"); > assert(!(ThisQuals & ~(Qualifiers::Const | Qualifiers::Volatile)) && > @@ -2497,9 +2483,6 @@ > ThisQuals & Qualifiers::Const, > ThisQuals & Qualifiers::Volatile); > > - if (ConstParamMatch) > - *ConstParamMatch = Result->hasConstParamMatch(); > - > return Result->getMethod(); > } > > > Modified: cfe/trunk/test/CXX/special/class.copy/implicit-move.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.copy/implicit-move.cpp?rev=155218&r1=155217&r2=155218&view=diff > ============================================================================== > --- cfe/trunk/test/CXX/special/class.copy/implicit-move.cpp (original) > +++ cfe/trunk/test/CXX/special/class.copy/implicit-move.cpp Fri Apr 20 > 13:46:14 2012 > @@ -198,15 +198,15 @@ > struct NoMove4 : NonTrivialCopyAssign {}; // expected-note 2{{'const > DR1402::NoMove4 &'}} > struct NoMove5 : virtual NonTrivialCopyCtor {}; // expected-note 2{{'const > DR1402::NoMove5 &'}} > struct NoMove6 : virtual NonTrivialCopyAssign {}; // expected-note > 2{{'const DR1402::NoMove6 &'}} > - struct NoMove7 : NonTrivialCopyCtorVBase {}; // expected-note > 2{{'DR1402::NoMove7 &'}} > - struct NoMove8 : NonTrivialCopyAssignVBase {}; // expected-note > 2{{'DR1402::NoMove8 &'}} > + struct NoMove7 : NonTrivialCopyCtorVBase {}; // expected-note 2{{'const > DR1402::NoMove7 &'}} > + struct NoMove8 : NonTrivialCopyAssignVBase {}; // expected-note 2{{'const > DR1402::NoMove8 &'}} > > // A non-trivially-move-assignable virtual base class inhibits the > declaration > // of a move assignment (which might move-assign the base class multiple > // times). > struct NoMove9 : NonTrivialMoveAssign {}; > - struct NoMove10 : virtual NonTrivialMoveAssign {}; // expected-note > {{'DR1402::NoMove10 &'}} > - struct NoMove11 : NonTrivialMoveAssignVBase {}; // expected-note > {{'DR1402::NoMove11 &'}} > + struct NoMove10 : virtual NonTrivialMoveAssign {}; // expected-note > {{'const DR1402::NoMove10 &'}} > + struct NoMove11 : NonTrivialMoveAssignVBase {}; // expected-note {{'const > DR1402::NoMove11 &'}} > > struct Test { > friend NoMove1::NoMove1(NoMove1 &&); // expected-error {{no matching > function}} > > Added: cfe/trunk/test/CXX/special/class.copy/p8-cxx11.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.copy/p8-cxx11.cpp?rev=155218&view=auto > ============================================================================== > --- cfe/trunk/test/CXX/special/class.copy/p8-cxx11.cpp (added) > +++ cfe/trunk/test/CXX/special/class.copy/p8-cxx11.cpp Fri Apr 20 13:46:14 > 2012 > @@ -0,0 +1,48 @@ > +// RUN: %clang_cc1 -std=c++11 %s -verify > + > +// C++98 [class.copy]p5 / C++11 [class.copy]p8. > + > +// The implicitly-declared copy constructor for a class X will have the form > +// X::X(const X&) > +// if [every direct subobject] has a copy constructor whose first parameter > is > +// of type 'const volatile[opt] T &'. Otherwise, it will have the form > +// X::X(X&) > + > +struct ConstCopy { > + ConstCopy(const ConstCopy &); > +}; > + > +struct NonConstCopy { > + NonConstCopy(NonConstCopy &); > +}; > + > +struct DeletedConstCopy { > + DeletedConstCopy(const DeletedConstCopy &) = delete; > +}; > + > +struct DeletedNonConstCopy { > + DeletedNonConstCopy(DeletedNonConstCopy &) = delete; > +}; > + > +struct ImplicitlyDeletedConstCopy { > + ImplicitlyDeletedConstCopy(ImplicitlyDeletedConstCopy &&); > +}; > + > + > +struct A : ConstCopy {}; > +struct B : NonConstCopy { ConstCopy a; }; > +struct C : ConstCopy { NonConstCopy a; }; > +struct D : DeletedConstCopy {}; > +struct E : DeletedNonConstCopy {}; > +struct F { ImplicitlyDeletedConstCopy a; }; > +struct G : virtual B {}; > + > +struct Test { > + friend A::A(const A &); > + friend B::B(B &); > + friend C::C(C &); > + friend D::D(const D &); > + friend E::E(E &); > + friend F::F(const F &); > + friend G::G(G &); > +}; > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
