[Sema] -Wtautological-constant-compare is too good. Cripple it.
Summary:
The diagnostic was mostly introduced in D38101 by me, as a reaction to wasting a lot of time, see [[ https://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20171009/206427.html | mail ]].
However, the diagnostic is pretty dumb. While it works with no false-positives,
there are some questionable cases that are diagnosed when one would argue that they should not be.
The common complaint is that it diagnoses the comparisons between an `int` and
`long` when compiling for a 32-bit target as tautological, but not when
compiling for 64-bit targets. The underlying problem is obvious: data model.
In most cases, 64-bit target is `LP64` (`int` is 32-bit, `long` and pointer are
64-bit), and the 32-bit target is `ILP32` (`int`, `long`, and pointer are 32-bit).
I.e. the common pattern is: (pseudocode)
```
#include <limits>
#include <cstdint>
int main() {
using T1 = long;
using T2 = int;
T1 r;
if (r < std::numeric_limits<T2>::min()) {}
if (r > std::numeric_limits<T2>::max()) {}
}
```
As an example, D39149 was trying to fix this diagnostic in libc++, and it was not well-received.
This *could* be "fixed", by changing the diagnostics logic to something like
`if the types of the values being compared are different, but are of the same size, then do diagnose`,
and i even attempted to do so in D39462, but as @rjmccall rightfully commented,
that implementation is incomplete to say the least.
So to stop causing trouble, and avoid contaminating upcoming release, lets do this workaround:
* move these three diags (`warn_unsigned_always_true_comparison`, `warn_unsigned_enum_always_true_comparison`, `warn_tautological_constant_compare`) into it's own `-Wtautological-constant-in-range-compare`
* Disable them by default
* Make them part of `-Wextra`
* Additionally, give `warn_tautological_constant_compare` it's own flag `-Wtautological-type-limit-compare`.
I'm not happy about that name, but i can't come up with anything better.
This way all three of them can be enabled/disabled either altogether, or one-by-one.
Reviewers: aaron.ballman, rsmith, smeenai, rjmccall, rnk, mclow.lists, dim
Reviewed By: aaron.ballman, rsmith, dim
Subscribers: thakis, compnerd, mehdi_amini, dim, hans, cfe-commits, rjmccall
Tags: #clang
Differential Revision: https://reviews.llvm.org/D41512
llvm-svn: 321691
2018-01-03 16:45:19 +08:00
|
|
|
// RUN: %clang_cc1 -fsyntax-only \
|
|
|
|
|
// RUN: -Wtautological-unsigned-zero-compare \
|
|
|
|
|
// RUN: -verify %s
|
|
|
|
|
// RUN: %clang_cc1 -fsyntax-only \
|
|
|
|
|
// RUN: -verify=silence %s
|
|
|
|
|
// RUN: %clang_cc1 -fsyntax-only \
|
|
|
|
|
// RUN: -Wtautological-unsigned-zero-compare \
|
|
|
|
|
// RUN: -verify -x c++ %s
|
|
|
|
|
// RUN: %clang_cc1 -fsyntax-only \
|
|
|
|
|
// RUN: -verify=silence -x c++ %s
|
2017-09-08 21:56:45 +08:00
|
|
|
|
2017-10-16 04:13:17 +08:00
|
|
|
unsigned uvalue(void);
|
|
|
|
|
signed int svalue(void);
|
2017-09-08 21:56:45 +08:00
|
|
|
|
2017-10-16 04:13:17 +08:00
|
|
|
#define macro(val) val
|
2017-09-08 21:56:45 +08:00
|
|
|
|
2017-10-16 04:13:17 +08:00
|
|
|
#ifdef __cplusplus
|
|
|
|
|
template<typename T>
|
|
|
|
|
void TFunc() {
|
|
|
|
|
// Make sure that we do warn for normal variables in template functions !
|
|
|
|
|
unsigned char c = svalue();
|
|
|
|
|
if (c < 0) // expected-warning {{comparison of unsigned expression < 0 is always false}}
|
|
|
|
|
return;
|
|
|
|
|
|
|
|
|
|
if (c < macro(0))
|
|
|
|
|
return;
|
|
|
|
|
|
|
|
|
|
T v = svalue();
|
|
|
|
|
if (v < 0)
|
|
|
|
|
return;
|
|
|
|
|
}
|
|
|
|
|
#endif
|
|
|
|
|
|
|
|
|
|
int main()
|
|
|
|
|
{
|
|
|
|
|
#ifdef __cplusplus
|
|
|
|
|
TFunc<unsigned char>();
|
|
|
|
|
TFunc<unsigned short>();
|
|
|
|
|
#endif
|
|
|
|
|
|
[Sema] -Wtautological-constant-compare is too good. Cripple it.
Summary:
The diagnostic was mostly introduced in D38101 by me, as a reaction to wasting a lot of time, see [[ https://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20171009/206427.html | mail ]].
However, the diagnostic is pretty dumb. While it works with no false-positives,
there are some questionable cases that are diagnosed when one would argue that they should not be.
The common complaint is that it diagnoses the comparisons between an `int` and
`long` when compiling for a 32-bit target as tautological, but not when
compiling for 64-bit targets. The underlying problem is obvious: data model.
In most cases, 64-bit target is `LP64` (`int` is 32-bit, `long` and pointer are
64-bit), and the 32-bit target is `ILP32` (`int`, `long`, and pointer are 32-bit).
I.e. the common pattern is: (pseudocode)
```
#include <limits>
#include <cstdint>
int main() {
using T1 = long;
using T2 = int;
T1 r;
if (r < std::numeric_limits<T2>::min()) {}
if (r > std::numeric_limits<T2>::max()) {}
}
```
As an example, D39149 was trying to fix this diagnostic in libc++, and it was not well-received.
This *could* be "fixed", by changing the diagnostics logic to something like
`if the types of the values being compared are different, but are of the same size, then do diagnose`,
and i even attempted to do so in D39462, but as @rjmccall rightfully commented,
that implementation is incomplete to say the least.
So to stop causing trouble, and avoid contaminating upcoming release, lets do this workaround:
* move these three diags (`warn_unsigned_always_true_comparison`, `warn_unsigned_enum_always_true_comparison`, `warn_tautological_constant_compare`) into it's own `-Wtautological-constant-in-range-compare`
* Disable them by default
* Make them part of `-Wextra`
* Additionally, give `warn_tautological_constant_compare` it's own flag `-Wtautological-type-limit-compare`.
I'm not happy about that name, but i can't come up with anything better.
This way all three of them can be enabled/disabled either altogether, or one-by-one.
Reviewers: aaron.ballman, rsmith, smeenai, rjmccall, rnk, mclow.lists, dim
Reviewed By: aaron.ballman, rsmith, dim
Subscribers: thakis, compnerd, mehdi_amini, dim, hans, cfe-commits, rjmccall
Tags: #clang
Differential Revision: https://reviews.llvm.org/D41512
llvm-svn: 321691
2018-01-03 16:45:19 +08:00
|
|
|
short s = svalue();
|
|
|
|
|
|
2017-10-16 04:13:17 +08:00
|
|
|
unsigned un = uvalue();
|
|
|
|
|
|
2017-12-16 10:23:22 +08:00
|
|
|
// silence-no-diagnostics
|
|
|
|
|
|
[Sema] -Wtautological-constant-compare is too good. Cripple it.
Summary:
The diagnostic was mostly introduced in D38101 by me, as a reaction to wasting a lot of time, see [[ https://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20171009/206427.html | mail ]].
However, the diagnostic is pretty dumb. While it works with no false-positives,
there are some questionable cases that are diagnosed when one would argue that they should not be.
The common complaint is that it diagnoses the comparisons between an `int` and
`long` when compiling for a 32-bit target as tautological, but not when
compiling for 64-bit targets. The underlying problem is obvious: data model.
In most cases, 64-bit target is `LP64` (`int` is 32-bit, `long` and pointer are
64-bit), and the 32-bit target is `ILP32` (`int`, `long`, and pointer are 32-bit).
I.e. the common pattern is: (pseudocode)
```
#include <limits>
#include <cstdint>
int main() {
using T1 = long;
using T2 = int;
T1 r;
if (r < std::numeric_limits<T2>::min()) {}
if (r > std::numeric_limits<T2>::max()) {}
}
```
As an example, D39149 was trying to fix this diagnostic in libc++, and it was not well-received.
This *could* be "fixed", by changing the diagnostics logic to something like
`if the types of the values being compared are different, but are of the same size, then do diagnose`,
and i even attempted to do so in D39462, but as @rjmccall rightfully commented,
that implementation is incomplete to say the least.
So to stop causing trouble, and avoid contaminating upcoming release, lets do this workaround:
* move these three diags (`warn_unsigned_always_true_comparison`, `warn_unsigned_enum_always_true_comparison`, `warn_tautological_constant_compare`) into it's own `-Wtautological-constant-in-range-compare`
* Disable them by default
* Make them part of `-Wextra`
* Additionally, give `warn_tautological_constant_compare` it's own flag `-Wtautological-type-limit-compare`.
I'm not happy about that name, but i can't come up with anything better.
This way all three of them can be enabled/disabled either altogether, or one-by-one.
Reviewers: aaron.ballman, rsmith, smeenai, rjmccall, rnk, mclow.lists, dim
Reviewed By: aaron.ballman, rsmith, dim
Subscribers: thakis, compnerd, mehdi_amini, dim, hans, cfe-commits, rjmccall
Tags: #clang
Differential Revision: https://reviews.llvm.org/D41512
llvm-svn: 321691
2018-01-03 16:45:19 +08:00
|
|
|
// Note: both sides are promoted to unsigned long prior to the comparison.
|
|
|
|
|
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;
|
|
|
|
|
|
2017-10-16 04:13:17 +08:00
|
|
|
if (un == 0)
|
|
|
|
|
return 0;
|
|
|
|
|
if (un != 0)
|
|
|
|
|
return 0;
|
2017-09-08 21:56:45 +08:00
|
|
|
if (un < 0) // expected-warning {{comparison of unsigned expression < 0 is always false}}
|
2017-10-16 04:13:17 +08:00
|
|
|
return 0;
|
|
|
|
|
if (un <= 0)
|
|
|
|
|
return 0;
|
|
|
|
|
if (un > 0)
|
|
|
|
|
return 0;
|
2017-09-08 21:56:45 +08:00
|
|
|
if (un >= 0) // expected-warning {{comparison of unsigned expression >= 0 is always true}}
|
2017-10-16 04:13:17 +08:00
|
|
|
return 0;
|
|
|
|
|
|
|
|
|
|
if (0 == un)
|
|
|
|
|
return 0;
|
|
|
|
|
if (0 != un)
|
|
|
|
|
return 0;
|
|
|
|
|
if (0 < un)
|
|
|
|
|
return 0;
|
2017-09-08 21:56:45 +08:00
|
|
|
if (0 <= un) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
|
2017-10-16 04:13:17 +08:00
|
|
|
return 0;
|
2017-09-08 21:56:45 +08:00
|
|
|
if (0 > un) // expected-warning {{comparison of 0 > unsigned expression is always false}}
|
2017-10-16 04:13:17 +08:00
|
|
|
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;
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
signed int a = svalue();
|
|
|
|
|
|
|
|
|
|
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;
|
2017-09-08 21:56:45 +08:00
|
|
|
|
2017-10-16 04:13:17 +08:00
|
|
|
|
|
|
|
|
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;
|
|
|
|
|
|
2017-09-08 21:56:45 +08:00
|
|
|
return 1;
|
|
|
|
|
}
|