Finesse 'idempotent operations' analyzer issues to include the opcode of the binary operator for clearer error reporting. Also remove the 'Idempotent operation' prefix in messages; it's redundant since the bug type is the same.

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

8 files changed, 60 insertions, 54 deletions

diff --git a/lib/Checker/IdempotentOperationChecker.cpp b/lib/Checker/IdempotentOperationChecker.cpp
index 95d488c6af..f8bf708e5f 100644
--- a/lib/Checker/IdempotentOperationChecker.cpp
+++ b/lib/Checker/IdempotentOperationChecker.cpp
@@ -310,7 +310,7 @@ void IdempotentOperationChecker::PreVisitBinaryOperator(
 }
 
 void IdempotentOperationChecker::VisitEndAnalysis(ExplodedGraph &G,
-                                                  BugReporter &B,
+                                                  BugReporter &BR,
                                                   bool hasWorkRemaining) {
   // If there is any work remaining we cannot be 100% sure about our warnings
   if (hasWorkRemaining)
@@ -322,22 +322,29 @@ void IdempotentOperationChecker::VisitEndAnalysis(ExplodedGraph &G,
       hash.begin(); i != hash.end(); ++i) {
     if (i->second != Impossible) {
       // Select the error message.
-      const char *msg = 0;
+      const BinaryOperator *B = i->first;
+      llvm::SmallString<128> buf;
+      llvm::raw_svector_ostream os(buf);
+
       switch (i->second) {
       case Equal:
-        msg = "idempotent operation; both operands are always equal in value";
+        if (B->getOpcode() == BinaryOperator::Assign)
+          os << "Assigned value is always the same as the existing value";
+        else
+          os << "Both operands to '" << B->getOpcodeStr()
+             << "' always have the same value";
         break;
       case LHSis1:
-        msg = "idempotent operation; the left operand is always 1";
+        os << "The left operand to '" << B->getOpcodeStr() << "' is always 1";
         break;
       case RHSis1:
-        msg = "idempotent operation; the right operand is always 1";
+        os << "The right operand to '" << B->getOpcodeStr() << "' is always 1";
         break;
       case LHSis0:
-        msg = "idempotent operation; the left operand is always 0";
+        os << "The left operand to '" << B->getOpcodeStr() << "' is always 0";
         break;
       case RHSis0:
-        msg = "idempotent operation; the right operand is always 0";
+        os << "The right operand to '" << B->getOpcodeStr() << "' is always 0";
         break;
       case Possible:
         llvm_unreachable("Operation was never marked with an assumption");
@@ -348,9 +355,8 @@ void IdempotentOperationChecker::VisitEndAnalysis(ExplodedGraph &G,
       // Create the SourceRange Arrays
       SourceRange S[2] = { i->first->getLHS()->getSourceRange(),
                            i->first->getRHS()->getSourceRange() };
-      B.EmitBasicReport("Idempotent operation", "Dead code",
-                        msg, i->first->getOperatorLoc(),
-          S, 2);
+      BR.EmitBasicReport("Idempotent operation", "Dead code",
+                         os.str(), i->first->getOperatorLoc(), S, 2);
     }
   }
 }
diff --git a/test/Analysis/constant-folding.c b/test/Analysis/constant-folding.c
index 5a7e6be1dc..b1ca3eeec9 100644
--- a/test/Analysis/constant-folding.c
+++ b/test/Analysis/constant-folding.c
@@ -18,10 +18,10 @@ void testComparisons (int a) {
 }
 
 void testSelfOperations (int a) {
-  if ((a|a) != a) WARN; // expected-warning{{idempotent operation}}
-  if ((a&a) != a) WARN; // expected-warning{{idempotent operation}}
-  if ((a^a) != 0) WARN; // expected-warning{{idempotent operation}}
-  if ((a-a) != 0) WARN; // expected-warning{{idempotent operation}}
+  if ((a|a) != a) WARN; // expected-warning{{Both operands to '|' always have the same value}}
+  if ((a&a) != a) WARN; // expected-warning{{Both operands to '&' always have the same value}}
+  if ((a^a) != 0) WARN; // expected-warning{{Both operands to '^' always have the same value}}
+  if ((a-a) != 0) WARN; // expected-warning{{Both operands to '-' always have the same value}}
 }
 
 void testIdempotent (int a) {
@@ -68,5 +68,5 @@ void testLocations (char *a) {
   if (b!=a) WARN;
   if (b>a) WARN;
   if (b<a) WARN;
-  if (b-a) WARN; // expected-warning{{idempotent operation}}
+  if (b-a) WARN; // expected-warning{{Both operands to '-' always have the same value}}
 }
diff --git a/test/Analysis/dead-stores.c b/test/Analysis/dead-stores.c
index a295d96464..cc8a3f5593 100644
--- a/test/Analysis/dead-stores.c
+++ b/test/Analysis/dead-stores.c
@@ -158,7 +158,7 @@ int f16(int x) {
 // Self-assignments should not be flagged as dead stores.
 void f17() {
   int x = 1;
-  x = x; // expected-warning{{idempotent operation}}
+  x = x; // expected-warning{{Assigned value is always the same as the existing value}}
 }
 
 // <rdar://problem/6506065>
@@ -458,7 +458,7 @@ void rdar8014335() {
     // Note that the next value stored to 'i' is never executed
     // because the next statement to be executed is the 'break'
     // in the increment code of the first loop.
-    i = i * 3; // expected-warning{{Value stored to 'i' is never read}} expected-warning{{idempotent operation}}
+    i = i * 3; // expected-warning{{Value stored to 'i' is never read}} expected-warning{{The left operand to '*' is always 1}}
   }
 }
 
diff --git a/test/Analysis/idempotent-operations.c b/test/Analysis/idempotent-operations.c
index 94e972c551..74d31e7239 100644
--- a/test/Analysis/idempotent-operations.c
+++ b/test/Analysis/idempotent-operations.c
@@ -9,47 +9,47 @@ void basic() {
   int x = 10, zero = 0, one = 1;
 
   // x op x
-  x = x;        // expected-warning {{idempotent operation; both operands are always equal in value}}
-  test(x - x);  // expected-warning {{idempotent operation; both operands are always equal in value}}
-  x -= x;       // expected-warning {{idempotent operation; both operands are always equal in value}}
+  x = x;        // expected-warning {{Assigned value is always the same as the existing value}}
+  test(x - x);  // expected-warning {{Both operands to '-' always have the same value}}
+  x -= x;       // expected-warning {{Both operands to '-=' always have the same value}}
   x = 10;       // no-warning
-  test(x / x);  // expected-warning {{idempotent operation; both operands are always equal in value}}
-  x /= x;       // expected-warning {{idempotent operation; both operands are always equal in value}}
+  test(x / x);  // expected-warning {{Both operands to '/' always have the same value}}
+  x /= x;       // expected-warning {{Both operands to '/=' always have the same value}}
   x = 10;       // no-warning
-  test(x & x);  // expected-warning {{idempotent operation; both operands are always equal in value}}
-  x &= x;       // expected-warning {{idempotent operation; both operands are always equal in value}}
-  test(x | x);  // expected-warning {{idempotent operation; both operands are always equal in value}}
-  x |= x;       // expected-warning {{idempotent operation; both operands are always equal in value}}
+  test(x & x);  // expected-warning {{Both operands to '&' always have the same value}}
+  x &= x;       // expected-warning {{Both operands to '&=' always have the same value}}
+  test(x | x);  // expected-warning {{Both operands to '|' always have the same value}}
+  x |= x;       // expected-warning {{Both operands to '|=' always have the same value}}
 
   // x op 1
-  test(x * one);  // expected-warning {{idempotent operation; the right operand is always 1}}
-  x *= one;       // expected-warning {{idempotent operation; the right operand is always 1}}
-  test(x / one);  // expected-warning {{idempotent operation; the right operand is always 1}}
-  x /= one;       // expected-warning {{idempotent operation; the right operand is always 1}}
+  test(x * one);  // expected-warning {{The right operand to '*' is always 1}}
+  x *= one;       // expected-warning {{The right operand to '*=' is always 1}}
+  test(x / one);  // expected-warning {{The right operand to '/' is always 1}}
+  x /= one;       // expected-warning {{The right operand to '/=' is always 1}}
 
   // 1 op x
-  test(one * x);   // expected-warning {{idempotent operation; the left operand is always 1}}
+  test(one * x);   // expected-warning {{The left operand to '*' is always 1}}
 
   // x op 0
-  test(x + zero);  // expected-warning {{idempotent operation; the right operand is always 0}}
-  test(x - zero);  // expected-warning {{idempotent operation; the right operand is always 0}}
-  test(x * zero);  // expected-warning {{idempotent operation; the right operand is always 0}}
-  test(x & zero);  // expected-warning {{idempotent operation; the right operand is always 0}}
-  test(x | zero);  // expected-warning {{idempotent operation; the right operand is always 0}}
-  test(x ^ zero);  // expected-warning {{idempotent operation; the right operand is always 0}}
-  test(x << zero); // expected-warning {{idempotent operation; the right operand is always 0}}
-  test(x >> zero); // expected-warning {{idempotent operation; the right operand is always 0}}
+  test(x + zero);  // expected-warning {{The right operand to '+' is always 0}}
+  test(x - zero);  // expected-warning {{The right operand to '-' is always 0}}
+  test(x * zero);  // expected-warning {{The right operand to '*' is always 0}}
+  test(x & zero);  // expected-warning {{The right operand to '&' is always 0}}
+  test(x | zero);  // expected-warning {{The right operand to '|' is always 0}}
+  test(x ^ zero);  // expected-warning {{The right operand to '^' is always 0}}
+  test(x << zero); // expected-warning {{The right operand to '<<' is always 0}}
+  test(x >> zero); // expected-warning {{The right operand to '>>' is always 0}}
 
   // 0 op x
-  test(zero + x);  // expected-warning {{idempotent operation; the left operand is always 0}}
-  test(zero - x);  // expected-warning {{idempotent operation; the left operand is always 0}}
-  test(zero / x);  // expected-warning {{idempotent operation; the left operand is always 0}}
-  test(zero * x);  // expected-warning {{idempotent operation; the left operand is always 0}}
-  test(zero & x);  // expected-warning {{idempotent operation; the left operand is always 0}}
-  test(zero | x);  // expected-warning {{idempotent operation; the left operand is always 0}}
-  test(zero ^ x);  // expected-warning {{idempotent operation; the left operand is always 0}}
-  test(zero << x); // expected-warning {{idempotent operation; the left operand is always 0}}
-  test(zero >> x); // expected-warning {{idempotent operation; the left operand is always 0}}
+  test(zero + x);  // expected-warning {{The left operand to '+' is always 0}}
+  test(zero - x);  // expected-warning {{The left operand to '-' is always 0}}
+  test(zero / x);  // expected-warning {{The left operand to '/' is always 0}}
+  test(zero * x);  // expected-warning {{The left operand to '*' is always 0}}
+  test(zero & x);  // expected-warning {{The left operand to '&' is always 0}}
+  test(zero | x);  // expected-warning {{The left operand to '|' is always 0}}
+  test(zero ^ x);  // expected-warning {{The left operand to '^' is always 0}}
+  test(zero << x); // expected-warning {{The left operand to '<<' is always 0}}
+  test(zero >> x); // expected-warning {{The left operand to '>>' is always 0}}
 }
 
 void floats(float x) {
diff --git a/test/Analysis/misc-ps.m b/test/Analysis/misc-ps.m
index c2099b04a9..be7f450de6 100644
--- a/test/Analysis/misc-ps.m
+++ b/test/Analysis/misc-ps.m
@@ -86,11 +86,11 @@ unsigned r6268365Aux();
 
 void r6268365() {
   unsigned x = 0;
-  x &= r6268365Aux(); // expected-warning{{idempotent operation}}
+  x &= r6268365Aux(); // expected-warning{{The left operand to '&=' is always 0}}
   unsigned j = 0;
     
   if (x == 0) ++j;
-  if (x == 0) x = x / j; // expected-warning{{idempotent operation}} expected-warning{{idempotent operation}}
+  if (x == 0) x = x / j; // expected-warning{{Assigned value is always the same as the existing value}} expected-warning{{The right operand to '/' is always 1}}
 }
 
 void divzeroassume(unsigned x, unsigned j) {  
diff --git a/test/Analysis/null-deref-ps.c b/test/Analysis/null-deref-ps.c
index 0b4153c834..9be73a8c0f 100644
--- a/test/Analysis/null-deref-ps.c
+++ b/test/Analysis/null-deref-ps.c
@@ -280,7 +280,7 @@ void f12(HF12ITEM i, char *q) {
 // Test handling of translating between integer "pointers" and back.
 void f13() {
   int *x = 0;
-  if (((((int) x) << 2) + 1) >> 1) *x = 1; // expected-warning{{idempotent operation}}
+  if (((((int) x) << 2) + 1) >> 1) *x = 1; // expected-warning{{he left operand to '<<' is always 0}}
 }
 
 // PR 4759 - Attribute non-null checking by the analyzer was not correctly
diff --git a/test/Analysis/rdar-6541136-region.c b/test/Analysis/rdar-6541136-region.c
index c1734eec9b..80c04ae8ca 100644
--- a/test/Analysis/rdar-6541136-region.c
+++ b/test/Analysis/rdar-6541136-region.c
@@ -9,7 +9,7 @@ extern kernel_tea_cheese_t _wonky_gesticulate_cheese;
 void test1( void ) {
   kernel_tea_cheese_t *wonky = &_wonky_gesticulate_cheese;
   struct load_wine *cmd = (void*) &wonky[1];
-  cmd = cmd; // expected-warning{{idempotent operation}}
+  cmd = cmd; // expected-warning{{Assigned value is always the same as the existing value}}
   char *p = (void*) &wonky[1];
   kernel_tea_cheese_t *q = &wonky[1];
   // This test case tests both the RegionStore logic (doesn't crash) and
@@ -21,7 +21,7 @@ void test1( void ) {
 void test1_b( void ) {
   kernel_tea_cheese_t *wonky = &_wonky_gesticulate_cheese;
   struct load_wine *cmd = (void*) &wonky[1];
-  cmd = cmd; // expected-warning{{idempotent operation}}
+  cmd = cmd; // expected-warning{{Assigned value is always the same as the existing value}}
   char *p = (void*) &wonky[1];
   *p = 1;  // expected-warning{{Access out-of-bound array element (buffer overflow)}}
 }
diff --git a/test/Analysis/rdar-6541136.c b/test/Analysis/rdar-6541136.c
index c729cb518f..911fb4aa2f 100644
--- a/test/Analysis/rdar-6541136.c
+++ b/test/Analysis/rdar-6541136.c
@@ -12,7 +12,7 @@ void foo( void )
 {
   kernel_tea_cheese_t *wonky = &_wonky_gesticulate_cheese;
   struct load_wine *cmd = (void*) &wonky[1];
-  cmd = cmd; // expected-warning{{idempotent operation}}
+  cmd = cmd; // expected-warning{{Assigned value is always the same as the existing value}}
   char *p = (void*) &wonky[1];
   *p = 1;
   kernel_tea_cheese_t *q = &wonky[1];