Unify implementation of our two different flavours of -Wtautological-compare,

and fold together into a single function.

In so doing, fix a handful of remaining bugs where we would report false
positives or false negatives if we promote a signed value to an unsigned type
for the comparison.

This re-commits r320122 and r320124, minus two changes:

 * Comparisons between a constant and a non-constant expression of enumeration
   type never warn, not even if the constant is out of range. We should be
   warning about the creation of such a constant, not about its use.

 * We do not use more precise bit-widths for comparisons against bit-fields.
   The more precise diagnostics probably are the right thing, but we should
   consider moving them under their own warning flag.

Other than the refactoring, this patch should only change the behavior for the
buggy cases (where the warnings didn't take into account that promotion from
signed to unsigned can leave a range of inaccessible values in the middle of
the promoted type).

llvm-svn: 320211

3 files changed, 174 insertions, 100 deletions

diff --git a/clang/test/Sema/tautological-constant-compare.c b/clang/test/Sema/tautological-constant-compare.c
index b9ade2a2db4..c48aa994445 100644
--- a/clang/test/Sema/tautological-constant-compare.c
+++ b/clang/test/Sema/tautological-constant-compare.c
@@ -94,15 +94,17 @@ int main()
   if (-32768 >= s)
       return 0;
 
+  // Note: both sides are promoted to unsigned long prior to the comparison, so
+  // it is perfectly possible for a short to compare greater than 32767UL.
   if (s == 32767UL)
       return 0;
   if (s != 32767UL)
       return 0;
   if (s < 32767UL)
       return 0;
-  if (s <= 32767UL) // expected-warning {{comparison 'short' <= 32767 is always true}}
+  if (s <= 32767UL)
       return 0;
-  if (s > 32767UL) // expected-warning {{comparison 'short' > 32767 is always false}}
+  if (s > 32767UL)
       return 0;
   if (s >= 32767UL)
       return 0;
@@ -111,13 +113,66 @@ int main()
       return 0;
   if (32767UL != s)
       return 0;
-  if (32767UL < s) // expected-warning {{comparison 32767 < 'short' is always false}}
+  if (32767UL < s)
       return 0;
   if (32767UL <= s)
       return 0;
   if (32767UL > s)
       return 0;
-  if (32767UL >= s) // expected-warning {{comparison 32767 >= 'short' is always true}}
+  if (32767UL >= s)
+      return 0;
+
+  if (s == 0UL)
+      return 0;
+  if (s != 0UL)
+      return 0;
+  if (s < 0UL) // expected-warning {{comparison of unsigned expression < 0 is always false}}
+      return 0;
+  if (s <= 0UL)
+      return 0;
+  if (s > 0UL)
+      return 0;
+  if (s >= 0UL) // expected-warning {{comparison of unsigned expression >= 0 is always true}}
+      return 0;
+
+  if (0UL == s)
+      return 0;
+  if (0UL != s)
+      return 0;
+  if (0UL < s)
+      return 0;
+  if (0UL <= s) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
+      return 0;
+  if (0UL > s) // expected-warning {{comparison of 0 > unsigned expression is always false}}
+      return 0;
+  if (0UL >= s)
+      return 0;
+
+  enum { ULONG_MAX = (2UL * (unsigned long)__LONG_MAX__ + 1UL) };
+  if (s == 2UL * (unsigned long)__LONG_MAX__ + 1UL)
+      return 0;
+  if (s != 2UL * (unsigned long)__LONG_MAX__ + 1UL)
+      return 0;
+  if (s < 2UL * (unsigned long)__LONG_MAX__ + 1UL)
+      return 0;
+  if (s <= 2UL * (unsigned long)__LONG_MAX__ + 1UL) // expected-warning-re {{comparison 'short' <= {{.*}} is always true}}
+      return 0;
+  if (s > 2UL * (unsigned long)__LONG_MAX__ + 1UL) // expected-warning-re {{comparison 'short' > {{.*}} is always false}}
+      return 0;
+  if (s >= 2UL * (unsigned long)__LONG_MAX__ + 1UL)
+      return 0;
+
+  if (2UL * (unsigned long)__LONG_MAX__ + 1UL == s)
+      return 0;
+  if (2UL * (unsigned long)__LONG_MAX__ + 1UL != s)
+      return 0;
+  if (2UL * (unsigned long)__LONG_MAX__ + 1UL < s) // expected-warning-re {{comparison {{.*}} < 'short' is always false}}
+      return 0;
+  if (2UL * (unsigned long)__LONG_MAX__ + 1UL <= s)
+      return 0;
+  if (2UL * (unsigned long)__LONG_MAX__ + 1UL > s)
+      return 0;
+  if (2UL * (unsigned long)__LONG_MAX__ + 1UL >= s) // expected-warning-re {{comparison {{.*}} >= 'short' is always true}}
       return 0;
 
   // FIXME: assumes two's complement
