teak-llvm/clang/test/Sema/tautological-unsigned-enum-zero-compare.cpp
Roman Lebedev c5417aafec [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 08:45:19 +00:00

149 lines
4.5 KiB
C++

// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-linux-gnu -fsyntax-only \
// RUN: -Wtautological-unsigned-enum-zero-compare \
// RUN: -verify=unsigned,unsigned-signed %s
// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only \
// RUN: -Wtautological-unsigned-enum-zero-compare \
// RUN: -verify=unsigned-signed %s
// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only \
// RUN: -verify=silence %s
// silence-no-diagnostics
int main() {
// On Windows, all enumerations have a fixed underlying type, which is 'int'
// if not otherwise specified, so A is identical to C on Windows. Otherwise,
// we follow the C++ rules, which say that the only valid values of A are 0
// and 1.
enum A { A_foo = 0, A_bar, };
enum A a;
enum B : unsigned { B_foo = 0, B_bar, };
enum B b;
enum C : signed { C_foo = 0, C_bar, };
enum C c;
if (a < 0) // unsigned-warning {{comparison of unsigned enum expression < 0 is always false}}
return 0;
if (0 >= a)
return 0;
if (a > 0)
return 0;
if (0 <= a) // unsigned-warning {{comparison of 0 <= unsigned enum expression is always true}}
return 0;
if (a <= 0)
return 0;
if (0 > a) // unsigned-warning {{comparison of 0 > unsigned enum expression is always false}}
return 0;
if (a >= 0) // unsigned-warning {{comparison of unsigned enum expression >= 0 is always true}}
return 0;
if (0 < a)
return 0;
// FIXME: As below, the issue here is that the enumeration is promoted to
// unsigned.
if (a < 0U) // unsigned-signed-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) // unsigned-signed-warning {{comparison of 0 <= unsigned enum expression is always true}}
return 0;
if (a <= 0U)
return 0;
if (0U > a) // unsigned-signed-warning {{comparison of 0 > unsigned enum expression is always false}}
return 0;
if (a >= 0U) // unsigned-signed-warning {{comparison of unsigned enum expression >= 0 is always true}}
return 0;
if (0U < a)
return 0;
if (b < 0) // unsigned-signed-warning {{comparison of unsigned enum expression < 0 is always false}}
return 0;
if (0 >= b)
return 0;
if (b > 0)
return 0;
if (0 <= b) // unsigned-signed-warning {{comparison of 0 <= unsigned enum expression is always true}}
return 0;
if (b <= 0)
return 0;
if (0 > b) // unsigned-signed-warning {{comparison of 0 > unsigned enum expression is always false}}
return 0;
if (b >= 0) // unsigned-signed-warning {{comparison of unsigned enum expression >= 0 is always true}}
return 0;
if (0 < b)
return 0;
if (b < 0U) // unsigned-signed-warning {{comparison of unsigned enum expression < 0 is always false}}
return 0;
if (0U >= b)
return 0;
if (b > 0U)
return 0;
if (0U <= b) // unsigned-signed-warning {{comparison of 0 <= unsigned enum expression is always true}}
return 0;
if (b <= 0U)
return 0;
if (0U > b) // unsigned-signed-warning {{comparison of 0 > unsigned enum expression is always false}}
return 0;
if (b >= 0U) // unsigned-signed-warning {{comparison of unsigned enum expression >= 0 is always true}}
return 0;
if (0U < b)
return 0;
if (c < 0)
return 0;
if (0 >= c)
return 0;
if (c > 0)
return 0;
if (0 <= c)
return 0;
if (c <= 0)
return 0;
if (0 > c)
return 0;
if (c >= 0)
return 0;
if (0 < c)
return 0;
// FIXME: These diagnostics are terrible. The issue here is that the signed
// enumeration value was promoted to an unsigned type.
if (c < 0U) // unsigned-signed-warning {{comparison of unsigned enum expression < 0 is always false}}
return 0;
if (0U >= c)
return 0;
if (c > 0U)
return 0;
if (0U <= c) // unsigned-signed-warning {{comparison of 0 <= unsigned enum expression is always true}}
return 0;
if (c <= 0U)
return 0;
if (0U > c) // unsigned-signed-warning {{comparison of 0 > unsigned enum expression is always false}}
return 0;
if (c >= 0U) // unsigned-signed-warning {{comparison of unsigned enum expression >= 0 is always true}}
return 0;
if (0U < c)
return 0;
return 1;
}
namespace crash_enum_zero_width {
int test() {
enum A : unsigned {
A_foo = 0
};
enum A a;
// used to crash in llvm::APSInt::getMaxValue()
if (a < 0) // unsigned-signed-warning {{comparison of unsigned enum expression < 0 is always false}}
return 0;
return 1;
}
} // namespace crash_enum_zero_width