Merging r155218:

------------------------------------------------------------------------
r155218 | rsmith | 2012-04-20 11:46:14 -0700 (Fri, 20 Apr 2012) | 5 lines

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).

------------------------------------------------------------------------


git-svn-id: https://llvm.org/svn/llvm-project/cfe/branches/release_31@156682 91177308-0d34-0410-b5e6-96231b3b80d8

5 files changed, 91 insertions, 66 deletions

diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 8ac7c3ee01..ca4b7096de 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -653,23 +653,19 @@ public:
   /// 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:
@@ -685,9 +681,6 @@ public:
 
     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
@@ -1909,11 +1902,9 @@ public:
   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);
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index cec20af2d7..fe83985b42 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -7579,8 +7579,9 @@ Sema::ComputeDefaultedCopyAssignmentExceptionSpecAndConst(
     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
@@ -7591,8 +7592,9 @@ Sema::ComputeDefaultedCopyAssignmentExceptionSpecAndConst(
       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);
     }
   }
   
@@ -7606,8 +7608,9 @@ Sema::ComputeDefaultedCopyAssignmentExceptionSpecAndConst(
        ++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);
     }
   }
   
@@ -8610,8 +8613,8 @@ Sema::ComputeDefaultedCopyCtorExceptionSpecAndConst(CXXRecordDecl *ClassDecl) {
     
     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(),
@@ -8620,8 +8623,8 @@ Sema::ComputeDefaultedCopyCtorExceptionSpecAndConst(CXXRecordDecl *ClassDecl) {
        ++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
@@ -8634,8 +8637,8 @@ Sema::ComputeDefaultedCopyCtorExceptionSpecAndConst(CXXRecordDecl *ClassDecl) {
        ++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
diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp
index f003bddc05..9f5138ba4a 100644
--- a/lib/Sema/SemaLookup.cpp
+++ b/lib/Sema/SemaLookup.cpp
@@ -2277,7 +2277,7 @@ Sema::SpecialMemberOverloadResult *Sema::LookupSpecialMember(CXXRecordDecl *RD,
     Result->setMethod(DD);
     Result->setKind(DD->isDeleted() ?
                     SpecialMemberOverloadResult::NoMemberOrDeleted :
-                    SpecialMemberOverloadResult::SuccessNonConst);
+                    SpecialMemberOverloadResult::Success);
     return Result;
   }
 
@@ -2288,6 +2288,9 @@ Sema::SpecialMemberOverloadResult *Sema::LookupSpecialMember(CXXRecordDecl *RD,
   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 @@ Sema::SpecialMemberOverloadResult *Sema::LookupSpecialMember(CXXRecordDecl *RD,
         DeclareImplicitMoveAssignment(RD);
     }
 
-    QualType ArgType = CanTy;
     if (ConstArg)
       ArgType.addConst();
     if (VolatileArg)
@@ -2321,14 +2323,17 @@ Sema::SpecialMemberOverloadResult *Sema::LookupSpecialMember(CXXRecordDecl *RD,
     // 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 @@ Sema::SpecialMemberOverloadResult *Sema::LookupSpecialMember(CXXRecordDecl *RD,
   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 @@ Sema::SpecialMemberOverloadResult *Sema::LookupSpecialMember(CXXRecordDecl *RD,
       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 @@ Sema::SpecialMemberOverloadResult *Sema::LookupSpecialMember(CXXRecordDecl *RD,
   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 @@ CXXConstructorDecl *Sema::LookupDefaultConstructor(CXXRecordDecl *Class) {
 
 /// \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 @@ DeclContext::lookup_result Sema::LookupConstructors(CXXRecordDecl *Class) {
 /// \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 @@ CXXMethodDecl *Sema::LookupCopyingAssignment(CXXRecordDecl *Class,
                         ThisQuals & Qualifiers::Const,
                         ThisQuals & Qualifiers::Volatile);
 
-  if (ConstParamMatch)
-    *ConstParamMatch = Result->hasConstParamMatch();
-
   return Result->getMethod();
 }
 
diff --git a/test/CXX/special/class.copy/implicit-move.cpp b/test/CXX/special/class.copy/implicit-move.cpp
index b1b298e893..3e9accfbf6 100644
--- a/test/CXX/special/class.copy/implicit-move.cpp
+++ b/test/CXX/special/class.copy/implicit-move.cpp
@@ -198,15 +198,15 @@ namespace DR1402 {
   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}}
diff --git a/test/CXX/special/class.copy/p8-cxx11.cpp b/test/CXX/special/class.copy/p8-cxx11.cpp
new file mode 100644
index 0000000000..02e6cd1c9a
--- /dev/null
+++ b/test/CXX/special/class.copy/p8-cxx11.cpp
@@ -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 &);
+};