Improve our handling of user-defined conversions when computing

implicit conversion sequences. In particular, model the "standard
conversion" from a class to its own type (or a base type) directly as
a standard conversion in the normal path *without* trying to determine
if there is a valid copy constructor. This appears to match the intent
of C++ [over.best.ics]p6 and more closely matches GCC and EDG.

As part of this, model non-lvalue reference initialization via
user-defined conversion in overloading the same way we handle it in
InitializationSequence, separating the "general user-defined
conversion" and "conversion to compatible class type" cases.

The churn in the overload-call-copycon.cpp test case is because the
test case was originally wrong; it assumed that we should do more
checking for copy constructors that we actually should, which affected
overload resolution.

Fixes PR7055. Bootstrapped okay.


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

include/clang/Basic/DiagnosticSemaKinds.td

@@ -1212,7 +1212,7 @@ def note_ovl_candidate_bad_cvr_this : Note<"candidate "
     "%select{|function|||function||||"
     "function (the implicit copy assignment operator)}0 not viable: "
     "'this' argument has type %2, but method is not marked "
-    "%select{const|volatile|const or volatile|restrict|const or restrict|"
+    "%select{const|restrict|const or restrict|volatile|const or volatile|"
     "volatile or restrict|const, volatile, or restrict}3">;
 def note_ovl_candidate_bad_cvr : Note<"candidate "
     "%select{function|function|constructor|"
@@ -1221,7 +1221,7 @@ def note_ovl_candidate_bad_cvr : Note<"candidate "
     "constructor (the implicit copy constructor)|"
     "function (the implicit copy assignment operator)}0%1 not viable: "
     "%ordinal4 argument (%2) would lose "
-    "%select{const|volatile|const and volatile|restrict|const and restrict|"
+    "%select{const|restrict|const and restrict|volatile|const and volatile|"
     "volatile and restrict|const, volatile, and restrict}3 qualifier"
     "%select{||s||s|s|s}3">;
 def note_ovl_candidate_bad_base_to_derived_conv : Note<"candidate "

lib/Sema/SemaOverload.cpp

@@ -685,24 +685,17 @@ Sema::TryImplicitConversion(Expr* From, QualType ToType,
     return ICS;
   }
 