@@ -281,8 +336,6 @@ int main()
   if (0 >= s)
     return 0;
 
-  // However the comparison with 0U would warn
-
   unsigned short us = value();
 
 #ifdef TEST
@@ -510,5 +563,23 @@ int main()
   if (maybe >= e)
       return 0;
 
+  // For the time being, use the declared type of bit-fields rather than their
+  // length when determining whether a value is in-range.
+  // FIXME: Reconsider this.
+  struct A {
+    int a : 3;
+    unsigned b : 3;
+    long c : 3;
+    unsigned long d : 3;
+  } a;
+  if (a.a < 3) {}
+  if (a.a < 4) {}
+  if (a.b < 7) {}
+  if (a.b < 8) {}
+  if (a.c < 3) {}
+  if (a.c < 4) {}
+  if (a.d < 7) {}
+  if (a.d < 8) {}
+
   return 1;
 }
diff --git a/clang/test/Sema/tautological-constant-enum-compare.c b/clang/test/Sema/tautological-constant-enum-compare.c
index 1f09865f13e..539439b817d 100644
--- a/clang/test/Sema/tautological-constant-enum-compare.c
+++ b/clang/test/Sema/tautological-constant-enum-compare.c
@@ -9,78 +9,96 @@ int main() {
 
 #ifdef SILENCE
   // expected-no-diagnostics
-#endif
+#else
+  // If we promote to unsigned, it doesn't matter whether the enum's underlying
+  // type was signed.
+  if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
+    return 0;
+  if (0U >= a)
+    return 0;
+  if (a > 0U)
+    return 0;
+  if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+    return 0;
+  if (a <= 0U)
+    return 0;
+  if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+    return 0;
+  if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+    return 0;
+  if (0U < a)
+    return 0;
 
-#ifdef UNSIGNED
-#ifndef SILENCE
-  if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
+  if (a < 4294967295U)
     return 0;
-  if (0 >= a)
+  if (4294967295U >= a) // expected-warning {{comparison 4294967295 >= 'enum A' is always true}}
     return 0;
-  if (a > 0)
+  if (a > 4294967295U) // expected-warning {{comparison 'enum A' > 4294967295 is always false}}
     return 0;
-  if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+  if (4294967295U <= a)
     return 0;
-  if (a <= 0)
+  if (a <= 4294967295U) // expected-warning {{comparison 'enum A' <= 4294967295 is always true}}
     return 0;
-  if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+  if (4294967295U > a)
     return 0;
-  if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+  if (a >= 4294967295U)
     return 0;
-  if (0 < a)
+  if (4294967295U < a) // expected-warning {{comparison 4294967295 < 'enum A' is always false}}
     return 0;
 
-  if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
+  if (a < 2147483647U)
     return 0;
-  if (0U >= a)
+  if (2147483647U >= a)
     return 0;
-  if (a > 0U)
+  if (a > 2147483647U)
     return 0;
-  if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+  if (2147483647U <= a)
     return 0;
-  if (a <= 0U)
+  if (a <= 2147483647U)
     return 0;
-  if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+  if (2147483647U > a)
     return 0;
-  if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+  if (a >= 2147483647U)
     return 0;
-  if (0U < a)
+  if (2147483647U < a)
     return 0;
+#endif
 
-  if (a < 4294967295)
+#if defined(UNSIGNED) && !defined(SILENCE)
+  if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
     return 0;
-  if (4294967295 >= a) // expected-warning {{comparison 4294967295 >= 'enum A' is always true}}
+  if (0 >= a)
     return 0;
-  if (a > 4294967295) // expected-warning {{comparison 'enum A' > 4294967295 is always false}}
+  if (a > 0)
     return 0;
-  if (4294967295 <= a)
+  if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
     return 0;
-  if (a <= 4294967295) // expected-warning {{comparison 'enum A' <= 4294967295 is always true}}
+  if (a <= 0)
     return 0;
-  if (4294967295 > a)
+  if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
     return 0;
-  if (a >= 4294967295)
+  if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
     return 0;
-  if (4294967295 < a) // expected-warning {{comparison 4294967295 < 'enum A' is always false}}
+  if (0 < a)
     return 0;
 
-  if (a < 4294967295U)
+  if (a < 4294967295)
     return 0;
-  if (4294967295U >= a) // expected-warning {{comparison 4294967295 >= 'enum A' is always true}}
+  if (4294967295 >= a) // expected-warning {{comparison 4294967295 >= 'enum A' is always true}}
     return 0;
-  if (a > 4294967295U) // expected-warning {{comparison 'enum A' > 4294967295 is always false}}
+  if (a > 4294967295) // expected-warning {{comparison 'enum A' > 4294967295 is always false}}
     return 0;
-  if (4294967295U <= a)
+  if (4294967295 <= a)
     return 0;
-  if (a <= 4294967295U) // expected-warning {{comparison 'enum A' <= 4294967295 is always true}}
+  if (a <= 4294967295) // expected-warning {{comparison 'enum A' <= 4294967295 is always true}}
     return 0;
-  if (4294967295U > a)
+  if (4294967295 > a)
     return 0;
-  if (a >= 4294967295U)
+  if (a >= 4294967295)
     return 0;
-  if (4294967295U < a) // expected-warning {{comparison 4294967295 < 'enum A' is always false}}
+  if (4294967295 < a) // expected-warning {{comparison 4294967295 < 'enum A' is always false}}
     return 0;
-#else // SILENCE
+#else // SIGNED || SILENCE
   if (a < 0)
     return 0;
   if (0 >= a)
@@ -98,23 +116,24 @@ int main() {
   if (0 < a)
     return 0;
 
-  if (a < 0U)
+#ifndef SILENCE
+  if (a < 4294967295) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always true}}
     return 0;
