Improve our handling of the second step in a user-defined conversion

sequence. Previously, we weren't permitting the second step to call
copy constructors, which left user-defined conversion sequences
surprisingly broken.

Now, we perform overload resolution among all of the constructors, but
only accept the result if it makes the conversion a standard
conversion. Note that this behavior is different from both GCC and EDG
(which don't agree with each other, either); I've submitted a core
issue on the matter.

llvm-svn: 63450

clang/lib/Sema/Sema.h

@@ -433,7 +433,8 @@ public:
   bool IsQualificationConversion(QualType FromType, QualType ToType);
   bool IsUserDefinedConversion(Expr *From, QualType ToType, 
                                UserDefinedConversionSequence& User,
-                               bool AllowExplicit = false);
+                               bool AllowConversionFunctions,
+                               bool AllowExplicit);
 
   ImplicitConversionSequence::CompareKind 
   CompareImplicitConversionSequences(const ImplicitConversionSequence& ICS1,

clang/lib/Sema/SemaDeclCXX.cpp

@@ -1867,7 +1867,6 @@ Sema::CheckReferenceInit(Expr *&Init, QualType &DeclType,
   //             the temporary or to a sub-object within the
   //             temporary.
   //
-  //
   //          The constructor that would be used to make the copy
   //          shall be callable whether or not the copy is actually
   //          done.
@@ -1915,6 +1914,20 @@ Sema::CheckReferenceInit(Expr *&Init, QualType &DeclType,
     return true;
   }
 
+  // If at least one of the types is a class type, the types are not
+  // related, and we aren't allowed any user conversions, the
+  // reference binding fails. This case is important for breaking
+  // recursion, since TryImplicitConversion below will attempt to
+  // create a temporary through the use of a copy constructor.
+  if (SuppressUserConversions && RefRelationship == Ref_Incompatible &&
+      (T1->isRecordType() || T2->isRecordType())) {
+    if (!ICS)
+      Diag(Init->getSourceRange().getBegin(),
+           diag::err_typecheck_convert_incompatible)
+        << DeclType << Init->getType() << "initializing" << Init->getSourceRange();
+    return true;
+  }
+
   // Actually try to convert the initializer to T1.
   if (ICS) {
     /// C++ [over.ics.ref]p2:

clang/lib/Sema/SemaOverload.cpp

@@ -366,13 +366,13 @@ Sema::IsOverload(FunctionDecl *New, Decl* OldD,
 ImplicitConversionSequence
 Sema::TryImplicitConversion(Expr* From, QualType ToType,
                             bool SuppressUserConversions,
-                            bool AllowExplict)
+                            bool AllowExplicit)
 {
   ImplicitConversionSequence ICS;
   if (IsStandardConversion(From, ToType, ICS.Standard))
     ICS.ConversionKind = ImplicitConversionSequence::StandardConversion;
-  else if (!SuppressUserConversions &&
-           IsUserDefinedConversion(From, ToType, ICS.UserDefined, AllowExplict)) {
+  else if (IsUserDefinedConversion(From, ToType, ICS.UserDefined, 
+                                   !SuppressUserConversions, AllowExplicit)) {
     ICS.ConversionKind = ImplicitConversionSequence::UserDefinedConversion;
     // C++ [over.ics.user]p4:
     //   A conversion of an expression of class type to the same class
@@ -396,6 +396,17 @@ Sema::TryImplicitConversion(Expr* From, QualType ToType,
           ICS.Standard.Second = ICK_Derived_To_Base;
       }
     }
+
+    // C++ [over.best.ics]p4:
+    //   However, when considering the argument of a user-defined
+    //   conversion function that is a candidate by 13.3.1.3 when
+    //   invoked for the copying of the temporary in the second step
+    //   of a class copy-initialization, or by 13.3.1.4, 13.3.1.5, or
+    //   13.3.1.6 in all cases, only standard conversion sequences and
+    //   ellipsis conversion sequences are allowed.
+    if (SuppressUserConversions &&
+        ICS.ConversionKind == ImplicitConversionSequence::UserDefinedConversion)
+      ICS.ConversionKind = ImplicitConversionSequence::BadConversion;
   } else
     ICS.ConversionKind = ImplicitConversionSequence::BadConversion;
 
@@ -1188,17 +1199,23 @@ Sema::IsQualificationConversion(QualType FromType, QualType ToType)
     FromType.getUnqualifiedType() == ToType.getUnqualifiedType();
 }
 
-/// IsUserDefinedConversion - Determines whether there is a
-/// user-defined conversion sequence (C++ [over.ics.user]) that
-/// converts expression From to the type ToType. If such a conversion
-/// exists, User will contain the user-defined conversion sequence
-/// that performs such a conversion and this routine will return
-/// true. Otherwise, this routine returns false and User is
-/// unspecified. AllowExplicit is true if the conversion should
-/// consider C++0x "explicit" conversion functions as well as
-/// non-explicit conversion functions (C++0x [class.conv.fct]p2).
+/// Determines whether there is a user-defined conversion sequence
+/// (C++ [over.ics.user]) that converts expression From to the type
+/// ToType. If such a conversion exists, User will contain the
+/// user-defined conversion sequence that performs such a conversion
+/// and this routine will return true. Otherwise, this routine returns
+/// false and User is unspecified.
+///
+/// \param AllowConversionFunctions true if the conversion should
+/// consider conversion functions at all. If false, only constructors
+/// will be considered.
+///
+/// \param AllowExplicit  true if the conversion should consider C++0x
+/// "explicit" conversion functions as well as non-explicit conversion
+/// functions (C++0x [class.conv.fct]p2).
 bool Sema::IsUserDefinedConversion(Expr *From, QualType ToType, 
                                    UserDefinedConversionSequence& User,
+                                   bool AllowConversionFunctions,
                                    bool AllowExplicit)
 {
   OverloadCandidateSet CandidateSet;
@@ -1226,8 +1243,11 @@ bool Sema::IsUserDefinedConversion(Expr *From, QualType ToType,
     }
   }
 
-  if (const CXXRecordType *FromRecordType
-        = dyn_cast_or_null<CXXRecordType>(From->getType()->getAsRecordType())) {
+  if (!AllowConversionFunctions) {
+    // Don't allow any conversion functions to enter the overload set.
+  } else if (const CXXRecordType *FromRecordType
+               = dyn_cast_or_null<CXXRecordType>(
+                                        From->getType()->getAsRecordType())) {
     // Add all of the conversion functions as candidates.
     // FIXME: Look for conversions in base classes!
     CXXRecordDecl *FromRecordDecl = FromRecordType->getDecl();

clang/test/SemaCXX/dcl_init_aggr.cpp

@@ -96,7 +96,7 @@ C2 c2;
 D2 d2;
 B2 b2 = { 4, a2, a2 };
 B2 b2_2 = { 4, d2, 0 };
-// FIXME: B2 b2_3 = { c2, a2, a2 };
+B2 b2_3 = { c2, a2, a2 };
 
 // C++ [dcl.init.aggr]p15:
 union u { int a; char* b; };

clang/test/SemaCXX/user-defined-conversions.cpp

@@ -35,3 +35,35 @@ void h(volatile A&);
 void h_test(C c) {
   h(c);
 }
+
+// Test conversion followed by copy-construction
+struct FunkyDerived;
+
+struct Base { 
+  Base(const FunkyDerived&);
+};
+
+struct Derived : Base { };
+
+struct FunkyDerived : Base { };
+
+struct ConvertibleToBase {
+  operator Base();
+};
+
+struct ConvertibleToDerived {
+  operator Derived();
+};
+
+struct ConvertibleToFunkyDerived {
+  operator FunkyDerived();
+};
+
+void test_conversion(ConvertibleToBase ctb, ConvertibleToDerived ctd,
+                     ConvertibleToFunkyDerived ctfd) {
+  Base b1 = ctb;
+  Base b2(ctb);
+  Base b3 = ctd;
+  Base b4(ctd);
+  Base b5 = ctfd; // expected-error{{cannot initialize 'b5' with an lvalue of type 'struct ConvertibleToFunkyDerived'}}
+}