DR1687: When overload resolution selects a built-in operator, implicit

conversions are only applied to operands of class type, and the second
standard conversion sequence is not applied.

When diagnosing an invalid builtin binary operator, talk about the
original types rather than the converted types. If these differ by a
user-defined conversion, tell the user what happened.

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

include/clang/AST/Expr.h

@@ -2850,6 +2850,10 @@ public:
     return const_cast<CastExpr *>(this)->getSubExprAsWritten();
   }
 
+  /// If this cast applies a user-defined conversion, retrieve the conversion
+  /// function that it invokes.
+  NamedDecl *getConversionFunction() const;
+
   typedef CXXBaseSpecifier **path_iterator;
   typedef const CXXBaseSpecifier * const *path_const_iterator;
   bool path_empty() const { return CastExprBits.BasePathSize == 0; }

include/clang/Basic/DiagnosticSemaKinds.td

@@ -5802,6 +5802,8 @@ def err_objc_object_assignment : Error<
   "cannot assign to class object (%0 invalid)">;
 def err_typecheck_invalid_operands : Error<
   "invalid operands to binary expression (%0 and %1)">;
+def note_typecheck_invalid_operands_converted : Note<
+  "%select{first|second}0 operand was implicitly converted to type %1">;
 def err_typecheck_logical_vector_expr_gnu_cpp_restrict : Error<
   "logical expression with vector %select{type %1 and non-vector type %2|types"
   " %1 and %2}0 is only supported in C++">;

include/clang/Sema/Sema.h

@@ -9316,9 +9316,16 @@ public:
     /// A functional-style cast.
     CCK_FunctionalCast,
     /// A cast other than a C-style cast.
-    CCK_OtherCast
+    CCK_OtherCast,
+    /// A conversion for an operand of a builtin overloaded operator.
+    CCK_ForBuiltinOverloadedOp
   };
 
+  static bool isCast(CheckedConversionKind CCK) {
+    return CCK == CCK_CStyleCast || CCK == CCK_FunctionalCast ||
+           CCK == CCK_OtherCast;
+  }
+
   /// ImpCastExprToType - If Expr is not of type 'Type', insert an implicit
   /// cast.  If there is already an implicit cast, merge into the existing one.
   /// If isLvalue, the result of the cast is an lvalue.

lib/AST/Expr.cpp

@@ -1673,23 +1673,22 @@ const char *CastExpr::getCastKindName(CastKind CK) {
 }
 
 namespace {
-  Expr *skipImplicitTemporary(Expr *expr) {
+  const Expr *skipImplicitTemporary(const Expr *E) {
     // Skip through reference binding to temporary.
-    if (MaterializeTemporaryExpr *Materialize
-                                  = dyn_cast<MaterializeTemporaryExpr>(expr))
-      expr = Materialize->GetTemporaryExpr();
+    if (auto *Materialize = dyn_cast<MaterializeTemporaryExpr>(E))
+      E = Materialize->GetTemporaryExpr();
 
     // Skip any temporary bindings; they're implicit.
-    if (CXXBindTemporaryExpr *Binder = dyn_cast<CXXBindTemporaryExpr>(expr))
-      expr = Binder->getSubExpr();
+    if (auto *Binder = dyn_cast<CXXBindTemporaryExpr>(E))
+      E = Binder->getSubExpr();
 
-    return expr;
+    return E;
   }
 }
 
 Expr *CastExpr::getSubExprAsWritten() {
-  Expr *SubExpr = nullptr;
-  CastExpr *E = this;
+  const Expr *SubExpr = nullptr;
+  const CastExpr *E = this;
   do {
     SubExpr = skipImplicitTemporary(E->getSubExpr());
 
@@ -1702,15 +1701,33 @@ Expr *CastExpr::getSubExprAsWritten() {
       assert((isa<CXXMemberCallExpr>(SubExpr) ||
               isa<BlockExpr>(SubExpr)) &&
              "Unexpected SubExpr for CK_UserDefinedConversion.");
-      if (isa<CXXMemberCallExpr>(SubExpr))
-        SubExpr = cast<CXXMemberCallExpr>(SubExpr)->getImplicitObjectArgument();
+      if (auto *MCE = dyn_cast<CXXMemberCallExpr>(SubExpr))
+        SubExpr = MCE->getImplicitObjectArgument();
     }
     
     // If the subexpression we're left with is an implicit cast, look
     // through that, too.
   } while ((E = dyn_cast<ImplicitCastExpr>(SubExpr)));  
   
-  return SubExpr;
+  return const_cast<Expr*>(SubExpr);
+}
+
+NamedDecl *CastExpr::getConversionFunction() const {
+  const Expr *SubExpr = nullptr;
+
+  for (const CastExpr *E = this; E; E = dyn_cast<ImplicitCastExpr>(SubExpr)) {
+    SubExpr = skipImplicitTemporary(E->getSubExpr());
+
+    if (E->getCastKind() == CK_ConstructorConversion)
+      return cast<CXXConstructExpr>(SubExpr)->getConstructor();
+
+    if (E->getCastKind() == CK_UserDefinedConversion) {
+      if (auto *MCE = dyn_cast<CXXMemberCallExpr>(SubExpr))
+        return MCE->getMethodDecl();
+    }
+  }
+
+  return nullptr;
 }
 
 CXXBaseSpecifier **CastExpr::path_buffer() {

lib/Sema/SemaExpr.cpp

@@ -8130,18 +8130,57 @@ Sema::CheckSingleAssignmentConstraints(QualType LHSType, ExprResult &CallerRHS,
       RHS = E;
       return Compatible;
     }
-    
+
     if (ConvertRHS)
       RHS = ImpCastExprToType(E, Ty, Kind);
   }
   return result;
 }
 
