[clang] d6492d8 - Add -Wtautological-value-range-compare warning.
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 6 13:28:59 PDT 2020
Author: Richard Smith
Date: 2020-08-06T13:28:50-07:00
New Revision: d6492d874478b1d3b1ce3adb4c3044618bec29e9
URL: https://github.com/llvm/llvm-project/commit/d6492d874478b1d3b1ce3adb4c3044618bec29e9
DIFF: https://github.com/llvm/llvm-project/commit/d6492d874478b1d3b1ce3adb4c3044618bec29e9.diff
LOG: Add -Wtautological-value-range-compare warning.
This warning diagnoses cases where an expression is compared to a
constant, and the comparison is tautological due to the form of the
expression (but not merely due to its type). This applies in cases such
as comparisons of bit-fields and the result of bit-masks.
The new warning is added to the Clang diagnostic group
-Wtautological-constant-in-range-compare but not to the
formerly-equivalent GCC-compatibility diagnostic group -Wtype-limits,
which retains its old meaning of diagnosing only tautological
comparisons to extremal values of a type (eg, int > INT_MAX).
Reviewed By: rtrieu
Differential Revision: https://reviews.llvm.org/D85256
Added:
Modified:
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/tautological-constant-compare.c
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index be62461faef48..5ddd37e9972af 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -555,12 +555,16 @@ def IntInBoolContext : DiagGroup<"int-in-bool-context">;
def TautologicalTypeLimitCompare : DiagGroup<"tautological-type-limit-compare">;
def TautologicalUnsignedZeroCompare : DiagGroup<"tautological-unsigned-zero-compare">;
def TautologicalUnsignedEnumZeroCompare : DiagGroup<"tautological-unsigned-enum-zero-compare">;
+// For compatibility with GCC. Tautological comparison warnings for constants
+// that are an extremal value of the type.
+def TypeLimits : DiagGroup<"type-limits", [TautologicalTypeLimitCompare,
+ TautologicalUnsignedZeroCompare,
+ TautologicalUnsignedEnumZeroCompare]>;
+// Additional tautological comparison warnings based on the expression, not
+// only on its type.
+def TautologicalValueRangeCompare : DiagGroup<"tautological-value-range-compare">;
def TautologicalInRangeCompare : DiagGroup<"tautological-constant-in-range-compare",
- [TautologicalTypeLimitCompare,
- TautologicalUnsignedZeroCompare,
- TautologicalUnsignedEnumZeroCompare]>;
-// For compatibility with GCC; -Wtype-limits = -Wtautological-constant-in-range-compare
-def TypeLimits : DiagGroup<"type-limits", [TautologicalInRangeCompare]>;
+ [TypeLimits, TautologicalValueRangeCompare]>;
def TautologicalOutOfRangeCompare : DiagGroup<"tautological-constant-out-of-range-compare">;
def TautologicalConstantCompare : DiagGroup<"tautological-constant-compare",
[TautologicalOutOfRangeCompare]>;
@@ -624,7 +628,6 @@ def ImplicitFallthrough : DiagGroup<"implicit-fallthrough",
def InvalidPPToken : DiagGroup<"invalid-pp-token">;
def Trigraphs : DiagGroup<"trigraphs">;
-def : DiagGroup<"type-limits">;
def UndefinedReinterpretCast : DiagGroup<"undefined-reinterpret-cast">;
def ReinterpretBaseClass : DiagGroup<"reinterpret-base-class">;
def Unicode : DiagGroup<"unicode">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 7bcff3eb4d8c5..1aeaf590cbb4b 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6588,6 +6588,12 @@ def warn_tautological_compare_objc_bool : Warning<
"result of comparison of constant %0 with expression of type 'BOOL'"
" is always %1, as the only well defined values for 'BOOL' are YES and NO">,
InGroup<TautologicalObjCBoolCompare>;
+def subst_int_range : TextSubstitution<"%0-bit %select{signed|unsigned}1 value">;
+def warn_tautological_compare_value_range : Warning<
+ "result of comparison of "
+ "%select{%4|%sub{subst_int_range}1,2}0 %3 "
+ "%select{%sub{subst_int_range}1,2|%4}0 is always %5">,
+ InGroup<TautologicalValueRangeCompare>, DefaultIgnore;
def warn_mixed_sign_comparison : Warning<
"comparison of integers of
diff erent signs: %0 and %1">,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 5c2092f1447ad..bbd856e9262e0 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10792,15 +10792,16 @@ static bool CheckTautologicalComparison(Sema &S, BinaryOperator *E,
S.Context.hasSameUnqualifiedType(Constant->getType(), Other->getType()))
return false;
- // TODO: Investigate using GetExprRange() to get tighter bounds
- // on the bit ranges.
+ IntRange OtherValueRange =
+ GetExprRange(S.Context, Other, S.isConstantEvaluated());
+
QualType OtherT = Other->getType();
if (const auto *AT = OtherT->getAs<AtomicType>())
OtherT = AT->getValueType();
- IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
+ IntRange OtherTypeRange = IntRange::forValueOfType(S.Context, OtherT);
// Special case for ObjC BOOL on targets where its a typedef for a signed char
- // (Namely, macOS).
+ // (Namely, macOS). FIXME: IntRange::forValueOfType should do this.
bool IsObjCSignedCharBool = S.getLangOpts().ObjC &&
S.NSAPIObj->isObjCBOOLType(OtherT) &&
OtherT->isSpecificBuiltinType(BuiltinType::SChar);
@@ -10810,17 +10811,32 @@ static bool CheckTautologicalComparison(Sema &S, BinaryOperator *E,
bool OtherIsBooleanDespiteType =
!OtherT->isBooleanType() && Other->isKnownToHaveBooleanValue();
if (OtherIsBooleanDespiteType || IsObjCSignedCharBool)
- OtherRange = IntRange::forBoolType();
+ OtherTypeRange = OtherValueRange = IntRange::forBoolType();
- // Determine the promoted range of the other type and see if a comparison of
- // the constant against that range is tautological.
- PromotedRange OtherPromotedRange(OtherRange, Value.getBitWidth(),
- Value.isUnsigned());
- auto Cmp = OtherPromotedRange.compare(Value);
+ // Check if all values in the range of possible values of this expression
+ // lead to the same comparison outcome.
+ PromotedRange OtherPromotedValueRange(OtherValueRange, Value.getBitWidth(),
+ Value.isUnsigned());
+ auto Cmp = OtherPromotedValueRange.compare(Value);
auto Result = PromotedRange::constantValue(E->getOpcode(), Cmp, RhsConstant);
if (!Result)
return false;
+ // Also consider the range determined by the type alone. This allows us to
+ // classify the warning under the proper diagnostic group.
+ bool TautologicalTypeCompare = false;
+ {
+ PromotedRange OtherPromotedTypeRange(OtherTypeRange, Value.getBitWidth(),
+ Value.isUnsigned());
+ auto TypeCmp = OtherPromotedTypeRange.compare(Value);
+ if (auto TypeResult = PromotedRange::constantValue(E->getOpcode(), TypeCmp,
+ RhsConstant)) {
+ TautologicalTypeCompare = true;
+ Cmp = TypeCmp;
+ Result = TypeResult;
+ }
+ }
+
// Suppress the diagnostic for an in-range comparison if the constant comes
// from a macro or enumerator. We don't want to diagnose
//
@@ -10849,6 +10865,14 @@ static bool CheckTautologicalComparison(Sema &S, BinaryOperator *E,
OS << Value;
}
+ if (!TautologicalTypeCompare) {
+ S.Diag(E->getOperatorLoc(), diag::warn_tautological_compare_value_range)
+ << RhsConstant << OtherValueRange.Width << OtherValueRange.NonNegative
+ << E->getOpcodeStr() << OS.str() << *Result
+ << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
+ return true;
+ }
+
if (IsObjCSignedCharBool) {
S.DiagRuntimeBehavior(E->getOperatorLoc(), E,
S.PDiag(diag::warn_tautological_compare_objc_bool)
diff --git a/clang/test/Sema/tautological-constant-compare.c b/clang/test/Sema/tautological-constant-compare.c
index 4f9b43b9f8982..7bf70eda7df4d 100644
--- a/clang/test/Sema/tautological-constant-compare.c
+++ b/clang/test/Sema/tautological-constant-compare.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST -verify %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST -verify -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST=2 -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST=2 -verify -x c++ %s
// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-type-limit-compare -DTEST -verify %s
// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-type-limit-compare -DTEST -verify -x c++ %s
// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtype-limits -DTEST -verify %s
@@ -545,9 +545,9 @@ int main()
if (maybe >= e)
return 0;
- // For the time being, use the declared type of bit-fields rather than their
- // length when determining whether a value is in-range.
- // FIXME: Reconsider this.
+ // We only warn on out-of-range bitfields and expressions with limited range
+ // under -Wtantological-in-range-compare, not under -Wtype-limits, because
+ // the warning is not based on the type alone.
struct A {
int a : 3;
unsigned b : 3;
@@ -555,13 +555,36 @@ int main()
unsigned long d : 3;
} a;
if (a.a < 3) {}
- if (a.a < 4) {}
+ if (a.a < 4) {} // #bitfield1
if (a.b < 7) {}
- if (a.b < 8) {}
+ if (a.b < 8) {} // #bitfield2
if (a.c < 3) {}
- if (a.c < 4) {}
+ if (a.c < 4) {} // #bitfield3
if (a.d < 7) {}
- if (a.d < 8) {}
+ if (a.d < 8) {} // #bitfield4
+#if TEST == 2
+ // expected-warning@#bitfield1 {{comparison of 3-bit signed value < 4 is always true}}
+ // expected-warning@#bitfield2 {{comparison of 3-bit unsigned value < 8 is always true}}
+ // expected-warning@#bitfield3 {{comparison of 3-bit signed value < 4 is always true}}
+ // expected-warning@#bitfield4 {{comparison of 3-bit unsigned value < 8 is always true}}
+#endif
+
+ if ((s & 0xff) < 0) {} // #valuerange1
+ if ((s & 0xff) < 1) {}
+ if ((s & -3) < -4) {} // #valuerange2
+ if ((s & -3) < -3) {}
+ if ((s & -3) < 4u) {} // (true if s non-negative)
+ if ((s & -3) > 4u) {} // (true if s negative)
+ if ((s & -3) == 4u) {} // #valuerange3 (never true)
+ if ((s & -3) == 3u) {}
+ if ((s & -3) == -5u) {} // #valuerange4
+ if ((s & -3) == -4u) {}
+#if TEST == 2
+ // expected-warning@#valuerange1 {{comparison of 8-bit unsigned value < 0 is always false}}
+ // expected-warning@#valuerange2 {{comparison of 3-bit signed value < -4 is always false}}
+ // expected-warning@#valuerange3 {{comparison of 3-bit signed value == 4 is always false}}
+ // expected-warning@#valuerange4 {{comparison of 3-bit signed value == 4294967291 is always false}}
+#endif
return 1;
}
More information about the cfe-commits
mailing list