Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -71,6 +71,10 @@ 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`` warning on comparison of unsigned integer and + ``0`` constant was adjusted to warn regardless of whether the constant is + signed or unsigned. + Non-comprehensive list of changes in this release ------------------------------------------------- Index: lib/Sema/SemaChecking.cpp =================================================================== --- lib/Sema/SemaChecking.cpp +++ lib/Sema/SemaChecking.cpp @@ -8567,31 +8567,43 @@ return E->getType()->isEnumeralType(); } -void CheckTrivialUnsignedComparison(Sema &S, BinaryOperator *E) { +bool isNonBooleanUnsignedValue(Expr *E) { + QualType Type = E->getType(); + + return !E->isKnownToHaveBooleanValue() && Type->isIntegerType() && + !Type->isSignedIntegerType(); +} + +void CheckTautologicalComparisonWithZero(Sema &S, BinaryOperator *E) { // Disable warning in template instantiations. 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(); + + 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(); } } @@ -8612,7 +8624,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 +8861,20 @@ (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); + CheckTautologicalComparisonWithZero(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 +8887,6 @@ signedOperand = RHS; unsignedOperand = LHS; } else { - CheckTrivialUnsignedComparison(S, E); return AnalyzeImpConvsInComparison(S, E); } @@ -8883,11 +8898,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: 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,13 +160,13 @@ 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) @@ -121,19 +175,92 @@ 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,