lebedev.ri updated this revision to Diff 114260.
lebedev.ri marked 3 inline comments as done.
lebedev.ri added a comment.
Address review notes.
Repository:
rL LLVM
https://reviews.llvm.org/D37565
Files:
docs/ReleaseNotes.rst
lib/Sema/SemaChecking.cpp
test/Sema/compare.c
test/Sema/outof-range-constant-compare.c
test/SemaCXX/compare.cpp
Index: test/SemaCXX/compare.cpp
===================================================================
--- test/SemaCXX/compare.cpp
+++ test/SemaCXX/compare.cpp
@@ -73,16 +73,16 @@
((int) a == (unsigned int) B) +
((short) a == (unsigned short) B) +
((signed char) a == (unsigned char) B) +
- (a < (unsigned long) B) + // expected-warning {{comparison of integers of different signs}}
+ (a < (unsigned long) B) + // expected-warning {{comparison of unsigned expression < 0 is always false}}
(a < (unsigned int) B) +
(a < (unsigned short) B) +
(a < (unsigned char) B) +
((long) a < B) +
((int) a < B) +
((short) a < B) +
((signed char) a < B) +
- ((long) a < (unsigned long) B) + // expected-warning {{comparison of integers of different signs}}
- ((int) a < (unsigned int) B) + // expected-warning {{comparison of integers of different signs}}
+ ((long) a < (unsigned long) B) + // expected-warning {{comparison of unsigned expression < 0 is always false}}
+ ((int) a < (unsigned int) B) + // expected-warning {{comparison of unsigned expression < 0 is always false}}
((short) a < (unsigned short) B) +
((signed char) a < (unsigned char) B) +
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) // expected-warning {{comparison of unsigned expression < 0 is always false}}
+ return 0;
+ if (a <= 0x0000000000000000UL)
+ return 0;
+ if (a > 0x0000000000000000UL)
+ return 0;
+ if (a >= 0x0000000000000000UL) // expected-warning {{comparison of unsigned expression >= 0 is always true}}
+ return 0;
+
+ if (0x0000000000000000UL == a)
+ return 0;
+ if (0x0000000000000000UL != a)
+ return 0;
+ if (0x0000000000000000UL < a)
+ return 0;
+ if (0x0000000000000000UL <= a) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
+ return 0;
+ if (0x0000000000000000UL > a) // expected-warning {{comparison of 0 > unsigned expression is always false}}
+ 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: test/Sema/compare.c
===================================================================
--- test/Sema/compare.c
+++ test/Sema/compare.c
@@ -77,16 +77,16 @@
((int) a == (unsigned int) B) +
((short) a == (unsigned short) B) +
((signed char) a == (unsigned char) B) +
- (a < (unsigned long) B) + // expected-warning {{comparison of integers of different signs}}
+ (a < (unsigned long) B) + // expected-warning {{comparison of unsigned expression < 0 is always false}}
(a < (unsigned int) B) +
(a < (unsigned short) B) +
(a < (unsigned char) B) +
((long) a < B) +
((int) a < B) +
((short) a < B) +
((signed char) a < B) +
- ((long) a < (unsigned long) B) + // expected-warning {{comparison of integers of different signs}}
- ((int) a < (unsigned int) B) + // expected-warning {{comparison of integers of different signs}}
+ ((long) a < (unsigned long) B) + // expected-warning {{comparison of unsigned expression < 0 is always false}}
+ ((int) a < (unsigned int) B) + // expected-warning {{comparison of unsigned expression < 0 is always false}}
((short) a < (unsigned short) B) +
((signed char) a < (unsigned char) B) +
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -8567,32 +8567,51 @@
return E->getType()->isEnumeralType();
}
-void CheckTrivialUnsignedComparison(Sema &S, BinaryOperator *E) {
+bool isNonBooleanUnsignedValue(Expr *E) {
+ // We are checking that the expression is not known to have boolean value,
+ // is an integer type; and is either unsigned after implicit casts,
+ // or was unsigned before implicit casts.
+ return !E->isKnownToHaveBooleanValue() && E->getType()->isIntegerType() &&
+ (!E->getType()->isSignedIntegerType() ||
+ !E->IgnoreParenImpCasts()->getType()->isSignedIntegerType());
+}
+
+bool CheckTautologicalComparisonWithZero(Sema &S, BinaryOperator *E) {
// Disable warning in template instantiations.
if (S.inTemplateInstantiation())
- return;
+ return false;
+
+ // bool values are handled by DiagnoseOutOfRangeComparison().
BinaryOperatorKind op = E->getOpcode();
if (E->isValueDependent())
- return;
+ return false;
- if (op == BO_LT && IsZero(S, E->getRHS())) {
+ Expr *LHS = E->getLHS();
+ Expr *RHS = E->getRHS();
+
+ bool Match = true;
+
+ if (op == BO_LT && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) {
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 && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) {
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 && isNonBooleanUnsignedValue(RHS) && IsZero(S, LHS)) {
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 && isNonBooleanUnsignedValue(RHS) && IsZero(S, LHS)) {
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();
+ } else
+ Match = false;
+
+ return Match;
}
void DiagnoseOutOfRangeComparison(Sema &S, BinaryOperator *E, Expr *Constant,
@@ -8612,7 +8631,7 @@
bool OtherIsBooleanType = Other->isKnownToHaveBooleanValue();
- // 0 values are handled later by CheckTrivialUnsignedComparison().
+ // 0 values are handled later by CheckTautologicalComparisonWithZero().
if ((Value == 0) && (!OtherIsBooleanType))
return;
@@ -8849,16 +8868,22 @@
(IsRHSIntegralLiteral && IsLHSIntegralLiteral);
} else if (!T->hasUnsignedIntegerRepresentation())
IsComparisonConstant = E->isIntegerConstantExpr(S.Context);
-
+
+ // We don't care about value-dependent expressions or expressions
+ // whose result is a constant.
+ if (IsComparisonConstant)
+ return AnalyzeImpConvsInComparison(S, E);
+
+ // If this is a tautological comparison, suppress -Wsign-compare.
+ if (CheckTautologicalComparisonWithZero(S, E))
+ return AnalyzeImpConvsInComparison(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.
- //
- // We also don't care about value-dependent expressions or expressions
- // whose result is a constant.
- if (!T->hasUnsignedIntegerRepresentation() || IsComparisonConstant)
+ if (!T->hasUnsignedIntegerRepresentation())
return AnalyzeImpConvsInComparison(S, E);
-
+
// Check to see if one of the (unmodified) operands is of different
// signedness.
Expr *signedOperand, *unsignedOperand;
@@ -8871,7 +8896,6 @@
signedOperand = RHS;
unsignedOperand = LHS;
} else {
- CheckTrivialUnsignedComparison(S, E);
return AnalyzeImpConvsInComparison(S, E);
}
@@ -8883,11 +8907,9 @@
AnalyzeImplicitConversions(S, LHS, E->getOperatorLoc());
AnalyzeImplicitConversions(S, RHS, E->getOperatorLoc());
- // If the signed range is non-negative, -Wsign-compare won't fire,
- // but we should still check for comparisons which are always true
- // or false.
+ // If the signed range is non-negative, -Wsign-compare won't fire.
if (signedRange.NonNegative)
- return CheckTrivialUnsignedComparison(S, E);
+ return;
// For (in)equality comparisons, if the unsigned operand is a
// constant which cannot collide with a overflowed signed operand,
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -71,6 +71,13 @@
errors/warnings, as the system frameworks might add a method with the same
selector which could make the message send to ``id`` ambiguous.
+- ``-Wtautological-compare`` now warns when comparing an unsigned integer and 0
+ regardless of whether the constant is signed or unsigned."
+
+- ``-Wtautological-compare`` now warns about comparing a signed integer and 0
+ when the signed integer is coerced to an unsigned type for the comparison.
+ ``-Wsign-compare`` was adjusted not to warn in this case.
+
Non-comprehensive list of changes in this release
-------------------------------------------------
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