-  if (0U >= a)
+  if (4294967295 >= a) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always true}}
     return 0;
-  if (a > 0U)
+  if (a > 4294967295) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always false}}
     return 0;
-  if (0U <= a)
+  if (4294967295 <= a) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always false}}
     return 0;
-  if (a <= 0U)
+  if (a <= 4294967295) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always true}}
     return 0;
-  if (0U > a)
+  if (4294967295 > a) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always true}}
     return 0;
-  if (a >= 0U)
+  if (a >= 4294967295) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always false}}
     return 0;
-  if (0U < a)
+  if (4294967295 < a) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always false}}
     return 0;
-
+#else
   if (a < 4294967295)
     return 0;
   if (4294967295 >= a)
@@ -131,26 +150,10 @@ int main() {
     return 0;
   if (4294967295 < a)
     return 0;
-
-  if (a < 4294967295U)
-    return 0;
-  if (4294967295U >= a)
-    return 0;
-  if (a > 4294967295U)
-    return 0;
-  if (4294967295U <= a)
-    return 0;
-  if (a <= 4294967295U)
-    return 0;
-  if (4294967295U > a)
-    return 0;
-  if (a >= 4294967295U)
-    return 0;
-  if (4294967295U < a)
-    return 0;
 #endif
-#elif defined(SIGNED)
-#ifndef SILENCE
+#endif
+
+#if defined(SIGNED) && !defined(SILENCE)
   if (a < -2147483648) // expected-warning {{comparison 'enum A' < -2147483648 is always false}}
     return 0;
   if (-2147483648 >= a)
@@ -184,24 +187,25 @@ int main() {
     return 0;
   if (2147483647 < a) // expected-warning {{comparison 2147483647 < 'enum A' is always false}}
     return 0;
-
-  if (a < 2147483647U)
+#elif defined(UNSIGNED) && !defined(SILENCE)
+#ifndef SILENCE
+  if (a < -2147483648) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always false}}
     return 0;
