[cfe-commits] r155218 - in /cfe/trunk: include/clang/Sema/Sema.h lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaLookup.cpp test/CXX/special/class.copy/implicit-move.cpp test/CXX/special/class.copy/p8-cxx11.cpp

Richard Smith richard at metafoo.co.uk
Fri Apr 20 13:20:17 PDT 2012


Without this, std::pair<std::unique_ptr<int>, int> fails to instantiate
when using libstdc++4.7. Integrate to branch requested.

On Fri, Apr 20, 2012 at 11:46 AM, Richard Smith
<richard-llvm at metafoo.co.uk>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
> 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/20120420/ef9d0561/attachment.html>


More information about the cfe-commits mailing list