lebedev.ri updated this revision to Diff 117802.
lebedev.ri marked 2 inline comments as done.
lebedev.ri added a subscriber: rtrieu.
lebedev.ri added a comment.
- Address review notes
- ~~Avoid re-evaluating ICE when called from `DiagnoseOutOfRangeComparison()`
which already knows the evaluated value~~
Don't even check if this is a Type Limit in `DiagnoseOutOfRangeComparison()`.
Perhaps @rtrieu can shed some light on this.
- Enhance test coverage a little (add a test with macro, with template)
Repository:
rL LLVM
https://reviews.llvm.org/D38101
Files:
docs/ReleaseNotes.rst
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaChecking.cpp
test/Sema/outof-range-constant-compare.c
test/Sema/tautological-constant-compare.c
test/Sema/tautological-unsigned-zero-compare.c
Index: test/Sema/tautological-unsigned-zero-compare.c
===================================================================
--- test/Sema/tautological-unsigned-zero-compare.c
+++ test/Sema/tautological-unsigned-zero-compare.c
@@ -1,47 +1,370 @@
// RUN: %clang_cc1 -fsyntax-only -DTEST -verify %s
// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify %s
+// RUN: %clang_cc1 -fsyntax-only -DTEST -verify -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify -x c++ %s
-unsigned value(void);
+unsigned uvalue(void);
+signed int svalue(void);
-int main() {
- unsigned un = value();
+#define macro(val) val
+
+#ifdef __cplusplus
+template<typename T>
+void TFunc() {
+ // Make sure that we do warn for normal variables in template functions !
+ unsigned char c = svalue();
+#ifdef TEST
+ if (c < 0) // expected-warning {{comparison of unsigned expression < 0 is always false}}
+ return;
+#else
+ if (c < 0)
+ return;
+#endif
+
+ if (c < macro(0))
+ return;
+
+ T v = svalue();
+ if (v < 0)
+ return;
+}
+#endif
+
+int main()
+{
+#ifdef __cplusplus
+ TFunc<unsigned char>();
+ TFunc<unsigned short>();
+#endif
+
+ unsigned un = uvalue();
#ifdef TEST
+ if (un == 0)
+ return 0;
+ if (un != 0)
+ return 0;
if (un < 0) // expected-warning {{comparison of unsigned expression < 0 is always false}}
- return 0;
+ return 0;
+ if (un <= 0)
+ return 0;
+ if (un > 0)
+ return 0;
if (un >= 0) // expected-warning {{comparison of unsigned expression >= 0 is always true}}
- return 0;
+ return 0;
+
+ if (0 == un)
+ return 0;
+ if (0 != un)
+ return 0;
+ if (0 < un)
+ return 0;
if (0 <= un) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
- return 0;
+ return 0;
if (0 > un) // expected-warning {{comparison of 0 > unsigned expression is always false}}
- return 0;
- if (un < 0U) // expected-warning {{comparison of unsigned expression < 0 is always false}}
- return 0;
- if (un >= 0U) // expected-warning {{comparison of unsigned expression >= 0 is always true}}
- return 0;
- if (0U <= un) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
- return 0;
- if (0U > un) // expected-warning {{comparison of 0 > unsigned expression is always false}}
- return 0;
+ return 0;
+ if (0 >= un)
+ return 0;
+
+ if (un == 0UL)
+ return 0;
+ if (un != 0UL)
+ return 0;
+ if (un < 0UL) // expected-warning {{comparison of unsigned expression < 0 is always false}}
+ return 0;
+ if (un <= 0UL)
+ return 0;
+ if (un > 0UL)
+ return 0;
+ if (un >= 0UL) // expected-warning {{comparison of unsigned expression >= 0 is always true}}
+ return 0;
+
+ if (0UL == un)
+ return 0;
+ if (0UL != un)
+ return 0;
+ if (0UL < un)
+ return 0;
+ if (0UL <= un) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
+ return 0;
+ if (0UL > un) // expected-warning {{comparison of 0 > unsigned expression is always false}}
+ return 0;
+ if (0UL >= un)
+ return 0;
#else
// expected-no-diagnostics
+ if (un == 0)
+ return 0;
+ if (un != 0)
+ return 0;
if (un < 0)
- return 0;
+ return 0;
+ if (un <= 0)
+ return 0;
+ if (un > 0)
+ return 0;
if (un >= 0)
- return 0;
+ return 0;
+
+ if (0 == un)
+ return 0;
+ if (0 != un)
+ return 0;
+ if (0 < un)
+ return 0;
if (0 <= un)
- return 0;
+ return 0;
if (0 > un)
- return 0;
- if (un < 0U)
- return 0;
- if (un >= 0U)
- return 0;
- if (0U <= un)
- return 0;
- if (0U > un)
- return 0;
+ return 0;
+ if (0 >= un)
+ return 0;
+
+ if (un == 0UL)
+ return 0;
+ if (un != 0UL)
+ return 0;
+ if (un < 0UL)
+ return 0;
+ if (un <= 0UL)
+ return 0;
+ if (un > 0UL)
+ return 0;
+ if (un >= 0UL)
+ return 0;
+
+ if (0UL == un)
+ return 0;
+ if (0UL != un)
+ return 0;
+ if (0UL < un)
+ return 0;
+ if (0UL <= un)
+ return 0;
+ if (0UL > un)
+ return 0;
+ if (0UL >= un)
+ return 0;
#endif
+
+ signed int a = svalue();
+
+#ifdef TEST
+ if (a == 0)
+ return 0;
+ if (a != 0)
+ return 0;
+ if (a < 0)
+ return 0;
+ if (a <= 0)
+ return 0;
+ if (a > 0)
+ return 0;
+ if (a >= 0)
+ return 0;
+
+ if (0 == a)
+ return 0;
+ if (0 != a)
+ return 0;
+ if (0 < a)
+ return 0;
+ if (0 <= a)
+ return 0;
+ if (0 > a)
+ return 0;
+ if (0 >= a)
+ return 0;
+
+ if (a == 0UL)
+ return 0;
+ if (a != 0UL)
+ return 0;
+ if (a < 0UL) // expected-warning {{comparison of unsigned expression < 0 is always false}}
+ return 0;
+ if (a <= 0UL)
+ return 0;
+ if (a > 0UL)
+ return 0;
+ if (a >= 0UL) // expected-warning {{comparison of unsigned expression >= 0 is always true}}
+ return 0;
+
+ if (0UL == a)
+ return 0;
+ if (0UL != a)
+ return 0;
+ if (0UL < a)
+ return 0;
+ if (0UL <= a) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
+ return 0;
+ if (0UL > a) // expected-warning {{comparison of 0 > unsigned expression is always false}}
+ return 0;
+ if (0UL >= a)
+ return 0;
+#else
+// expected-no-diagnostics
+ if (a == 0)
+ return 0;
+ if (a != 0)
+ return 0;
+ if (a < 0)
+ return 0;
+ if (a <= 0)
+ return 0;
+ if (a > 0)
+ return 0;
+ if (a >= 0)
+ return 0;
+
+ if (0 == a)
+ return 0;
+ if (0 != a)
+ return 0;
+ if (0 < a)
+ return 0;
+ if (0 <= a)
+ return 0;
+ if (0 > a)
+ return 0;
+ if (0 >= a)
+ return 0;
+
+ if (a == 0UL)
+ return 0;
+ if (a != 0UL)
+ return 0;
+ if (a < 0UL)
+ return 0;
+ if (a <= 0UL)
+ return 0;
+ if (a > 0UL)
+ return 0;
+ if (a >= 0UL)
+ return 0;
+
+ if (0UL == a)
+ return 0;
+ if (0UL != a)
+ return 0;
+ if (0UL < a)
+ return 0;
+ if (0UL <= a)
+ return 0;
+ if (0UL > a)
+ return 0;
+ if (0UL >= a)
+ return 0;
+#endif
+
+
+ float fl = 0;
+
+ if (fl == 0)
+ return 0;
+ if (fl != 0)
+ return 0;
+ if (fl < 0)
+ return 0;
+ if (fl <= 0)
+ return 0;
+ if (fl > 0)
+ return 0;
+ if (fl >= 0)
+ return 0;
+
+ if (0 == fl)
+ return 0;
+ if (0 != fl)
+ return 0;
+ if (0 < fl)
+ return 0;
+ if (0 <= fl)
+ return 0;
+ if (0 > fl)
+ return 0;
+ if (0 >= fl)
+ return 0;
+
+ if (fl == 0UL)
+ return 0;
+ if (fl != 0UL)
+ return 0;
+ if (fl < 0UL)
+ return 0;
+ if (fl <= 0UL)
+ return 0;
+ if (fl > 0UL)
+ return 0;
+ if (fl >= 0UL)
+ return 0;
+
+ if (0UL == fl)
+ return 0;
+ if (0UL != fl)
+ return 0;
+ if (0UL < fl)
+ return 0;
+ if (0UL <= fl)
+ return 0;
+ if (0UL > fl)
+ return 0;
+ if (0UL >= fl)
+ return 0;
+
+
+ double dl = 0;
+
+ if (dl == 0)
+ return 0;
+ if (dl != 0)
+ return 0;
+ if (dl < 0)
+ return 0;
+ if (dl <= 0)
+ return 0;
+ if (dl > 0)
+ return 0;
+ if (dl >= 0)
+ return 0;
+
+ if (0 == dl)
+ return 0;
+ if (0 != dl)
+ return 0;
+ if (0 < dl)
+ return 0;
+ if (0 <= dl)
+ return 0;
+ if (0 > dl)
+ return 0;
+ if (0 >= dl)
+ return 0;
+
+ if (dl == 0UL)
+ return 0;
+ if (dl != 0UL)
+ return 0;
+ if (dl < 0UL)
+ return 0;
+ if (dl <= 0UL)
+ return 0;
+ if (dl > 0UL)
+ return 0;
+ if (dl >= 0UL)
+ return 0;
+
+ if (0UL == dl)
+ return 0;
+ if (0UL != dl)
+ return 0;
+ if (0UL < dl)
+ return 0;
+ if (0UL <= dl)
+ return 0;
+ if (0UL > dl)
+ return 0;
+ if (0UL >= dl)
+ return 0;
+
return 1;
}
Index: test/Sema/tautological-constant-compare.c
===================================================================
--- /dev/null
+++ test/Sema/tautological-constant-compare.c
@@ -0,0 +1,514 @@
+// RUN: %clang_cc1 -fsyntax-only -DTEST -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-constant-compare -verify %s
+// RUN: %clang_cc1 -fsyntax-only -DTEST -verify -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-constant-compare -verify -x c++ %s
+
+int value(void);
+
+#define macro(val) val
+
+#ifdef __cplusplus
+template<typename T>
+void TFunc() {
+ // Make sure that we do warn for normal variables in template functions !
+ unsigned char c = value();
+#ifdef TEST
+ if (c > 255) // expected-warning {{comparison 'unsigned char' > 255 is always false}}
+ return;
+#else
+ if (c > 255)
+ return;
+#endif
+
+ if (c > macro(255))
+ return;
+
+ T v = value();
+ if (v > 255)
+ return;
+ if (v > 32767)
+ return;
+}
+#endif
+
+int main()
+{
+#ifdef __cplusplus
+ TFunc<unsigned char>();
+ TFunc<signed short>();
+#endif
+
+ short s = value();
+
+#ifdef TEST
+ if (s == 32767)
+ return 0;
+ if (s != 32767)
+ return 0;
+ if (s < 32767)
+ return 0;
+ if (s <= 32767) // expected-warning {{comparison 'short' <= 32767 is always true}}
+ return 0;
+ if (s > 32767) // expected-warning {{comparison 'short' > 32767 is always false}}
+ return 0;
+ if (s >= 32767)
+ return 0;
+
+ if (32767 == s)
+ return 0;
+ if (32767 != s)
+ return 0;
+ if (32767 < s) // expected-warning {{comparison 32767 < 'short' is always false}}
+ return 0;
+ if (32767 <= s)
+ return 0;
+ if (32767 > s)
+ return 0;
+ if (32767 >= s) // expected-warning {{comparison 32767 >= 'short' is always true}}
+ return 0;
+
+ // FIXME: assumes two's complement
+ if (s == -32768)
+ return 0;
+ if (s != -32768)
+ return 0;
+ if (s < -32768) // expected-warning {{comparison 'short' < -32768 is always false}}
+ return 0;
+ if (s <= -32768)
+ return 0;
+ if (s > -32768)
+ return 0;
+ if (s >= -32768) // expected-warning {{comparison 'short' >= -32768 is always true}}
+ return 0;
+
+ if (-32768 == s)
+ return 0;
+ if (-32768 != s)
+ return 0;
+ if (-32768 < s)
+ return 0;
+ if (-32768 <= s) // expected-warning {{comparison -32768 <= 'short' is always true}}
+ return 0;
+ if (-32768 > s) // expected-warning {{comparison -32768 > 'short' is always false}}
+ return 0;
+ if (-32768 >= s)
+ return 0;
+
+ 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}}
+ return 0;
+ if (s > 32767UL) // expected-warning {{comparison 'short' > 32767 is always false}}
+ return 0;
+ if (s >= 32767UL)
+ return 0;
+
+ if (32767UL == s)
+ return 0;
+ if (32767UL != s)
+ return 0;
+ if (32767UL < s) // expected-warning {{comparison 32767 < 'short' is always false}}
+ return 0;
+ if (32767UL <= s)
+ return 0;
+ if (32767UL > s)
+ return 0;
+ if (32767UL >= s) // expected-warning {{comparison 32767 >= 'short' is always true}}
+ return 0;
+
+ // FIXME: assumes two's complement
+ if (s == -32768L)
+ return 0;
+ if (s != -32768L)
+ return 0;
+ if (s < -32768L) // expected-warning {{comparison 'short' < -32768 is always false}}
+ return 0;
+ if (s <= -32768L)
+ return 0;
+ if (s > -32768L)
+ return 0;
+ if (s >= -32768L) // expected-warning {{comparison 'short' >= -32768 is always true}}
+ return 0;
+
+ if (-32768L == s)
+ return 0;
+ if (-32768L != s)
+ return 0;
+ if (-32768L < s)
+ return 0;
+ if (-32768L <= s) // expected-warning {{comparison -32768 <= 'short' is always true}}
+ return 0;
+ if (-32768L > s) // expected-warning {{comparison -32768 > 'short' is always false}}
+ return 0;
+ if (-32768L >= s)
+ return 0;
+#else
+ // expected-no-diagnostics
+ if (s == 32767)
+ return 0;
+ if (s != 32767)
+ return 0;
+ if (s < 32767)
+ return 0;
+ if (s <= 32767)
+ return 0;
+ if (s > 32767)
+ return 0;
+ if (s >= 32767)
+ return 0;
+
+ if (32767 == s)
+ return 0;
+ if (32767 != s)
+ return 0;
+ if (32767 < s)
+ return 0;
+ if (32767 <= s)
+ return 0;
+ if (32767 > s)
+ return 0;
+ if (32767 >= s)
+ return 0;
+
+ // FIXME: assumes two's complement
+ if (s == -32768)
+ return 0;
+ if (s != -32768)
+ return 0;
+ if (s < -32768)
+ return 0;
+ if (s <= -32768)
+ return 0;
+ if (s > -32768)
+ return 0;
+ if (s >= -32768)
+ return 0;
+
+ if (-32768 == s)
+ return 0;
+ if (-32768 != s)
+ return 0;
+ if (-32768 < s)
+ return 0;
+ if (-32768 <= s)
+ return 0;
+ if (-32768 > s)
+ return 0;
+ if (-32768 >= s)
+ return 0;
+
+ if (s == 32767UL)
+ return 0;
+ if (s != 32767UL)
+ return 0;
+ if (s < 32767UL)
+ return 0;
+ if (s <= 32767UL)
+ return 0;
+ if (s > 32767UL)
+ return 0;
+ if (s >= 32767UL)
+ return 0;
+
+ if (32767UL == s)
+ return 0;
+ if (32767UL != s)
+ return 0;
+ if (32767UL < s)
+ return 0;
+ if (32767UL <= s)
+ return 0;
+ if (32767UL > s)
+ return 0;
+ if (32767UL >= s)
+ return 0;
+
+ // FIXME: assumes two's complement
+ if (s == -32768L)
+ return 0;
+ if (s != -32768L)
+ return 0;
+ if (s < -32768L)
+ return 0;
+ if (s <= -32768L)
+ return 0;
+ if (s > -32768L)
+ return 0;
+ if (s >= -32768L)
+ return 0;
+
+ if (-32768L == s)
+ return 0;
+ if (-32768L != s)
+ return 0;
+ if (-32768L < s)
+ return 0;
+ if (-32768L <= s)
+ return 0;
+ if (-32768L > s)
+ return 0;
+ if (-32768L >= s)
+ return 0;
+#endif
+
+ if (s == 0)
+ return 0;
+ if (s != 0)
+ return 0;
+ if (s < 0)
+ return 0;
+ if (s <= 0)
+ return 0;
+ if (s > 0)
+ return 0;
+ if (s >= 0)
+ return 0;
+
+ if (0 == s)
+ return 0;
+ if (0 != s)
+ return 0;
+ if (0 < s)
+ return 0;
+ if (0 <= s)
+ return 0;
+ if (0 > s)
+ return 0;
+ if (0 >= s)
+ return 0;
+
+ // However the comparison with 0U would warn
+
+ unsigned short us = value();
+
+#ifdef TEST
+ if (us == 65535)
+ return 0;
+ if (us != 65535)
+ return 0;
+ if (us < 65535)
+ return 0;
+ if (us <= 65535) // expected-warning {{comparison 'unsigned short' <= 65535 is always true}}
+ return 0;
+ if (us > 65535) // expected-warning {{comparison 'unsigned short' > 65535 is always false}}
+ return 0;
+ if (us >= 65535)
+ return 0;
+
+ if (65535 == us)
+ return 0;
+ if (65535 != us)
+ return 0;
+ if (65535 < us) // expected-warning {{comparison 65535 < 'unsigned short' is always false}}
+ return 0;
+ if (65535 <= us)
+ return 0;
+ if (65535 > us)
+ return 0;
+ if (65535 >= us) // expected-warning {{comparison 65535 >= 'unsigned short' is always true}}
+ return 0;
+
+ if (us == 65535UL)
+ return 0;
+ if (us != 65535UL)
+ return 0;
+ if (us < 65535UL)
+ return 0;
+ if (us <= 65535UL) // expected-warning {{comparison 'unsigned short' <= 65535 is always true}}
+ return 0;
+ if (us > 65535UL) // expected-warning {{comparison 'unsigned short' > 65535 is always false}}
+ return 0;
+ if (us >= 65535UL)
+ return 0;
+
+ if (65535UL == us)
+ return 0;
+ if (65535UL != us)
+ return 0;
+ if (65535UL < us) // expected-warning {{comparison 65535 < 'unsigned short' is always false}}
+ return 0;
+ if (65535UL <= us)
+ return 0;
+ if (65535UL > us)
+ return 0;
+ if (65535UL >= us) // expected-warning {{comparison 65535 >= 'unsigned short' is always true}}
+ return 0;
+#else
+ // expected-no-diagnostics
+ if (us == 65535)
+ return 0;
+ if (us != 65535)
+ return 0;
+ if (us < 65535)
+ return 0;
+ if (us <= 65535)
+ return 0;
+ if (us > 65535)
+ return 0;
+ if (us >= 65535)
+ return 0;
+
+ if (65535 == us)
+ return 0;
+ if (65535 != us)
+ return 0;
+ if (65535 < us)
+ return 0;
+ if (65535 <= us)
+ return 0;
+ if (65535 > us)
+ return 0;
+ if (65535 >= us)
+ return 0;
+
+ if (us == 65535UL)
+ return 0;
+ if (us != 65535UL)
+ return 0;
+ if (us < 65535UL)
+ return 0;
+ if (us <= 65535UL)
+ return 0;
+ if (us > 65535UL)
+ return 0;
+ if (us >= 65535UL)
+ return 0;
+
+ if (65535UL == us)
+ return 0;
+ if (65535UL != us)
+ return 0;
+ if (65535UL < us)
+ return 0;
+ if (65535UL <= us)
+ return 0;
+ if (65535UL > us)
+ return 0;
+ if (65535UL >= us)
+ return 0;
+#endif
+
+ if (us == 32767)
+ return 0;
+ if (us != 32767)
+ return 0;
+ if (us < 32767)
+ return 0;
+ if (us <= 32767)
+ return 0;
+ if (us > 32767)
+ return 0;
+ if (us >= 32767)
+ return 0;
+
+ if (32767 == us)
+ return 0;
+ if (32767 != us)
+ return 0;
+ if (32767 < us)
+ return 0;
+ if (32767 <= us)
+ return 0;
+ if (32767 > us)
+ return 0;
+ if (32767 >= us)
+ return 0;
+
+ if (us == 32767UL)
+ return 0;
+ if (us != 32767UL)
+ return 0;
+ if (us < 32767UL)
+ return 0;
+ if (us <= 32767UL)
+ return 0;
+ if (us > 32767UL)
+ return 0;
+ if (us >= 32767UL)
+ return 0;
+
+ if (32767UL == us)
+ return 0;
+ if (32767UL != us)
+ return 0;
+ if (32767UL < us)
+ return 0;
+ if (32767UL <= us)
+ return 0;
+ if (32767UL > us)
+ return 0;
+ if (32767UL >= us)
+ return 0;
+
+#if __SIZEOF_INT128__
+ __int128 i128;
+ if (i128 == -1) // used to crash
+ return 0;
+#endif
+
+
+ enum E {
+ yes,
+ no,
+ maybe
+ };
+ enum E e;
+
+ if (e == yes)
+ return 0;
+ if (e != yes)
+ return 0;
+ if (e < yes)
+ return 0;
+ if (e <= yes)
+ return 0;
+ if (e > yes)
+ return 0;
+ if (e >= yes)
+ return 0;
+
+ if (yes == e)
+ return 0;
+ if (yes != e)
+ return 0;
+ if (yes < e)
+ return 0;
+ if (yes <= e)
+ return 0;
+ if (yes > e)
+ return 0;
+ if (yes >= e)
+ return 0;
+
+ if (e == maybe)
+ return 0;
+ if (e != maybe)
+ return 0;
+ if (e < maybe)
+ return 0;
+ if (e <= maybe)
+ return 0;
+ if (e > maybe)
+ return 0;
+ if (e >= maybe)
+ return 0;
+
+ if (maybe == e)
+ return 0;
+ if (maybe != e)
+ return 0;
+ if (maybe < e)
+ return 0;
+ if (maybe <= e)
+ return 0;
+ if (maybe > e)
+ return 0;
+ if (maybe >= e)
+ return 0;
+
+ return 1;
+}
Index: test/Sema/outof-range-constant-compare.c
===================================================================
--- test/Sema/outof-range-constant-compare.c
+++ test/Sema/outof-range-constant-compare.c
@@ -7,58 +7,6 @@
{
int a = value();
- if (a == 0x0000000000000000L)
- return 0;
- if (a != 0x0000000000000000L)
- return 0;
- if (a < 0x0000000000000000L)
- return 0;
- if (a <= 0x0000000000000000L)
- return 0;
- if (a > 0x0000000000000000L)
- return 0;
- if (a >= 0x0000000000000000L)
- return 0;
-
- if (0x0000000000000000L == a)
- return 0;
- if (0x0000000000000000L != a)
- return 0;
- if (0x0000000000000000L < a)
- return 0;
- if (0x0000000000000000L <= a)
- return 0;
- if (0x0000000000000000L > a)
- return 0;
- if (0x0000000000000000L >= a)
- return 0;
-
- if (a == 0x0000000000000000UL)
- return 0;
- if (a != 0x0000000000000000UL)
- return 0;
- if (a < 0x0000000000000000UL) // expected-warning {{comparison of unsigned expression < 0 is always false}}
- return 0;
- if (a <= 0x0000000000000000UL)
- return 0;
- if (a > 0x0000000000000000UL)
- return 0;
- if (a >= 0x0000000000000000UL) // expected-warning {{comparison of unsigned expression >= 0 is always true}}
- return 0;
-
- if (0x0000000000000000UL == a)
- return 0;
- if (0x0000000000000000UL != a)
- return 0;
- if (0x0000000000000000UL < a)
- return 0;
- if (0x0000000000000000UL <= a) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
- return 0;
- if (0x0000000000000000UL > a) // expected-warning {{comparison of 0 > unsigned expression is always false}}
- return 0;
- if (0x0000000000000000UL >= a)
- return 0;
-
if (a == 0x1234567812345678L) // expected-warning {{comparison of constant 1311768465173141112 with expression of type 'int' is always false}}
return 0;
if (a != 0x1234567812345678L) // expected-warning {{comparison of constant 1311768465173141112 with expression of type 'int' is always true}}
@@ -155,113 +103,6 @@
if (0x1234567812345678L >= l)
return 0;
- unsigned un = 0;
- if (un == 0x0000000000000000L)
- return 0;
- if (un != 0x0000000000000000L)
- return 0;
- if (un < 0x0000000000000000L) // expected-warning {{comparison of unsigned expression < 0 is always false}}
- return 0;
- if (un <= 0x0000000000000000L)
- return 0;
- if (un > 0x0000000000000000L)
- return 0;
- if (un >= 0x0000000000000000L) // expected-warning {{comparison of unsigned expression >= 0 is always true}}
- return 0;
-
- if (0x0000000000000000L == un)
- return 0;
- if (0x0000000000000000L != un)
- return 0;
- if (0x0000000000000000L < un)
- return 0;
- if (0x0000000000000000L <= un) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
- return 0;
- if (0x0000000000000000L > un) // expected-warning {{comparison of 0 > unsigned expression is always false}}
- return 0;
- if (0x0000000000000000L >= un)
- return 0;
-
- if (un == 0x0000000000000000UL)
- return 0;
- if (un != 0x0000000000000000UL)
- return 0;
- if (un < 0x0000000000000000UL) // expected-warning {{comparison of unsigned expression < 0 is always false}}
- return 0;
- if (un <= 0x0000000000000000UL)
- return 0;
- if (un > 0x0000000000000000UL)
- return 0;
- if (un >= 0x0000000000000000UL) // expected-warning {{comparison of unsigned expression >= 0 is always true}}
- return 0;
-
- if (0x0000000000000000UL == un)
- return 0;
- if (0x0000000000000000UL != un)
- return 0;
- if (0x0000000000000000UL < un)
- return 0;
- if (0x0000000000000000UL <= un) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
- return 0;
- if (0x0000000000000000UL > un) // expected-warning {{comparison of 0 > unsigned expression is always false}}
- return 0;
- if (0x0000000000000000UL >= un)
- return 0;
-
- float fl = 0;
- if (fl == 0x0000000000000000L)
- return 0;
- if (fl != 0x0000000000000000L)
- return 0;
- if (fl < 0x0000000000000000L)
- return 0;
- if (fl <= 0x0000000000000000L)
- return 0;
- if (fl > 0x0000000000000000L)
- return 0;
- if (fl >= 0x0000000000000000L)
- return 0;
-
- if (0x0000000000000000L == fl)
- return 0;
- if (0x0000000000000000L != fl)
- return 0;
- if (0x0000000000000000L < fl)
- return 0;
- if (0x0000000000000000L <= fl)
- return 0;
- if (0x0000000000000000L > fl)
- return 0;
- if (0x0000000000000000L >= fl)
- return 0;
-
- double dl = 0;
- if (dl == 0x0000000000000000L)
- return 0;
- if (dl != 0x0000000000000000L)
- return 0;
- if (dl < 0x0000000000000000L)
- return 0;
- if (dl <= 0x0000000000000000L)
- return 0;
- if (dl > 0x0000000000000000L)
- return 0;
- if (dl >= 0x0000000000000000L)
- return 0;
-
- if (0x0000000000000000L == dl)
- return 0;
- if (0x0000000000000000L != dl)
- return 0;
- if (0x0000000000000000L < dl)
- return 0;
- if (0x0000000000000000L <= dl)
- return 0;
- if (0x0000000000000000L > dl)
- return 0;
- if (0x0000000000000000L >= dl)
- return 0;
-
enum E {
yes,
no,
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -8553,19 +8553,75 @@
void AnalyzeImplicitConversions(Sema &S, Expr *E, SourceLocation CC);
-bool IsZero(Sema &S, Expr *E) {
+bool IsEnumConstOrFromMacro(Sema &S, Expr *E) {
// Suppress cases where we are comparing against an enum constant.
if (const DeclRefExpr *DR =
dyn_cast<DeclRefExpr>(E->IgnoreParenImpCasts()))
if (isa<EnumConstantDecl>(DR->getDecl()))
- return false;
+ return true;
// Suppress cases where the '0' value is expanded from a macro.
if (E->getLocStart().isMacroID())
- return false;
+ return true;
- llvm::APSInt Value;
- return E->isIntegerConstantExpr(Value, S.Context) && Value == 0;
+ return false;
+}
+
+bool isNonBooleanIntegerValue(Expr *E) {
+ return !E->isKnownToHaveBooleanValue() && E->getType()->isIntegerType();
+}
+
+bool isNonBooleanUnsignedValue(Expr *E) {
+ // We are checking that the expression is not known to have boolean value,
+ // is an integer type; and is either unsigned after implicit casts,
+ // or was unsigned before implicit casts.
+ return isNonBooleanIntegerValue(E) &&
+ (!E->getType()->isSignedIntegerType() ||
+ !E->IgnoreParenImpCasts()->getType()->isSignedIntegerType());
+}
+
+enum class LimitType {
+ Max, // e.g. 32767 for short
+ Min // e.g. -32768 for short
+};
+
+/// Checks whether Expr 'Constant' may be the
+/// std::numeric_limits<>::max() or std::numeric_limits<>::min()
+/// of the Expr 'Other'. If true, then returns the limit type (min or max).
+/// The Value will be set to the evaluation of Constant
+llvm::Optional<LimitType> IsTypeLimit(Sema &S, Expr *Other, Expr *Constant,
+ llvm::APSInt &Value) {
+ if (IsEnumConstOrFromMacro(S, Constant))
+ return llvm::Optional<LimitType>();
+
+ // Skip cases where this 'constant' is not an integer constant.
+ if (!Constant->isIntegerConstantExpr(Value, S.Context))
+ return llvm::Optional<LimitType>();
+
+ if (isNonBooleanUnsignedValue(Other) && Value == 0)
+ return LimitType::Min;
+
+ // TODO: Investigate using GetExprRange() to get tighter bounds
+ // on the bit ranges.
+ QualType OtherT = Other->IgnoreParenImpCasts()->getType();
+ if (const auto *AT = OtherT->getAs<AtomicType>())
+ OtherT = AT->getValueType();
+
+ IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
+
+ if (llvm::APSInt::isSameValue(
+ llvm::APSInt::getMaxValue(OtherRange.Width,
+ OtherT->isUnsignedIntegerType()),
+ Value))
+ return LimitType::Max;
+
+ if (llvm::APSInt::isSameValue(
+ llvm::APSInt::getMinValue(OtherRange.Width,
+ OtherT->isUnsignedIntegerType()),
+ Value))
+ return LimitType::Min;
+
+ return llvm::Optional<LimitType>();
}
bool HasEnumType(Expr *E) {
@@ -8580,55 +8636,66 @@
return E->getType()->isEnumeralType();
}
-bool isNonBooleanUnsignedValue(Expr *E) {
- // We are checking that the expression is not known to have boolean value,
- // is an integer type; and is either unsigned after implicit casts,
- // or was unsigned before implicit casts.
- return !E->isKnownToHaveBooleanValue() && E->getType()->isIntegerType() &&
- (!E->getType()->isSignedIntegerType() ||
- !E->IgnoreParenImpCasts()->getType()->isSignedIntegerType());
-}
-
-bool CheckTautologicalComparisonWithZero(Sema &S, BinaryOperator *E) {
- // Disable warning in template instantiations.
- if (S.inTemplateInstantiation())
+bool CheckTautologicalComparison(Sema &S, BinaryOperator *E) {
+ // Disable warning in template instantiations, don't analyze value-dependent
+ // comparisons, and only analyze <, >, <= and >= operations.
+ if (S.inTemplateInstantiation() || E->isValueDependent() ||
+ !E->isRelationalOp())
return false;
- // bool values are handled by DiagnoseOutOfRangeComparison().
+ // NOTE: bool values are handled by DiagnoseOutOfRangeComparison().
BinaryOperatorKind Op = E->getOpcode();
- if (E->isValueDependent())
- return false;
Expr *LHS = E->getLHS();
Expr *RHS = E->getRHS();
- bool Match = true;
+ llvm::APSInt Value; // The constant value
+ llvm::Optional<LimitType> ValueType; // Which limit (min/max) is the constant?
+ bool ConstOnRight; // Is the constant on the RHS or LHS?
- if (Op == BO_LT && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) {
- S.Diag(E->getOperatorLoc(),
- HasEnumType(LHS) ? diag::warn_lunsigned_enum_always_true_comparison
- : diag::warn_lunsigned_always_true_comparison)
- << "< 0" << false << LHS->getSourceRange() << RHS->getSourceRange();
- } else if (Op == BO_GE && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) {
- S.Diag(E->getOperatorLoc(),
- HasEnumType(LHS) ? diag::warn_lunsigned_enum_always_true_comparison
- : diag::warn_lunsigned_always_true_comparison)
- << ">= 0" << true << LHS->getSourceRange() << RHS->getSourceRange();
- } else if (Op == BO_GT && isNonBooleanUnsignedValue(RHS) && IsZero(S, LHS)) {
- S.Diag(E->getOperatorLoc(),
- HasEnumType(RHS) ? diag::warn_runsigned_enum_always_true_comparison
- : diag::warn_runsigned_always_true_comparison)
- << "0 >" << false << LHS->getSourceRange() << RHS->getSourceRange();
- } else if (Op == BO_LE && isNonBooleanUnsignedValue(RHS) && IsZero(S, LHS)) {
- S.Diag(E->getOperatorLoc(),
- HasEnumType(RHS) ? diag::warn_runsigned_enum_always_true_comparison
- : diag::warn_runsigned_always_true_comparison)
- << "0 <=" << true << LHS->getSourceRange() << RHS->getSourceRange();
+ if (isNonBooleanIntegerValue(LHS) &&
+ (ValueType = IsTypeLimit(S, LHS, RHS, Value))) {
+ ConstOnRight = true;
+ } else if (isNonBooleanIntegerValue(RHS) &&
+ (ValueType = IsTypeLimit(S, RHS, LHS, Value))) {
+ ConstOnRight = false;
} else
- Match = false;
+ return false;
- return Match;
+ // Expr *Const = ConstOnRight ? RHS : LHS;
+ Expr *Other = ConstOnRight ? LHS : RHS;
+ QualType OType = Other->IgnoreParenImpCasts()->getType();
+
+ bool Result; // The result of the comparison
+ if ((Op == BO_GT && ValueType == LimitType::Max && ConstOnRight) ||
+ (Op == BO_GT && ValueType == LimitType::Min && !ConstOnRight) ||
+ (Op == BO_LT && ValueType == LimitType::Max && !ConstOnRight) ||
+ (Op == BO_LT && ValueType == LimitType::Min && ConstOnRight)) {
+ Result = false;
+ } else if ((Op == BO_GE && ValueType == LimitType::Max && !ConstOnRight) ||
+ (Op == BO_GE && ValueType == LimitType::Min && ConstOnRight) ||
+ (Op == BO_LE && ValueType == LimitType::Max && ConstOnRight) ||
+ (Op == BO_LE && ValueType == LimitType::Min && !ConstOnRight)) {
+ Result = true;
+ } else // The comparison is not tautological.
+ return false;
+
+ unsigned Diag = (isNonBooleanUnsignedValue(Other) && Value == 0)
+ ? (HasEnumType(Other)
+ ? diag::warn_unsigned_enum_always_true_comparison
+ : diag::warn_unsigned_always_true_comparison)
+ : diag::warn_tautological_constant_compare;
+
+ SmallString<32> PrettySourceValue;
+ llvm::raw_svector_ostream OS(PrettySourceValue);
+ OS << Value;
+
+ S.Diag(E->getOperatorLoc(), Diag)
+ << ConstOnRight << OType << E->getOpcodeStr() << OS.str() << Result
+ << LHS->getSourceRange() << RHS->getSourceRange();
+
+ return true;
}
void DiagnoseOutOfRangeComparison(Sema &S, BinaryOperator *E, Expr *Constant,
@@ -8648,10 +8715,6 @@
bool OtherIsBooleanType = Other->isKnownToHaveBooleanValue();
- // 0 values are handled later by CheckTautologicalComparisonWithZero().
- if ((Value == 0) && (!OtherIsBooleanType))
- return;
-
BinaryOperatorKind op = E->getOpcode();
bool IsTrue = true;
@@ -8892,7 +8955,7 @@
return AnalyzeImpConvsInComparison(S, E);
// If this is a tautological comparison, suppress -Wsign-compare.
- if (CheckTautologicalComparisonWithZero(S, E))
+ if (CheckTautologicalComparison(S, E))
return AnalyzeImpConvsInComparison(S, E);
// We don't do anything special if this isn't an unsigned integral
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5935,18 +5935,18 @@
"member function %q1 is declared const here|"
"%select{|nested }1data member %2 declared const here}0">;
-def warn_lunsigned_always_true_comparison : Warning<
- "comparison of unsigned expression %0 is always %select{false|true}1">,
+def warn_unsigned_always_true_comparison : Warning<
+ "comparison of %select{%3|unsigned expression}0 %2 "
+ "%select{unsigned expression|%3}0 is always %select{false|true}4">,
InGroup<TautologicalUnsignedZeroCompare>;
-def warn_runsigned_always_true_comparison : Warning<
- "comparison of %0 unsigned expression is always %select{false|true}1">,
- InGroup<TautologicalUnsignedZeroCompare>;
-def warn_lunsigned_enum_always_true_comparison : Warning<
- "comparison of unsigned enum expression %0 is always %select{false|true}1">,
- InGroup<TautologicalUnsignedEnumZeroCompare>;
-def warn_runsigned_enum_always_true_comparison : Warning<
- "comparison of %0 unsigned enum expression is always %select{false|true}1">,
+def warn_unsigned_enum_always_true_comparison : Warning<
+ "comparison of %select{%3|unsigned enum expression}0 %2 "
+ "%select{unsigned enum expression|%3}0 is always %select{false|true}4">,
InGroup<TautologicalUnsignedEnumZeroCompare>;
+def warn_tautological_constant_compare : Warning<
+ "comparison %select{%3|%1}0 %2 "
+ "%select{%1|%3}0 is always %select{false|true}4">,
+ InGroup<TautologicalConstantCompare>;
def warn_mixed_sign_comparison : Warning<
"comparison of integers of different signs: %0 and %1">,
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -432,13 +432,15 @@
def TautologicalUnsignedZeroCompare : DiagGroup<"tautological-unsigned-zero-compare">;
def TautologicalUnsignedEnumZeroCompare : DiagGroup<"tautological-unsigned-enum-zero-compare">;
def TautologicalOutOfRangeCompare : DiagGroup<"tautological-constant-out-of-range-compare">;
+def TautologicalConstantCompare : DiagGroup<"tautological-constant-compare",
+ [TautologicalUnsignedZeroCompare,
+ TautologicalUnsignedEnumZeroCompare,
+ TautologicalOutOfRangeCompare]>;
def TautologicalPointerCompare : DiagGroup<"tautological-pointer-compare">;
def TautologicalOverlapCompare : DiagGroup<"tautological-overlap-compare">;
def TautologicalUndefinedCompare : DiagGroup<"tautological-undefined-compare">;
def TautologicalCompare : DiagGroup<"tautological-compare",
- [TautologicalUnsignedZeroCompare,
- TautologicalUnsignedEnumZeroCompare,
- TautologicalOutOfRangeCompare,
+ [TautologicalConstantCompare,
TautologicalPointerCompare,
TautologicalOverlapCompare,
TautologicalUndefinedCompare]>;
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -78,6 +78,10 @@
when the signed integer is coerced to an unsigned type for the comparison.
``-Wsign-compare`` was adjusted not to warn in this case.
+- ``-Wtautological-constant-compare`` is a new warning that warns on
+ tautological comparisons between integer variable of the type ``T`` and the
+ largest/smallest possible integer constant of that same type.
+
- ``-Wnull-pointer-arithmetic`` now warns about performing pointer arithmetic
on a null pointer. Such pointer arithmetic has an undefined behavior if the
offset is nonzero. It also now warns about arithmetic on a null pointer
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
