Fixed the Fix-It hints for comparison against a string literal. Thanks, Chris!

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

diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h
index f9270cf64f4f8d0ec57b6e84cb5540b42cf94793..c71d8d726f6eb6100c15575dd018ea93248b6692 100644 (file)
--- a/lib/Sema/Sema.h
+++ b/lib/Sema/Sema.h
@@ -2341,7 +2341,7 @@ public:
   QualType CheckShiftOperands( // C99 6.5.7
     Expr *&lex, Expr *&rex, SourceLocation OpLoc, bool isCompAssign = false);
   QualType CheckCompareOperands( // C99 6.5.8/9
-    Expr *&lex, Expr *&rex, SourceLocation OpLoc, bool isRelational);
+    Expr *&lex, Expr *&rex, SourceLocation OpLoc, unsigned Opc, bool isRelational);
   QualType CheckBitwiseOperands( // C99 6.5.[10...12]
     Expr *&lex, Expr *&rex, SourceLocation OpLoc, bool isCompAssign = false);
   QualType CheckLogicalOperands( // C99 6.5.[13,14]

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 602f7fcfd572be1f5d2f15bc3f0a7b98308d2756..e836a58432fbe1bb869457f455b0d1a17f72b397 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -3374,7 +3374,9 @@ QualType Sema::CheckShiftOperands(Expr *&lex, Expr *&rex, SourceLocation Loc,
 
 // C99 6.5.8
 QualType Sema::CheckCompareOperands(Expr *&lex, Expr *&rex, SourceLocation Loc,
-                                    bool isRelational) {
+                                    unsigned OpaqueOpc, bool isRelational) {
+  BinaryOperator::Opcode Opc = (BinaryOperator::Opcode)OpaqueOpc;
+
   if (lex->getType()->isVectorType() || rex->getType()->isVectorType())
     return CheckVectorCompareOperands(lex, rex, Loc, isRelational);
 
@@ -3409,29 +3411,41 @@ QualType Sema::CheckCompareOperands(Expr *&lex, Expr *&rex, SourceLocation Loc,
     
     // Warn about comparisons against a string constant (unless the other
     // operand is null), the user probably wants strcmp.
+    Expr *literalString = 0;
+    Expr *literalStringStripped = 0;
     if ((isa<StringLiteral>(LHSStripped) || isa<ObjCEncodeExpr>(LHSStripped)) &&
-        !RHSStripped->isNullPointerConstant(Context))
-      Diag(Loc, diag::warn_stringcompare)
-        << isa<ObjCEncodeExpr>(LHSStripped)
-        << lex->getSourceRange()
-        << CodeModificationHint::CreateReplacement(SourceRange(Loc), ", ")
-        << CodeModificationHint::CreateInsertion(lex->getLocStart(),
-                                                 "strcmp(")
-        << CodeModificationHint::CreateInsertion(
-                                       PP.getLocForEndOfToken(rex->getLocEnd()),
-                                       ") == 0");
+        !RHSStripped->isNullPointerConstant(Context)) {
+      literalString = lex;
+      literalStringStripped = LHSStripped;
+    }
     else if ((isa<StringLiteral>(RHSStripped) ||
               isa<ObjCEncodeExpr>(RHSStripped)) &&
-             !LHSStripped->isNullPointerConstant(Context))
-      Diag(Loc, diag::warn_stringcompare) 
-        << isa<ObjCEncodeExpr>(RHSStripped)
-        << rex->getSourceRange()
+             !LHSStripped->isNullPointerConstant(Context)) {
+      literalString = rex;
+      literalStringStripped = RHSStripped;
+    }
+
+    if (literalString) {
+      std::string resultComparison;
+      switch (Opc) {
+      case BinaryOperator::LT: resultComparison = ") < 0"; break;
+      case BinaryOperator::GT: resultComparison = ") > 0"; break;
+      case BinaryOperator::LE: resultComparison = ") <= 0"; break;
+      case BinaryOperator::GE: resultComparison = ") >= 0"; break;
+      case BinaryOperator::EQ: resultComparison = ") == 0"; break;
+      case BinaryOperator::NE: resultComparison = ") != 0"; break;
+      default: assert(false && "Invalid comparison operator");
+      }
+      Diag(Loc, diag::warn_stringcompare)
+        << isa<ObjCEncodeExpr>(literalStringStripped)
+        << literalString->getSourceRange()
         << CodeModificationHint::CreateReplacement(SourceRange(Loc), ", ")
         << CodeModificationHint::CreateInsertion(lex->getLocStart(),
                                                  "strcmp(")
         << CodeModificationHint::CreateInsertion(
                                        PP.getLocForEndOfToken(rex->getLocEnd()),
-                                       ") == 0");
+                                       resultComparison);
+    }
   }
 
   // The result of comparisons is 'bool' in C++, 'int' in C.
@@ -4146,11 +4160,11 @@ Action::OwningExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
   case BinaryOperator::LT:
   case BinaryOperator::GE:
   case BinaryOperator::GT:
-    ResultTy = CheckCompareOperands(lhs, rhs, OpLoc, true);
+    ResultTy = CheckCompareOperands(lhs, rhs, OpLoc, Opc, true);
     break;
   case BinaryOperator::EQ:
   case BinaryOperator::NE:
-    ResultTy = CheckCompareOperands(lhs, rhs, OpLoc, false);
+    ResultTy = CheckCompareOperands(lhs, rhs, OpLoc, Opc, false);
     break;
   case BinaryOperator::And:
   case BinaryOperator::Xor:

diff --git a/test/FixIt/fixit.c b/test/FixIt/fixit.c
index d1631091b4420e467ed6095055083e58e41fdee7..1378df4de152348a87f9423617b8b8b372f32d72 100644 (file)
--- a/test/FixIt/fixit.c
+++ b/test/FixIt/fixit.c
@@ -4,6 +4,7 @@
    provided as part of warning or extension diagnostics. All of the
    warnings will be fixed by -fixit, and the resulting file should
    compile cleanly with -Werror -pedantic. */
+#include <string.h> // FIXME: FIX-IT hint should add this for us!
 
 void f0(void) { };
 
@@ -24,6 +25,5 @@ int i0 = { 17 };
 
 int f2(const char *my_string) {
   // FIXME: terminal output isn't so good when "my_string" is shorter
-  // FIXME: Needs an #include hint, too!
-  //  return my_string == "foo";
+  return my_string == "foo";
 }