This revision was automatically updated to reflect the committed changes. Closed by commit rL315614: [Sema] Diagnose tautological comparison with type's min/max values (authored by lebedevri).
Changed prior to commit: https://reviews.llvm.org/D38101?vs=118833&id=118835#toc Repository: rL LLVM https://reviews.llvm.org/D38101 Files: cfe/trunk/docs/ReleaseNotes.rst cfe/trunk/include/clang/Basic/DiagnosticGroups.td cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/Sema/SemaChecking.cpp cfe/trunk/test/Sema/outof-range-constant-compare.c cfe/trunk/test/Sema/tautological-constant-compare.c cfe/trunk/test/Sema/tautological-unsigned-zero-compare.c
Index: cfe/trunk/docs/ReleaseNotes.rst =================================================================== --- cfe/trunk/docs/ReleaseNotes.rst +++ cfe/trunk/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 Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td @@ -5938,18 +5938,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: cfe/trunk/include/clang/Basic/DiagnosticGroups.td =================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td +++ cfe/trunk/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: cfe/trunk/test/Sema/outof-range-constant-compare.c =================================================================== --- cfe/trunk/test/Sema/outof-range-constant-compare.c +++ cfe/trunk/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: cfe/trunk/test/Sema/tautological-constant-compare.c =================================================================== --- cfe/trunk/test/Sema/tautological-constant-compare.c +++ cfe/trunk/test/Sema/tautological-constant-compare.c @@ -0,0 +1,514 @@ +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -DTEST -verify %s +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wno-tautological-constant-compare -verify %s +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -DTEST -verify -x c++ %s +// RUN: %clang_cc1 -triple x86_64-linux-gnu -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: cfe/trunk/test/Sema/tautological-unsigned-zero-compare.c =================================================================== --- cfe/trunk/test/Sema/tautological-unsigned-zero-compare.c +++ cfe/trunk/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: cfe/trunk/lib/Sema/SemaChecking.cpp =================================================================== --- cfe/trunk/lib/Sema/SemaChecking.cpp +++ cfe/trunk/lib/Sema/SemaChecking.cpp @@ -8553,19 +8553,71 @@ 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 is the evaluation of Constant +llvm::Optional<LimitType> IsTypeLimit(Sema &S, Expr *Constant, Expr *Other, + const llvm::APSInt &Value) { + if (IsEnumConstOrFromMacro(S, Constant)) + 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,63 +8632,59 @@ 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, Expr *Constant, + Expr *Other, const llvm::APSInt &Value, + bool RhsConstant) { + // Disable warning in template instantiations + // and only analyze <, >, <= and >= operations. + if (S.inTemplateInstantiation() || !E->isRelationalOp()) return false; - // bool values are handled by DiagnoseOutOfRangeComparison(). - BinaryOperatorKind Op = E->getOpcode(); - if (E->isValueDependent()) + + QualType OType = Other->IgnoreParenImpCasts()->getType(); + + llvm::Optional<LimitType> ValueType; // Which limit (min/max) is the constant? + + if (!(isNonBooleanIntegerValue(Other) && + (ValueType = IsTypeLimit(S, Constant, Other, Value)))) return false; - Expr *LHS = E->getLHS(); - Expr *RHS = E->getRHS(); + bool ConstIsLowerBound = (Op == BO_LT || Op == BO_LE) ^ RhsConstant; + bool ResultWhenConstEqualsOther = (Op == BO_LE || Op == BO_GE); + bool ResultWhenConstNeOther = ConstIsLowerBound ^ ValueType == LimitType::Max; + if (ResultWhenConstEqualsOther != ResultWhenConstNeOther) + return false; // The comparison is not tautological. - bool Match = true; + const bool Result = ResultWhenConstEqualsOther; - 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(); - } else - Match = 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; + + // Should be enough for uint128 (39 decimal digits) + SmallString<64> PrettySourceValue; + llvm::raw_svector_ostream OS(PrettySourceValue); + OS << Value; + + S.Diag(E->getOperatorLoc(), Diag) + << RhsConstant << OType << E->getOpcodeStr() << OS.str() << Result + << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange(); - return Match; + return true; } -void DiagnoseOutOfRangeComparison(Sema &S, BinaryOperator *E, Expr *Constant, +bool DiagnoseOutOfRangeComparison(Sema &S, BinaryOperator *E, Expr *Constant, Expr *Other, const llvm::APSInt &Value, bool RhsConstant) { // Disable warning in template instantiations. if (S.inTemplateInstantiation()) - return; + return false; + + Constant = Constant->IgnoreParenImpCasts(); + Other = Other->IgnoreParenImpCasts(); // TODO: Investigate using GetExprRange() to get tighter bounds // on the bit ranges. @@ -8648,10 +8696,6 @@ bool OtherIsBooleanType = Other->isKnownToHaveBooleanValue(); - // 0 values are handled later by CheckTautologicalComparisonWithZero(). - if ((Value == 0) && (!OtherIsBooleanType)) - return; - BinaryOperatorKind op = E->getOpcode(); bool IsTrue = true; @@ -8667,7 +8711,7 @@ QualType CommonT = E->getLHS()->getType(); if (S.Context.hasSameUnqualifiedType(OtherT, ConstantT)) - return; + return false; assert((OtherT->isIntegerType() && ConstantT->isIntegerType()) && "comparison with non-integer type"); @@ -8682,38 +8726,38 @@ // Check that the constant is representable in type OtherT. if (ConstantSigned) { if (OtherWidth >= Value.getMinSignedBits()) - return; + return false; } else { // !ConstantSigned if (OtherWidth >= Value.getActiveBits() + 1) - return; + return false; } } else { // !OtherSigned // Check that the constant is representable in type OtherT. // Negative values are out of range. if (ConstantSigned) { if (Value.isNonNegative() && OtherWidth >= Value.getActiveBits()) - return; + return false; } else { // !ConstantSigned if (OtherWidth >= Value.getActiveBits()) - return; + return false; } } } else { // !CommonSigned if (OtherRange.NonNegative) { if (OtherWidth >= Value.getActiveBits()) - return; + return false; } else { // OtherSigned assert(!ConstantSigned && "Two signed types converted to unsigned types."); // Check to see if the constant is representable in OtherT. if (OtherWidth > Value.getActiveBits()) - return; + return false; // Check to see if the constant is equivalent to a negative value // cast to CommonT. if (S.Context.getIntWidth(ConstantT) == S.Context.getIntWidth(CommonT) && Value.isNegative() && Value.getMinSignedBits() <= OtherWidth) - return; + return false; // The constant value rests between values that OtherT can represent // after conversion. Relational comparison still works, but equality // comparisons will be tautological. @@ -8726,7 +8770,7 @@ if (op == BO_EQ || op == BO_NE) { IsTrue = op == BO_NE; } else if (EqualityOnly) { - return; + return false; } else if (RhsConstant) { if (op == BO_GT || op == BO_GE) IsTrue = !PositiveConstant; @@ -8814,7 +8858,7 @@ } else if (CmpRes == ATrue) { IsTrue = true; } else { - return; + return false; } } @@ -8837,6 +8881,8 @@ << OS.str() << LiteralOrBoolConstant << OtherT << (OtherIsBooleanType && !OtherT->isBooleanType()) << IsTrue << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange()); + + return true; } /// Analyze the operands of the given comparison. Implements the @@ -8862,44 +8908,48 @@ if (E->isValueDependent()) return AnalyzeImpConvsInComparison(S, E); - Expr *LHS = E->getLHS()->IgnoreParenImpCasts(); - Expr *RHS = E->getRHS()->IgnoreParenImpCasts(); - - bool IsComparisonConstant = false; - - // Check whether an integer constant comparison results in a value - // of 'true' or 'false'. + Expr *LHS = E->getLHS(); + Expr *RHS = E->getRHS(); + if (T->isIntegralType(S.Context)) { llvm::APSInt RHSValue; - bool IsRHSIntegralLiteral = - RHS->isIntegerConstantExpr(RHSValue, S.Context); llvm::APSInt LHSValue; - bool IsLHSIntegralLiteral = - LHS->isIntegerConstantExpr(LHSValue, S.Context); - if (IsRHSIntegralLiteral && !IsLHSIntegralLiteral) - DiagnoseOutOfRangeComparison(S, E, RHS, LHS, RHSValue, true); - else if (!IsRHSIntegralLiteral && IsLHSIntegralLiteral) - DiagnoseOutOfRangeComparison(S, E, LHS, RHS, LHSValue, false); - else - IsComparisonConstant = - (IsRHSIntegralLiteral && IsLHSIntegralLiteral); - } else if (!T->hasUnsignedIntegerRepresentation()) - IsComparisonConstant = E->isIntegerConstantExpr(S.Context); - - // We don't care about value-dependent expressions or expressions - // whose result is a constant. - if (IsComparisonConstant) - return AnalyzeImpConvsInComparison(S, E); - // If this is a tautological comparison, suppress -Wsign-compare. - if (CheckTautologicalComparisonWithZero(S, E)) - return AnalyzeImpConvsInComparison(S, E); + bool IsRHSIntegralLiteral = RHS->isIntegerConstantExpr(RHSValue, S.Context); + bool IsLHSIntegralLiteral = LHS->isIntegerConstantExpr(LHSValue, S.Context); + + // We don't care about expressions whose result is a constant. + if (IsRHSIntegralLiteral && IsLHSIntegralLiteral) + return AnalyzeImpConvsInComparison(S, E); + + // We only care about expressions where just one side is literal + if (IsRHSIntegralLiteral ^ IsLHSIntegralLiteral) { + // Is the constant on the RHS or LHS? + const bool RhsConstant = IsRHSIntegralLiteral; + Expr *Const = RhsConstant ? RHS : LHS; + Expr *Other = RhsConstant ? LHS : RHS; + const llvm::APSInt &Value = RhsConstant ? RHSValue : LHSValue; - // We don't do anything special if this isn't an unsigned integral - // comparison: we're only interested in integral comparisons, and - // signed comparisons only happen in cases we don't care to warn about. - if (!T->hasUnsignedIntegerRepresentation()) + // Check whether an integer constant comparison results in a value + // of 'true' or 'false'. + + if (CheckTautologicalComparison(S, E, Const, Other, Value, RhsConstant)) + return AnalyzeImpConvsInComparison(S, E); + + if (DiagnoseOutOfRangeComparison(S, E, Const, Other, Value, RhsConstant)) + return AnalyzeImpConvsInComparison(S, E); + } + } + + if (!T->hasUnsignedIntegerRepresentation()) { + // We don't do anything special if this isn't an unsigned integral + // comparison: we're only interested in integral comparisons, and + // signed comparisons only happen in cases we don't care to warn about. return AnalyzeImpConvsInComparison(S, E); + } + + LHS = LHS->IgnoreParenImpCasts(); + RHS = RHS->IgnoreParenImpCasts(); // Check to see if one of the (unmodified) operands is of different // signedness.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits