Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -81,6 +81,8 @@ - ``-Wtautological-constant-compare`` is a new warning that warns on tautological comparisons between integer variable of the type ``T`` and the largest/smallest possible integer constant of that same type. + If the tautologicalness of the comparison is data model dependent, then the + ``-Wmaybe-tautological-constant-compare`` (disabled by default) is used. - For C code, ``-Wsign-compare``, ``-Wtautological-constant-compare`` and ``-Wtautological-constant-out-of-range-compare`` were adjusted to use the Index: include/clang/Basic/DiagnosticGroups.td =================================================================== --- include/clang/Basic/DiagnosticGroups.td +++ include/clang/Basic/DiagnosticGroups.td @@ -438,10 +438,12 @@ def TautologicalUnsignedZeroCompare : DiagGroup<"tautological-unsigned-zero-compare">; def TautologicalUnsignedEnumZeroCompare : DiagGroup<"tautological-unsigned-enum-zero-compare">; def TautologicalOutOfRangeCompare : DiagGroup<"tautological-constant-out-of-range-compare">; +def MaybeTautologicalConstantCompare : DiagGroup<"maybe-tautological-constant-compare">; def TautologicalConstantCompare : DiagGroup<"tautological-constant-compare", [TautologicalUnsignedZeroCompare, TautologicalUnsignedEnumZeroCompare, - TautologicalOutOfRangeCompare]>; + TautologicalOutOfRangeCompare, + MaybeTautologicalConstantCompare]>; def TautologicalPointerCompare : DiagGroup<"tautological-pointer-compare">; def TautologicalOverlapCompare : DiagGroup<"tautological-overlap-compare">; def TautologicalUndefinedCompare : DiagGroup<"tautological-undefined-compare">; Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -5972,6 +5972,10 @@ "comparison %select{%3|%1}0 %2 " "%select{%1|%3}0 is always %select{false|true}4">, InGroup; +def warn_maybe_tautological_constant_compare : Warning< + "with current data model, comparison %select{%3|%1}0 %2 " + "%select{%1|%3}0 is always %select{false|true}4">, + InGroup, DefaultIgnore; def warn_mixed_sign_comparison : Warning< "comparison of integers of different signs: %0 and %1">, Index: lib/Sema/SemaChecking.cpp =================================================================== --- lib/Sema/SemaChecking.cpp +++ lib/Sema/SemaChecking.cpp @@ -8159,6 +8159,11 @@ : Width(Width), NonNegative(NonNegative) {} + /// Indicate whether the specified integer ranges are identical. + friend bool operator==(const IntRange &LHS, const IntRange &RHS) { + return LHS.Width == RHS.Width && LHS.NonNegative == RHS.NonNegative; + } + /// Returns the range of the bool type. static IntRange forBoolType() { return IntRange(1, true); @@ -8588,12 +8593,42 @@ } enum class LimitType { - Max = 1U << 0U, // e.g. 32767 for short - Min = 1U << 1U, // e.g. -32768 for short - Both = Max | Min // When the value is both the Min and the Max limit at the - // same time; e.g. in C++, A::a in enum A { a = 0 }; + None = 0, // Just a zero-initalizer + Max = 1U << 0U, // e.g. 32767 for short + Min = 1U << 1U, // e.g. -32768 for short + Both = Max | Min, // When the value is both the Min and the Max limit at the + // same time; e.g. in C++, A::a in enum A { a = 0 }; + DataModelDependent = 1U << 2U, // is this limit is always the limit, or only + // for the current data model }; +/// LimitType is a bitset, thus a few helpers are needed +LimitType operator|(LimitType LHS, LimitType RHS) { + return static_cast( + static_cast::type>(LHS) | + static_cast::type>(RHS)); +} +LimitType operator&(LimitType LHS, LimitType RHS) { + return static_cast( + static_cast::type>(LHS) & + static_cast::type>(RHS)); +} +bool IsSet(llvm::Optional LHS, LimitType RHS) { + return LHS && ((*LHS) & RHS) == RHS; +} + +bool HasEnumType(Expr *E) { + // Strip off implicit integral promotions. + while (ImplicitCastExpr *ICE = dyn_cast(E)) { + if (ICE->getCastKind() != CK_IntegralCast && + ICE->getCastKind() != CK_NoOp) + break; + E = ICE->getSubExpr(); + } + + return E->getType()->isEnumeralType(); +} + /// Checks whether Expr 'Constant' may be the /// std::numeric_limits<>::max() or std::numeric_limits<>::min() /// of the Expr 'Other'. If true, then returns the limit type (min or max). @@ -8608,43 +8643,36 @@ // TODO: Investigate using GetExprRange() to get tighter bounds // on the bit ranges. - QualType OtherT = Other->IgnoreParenImpCasts()->getType(); - if (const auto *AT = OtherT->getAs()) - OtherT = AT->getValueType(); - + QualType ConstT = + Constant->IgnoreParenImpCasts()->getType()->getCanonicalTypeInternal(); + QualType OtherT = + Other->IgnoreParenImpCasts()->getType()->getCanonicalTypeInternal(); + IntRange ConstRange = IntRange::forValueOfType(S.Context, ConstT); IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT); // Special-case for C++ for enum with one enumerator with value of 0. if (OtherRange.Width == 0) return Value == 0 ? LimitType::Both : llvm::Optional(); + LimitType LT = LimitType::None; + if(ConstRange == OtherRange && ConstT != OtherT && !HasEnumType(Other)) + LT = LimitType::DataModelDependent; + if (llvm::APSInt::isSameValue( llvm::APSInt::getMaxValue(OtherRange.Width, OtherT->isUnsignedIntegerType()), Value)) - return LimitType::Max; + return LimitType::Max | LT; if (llvm::APSInt::isSameValue( llvm::APSInt::getMinValue(OtherRange.Width, OtherT->isUnsignedIntegerType()), Value)) - return LimitType::Min; + return LimitType::Min | LT; return llvm::None; } -bool HasEnumType(Expr *E) { - // Strip off implicit integral promotions. - while (ImplicitCastExpr *ICE = dyn_cast(E)) { - if (ICE->getCastKind() != CK_IntegralCast && - ICE->getCastKind() != CK_NoOp) - break; - E = ICE->getSubExpr(); - } - - return E->getType()->isEnumeralType(); -} - bool CheckTautologicalComparison(Sema &S, BinaryOperator *E, Expr *Constant, Expr *Other, const llvm::APSInt &Value, bool RhsConstant) { @@ -8665,9 +8693,9 @@ bool ConstIsLowerBound = (Op == BO_LT || Op == BO_LE) ^ RhsConstant; bool ResultWhenConstEqualsOther = (Op == BO_LE || Op == BO_GE); - if (ValueType != LimitType::Both) { + if (!IsSet(ValueType, LimitType::Both)) { bool ResultWhenConstNeOther = - ConstIsLowerBound ^ (ValueType == LimitType::Max); + ConstIsLowerBound ^ IsSet(ValueType, LimitType::Max); if (ResultWhenConstEqualsOther != ResultWhenConstNeOther) return false; // The comparison is not tautological. } else if (ResultWhenConstEqualsOther == ConstIsLowerBound) @@ -8679,7 +8707,9 @@ ? (HasEnumType(Other) ? diag::warn_unsigned_enum_always_true_comparison : diag::warn_unsigned_always_true_comparison) - : diag::warn_tautological_constant_compare; + : IsSet(ValueType, LimitType::DataModelDependent) + ? diag::warn_maybe_tautological_constant_compare + : diag::warn_tautological_constant_compare; // Should be enough for uint128 (39 decimal digits) SmallString<64> PrettySourceValue; Index: test/Sema/maybe-tautological-constant-compare.c =================================================================== --- /dev/null +++ test/Sema/maybe-tautological-constant-compare.c @@ -0,0 +1,342 @@ +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -DLP64 -Wmaybe-tautological-constant-compare -DTEST -verify %s +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -DLP64 -verify %s +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -DLP64 -Wmaybe-tautological-constant-compare -DTEST -verify -x c++ %s +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -DLP64 -verify -x c++ %s +// RUN: %clang_cc1 -triple i386-linux-gnu -fsyntax-only -DILP32 -Wmaybe-tautological-constant-compare -DTEST -verify %s +// RUN: %clang_cc1 -triple i386-linux-gnu -fsyntax-only -DILP32 -verify %s +// R1UN: %clang_cc1 -triple i386-linux-gnu -fsyntax-only -DILP32 -Wmaybe-tautological-constant-compare -DTEST -verify -x c++ %s +// R1UN: %clang_cc1 -triple i386-linux-gnu -fsyntax-only -DILP32 -verify -x c++ %s + +int value(void); + +int main() { + unsigned long ul = value(); + +#if defined(LP64) +#if __SIZEOF_INT__ != 4 +#error Expecting 32-bit int +#endif +#if __SIZEOF_LONG__ != 8 +#error Expecting 64-bit int +#endif +#elif defined(ILP32) +#if __SIZEOF_INT__ != 4 +#error Expecting 32-bit int +#endif +#if __SIZEOF_LONG__ != 4 +#error Expecting 32-bit int +#endif +#else +#error LP64 or ILP32 should be defined +#endif + +#if defined(LP64) + // expected-no-diagnostics + + if (ul < 4294967295) + return 0; + if (4294967295 >= ul) + return 0; + if (ul > 4294967295) + return 0; + if (4294967295 <= ul) + return 0; + if (ul <= 4294967295) + return 0; + if (4294967295 > ul) + return 0; + if (ul >= 4294967295) + return 0; + if (4294967295 < ul) + return 0; + if (ul == 4294967295) + return 0; + if (4294967295 != ul) + return 0; + if (ul != 4294967295) + return 0; + if (4294967295 == ul) + return 0; + + if (ul < 4294967295U) + return 0; + if (4294967295U >= ul) + return 0; + if (ul > 4294967295U) + return 0; + if (4294967295U <= ul) + return 0; + if (ul <= 4294967295U) + return 0; + if (4294967295U > ul) + return 0; + if (ul >= 4294967295U) + return 0; + if (4294967295U < ul) + return 0; + if (ul == 4294967295U) + return 0; + if (4294967295U != ul) + return 0; + if (ul != 4294967295U) + return 0; + if (4294967295U == ul) + return 0; + + if (ul < 4294967295L) + return 0; + if (4294967295L >= ul) + return 0; + if (ul > 4294967295L) + return 0; + if (4294967295L <= ul) + return 0; + if (ul <= 4294967295L) + return 0; + if (4294967295L > ul) + return 0; + if (ul >= 4294967295L) + return 0; + if (4294967295L < ul) + return 0; + if (ul == 4294967295L) + return 0; + if (4294967295L != ul) + return 0; + if (ul != 4294967295L) + return 0; + if (4294967295L == ul) + return 0; + + if (ul < 4294967295UL) + return 0; + if (4294967295UL >= ul) + return 0; + if (ul > 4294967295UL) + return 0; + if (4294967295UL <= ul) + return 0; + if (ul <= 4294967295UL) + return 0; + if (4294967295UL > ul) + return 0; + if (ul >= 4294967295UL) + return 0; + if (4294967295UL < ul) + return 0; + if (ul == 4294967295UL) + return 0; + if (4294967295UL != ul) + return 0; + if (ul != 4294967295UL) + return 0; + if (4294967295UL == ul) + return 0; +#elif defined(ILP32) +#if defined(TEST) + if (ul < 4294967295) + return 0; + if (4294967295 >= ul) // expected-warning {{comparison 4294967295 >= 'unsigned long' is always true}} + return 0; + if (ul > 4294967295) // expected-warning {{comparison 'unsigned long' > 4294967295 is always false}} + return 0; + if (4294967295 <= ul) + return 0; + if (ul <= 4294967295) // expected-warning {{comparison 'unsigned long' <= 4294967295 is always true}} + return 0; + if (4294967295 > ul) + return 0; + if (ul >= 4294967295) + return 0; + if (4294967295 < ul) // expected-warning {{comparison 4294967295 < 'unsigned long' is always false}} + return 0; + if (ul == 4294967295) + return 0; + if (4294967295 != ul) + return 0; + if (ul != 4294967295) + return 0; + if (4294967295 == ul) + return 0; + + if (ul < 4294967295U) + return 0; + if (4294967295U >= ul) // expected-warning {{with current data model, comparison 4294967295 >= 'unsigned long' is always true}} + return 0; + if (ul > 4294967295U) // expected-warning {{with current data model, comparison 'unsigned long' > 4294967295 is always false}} + return 0; + if (4294967295U <= ul) + return 0; + if (ul <= 4294967295U) // expected-warning {{with current data model, comparison 'unsigned long' <= 4294967295 is always true}} + return 0; + if (4294967295U > ul) + return 0; + if (ul >= 4294967295U) + return 0; + if (4294967295U < ul) // expected-warning {{with current data model, comparison 4294967295 < 'unsigned long' is always false}} + return 0; + if (ul == 4294967295U) + return 0; + if (4294967295U != ul) + return 0; + if (ul != 4294967295U) + return 0; + if (4294967295U == ul) + return 0; + + if (ul < 4294967295L) + return 0; + if (4294967295L >= ul) // expected-warning {{comparison 4294967295 >= 'unsigned long' is always true}} + return 0; + if (ul > 4294967295L) // expected-warning {{comparison 'unsigned long' > 4294967295 is always false}} + return 0; + if (4294967295L <= ul) + return 0; + if (ul <= 4294967295L) // expected-warning {{comparison 'unsigned long' <= 4294967295 is always true}} + return 0; + if (4294967295L > ul) + return 0; + if (ul >= 4294967295L) + return 0; + if (4294967295L < ul) // expected-warning {{comparison 4294967295 < 'unsigned long' is always false}} + return 0; + if (ul == 4294967295L) + return 0; + if (4294967295L != ul) + return 0; + if (ul != 4294967295L) + return 0; + if (4294967295L == ul) + return 0; + + if (ul < 4294967295UL) + return 0; + if (4294967295UL >= ul) // expected-warning {{comparison 4294967295 >= 'unsigned long' is always true}} + return 0; + if (ul > 4294967295UL) // expected-warning {{comparison 'unsigned long' > 4294967295 is always false}} + return 0; + if (4294967295UL <= ul) + return 0; + if (ul <= 4294967295UL) // expected-warning {{comparison 'unsigned long' <= 4294967295 is always true}} + return 0; + if (4294967295UL > ul) + return 0; + if (ul >= 4294967295UL) + return 0; + if (4294967295UL < ul) // expected-warning {{comparison 4294967295 < 'unsigned long' is always false}} + return 0; + if (ul == 4294967295UL) + return 0; + if (4294967295UL != ul) + return 0; + if (ul != 4294967295UL) + return 0; + if (4294967295UL == ul) + return 0; +#else // defined(TEST) + if (ul < 4294967295) + return 0; + if (4294967295 >= ul) // expected-warning {{comparison 4294967295 >= 'unsigned long' is always true}} + return 0; + if (ul > 4294967295) // expected-warning {{comparison 'unsigned long' > 4294967295 is always false}} + return 0; + if (4294967295 <= ul) + return 0; + if (ul <= 4294967295) // expected-warning {{comparison 'unsigned long' <= 4294967295 is always true}} + return 0; + if (4294967295 > ul) + return 0; + if (ul >= 4294967295) + return 0; + if (4294967295 < ul) // expected-warning {{comparison 4294967295 < 'unsigned long' is always false}} + return 0; + if (ul == 4294967295) + return 0; + if (4294967295 != ul) + return 0; + if (ul != 4294967295) + return 0; + if (4294967295 == ul) + return 0; + + if (ul < 4294967295U) + return 0; + if (4294967295U >= ul) + return 0; + if (ul > 4294967295U) + return 0; + if (4294967295U <= ul) + return 0; + if (ul <= 4294967295U) + return 0; + if (4294967295U > ul) + return 0; + if (ul >= 4294967295U) + return 0; + if (4294967295U < ul) + return 0; + if (ul == 4294967295U) + return 0; + if (4294967295U != ul) + return 0; + if (ul != 4294967295U) + return 0; + if (4294967295U == ul) + return 0; + + if (ul < 4294967295L) + return 0; + if (4294967295L >= ul) // expected-warning {{comparison 4294967295 >= 'unsigned long' is always true}} + return 0; + if (ul > 4294967295L) // expected-warning {{comparison 'unsigned long' > 4294967295 is always false}} + return 0; + if (4294967295L <= ul) + return 0; + if (ul <= 4294967295L) // expected-warning {{comparison 'unsigned long' <= 4294967295 is always true}} + return 0; + if (4294967295L > ul) + return 0; + if (ul >= 4294967295L) + return 0; + if (4294967295L < ul) // expected-warning {{comparison 4294967295 < 'unsigned long' is always false}} + return 0; + if (ul == 4294967295L) + return 0; + if (4294967295L != ul) + return 0; + if (ul != 4294967295L) + return 0; + if (4294967295L == ul) + return 0; + + if (ul < 4294967295UL) + return 0; + if (4294967295UL >= ul) // expected-warning {{comparison 4294967295 >= 'unsigned long' is always true}} + return 0; + if (ul > 4294967295UL) // expected-warning {{comparison 'unsigned long' > 4294967295 is always false}} + return 0; + if (4294967295UL <= ul) + return 0; + if (ul <= 4294967295UL) // expected-warning {{comparison 'unsigned long' <= 4294967295 is always true}} + return 0; + if (4294967295UL > ul) + return 0; + if (ul >= 4294967295UL) + return 0; + if (4294967295UL < ul) // expected-warning {{comparison 4294967295 < 'unsigned long' is always false}} + return 0; + if (ul == 4294967295UL) + return 0; + if (4294967295UL != ul) + return 0; + if (ul != 4294967295UL) + return 0; + if (4294967295UL == ul) + return 0; +#endif // defined(TEST) +#else +#error LP64 or ILP32 should be defined +#endif + + return 1; +}