+namespace {
+/// The original operand to an operator, prior to the application of the usual
+/// arithmetic conversions and converting the arguments of a builtin operator
+/// candidate.
+struct OriginalOperand {
+  explicit OriginalOperand(Expr *Op) : Orig(Op), Conversion(nullptr) {
+    if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(Op))
+      Op = MTE->GetTemporaryExpr();
+    if (auto *BTE = dyn_cast<CXXBindTemporaryExpr>(Op))
+      Op = BTE->getSubExpr();
+    if (auto *ICE = dyn_cast<ImplicitCastExpr>(Op)) {
+      Orig = ICE->getSubExprAsWritten();
+      Conversion = ICE->getConversionFunction();
+    }
+  }
+
+  QualType getType() const { return Orig->getType(); }
+
+  Expr *Orig;
+  NamedDecl *Conversion;
+};
+}
+
 QualType Sema::InvalidOperands(SourceLocation Loc, ExprResult &LHS,
                                ExprResult &RHS) {
+  OriginalOperand OrigLHS(LHS.get()), OrigRHS(RHS.get());
+
   Diag(Loc, diag::err_typecheck_invalid_operands)
-    << LHS.get()->getType() << RHS.get()->getType()
+    << OrigLHS.getType() << OrigRHS.getType()
     << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
+
+  // If a user-defined conversion was applied to either of the operands prior
+  // to applying the built-in operator rules, tell the user about it.
+  if (OrigLHS.Conversion) {
+    Diag(OrigLHS.Conversion->getLocation(),
+         diag::note_typecheck_invalid_operands_converted)
+      << 0 << LHS.get()->getType();
+  }
+  if (OrigRHS.Conversion) {
+    Diag(OrigRHS.Conversion->getLocation(),
+         diag::note_typecheck_invalid_operands_converted)
+      << 1 << RHS.get()->getType();
+  }
+
   return QualType();
 }
 
