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

13 files changed, 168 insertions, 74 deletions

diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h
index d71fda7430..df3cd182e0 100644
--- a/include/clang/AST/Expr.h
+++ b/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; }
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index f08cd00c62..c02e7d2780 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/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++">;
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 4897a2613e..527c376bd2 100644
--- a/include/clang/Sema/Sema.h
+++ b/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.
diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp
index 47cf026e28..3d11619ec4 100644
--- a/lib/AST/Expr.cpp
+++ b/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() {
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 74f84862e6..a438480a4b 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/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 =
diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
index 02cb78b9de..db47931348 100644
--- a/lib/Sema/SemaExprCXX.cpp
+++ b/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());
 
diff --git a/lib/Sema/SemaExprObjC.cpp b/lib/Sema/SemaExprObjC.cpp
index ce73e222b6..bf9d081bb9 100644
--- a/lib/Sema/SemaExprObjC.cpp
+++ b/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
diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp
index d730bb27a1..e9d2eb6cc5 100644
--- a/lib/Sema/SemaOverload.cpp
+++ b/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();
diff --git a/test/CXX/drs/dr15xx.cpp b/test/CXX/drs/dr15xx.cpp
index 2060047571..cca4509fa0 100644
--- a/test/CXX/drs/dr15xx.cpp
+++ b/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
diff --git a/test/CXX/drs/dr16xx.cpp b/test/CXX/drs/dr16xx.cpp
index 898672ab80..e0af95ac39 100644
--- a/test/CXX/drs/dr16xx.cpp
+++ b/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
+}
diff --git a/test/CXX/over/over.built/p15.cpp b/test/CXX/over/over.built/p15.cpp
index 64ed3e7b83..9b223bcbc2 100644
--- a/test/CXX/over/over.built/p15.cpp
+++ b/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);
diff --git a/test/Misc/diag-macro-backtrace2.c b/test/Misc/diag-macro-backtrace2.c
index af79bbde60..64fc5f6307 100644
--- a/test/Misc/diag-macro-backtrace2.c
+++ b/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'
diff --git a/test/SemaOpenCL/logical-ops.cl b/test/SemaOpenCL/logical-ops.cl
index 42501b1441..f6972c46e2 100644
--- a/test/SemaOpenCL/logical-ops.cl
+++ b/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;