-  if (2147483647U >= a) // expected-warning {{comparison 2147483647 >= 'enum A' is always true}}
+  if (-2147483648 >= a) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always false}}
     return 0;
-  if (a > 2147483647U) // expected-warning {{comparison 'enum A' > 2147483647 is always false}}
+  if (a > -2147483648) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always true}}
     return 0;
-  if (2147483647U <= a)
+  if (-2147483648 <= a) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always true}}
     return 0;
-  if (a <= 2147483647U) // expected-warning {{comparison 'enum A' <= 2147483647 is always true}}
+  if (a <= -2147483648) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always false}}
     return 0;
-  if (2147483647U > a)
+  if (-2147483648 > a) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always false}}
     return 0;
-  if (a >= 2147483647U)
+  if (a >= -2147483648) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always true}}
     return 0;
-  if (2147483647U < a) // expected-warning {{comparison 2147483647 < 'enum A' is always false}}
+  if (-2147483648 < a) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always true}}
     return 0;
-#else // SILENCE
+#else
   if (a < -2147483648)
     return 0;
   if (-2147483648 >= a)
@@ -218,6 +222,7 @@ int main() {
     return 0;
   if (-2147483648 < a)
     return 0;
+#endif
 
   if (a < 2147483647)
     return 0;
@@ -235,24 +240,6 @@ int main() {
     return 0;
   if (2147483647 < a)
     return 0;
-
-  if (a < 2147483647U)
-    return 0;
-  if (2147483647U >= a)
-    return 0;
-  if (a > 2147483647U)
-    return 0;
-  if (2147483647U <= a)
-    return 0;
-  if (a <= 2147483647U)
-    return 0;
-  if (2147483647U > a)
-    return 0;
-  if (a >= 2147483647U)
-    return 0;
-  if (2147483647U < a)
-    return 0;
-#endif
 #endif
 
   return 1;
diff --git a/clang/test/SemaCXX/compare.cpp b/clang/test/SemaCXX/compare.cpp
index 1d940be629a..d852180c066 100644
--- a/clang/test/SemaCXX/compare.cpp
+++ b/clang/test/SemaCXX/compare.cpp
@@ -245,8 +245,8 @@ void test4(short s) {
   // unsigned.
   const unsigned B = -1;
   void (s < B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
-  void (s > B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
-  void (s <= B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
+  void (s > B); // expected-warning{{comparison 'short' > 4294967295 is always false}}
+  void (s <= B); // expected-warning{{comparison 'short' <= 4294967295 is always true}}
   void (s >= B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
   void (s == B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
   void (s != B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
@@ -423,3 +423,19 @@ namespace templates {
     testx<B>();
   }
 }
+
+namespace tautological_enum {
+  enum E { a, b, c } e;
+
+  // FIXME: We should warn about constructing this out-of-range numeration value.
+  const E invalid = (E)-1;
+  // ... but we should not warn about comparing against it.
+  bool x = e == invalid;
+
+  // We should not warn about relational comparisons for enumerators, even if
+  // they're tautological.
+  bool y = e >= a && e <= b;
+  const E first_in_range = a;
+  const E last_in_range = b;
+  bool z = e >= first_in_range && e <= last_in_range;
+}