@@ -9887,7 +9926,7 @@ static QualType checkArithmeticOrEnumeralThreeWayCompare(Sema &S,
     // type E, the operator yields the result of converting the operands
     // to the underlying type of E and applying <=> to the converted operands.
     if (!S.Context.hasSameUnqualifiedType(LHSStrippedType, RHSStrippedType)) {
-      S.InvalidOperands(Loc, LHSStripped, RHSStripped);
+      S.InvalidOperands(Loc, LHS, RHS);
       return QualType();
     }
     QualType IntType =

lib/Sema/SemaExprCXX.cpp

@@ -3767,6 +3767,10 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
                                 const ImplicitConversionSequence &ICS,
                                 AssignmentAction Action,
                                 CheckedConversionKind CCK) {
+  // C++ [over.match.oper]p7: [...] operands of class type are converted [...]
+  if (CCK == CCK_ForBuiltinOverloadedOp && !From->getType()->isRecordType())
+    return From;
+
   switch (ICS.getKind()) {
   case ImplicitConversionSequence::StandardConversion: {
     ExprResult Res = PerformImplicitConversion(From, ToType, ICS.Standard,
@@ -3826,6 +3830,12 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
 
       From = CastArg.get();
 
+      // C++ [over.match.oper]p7:
+      //   [...] the second standard conversion sequence of a user-defined
+      //   conversion sequence is not applied.
+      if (CCK == CCK_ForBuiltinOverloadedOp)
+        return From;
+
       return PerformImplicitConversion(From, ToType, ICS.UserDefined.After,
                                        AA_Converting, CCK);
   }
@@ -4289,7 +4299,7 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
 
   // If this conversion sequence succeeded and involved implicitly converting a
   // _Nullable type to a _Nonnull one, complain.
-  if (CCK == CCK_ImplicitConversion)
+  if (!isCast(CCK))
     diagnoseNullableToNonnullConversion(ToType, InitialFromType,
                                         From->getLocStart());
 

lib/Sema/SemaExprObjC.cpp

@@ -3498,6 +3498,7 @@ static void addFixitForObjCARCConversion(Sema &S,
   // We handle C-style and implicit casts here.
   switch (CCK) {
   case Sema::CCK_ImplicitConversion:
+  case Sema::CCK_ForBuiltinOverloadedOp:
   case Sema::CCK_CStyleCast:
   case Sema::CCK_OtherCast:
     break;
@@ -3651,11 +3652,13 @@ diagnoseObjCARCConversion(Sema &S, SourceRange castRange,
   SourceLocation afterLParen = S.getLocForEndOfToken(castRange.getBegin());
   SourceLocation noteLoc = afterLParen.isValid() ? afterLParen : loc;
 
+  unsigned convKindForDiag = Sema::isCast(CCK) ? 0 : 1;
+
   // Bridge from an ARC type to a CF type.
   if (castACTC == ACTC_retainable && isAnyRetainable(exprACTC)) {
 
     S.Diag(loc, diag::err_arc_cast_requires_bridge)
-      << unsigned(CCK == Sema::CCK_ImplicitConversion) // cast|implicit
+      << convKindForDiag
       << 2 // of C pointer type
       << castExprType
       << unsigned(castType->isBlockPointerType()) // to ObjC|block type
@@ -3697,7 +3700,7 @@ diagnoseObjCARCConversion(Sema &S, SourceRange castRange,
   if (exprACTC == ACTC_retainable && isAnyRetainable(castACTC)) {
     bool br = S.isKnownName("CFBridgingRetain");
     S.Diag(loc, diag::err_arc_cast_requires_bridge)
-      << unsigned(CCK == Sema::CCK_ImplicitConversion) // cast|implicit
+      << convKindForDiag
       << unsigned(castExprType->isBlockPointerType()) // of ObjC|block type
       << castExprType
       << 2 // to C pointer type
@@ -3734,7 +3737,7 @@ diagnoseObjCARCConversion(Sema &S, SourceRange castRange,
   }
   
   S.Diag(loc, diag::err_arc_mismatched_cast)
-    << (CCK != Sema::CCK_ImplicitConversion)
+    << !convKindForDiag
     << srcKind << castExprType << castType
     << castRange << castExpr->getSourceRange();
 }
@@ -4187,7 +4190,7 @@ Sema::CheckObjCConversion(SourceRange castRange, QualType castType,
   if (exprACTC == ACTC_indirectRetainable && castACTC == ACTC_voidPtr)
     return ACR_okay;
   if (castACTC == ACTC_indirectRetainable && exprACTC == ACTC_voidPtr &&
-      CCK != CCK_ImplicitConversion)
+      isCast(CCK))
     return ACR_okay;
 
   switch (ARCCastChecker(Context, exprACTC, castACTC, false).Visit(castExpr)) {
@@ -4212,8 +4215,7 @@ Sema::CheckObjCConversion(SourceRange castRange, QualType castType,
   // If this is a non-implicit cast from id or block type to a
   // CoreFoundation type, delay complaining in case the cast is used
   // in an acceptable context.
-  if (exprACTC == ACTC_retainable && isAnyRetainable(castACTC) &&
-      CCK != CCK_ImplicitConversion)
+  if (exprACTC == ACTC_retainable && isAnyRetainable(castACTC) && isCast(CCK))
     return ACR_unbridged;
 
   // Issue a diagnostic about a missing @-sign when implicit casting a cstring

lib/Sema/SemaOverload.cpp

@@ -12191,7 +12191,8 @@ Sema::CreateOverloadedUnaryOp(SourceLocation OpLoc, UnaryOperatorKind Opc,
       // break out so that we will build the appropriate built-in
       // operator node.
       ExprResult InputRes = PerformImplicitConversion(
-          Input, Best->BuiltinParamTypes[0], Best->Conversions[0], AA_Passing);
+          Input, Best->BuiltinParamTypes[0], Best->Conversions[0], AA_Passing,
+          CCK_ForBuiltinOverloadedOp);
       if (InputRes.isInvalid())
         return ExprError();
       Input = InputRes.get();
@@ -12435,16 +12436,16 @@ Sema::CreateOverloadedBinOp(SourceLocation OpLoc,
         // We matched a built-in operator. Convert the arguments, then
         // break out so that we will build the appropriate built-in
         // operator node.
-        ExprResult ArgsRes0 =
-            PerformImplicitConversion(Args[0], Best->BuiltinParamTypes[0],
-                                      Best->Conversions[0], AA_Passing);
+        ExprResult ArgsRes0 = PerformImplicitConversion(
+            Args[0], Best->BuiltinParamTypes[0], Best->Conversions[0],
+            AA_Passing, CCK_ForBuiltinOverloadedOp);
         if (ArgsRes0.isInvalid())
           return ExprError();
         Args[0] = ArgsRes0.get();
 
-        ExprResult ArgsRes1 =
-            PerformImplicitConversion(Args[1], Best->BuiltinParamTypes[1],
-                                      Best->Conversions[1], AA_Passing);
+        ExprResult ArgsRes1 = PerformImplicitConversion(
+            Args[1], Best->BuiltinParamTypes[1], Best->Conversions[1],
+            AA_Passing, CCK_ForBuiltinOverloadedOp);
         if (ArgsRes1.isInvalid())
           return ExprError();
         Args[1] = ArgsRes1.get();
@@ -12647,16 +12648,16 @@ Sema::CreateOverloadedArraySubscriptExpr(SourceLocation LLoc,
         // We matched a built-in operator. Convert the arguments, then
         // break out so that we will build the appropriate built-in
         // operator node.
-        ExprResult ArgsRes0 =
-            PerformImplicitConversion(Args[0], Best->BuiltinParamTypes[0],
-                                      Best->Conversions[0], AA_Passing);
+        ExprResult ArgsRes0 = PerformImplicitConversion(
+            Args[0], Best->BuiltinParamTypes[0], Best->Conversions[0],
+            AA_Passing, CCK_ForBuiltinOverloadedOp);
         if (ArgsRes0.isInvalid())
           return ExprError();
         Args[0] = ArgsRes0.get();
 
-        ExprResult ArgsRes1 =
-            PerformImplicitConversion(Args[1], Best->BuiltinParamTypes[1],
-                                      Best->Conversions[1], AA_Passing);
+        ExprResult ArgsRes1 = PerformImplicitConversion(
+            Args[1], Best->BuiltinParamTypes[1], Best->Conversions[1],
+            AA_Passing, CCK_ForBuiltinOverloadedOp);
         if (ArgsRes1.isInvalid())
           return ExprError();
         Args[1] = ArgsRes1.get();

test/CXX/drs/dr15xx.cpp

@@ -112,9 +112,9 @@ namespace dr1512 { // dr1512: 4
 #endif
   }
 
-  template<typename T> struct Wrap { operator T(); };
-  void test_overload() {
 #if __cplusplus >= 201103L
+  template<typename T> struct Wrap { operator T(); }; // expected-note 4{{converted to type 'nullptr_t'}} expected-note 4{{converted to type 'int *'}}
+  void test_overload() {
     using nullptr_t = decltype(nullptr);
     void(Wrap<nullptr_t>() == Wrap<nullptr_t>());
     void(Wrap<nullptr_t>() != Wrap<nullptr_t>());
@@ -123,16 +123,16 @@ namespace dr1512 { // dr1512: 4
     void(Wrap<nullptr_t>() <= Wrap<nullptr_t>()); // expected-error {{invalid operands}}
     void(Wrap<nullptr_t>() >= Wrap<nullptr_t>()); // expected-error {{invalid operands}}
 
-    // The wording change fails to actually disallow this. This is valid
-    // via the builtin operator<(int*, int*) etc.
+    // Under dr1213, this is ill-formed: we select the builtin operator<(int*, int*)
+    // but then only convert as far as 'nullptr_t', which we then can't convert to 'int*'.
     void(Wrap<nullptr_t>() == Wrap<int*>());
     void(Wrap<nullptr_t>() != Wrap<int*>());
-    void(Wrap<nullptr_t>() < Wrap<int*>());
-    void(Wrap<nullptr_t>() > Wrap<int*>());
-    void(Wrap<nullptr_t>() <= Wrap<int*>());
-    void(Wrap<nullptr_t>() >= Wrap<int*>());
-#endif
+    void(Wrap<nullptr_t>() < Wrap<int*>()); // expected-error {{invalid operands to binary expression ('Wrap<nullptr_t>' and 'Wrap<int *>')}}
+    void(Wrap<nullptr_t>() > Wrap<int*>()); // expected-error {{invalid operands}}
+    void(Wrap<nullptr_t>() <= Wrap<int*>()); // expected-error {{invalid operands}}
+    void(Wrap<nullptr_t>() >= Wrap<int*>()); // expected-error {{invalid operands}}
   }
+#endif
 }
 
 namespace dr1518 { // dr1518: 4

test/CXX/drs/dr16xx.cpp

@@ -1,7 +1,8 @@
 // RUN: %clang_cc1 -std=c++98 -triple x86_64-unknown-unknown %s -verify -fexceptions -fcxx-exceptions -pedantic-errors
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown %s -verify -fexceptions -fcxx-exceptions -pedantic-errors
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown %s -verify -fexceptions -fcxx-exceptions -pedantic-errors
-// RUN: %clang_cc1 -std=c++1z -triple x86_64-unknown-unknown %s -verify -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-unknown %s -verify -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++2a -triple x86_64-unknown-unknown %s -verify -fexceptions -fcxx-exceptions -pedantic-errors
 
 #if __cplusplus < 201103L
 // expected-error@+1 {{variadic macro}}
@@ -249,3 +250,22 @@ namespace dr1672 { // dr1672: 7
   static_assert(!__is_standard_layout(Y<H>), "");
   static_assert(!__is_standard_layout(Y<X>), "");
 }
+
+namespace dr1687 { // dr1687: 7
+  template<typename T> struct To {
+    operator T(); // expected-note 2{{first operand was implicitly converted to type 'int *'}}
+    // expected-note@-1 {{second operand was implicitly converted to type 'double'}}
+#if __cplusplus > 201703L
+    // expected-note@-3 2{{operand was implicitly converted to type 'dr1687::E}}
+#endif
+  };
+
+  int *a = To<int*>() + 100.0; // expected-error {{invalid operands to binary expression ('To<int *>' and 'double')}}
+  int *b = To<int*>() + To<double>(); // expected-error {{invalid operands to binary expression ('To<int *>' and 'To<double>')}}
+
+#if __cplusplus > 201703L
+  enum E1 {};
+  enum E2 {};
+  auto c = To<E1>() <=> To<E2>(); // expected-error {{invalid operands to binary expression ('To<dr1687::E1>' and 'To<dr1687::E2>')}}
+#endif
+}

test/CXX/over/over.built/p15.cpp

@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -std=c++11 -verify %s -Wno-tautological-compare
 
-struct A { operator decltype(nullptr)(); };
-struct B { operator const int *(); };
+struct A { operator decltype(nullptr)(); }; // expected-note 16{{implicitly converted}}
+struct B { operator const int *(); }; // expected-note 8{{implicitly converted}}
 void f(A a, B b, volatile int *pi) {
   (void)(a == a);
   (void)(a != a);
@@ -12,39 +12,31 @@ void f(A a, B b, volatile int *pi) {
 
   (void)(a == b);
   (void)(a != b);
-  // FIXME: These cases were intended to be made ill-formed by N3624, but it
-  // fails to actually achieve this goal.
-  (void)(a < b);
-  (void)(a > b);
-  (void)(a <= b);
-  (void)(a >= b);
+  (void)(a < b); // expected-error {{invalid operands}}
+  (void)(a > b); // expected-error {{invalid operands}}
+  (void)(a <= b); // expected-error {{invalid operands}}
+  (void)(a >= b); // expected-error {{invalid operands}}
 
   (void)(b == a);
   (void)(b != a);
-  // FIXME: These cases were intended to be made ill-formed by N3624, but it
-  // fails to actually achieve this goal.
-  (void)(b < a);
-  (void)(b > a);
-  (void)(b <= a);
-  (void)(b >= a);
+  (void)(b < a); // expected-error {{invalid operands}}
+  (void)(b > a); // expected-error {{invalid operands}}
+  (void)(b <= a); // expected-error {{invalid operands}}
+  (void)(b >= a); // expected-error {{invalid operands}}
 
   (void)(a == pi);
   (void)(a != pi);
-  // FIXME: These cases were intended to be made ill-formed by N3624, but it
-  // fails to actually achieve this goal.
-  (void)(a < pi);
-  (void)(a > pi);
-  (void)(a <= pi);
-  (void)(a >= pi);
+  (void)(a < pi); // expected-error {{invalid operands}}
+  (void)(a > pi); // expected-error {{invalid operands}}
+  (void)(a <= pi); // expected-error {{invalid operands}}
+  (void)(a >= pi); // expected-error {{invalid operands}}
 
   (void)(pi == a);
   (void)(pi != a);
-  // FIXME: These cases were intended to be made ill-formed by N3624, but it
-  // fails to actually achieve this goal.
-  (void)(pi < a);
-  (void)(pi > a);
-  (void)(pi <= a);
-  (void)(pi >= a);
+  (void)(pi < a); // expected-error {{invalid operands}}
+  (void)(pi > a); // expected-error {{invalid operands}}
+  (void)(pi <= a); // expected-error {{invalid operands}}
+  (void)(pi >= a); // expected-error {{invalid operands}}
 
   (void)(b == pi);
   (void)(b != pi);

test/Misc/diag-macro-backtrace2.c

@@ -14,7 +14,7 @@ void PR16799() {
   const char str[] = "string";
   a(str);
   // CHECK: :15:3: error: invalid operands to binary expression
-  // CHECK:       ('const char *' and 'int')
+  // CHECK:       ('const char [7]' and 'int')
   // CHECK:   a(str);
   // CHECK:   ^~~~~~
   // CHECK: :3:11: note: expanded from macro 'a'

test/SemaOpenCL/logical-ops.cl

@@ -29,7 +29,7 @@ kernel void float_ops() {
 #if __OPENCL_C_VERSION__ < 120
 // expected-error@-2{{invalid operands}}
 #endif
-  float ibaf = 0 & 0.0f; // expected-error {{invalid operands}}
+  float ibaf = 0 & 0.0f; // expected-error {{invalid operands to binary expression ('int' and 'float')}}
   float ibof = 0 | 0.0f; // expected-error {{invalid operands}}
   float bnf = ~0.0f;// expected-error {{invalid argument type}}
   float lnf = !0.0f;