lebedev.ri created this revision.
lebedev.ri added a project: clang.
This is a first half(?) of a fix for the following bug:
https://bugs.llvm.org/show_bug.cgi?id=34147 (gcc -Wtype-limits)
GCC's -Wtype-limits does warn on comparison of unsigned value
with signed zero (as in, with 0), but clang only warns if the
zero is unsigned (i.e. 0U).
Also, be careful not to double-warn, or falsely warn on
comparison of signed/fp variable and signed 0.
Yes, all these testcases are needed.
Testing: $ ninja check-clang-sema check-clang-semacxx
Also, no new warnings for clang stage-2 build.
Repository:
rL LLVM
https://reviews.llvm.org/D37565
Files:
lib/Sema/SemaChecking.cpp
test/Sema/outof-range-constant-compare.c
Index: test/Sema/outof-range-constant-compare.c
===================================================================
--- test/Sema/outof-range-constant-compare.c
+++ test/Sema/outof-range-constant-compare.c
@@ -6,6 +6,59 @@
int main()
{
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)
+ return 0;
+ if (a <= 0x0000000000000000UL)
+ return 0;
+ if (a > 0x0000000000000000UL)
+ return 0;
+ if (a >= 0x0000000000000000UL)
+ return 0;
+
+ if (0x0000000000000000UL == a)
+ return 0;
+ if (0x0000000000000000UL != a)
+ return 0;
+ if (0x0000000000000000UL < a)
+ return 0;
+ if (0x0000000000000000UL <= a)
+ return 0;
+ if (0x0000000000000000UL > a)
+ 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}}
@@ -74,6 +127,7 @@
return 0;
if (0x1234567812345678L >= s) // expected-warning {{comparison of constant 1311768465173141112 with expression of type 'short' is always true}}
return 0;
+
long l = value();
if (l == 0x1234567812345678L)
return 0;
@@ -106,34 +160,107 @@
return 0;
if (un != 0x0000000000000000L)
return 0;
- if (un < 0x0000000000000000L)
+ 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)
+ 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)
+ if (0x0000000000000000L <= un) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
return 0;
- if (0x0000000000000000L > un)
+ if (0x0000000000000000L > un) // expected-warning {{comparison of 0 > unsigned expression is always false}}
return 0;
if (0x0000000000000000L >= un)
return 0;
- float fl = 0;
- if (fl == 0x0000000000000000L) // no warning
- return 0;
- float dl = 0;
- if (dl == 0x0000000000000000L) // no warning
- 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,
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -8572,26 +8572,49 @@
if (S.inTemplateInstantiation())
return;
+ // bool values are handled by DiagnoseOutOfRangeComparison().
+
BinaryOperatorKind op = E->getOpcode();
if (E->isValueDependent())
return;
- if (op == BO_LT && IsZero(S, E->getRHS())) {
+ Expr *LHS = E->getLHS()->IgnoreParenImpCasts();
+ Expr *RHS = E->getRHS()->IgnoreParenImpCasts();
+
+ QualType LType = LHS->getType();
+ QualType RType = RHS->getType();
+
+ // if the non-constant size is signed/boolean value/not integer,
+ // then we don't warn here
+ bool BadL = !LType->isIntegerType() || LType->isSignedIntegerType() ||
+ LHS->isKnownToHaveBooleanValue();
+ bool BadR = !RType->isIntegerType() || RType->isSignedIntegerType() ||
+ RHS->isKnownToHaveBooleanValue();
+
+ if (op == BO_LT && IsZero(S, RHS)) {
+ if (BadL)
+ return;
S.Diag(E->getOperatorLoc(), diag::warn_lunsigned_always_true_comparison)
- << "< 0" << "false" << HasEnumType(E->getLHS())
- << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
- } else if (op == BO_GE && IsZero(S, E->getRHS())) {
+ << "< 0" << "false" << HasEnumType(LHS)
+ << LHS->getSourceRange() << RHS->getSourceRange();
+ } else if (op == BO_GE && IsZero(S, RHS)) {
+ if (BadL)
+ return;
S.Diag(E->getOperatorLoc(), diag::warn_lunsigned_always_true_comparison)
- << ">= 0" << "true" << HasEnumType(E->getLHS())
- << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
- } else if (op == BO_GT && IsZero(S, E->getLHS())) {
+ << ">= 0" << "true" << HasEnumType(LHS)
+ << LHS->getSourceRange() << RHS->getSourceRange();
+ } else if (op == BO_GT && IsZero(S, LHS)) {
+ if (BadR)
+ return;
S.Diag(E->getOperatorLoc(), diag::warn_runsigned_always_true_comparison)
- << "0 >" << "false" << HasEnumType(E->getRHS())
- << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
- } else if (op == BO_LE && IsZero(S, E->getLHS())) {
+ << "0 >" << "false" << HasEnumType(RHS)
+ << LHS->getSourceRange() << RHS->getSourceRange();
+ } else if (op == BO_LE && IsZero(S, LHS)) {
+ if (BadR)
+ return;
S.Diag(E->getOperatorLoc(), diag::warn_runsigned_always_true_comparison)
- << "0 <=" << "true" << HasEnumType(E->getRHS())
- << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
+ << "0 <=" << "true" << HasEnumType(RHS)
+ << LHS->getSourceRange() << RHS->getSourceRange();
}
}
@@ -8849,14 +8872,10 @@
(IsRHSIntegralLiteral && IsLHSIntegralLiteral);
} else if (!T->hasUnsignedIntegerRepresentation())
IsComparisonConstant = E->isIntegerConstantExpr(S.Context);
-
- // 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.
- //
- // We also don't care about value-dependent expressions or expressions
+
+ // We don't care about value-dependent expressions or expressions
// whose result is a constant.
- if (!T->hasUnsignedIntegerRepresentation() || IsComparisonConstant)
+ if (IsComparisonConstant)
return AnalyzeImpConvsInComparison(S, E);
// Check to see if one of the (unmodified) operands is of different
@@ -8889,6 +8908,12 @@
if (signedRange.NonNegative)
return CheckTrivialUnsignedComparison(S, E);
+ // 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())
+ return AnalyzeImpConvsInComparison(S, E);
+
// For (in)equality comparisons, if the unsigned operand is a
// constant which cannot collide with a overflowed signed operand,
// then reinterpreting the signed operand as unsigned will not
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
