[Sema] Fix conditional expression bug

[terrelln]

Summary

When determining the type of a conditional expression clang didn't correctly perform the copy initialization required by the standard section 5.16p3b3.1. This resulted in the incorrect constructor being called in the added test case.
See https://llvm.org/bugs/show_bug.cgi?id=24227.

Secondly, in the same test case the old code copy initializes twice. The second copy initialization calls the M(const M&&) constructor, which is deleted so the code didn't compile. I removed the call to the second copy initialization in this case, although I'm not certain if this is the correct behavior.

lib/Sema/SemaExprCXX.cpp

static bool FindConditionalOverload(Sema &Self, ExprResult &LHS, ExprResult &RHS,
   }
   return true;
 }
 
 /// \brief Perform an "extended" implicit conversion as returned by
 /// TryClassUnification.
 static bool ConvertForConditional(Sema &Self, ExprResult &E, QualType T) {
   InitializedEntity Entity = InitializedEntity::InitializeTemporary(T);
-  InitializationKind Kind = InitializationKind::CreateCopy(E.get()->getLocStart(),
-                                                           SourceLocation());
-  Expr *Arg = E.get();
-  InitializationSequence InitSeq(Self, Entity, Kind, Arg);
-  ExprResult Result = InitSeq.Perform(Self, Entity, Kind, Arg);
+  ExprResult Result = Self.PerformCopyInitialization(Entity,
+                                                     SourceLocation(),
+                                                     E);
   if (Result.isInvalid())
     return true;
 
   E = Result;
   return false;
 }
 
 /// \brief Check the operands of ?: under C++ semantics.
QualType Sema::CXXCheckConditionalOperands(ExprResult &Cond, ExprResult &LHS,
   }
 
   // Neither is void.
 
   // C++11 [expr.cond]p3
   //   Otherwise, if the second and third operand have different types, and
   //   either has (cv) class type [...] an attempt is made to convert each of
   //   those operands to the type of the other.
+  bool LHSConverted = false;
+  bool RHSConverted = false;
   if (!Context.hasSameType(LTy, RTy) &&
       (LTy->isRecordType() || RTy->isRecordType())) {
     // These return true if a single direction is already ambiguous.
     QualType L2RType, R2LType;
     bool HaveL2R, HaveR2L;
     if (TryClassUnification(*this, LHS.get(), RHS.get(), QuestionLoc, HaveL2R, L2RType))
       return QualType();
     if (TryClassUnification(*this, RHS.get(), LHS.get(), QuestionLoc, HaveR2L, R2LType))
       return QualType();
 
     //   If both can be converted, [...] the program is ill-formed.
     if (HaveL2R && HaveR2L) {
       Diag(QuestionLoc, diag::err_conditional_ambiguous)
         << LTy << RTy << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
       return QualType();
     }
 
     //   If exactly one conversion is possible, that conversion is applied to
     //   the chosen operand and the converted operands are used in place of the
     //   original operands for the remainder of this section.
     if (HaveL2R) {
       if (ConvertForConditional(*this, LHS, L2RType) || LHS.isInvalid())
         return QualType();
       LTy = LHS.get()->getType();
+      LHSConverted = true;
     } else if (HaveR2L) {
       if (ConvertForConditional(*this, RHS, R2LType) || RHS.isInvalid())
         return QualType();
       RTy = RHS.get()->getType();
+      RHSConverted = true;
     }
   }
 
   // C++11 [expr.cond]p3
   //   if both are glvalues of the same value category and the same type except
   //   for cv-qualification, an attempt is made to convert each of those
   //   operands to the type of the other.
   ExprValueKind LVK = LHS.get()->getValueKind();
QualType Sema::CXXCheckConditionalOperands(ExprResult &Cond, ExprResult &LHS,
   if (Context.getCanonicalType(LTy) == Context.getCanonicalType(RTy)) {
     if (LTy->isRecordType()) {
       // The operands have class type. Make a temporary copy.
       if (RequireNonAbstractType(QuestionLoc, LTy,
                                  diag::err_allocation_of_abstract_type))
         return QualType();
       InitializedEntity Entity = InitializedEntity::InitializeTemporary(LTy);
 
-      ExprResult LHSCopy = PerformCopyInitialization(Entity,
+      ExprResult LHSCopy = LHSConverted ? LHS : PerformCopyInitialization(Entity,
                                                      SourceLocation(),
                                                      LHS);
       if (LHSCopy.isInvalid())
         return QualType();
 
-      ExprResult RHSCopy = PerformCopyInitialization(Entity,
+      ExprResult RHSCopy = RHSConverted ? RHS : PerformCopyInitialization(Entity,
                                                      SourceLocation(),
                                                      RHS);
       if (RHSCopy.isInvalid())
         return QualType();
 
       LHS = LHSCopy;
       RHS = RHSCopy;
     }

test/SemaCXX/conditional-expr.cpp

 };
 
 struct Derived1: Abstract {
 };
 
 struct Derived2: Abstract {
 };
 
+struct M {
+    M();
+    M(const M&);
+    M(M&&);
+    M(const M&&) = delete;
+};
+
 void test()
 {
   // This function tests C++0x 5.16
 
   // p1 (contextually convert to bool)
   int i1 = ToBool() ? 0 : 1;
 
   // p2 (one or both void, and throwing)
void test()
   // should use non-const overload
   i1 = (i1 ? Base() : Base()).trick();
   i1 = (i1 ? Base() : Base()).trick();
   i1 = (i1 ? Base() : Derived()).trick();
   i1 = (i1 ? Derived() : Base()).trick();
   // should fail: const lost
   (void)(i1 ? Base() : constder()); // expected-error {{incompatible operand types ('Base' and 'const Derived')}}
   (void)(i1 ? constder() : Base()); // expected-error {{incompatible operand types ('const Derived' and 'Base')}}
+  // should succeed: const applied after copy initialization
+  const M constM;
+  (void)(i1 ? constM : M());
 
   Priv priv;
   Fin fin;
   (void)(i1 ? Base() : Priv()); // expected-error{{private base class}}
   (void)(i1 ? Priv() : Base()); // expected-error{{private base class}}
   (void)(i1 ? Base() : Fin()); // expected-error{{ambiguous conversion from derived class 'Fin' to base class 'Base':}}
   (void)(i1 ? Fin() : Base()); // expected-error{{ambiguous conversion from derived class 'Fin' to base class 'Base':}}
   (void)(i1 ? base : priv); // expected-error {{private base class}}