[cfe-commits] r68454 - in /cfe/trunk: lib/Sema/Sema.h lib/Sema/SemaExpr.cpp test/FixIt/fixit.c

[Douglas Gregor]

Author: dgregor
Date: Mon Apr  6 13:45:53 2009
New Revision: 68454

URL: http://llvm.org/viewvc/llvm-project?rev=68454&view=rev
Log:
Fixed the Fix-It hints for comparison against a string literal. Thanks, Chris!

Modified:
    cfe/trunk/lib/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/test/FixIt/fixit.c

Modified: cfe/trunk/lib/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=68454&r1=68453&r2=68454&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/Sema.h (original)
+++ cfe/trunk/lib/Sema/Sema.h Mon Apr  6 13:45:53 2009
@@ -2341,7 +2341,7 @@
   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]

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=68454&r1=68453&r2=68454&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Apr  6 13:45:53 2009
@@ -3374,7 +3374,9 @@
 
 // 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 @@
     
     // 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 @@
   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:

Modified: cfe/trunk/test/FixIt/fixit.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/fixit.c?rev=68454&r1=68453&r2=68454&view=diff

==============================================================================
--- cfe/trunk/test/FixIt/fixit.c (original)
+++ cfe/trunk/test/FixIt/fixit.c Mon Apr  6 13:45:53 2009
@@ -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 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";
 }