Now that -Wobjc-literal-compare is a warning, put the fixit on a note.

Recovering as if the user had actually called -isEqual: is a bit too far from
the semantics of the program as written, /even though/ it's probably what they
intended.

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

1 files changed, 66 insertions, 100 deletions

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 9c5ecc405b..0548cccd1f 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -6688,11 +6688,60 @@ static bool isObjCObjectLiteral(ExprResult &E) {
   }
 }
 
-static DiagnosticBuilder diagnoseObjCLiteralComparison(Sema &S,
-                                                       SourceLocation Loc,
-                                                       ExprResult &LHS,
-                                                       ExprResult &RHS,
-                                                       bool CanFix = false) {
+static bool hasIsEqualMethod(Sema &S, const Expr *LHS, const Expr *RHS) {
+  // Get the LHS object's interface type.
+  QualType Type = LHS->getType();
+  QualType InterfaceType;
+  if (const ObjCObjectPointerType *PTy = Type->getAs<ObjCObjectPointerType>()) {
+    InterfaceType = PTy->getPointeeType();
+    if (const ObjCObjectType *iQFaceTy =
+        InterfaceType->getAsObjCQualifiedInterfaceType())
+      InterfaceType = iQFaceTy->getBaseType();
+  } else {
+    // If this is not actually an Objective-C object, bail out.
+    return false;
+  }
+
+  // If the RHS isn't an Objective-C object, bail out.
+  if (!RHS->getType()->isObjCObjectPointerType())
+    return false;
+
+  // Try to find the -isEqual: method.
+  Selector IsEqualSel = S.NSAPIObj->getIsEqualSelector();
+  ObjCMethodDecl *Method = S.LookupMethodInObjectType(IsEqualSel,
+                                                      InterfaceType,
+                                                      /*instance=*/true);
+  if (!Method) {
+    if (Type->isObjCIdType()) {
+      // For 'id', just check the global pool.
+      Method = S.LookupInstanceMethodInGlobalPool(IsEqualSel, SourceRange(),
+                                                  /*receiverId=*/true,
+                                                  /*warn=*/false);
+    } else {
+      // Check protocols.
+      Method = S.LookupMethodInQualifiedType(IsEqualSel,
+                                             cast<ObjCObjectPointerType>(Type),
+                                             /*instance=*/true);
+    }
+  }
+
+  if (!Method)
+    return false;
+
+  QualType T = Method->param_begin()[0]->getType();
+  if (!T->isObjCObjectPointerType())
+    return false;
+  
+  QualType R = Method->getResultType();
+  if (!R->isScalarType())
+    return false;
+
+  return true;
+}
+
+static void diagnoseObjCLiteralComparison(Sema &S, SourceLocation Loc,
+                                          ExprResult &LHS, ExprResult &RHS,
+                                          BinaryOperator::Opcode Opc){
   Expr *Literal = (isObjCObjectLiteral(LHS) ? LHS : RHS).get();
 
   unsigned LiteralKind;
@@ -6740,94 +6789,20 @@ static DiagnosticBuilder diagnoseObjCLiteralComparison(Sema &S,
     llvm_unreachable("Unknown Objective-C object literal kind");
   }
 
-  return S.Diag(Loc, diag::warn_objc_literal_comparison)
-           << LiteralKind << CanFix << Literal->getSourceRange();
-}
-
-static ExprResult fixObjCLiteralComparison(Sema &S, SourceLocation OpLoc,
-                                           ExprResult &LHS,
-                                           ExprResult &RHS,
-                                           BinaryOperatorKind Op) {
-  // Check early to see if the warning's on.
-  // If it's off, we should /not/ be auto-applying the accompanying fixit.
-  DiagnosticsEngine::Level Level =
-    S.getDiagnostics().getDiagnosticLevel(diag::warn_objc_literal_comparison,
-                                          OpLoc);
-  if (Level == DiagnosticsEngine::Ignored)
-    return ExprEmpty();
-
-  assert((Op == BO_EQ || Op == BO_NE) && "Cannot fix other operations.");
+  S.Diag(Loc, diag::warn_objc_literal_comparison)
+    << LiteralKind << Literal->getSourceRange();
 
-  // Get the LHS object's interface type.
-  QualType Type = LHS.get()->getType();
-  QualType InterfaceType;
-  if (const ObjCObjectPointerType *PTy = Type->getAs<ObjCObjectPointerType>()) {
-    InterfaceType = PTy->getPointeeType();
-    if (const ObjCObjectType *iQFaceTy =
-        InterfaceType->getAsObjCQualifiedInterfaceType())
-      InterfaceType = iQFaceTy->getBaseType();
-  } else {
-    // If this is not actually an Objective-C object, bail out.
-    return ExprEmpty();
-  }
-
-  // If the RHS isn't an Objective-C object, bail out.
-  if (!RHS.get()->getType()->isObjCObjectPointerType())
-    return ExprEmpty();
-
-  // Try to find the -isEqual: method.
-  Selector IsEqualSel = S.NSAPIObj->getIsEqualSelector();
-  ObjCMethodDecl *Method = S.LookupMethodInObjectType(IsEqualSel,
-                                                      InterfaceType,
-                                                      /*instance=*/true);
-  bool ReceiverIsId = (Type->isObjCIdType() || Type->isObjCQualifiedIdType());
+  if (BinaryOperator::isEqualityOp(Opc) &&
+      hasIsEqualMethod(S, LHS.get(), RHS.get())) {
+    SourceLocation Start = LHS.get()->getLocStart();
+    SourceLocation End = S.PP.getLocForEndOfToken(RHS.get()->getLocEnd());
+    SourceRange OpRange(Loc, S.PP.getLocForEndOfToken(Loc));
 
-  if (!Method && ReceiverIsId) {
-    Method = S.LookupInstanceMethodInGlobalPool(IsEqualSel, SourceRange(),
-                                                /*receiverId=*/true,
-                                                /*warn=*/false);
+    S.Diag(Loc, diag::note_objc_literal_comparison_isequal)
+      << FixItHint::CreateInsertion(Start, Opc == BO_EQ ? "[" : "![")
+      << FixItHint::CreateReplacement(OpRange, "isEqual:")
+      << FixItHint::CreateInsertion(End, "]");
   }
-
-  if (!Method)
-    return ExprEmpty();
-
-  QualType T = Method->param_begin()[0]->getType();
-  if (!T->isObjCObjectPointerType())
-    return ExprEmpty();
-
-  QualType R = Method->getResultType();
-  if (!R->isScalarType())
-    return ExprEmpty();
-
-  // At this point we know we have a good -isEqual: method.
-  // Emit the diagnostic and fixit.
-  DiagnosticBuilder Diag = diagnoseObjCLiteralComparison(S, OpLoc,
-                                                         LHS, RHS, true);
-
-  Expr *LHSExpr = LHS.take();
-  Expr *RHSExpr = RHS.take();
-
-  SourceLocation Start = LHSExpr->getLocStart();
-  SourceLocation End = S.PP.getLocForEndOfToken(RHSExpr->getLocEnd());
-  SourceRange OpRange(OpLoc, S.PP.getLocForEndOfToken(OpLoc));
-
-  Diag << FixItHint::CreateInsertion(Start, Op == BO_EQ ? "[" : "![")
-       << FixItHint::CreateReplacement(OpRange, "isEqual:")
-       << FixItHint::CreateInsertion(End, "]");
-
-  // Finally, build the call to -isEqual: (and possible logical not).
-  ExprResult Call = S.BuildInstanceMessage(LHSExpr, LHSExpr->getType(),
-                                           /*SuperLoc=*/SourceLocation(),
-                                           IsEqualSel, Method,
-                                           OpLoc, OpLoc, OpLoc,
-                                           MultiExprArg(S, &RHSExpr, 1),
-                                           /*isImplicit=*/false);
-
-  ExprResult CallCond = S.CheckBooleanCondition(Call.get(), OpLoc);
-
-  if (Op == BO_NE)
-    return S.CreateBuiltinUnaryOp(OpLoc, UO_LNot, CallCond.get());
-  return CallCond;
 }
 
 // C99 6.5.8, C++ [expr.rel]
@@ -7155,7 +7130,7 @@ QualType Sema::CheckCompareOperands(ExprResult &LHS, ExprResult &RHS,
         diagnoseDistinctPointerComparison(*this, Loc, LHS, RHS,
                                           /*isError*/false);
       if (isObjCObjectLiteral(LHS) || isObjCObjectLiteral(RHS))
-        diagnoseObjCLiteralComparison(*this, Loc, LHS, RHS);
+        diagnoseObjCLiteralComparison(*this, Loc, LHS, RHS, Opc);
 
       if (LHSIsNull && !RHSIsNull)
         LHS = ImpCastExprToType(LHS.take(), RHSType, CK_BitCast);
@@ -8237,15 +8212,6 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
     break;
   case BO_EQ:
   case BO_NE:
-    if (getLangOpts().ObjC1) {
-      if (isObjCObjectLiteral(LHS) || isObjCObjectLiteral(RHS)) {
-        ExprResult IsEqualCall = fixObjCLiteralComparison(*this, OpLoc,
-                                                          LHS, RHS, Opc);
-        if (IsEqualCall.isUsable())
-          return IsEqualCall;
-        // Otherwise, fall back to the normal warning in CheckCompareOperands.
-      }
-    }
     ResultTy = CheckCompareOperands(LHS, RHS, OpLoc, Opc, false);
     break;
   case BO_And: