diff options
author | Roman Lebedev <lebedev.ri@gmail.com> | 2018-01-03 08:45:19 +0000 |
---|---|---|
committer | Roman Lebedev <lebedev.ri@gmail.com> | 2018-01-03 08:45:19 +0000 |
commit | b5b3d436a1831ddb4fdf2e4c9c07fb60807b70e5 (patch) | |
tree | 7b0db6a5a22fba468dec7fdb8ea36da731460823 /include | |
parent | ccd63c08661e585a27f9ec1c6aa51f81f9e2623a (diff) |
[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
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@321691 91177308-0d34-0410-b5e6-96231b3b80d8
Diffstat (limited to 'include')
-rw-r--r-- | include/clang/Basic/DiagnosticGroups.td | 9 | ||||
-rw-r--r-- | include/clang/Basic/DiagnosticSemaKinds.td | 6 |
2 files changed, 10 insertions, 5 deletions
diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td index c23183c81a..79b2766ae1 100644 --- a/include/clang/Basic/DiagnosticGroups.td +++ b/include/clang/Basic/DiagnosticGroups.td @@ -435,12 +435,16 @@ def StringCompare : DiagGroup<"string-compare">; def StringPlusInt : DiagGroup<"string-plus-int">; def StringPlusChar : DiagGroup<"string-plus-char">; def StrncatSize : DiagGroup<"strncat-size">; +def TautologicalTypeLimitCompare : DiagGroup<"tautological-type-limit-compare">; def TautologicalUnsignedZeroCompare : DiagGroup<"tautological-unsigned-zero-compare">; def TautologicalUnsignedEnumZeroCompare : DiagGroup<"tautological-unsigned-enum-zero-compare">; +def TautologicalInRangeCompare : DiagGroup<"tautological-constant-in-range-compare", + [TautologicalTypeLimitCompare, + TautologicalUnsignedZeroCompare, + TautologicalUnsignedEnumZeroCompare]>; def TautologicalOutOfRangeCompare : DiagGroup<"tautological-constant-out-of-range-compare">; def TautologicalConstantCompare : DiagGroup<"tautological-constant-compare", - [TautologicalUnsignedZeroCompare, - TautologicalUnsignedEnumZeroCompare, + [TautologicalInRangeCompare, TautologicalOutOfRangeCompare]>; def TautologicalPointerCompare : DiagGroup<"tautological-pointer-compare">; def TautologicalOverlapCompare : DiagGroup<"tautological-overlap-compare">; @@ -715,6 +719,7 @@ def IntToPointerCast : DiagGroup<"int-to-pointer-cast", def Move : DiagGroup<"move", [PessimizingMove, RedundantMove, SelfMove]>; def Extra : DiagGroup<"extra", [ + TautologicalInRangeCompare, MissingFieldInitializers, IgnoredQualifiers, InitializerOverrides, diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index e4649d3a44..ec624a0cc1 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -5946,15 +5946,15 @@ def note_typecheck_assign_const : Note< def warn_unsigned_always_true_comparison : Warning< "result of comparison of %select{%3|unsigned expression}0 %2 " "%select{unsigned expression|%3}0 is always %4">, - InGroup<TautologicalUnsignedZeroCompare>; + InGroup<TautologicalUnsignedZeroCompare>, DefaultIgnore; def warn_unsigned_enum_always_true_comparison : Warning< "result of comparison of %select{%3|unsigned enum expression}0 %2 " "%select{unsigned enum expression|%3}0 is always %4">, - InGroup<TautologicalUnsignedEnumZeroCompare>; + InGroup<TautologicalUnsignedEnumZeroCompare>, DefaultIgnore; def warn_tautological_constant_compare : Warning< "result of comparison %select{%3|%1}0 %2 " "%select{%1|%3}0 is always %4">, - InGroup<TautologicalConstantCompare>; + InGroup<TautologicalTypeLimitCompare>, DefaultIgnore; def warn_mixed_sign_comparison : Warning< "comparison of integers of different signs: %0 and %1">, |