-  if (SuppressUserConversions) {
-    // C++ [over.ics.user]p4:
-    //   A conversion of an expression of class type to the same class
-    //   type is given Exact Match rank, and a conversion of an
-    //   expression of class type to a base class of that type is
-    //   given Conversion rank, in spite of the fact that a copy/move
-    //   constructor (i.e., a user-defined conversion function) is
-    //   called for those cases.
-    QualType FromType = From->getType();
-    if (!ToType->getAs<RecordType>() || !FromType->getAs<RecordType>() ||
-        !(Context.hasSameUnqualifiedType(FromType, ToType) ||
-          IsDerivedFrom(FromType, ToType))) {
-      // We're not in the case above, so there is no conversion that
-      // we can perform.
-      ICS.setBad(BadConversionSequence::no_conversion, From, ToType);
-      return ICS;
-    }
-
+  // C++ [over.ics.user]p4:
+  //   A conversion of an expression of class type to the same class
+  //   type is given Exact Match rank, and a conversion of an
+  //   expression of class type to a base class of that type is
+  //   given Conversion rank, in spite of the fact that a copy/move
+  //   constructor (i.e., a user-defined conversion function) is
+  //   called for those cases.
+  QualType FromType = From->getType();
+  if (ToType->getAs<RecordType>() && FromType->getAs<RecordType>() &&
+      (Context.hasSameUnqualifiedType(FromType, ToType) ||
+       IsDerivedFrom(FromType, ToType))) {
     ICS.setStandard();
     ICS.Standard.setAsIdentityConversion();
     ICS.Standard.setFromType(FromType);
@@ -713,11 +706,18 @@ Sema::TryImplicitConversion(Expr* From, QualType ToType,
     // exists. When we actually perform initialization, we'll find the
     // appropriate constructor to copy the returned object, if needed.
     ICS.Standard.CopyConstructor = 0;
-
+    
     // Determine whether this is considered a derived-to-base conversion.
     if (!Context.hasSameUnqualifiedType(FromType, ToType))
       ICS.Standard.Second = ICK_Derived_To_Base;
-
+    
+    return ICS;
+  }
+  
+  if (SuppressUserConversions) {
+    // We're not in the case above, so there is no conversion that
+    // we can perform.
+    ICS.setBad(BadConversionSequence::no_conversion, From, ToType);
     return ICS;
   }
 
@@ -2630,16 +2630,21 @@ Sema::CompareReferenceRelationship(SourceLocation Loc,
     return Ref_Related;
 }
 
-/// \brief Look for a user-defined conversion to an lvalue reference-compatible
+/// \brief Look for a user-defined conversion to an value reference-compatible
 ///        with DeclType. Return true if something definite is found.
 static bool
-FindConversionToLValue(Sema &S, ImplicitConversionSequence &ICS,
-                       QualType DeclType, SourceLocation DeclLoc,
-                       Expr *Init, QualType T2, bool AllowExplicit) {
+FindConversionForRefInit(Sema &S, ImplicitConversionSequence &ICS,
+                         QualType DeclType, SourceLocation DeclLoc,
+                         Expr *Init, QualType T2, bool AllowRvalues,
+                         bool AllowExplicit) {
   assert(T2->isRecordType() && "Can only find conversions of record types.");
   CXXRecordDecl *T2RecordDecl
     = dyn_cast<CXXRecordDecl>(T2->getAs<RecordType>()->getDecl());
 
+  QualType ToType
+    = AllowRvalues? DeclType->getAs<ReferenceType>()->getPointeeType()
+                  : DeclType;
+
   OverloadCandidateSet CandidateSet(DeclLoc);
   const UnresolvedSetImpl *Conversions
     = T2RecordDecl->getVisibleConversionFunctions();
@@ -2658,21 +2663,40 @@ FindConversionToLValue(Sema &S, ImplicitConversionSequence &ICS,
     else
       Conv = cast<CXXConversionDecl>(D);
 
-    // If the conversion function doesn't return a reference type,
-    // it can't be considered for this conversion. An rvalue reference
-    // is only acceptable if its referencee is a function type.
-    const ReferenceType *RefType =
-      Conv->getConversionType()->getAs<ReferenceType>();
-    if (RefType && (RefType->isLValueReferenceType() ||
-                    RefType->getPointeeType()->isFunctionType()) &&
-        (AllowExplicit || !Conv->isExplicit())) {
-      if (ConvTemplate)
-        S.AddTemplateConversionCandidate(ConvTemplate, I.getPair(), ActingDC,
-                                       Init, DeclType, CandidateSet);
-      else
-        S.AddConversionCandidate(Conv, I.getPair(), ActingDC, Init,
-                               DeclType, CandidateSet);
+    // If this is an explicit conversion, and we're not allowed to consider 
+    // explicit conversions, skip it.
+    if (!AllowExplicit && Conv->isExplicit())
+      continue;
+    
+    if (AllowRvalues) {
+      bool DerivedToBase = false;
+      bool ObjCConversion = false;
+      if (!ConvTemplate &&
+          S.CompareReferenceRelationship(DeclLoc,
+                                         Conv->getConversionType().getNonReferenceType().getUnqualifiedType(),
+                                         DeclType.getNonReferenceType().getUnqualifiedType(),
+                                         DerivedToBase, ObjCConversion)
+            == Sema::Ref_Incompatible)
+        continue;
+    } else {
+      // If the conversion function doesn't return a reference type,
+      // it can't be considered for this conversion. An rvalue reference
+      // is only acceptable if its referencee is a function type.
+
+      const ReferenceType *RefType =
+        Conv->getConversionType()->getAs<ReferenceType>();
+      if (!RefType ||
+          (!RefType->isLValueReferenceType() &&
+           !RefType->getPointeeType()->isFunctionType()))
+        continue;
     }
+    
+    if (ConvTemplate)
+      S.AddTemplateConversionCandidate(ConvTemplate, I.getPair(), ActingDC,
+                                       Init, ToType, CandidateSet);
+    else
+      S.AddConversionCandidate(Conv, I.getPair(), ActingDC, Init,
+                               ToType, CandidateSet);
   }
 
   OverloadCandidateSet::iterator Best;
@@ -2808,8 +2832,9 @@ TryReferenceInit(Sema &S, Expr *&Init, QualType DeclType,
     if (!SuppressUserConversions && T2->isRecordType() &&
         !S.RequireCompleteType(DeclLoc, T2, 0) && 
         RefRelationship == Sema::Ref_Incompatible) {
-      if (FindConversionToLValue(S, ICS, DeclType, DeclLoc,
-                                 Init, T2, AllowExplicit))
+      if (FindConversionForRefInit(S, ICS, DeclType, DeclLoc,
+                                   Init, T2, /*AllowRvalues=*/false,
+                                   AllowExplicit))
         return ICS;
     }
   }
@@ -2861,28 +2886,37 @@ TryReferenceInit(Sema &S, Expr *&Init, QualType DeclType,
   //          that is the result of the conversion in the second case
   //          (or, in either case, to the appropriate base class
   //          subobject of the object).
-  //
-  // We're only checking the first case here, which is a direct
-  // binding in C++0x but not in C++03.
-  if (InitCategory.isRValue() && T2->isRecordType() &&
-      RefRelationship >= Sema::Ref_Compatible_With_Added_Qualification) {
-    ICS.setStandard();
-    ICS.Standard.First = ICK_Identity;
-    ICS.Standard.Second = DerivedToBase? ICK_Derived_To_Base 
-                        : ObjCConversion? ICK_Compatible_Conversion
-                        : ICK_Identity;
-    ICS.Standard.Third = ICK_Identity;
-    ICS.Standard.FromTypePtr = T2.getAsOpaquePtr();
-    ICS.Standard.setToType(0, T2);
-    ICS.Standard.setToType(1, T1);
-    ICS.Standard.setToType(2, T1);
-    ICS.Standard.ReferenceBinding = true;
-    ICS.Standard.DirectBinding = S.getLangOptions().CPlusPlus0x;
-    ICS.Standard.RRefBinding = isRValRef;
-    ICS.Standard.CopyConstructor = 0;
-    return ICS;
+  if (T2->isRecordType()) {
+    // First case: "cv1 T1" is reference-compatible with "cv2 T2". This is a
+    // direct binding in C++0x but not in C++03.
+    if (InitCategory.isRValue() && 
+        RefRelationship >= Sema::Ref_Compatible_With_Added_Qualification) {
+      ICS.setStandard();
+      ICS.Standard.First = ICK_Identity;
+      ICS.Standard.Second = DerivedToBase? ICK_Derived_To_Base 
+                          : ObjCConversion? ICK_Compatible_Conversion
+                          : ICK_Identity;
+      ICS.Standard.Third = ICK_Identity;
+      ICS.Standard.FromTypePtr = T2.getAsOpaquePtr();
+      ICS.Standard.setToType(0, T2);
+      ICS.Standard.setToType(1, T1);
+      ICS.Standard.setToType(2, T1);
+      ICS.Standard.ReferenceBinding = true;
+      ICS.Standard.DirectBinding = S.getLangOptions().CPlusPlus0x;
+      ICS.Standard.RRefBinding = isRValRef;
+      ICS.Standard.CopyConstructor = 0;
+      return ICS;
+    }
+    
+    // Second case: not reference-related.
+    if (RefRelationship == Sema::Ref_Incompatible &&
+        !S.RequireCompleteType(DeclLoc, T2, 0) && 
+        FindConversionForRefInit(S, ICS, DeclType, DeclLoc,
+                                 Init, T2, /*AllowRvalues=*/true,
+                                 AllowExplicit))
+      return ICS;
   }
-
+  
   //       -- Otherwise, a temporary of type "cv1 T1" is created and
   //          initialized from the initializer expression using the
   //          rules for a non-reference copy initialization (8.5). The

test/CXX/dcl.dcl/dcl.spec/dcl.stc/p9.cpp

@@ -1,12 +1,11 @@
 // RUN: %clang_cc1 -verify %s
 
-struct S; // expected-note {{forward declaration of 'S'}}
+struct S; // expected-note 2{{forward declaration of 'S'}}
 extern S a;
 extern S f(); // expected-note {{'f' declared here}}
-extern void g(S a); // expected-note {{candidate function}}
+extern void g(S a);
 
 void h() {
-  // FIXME: This diagnostic could be better.
-  g(a); // expected-error {{no matching function for call to 'g'}}
+  g(a); // expected-error {{argument type 'S' is incomplete}}
   f(); // expected-error {{calling 'f' with incomplete return type 'S'}}
 }

test/SemaCXX/conversion-function.cpp

@@ -214,3 +214,37 @@ struct Other {
 void test_any() {
   Any any = Other(); // expected-error{{cannot pass object of non-POD type 'Other' through variadic constructor; call will abort at runtime}}
 }
+
+namespace PR7055 {
+  // Make sure that we don't allow too many conversions in an
+  // auto_ptr-like template. In particular, we can't create multiple
+  // temporary objects when binding to a reference.
+  struct auto_ptr {
+    struct auto_ptr_ref { };
+
+    auto_ptr(auto_ptr&);
+    auto_ptr(auto_ptr_ref);
+    explicit auto_ptr(int *);
+
+    operator auto_ptr_ref();
+  };
+
+  struct X {
+    X(auto_ptr);
+  };
+
+  X f() {
+    X x(auto_ptr(new int));
+    return X(auto_ptr(new int));
+  }
+
+  auto_ptr foo();
+
+  X e(foo());
+
+  struct Y {
+    Y(X);
+  };
+  
+  Y f2(foo());
+}

test/SemaCXX/overload-call-copycon.cpp

@@ -1,40 +1,44 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s -Wnon-pod-varargs
-class X { };
+class X { }; // expected-note {{the implicit copy constructor}} \
+             // expected-note{{the implicit default constructor}}
 
-int& copycon(X x);
+int& copycon(X x); // expected-note{{passing argument to parameter}}
 float& copycon(...);
 
 void test_copycon(X x, X const xc, X volatile xv) {
   int& i1 = copycon(x);
   int& i2 = copycon(xc);
-  float& f1 = copycon(xv);
+  copycon(xv); // expected-error{{no matching constructor}}
 }
 
 class A {
 public:
-  A(A&);
+  A(A&); // expected-note{{would lose const qualifier}} \
+         // expected-note{{no known conversion}}
 };
 
-class B : public A { };
+class B : public A { }; // expected-note{{would lose const qualifier}} \
+// expected-note{{would lose volatile qualifier}} \
+// expected-note 2{{requires 0 arguments}}
 
-short& copycon2(A a);
-int& copycon2(B b);
+short& copycon2(A a); // expected-note{{passing argument to parameter}}
+int& copycon2(B b); // expected-note 2{{passing argument to parameter}}
 float& copycon2(...);
 
 void test_copycon2(A a, const A ac, B b, B const bc, B volatile bv) {
   int& i1 = copycon2(b);
-  float& f1 = copycon2(bc); // expected-warning {{cannot pass object of non-POD type}}
-  float& f2 = copycon2(bv); // expected-warning {{cannot pass object of non-POD type}}
+  copycon2(bc); // expected-error{{no matching constructor}}
+  copycon2(bv); // expected-error{{no matching constructor}}
   short& s1 = copycon2(a);
-  float& f3 = copycon2(ac); // expected-warning {{cannot pass object of non-POD type}}
+  copycon2(ac); // expected-error{{no matching constructor}}
 }
 
-int& copycon3(A a);
+int& copycon3(A a); // expected-note{{passing argument to parameter 'a' here}}
 float& copycon3(...);
 
 void test_copycon3(B b, const B bc) {
   int& i1 = copycon3(b);
-  float& f1 = copycon3(bc); // expected-warning {{cannot pass object of non-POD type}}
+  copycon3(bc); // expected-error{{no matching constructor}}
 }
 
 class C : public B { };