For what it's worth, Chromium's bitfield warning is easy to fix. The "-1 vs enum" comparisons are more plentiful and less straightforward. I'm guessing it's the same for the self-host.
On Fri, Dec 8, 2017 at 1:33 PM, Richard Smith via cfe-commits <cfe-commits@lists.llvm.org> wrote: > I'm going to reland without the changes to bit-field handling. If we want > those changes (which I think we do, based on the bugs it found in selfhost), > that'll need to be done more carefully, and I don't want to get the > refactoring and bugfixes here tangled up with that. > > On 8 December 2017 at 13:27, Bill Seurer via cfe-commits > <cfe-commits@lists.llvm.org> wrote: >> >> It also caused all the sanitizer builds to fail. For example: >> >> http://lab.llvm.org:8011/builders/sanitizer-ppc64le-linux/builds/3654 >> >> >> /home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm/utils/TableGen/X86RecognizableInstr.cpp:789:21: >> error: comparison of constant -1 with expression of type >> 'llvm::X86Disassembler::OpcodeType' is always true >> [-Werror,-Wtautological-constant-out-of-range-compare] >> assert(opcodeType != (OpcodeType)-1 && >> ~~~~~~~~~~ ^ ~~~~~~~~~~~~~~ >> /usr/include/assert.h:86:5: note: expanded from macro 'assert' >> ((expr) \ >> ^~~~ >> 1 error generated. >> utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/build.make:1022: recipe for >> target >> 'utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/X86RecognizableInstr.cpp.o' >> failed >> make[3]: *** >> [utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/X86RecognizableInstr.cpp.o] >> Error 1 >> >> >> >> And there are lots more warnings (which are flagged as errors for the >> sanitizer builds) when I run a build by hand. For example: >> >> [4001/4008] Building CXX object >> tools/clang/tools/libclang/CMakeFiles/libclang.dir/CIndex.cpp.o >> In file included from >> /home/seurer/llvm/llvm-test2/tools/clang/tools/libclang/CIndex.cpp:23: >> In file included from >> /home/seurer/llvm/llvm-test2/tools/clang/tools/libclang/CursorVisitor.h:16: >> In file included from >> /home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/DeclVisitor.h:19: >> In file included from >> /home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/DeclCXX.h:21: >> In file included from >> /home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/Attr.h:19: >> /home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/Expr.h:2745:17: >> warning: comparison of constant -1 with expression of type 'const >> clang::CastKind' is always true >> [-Wtautological-constant-out-of-range-compare] >> assert(kind != CK_Invalid && "creating cast with invalid cast kind"); >> ~~~~ ^ ~~~~~~~~~~ >> /usr/include/assert.h:89:5: note: expanded from macro 'assert' >> ((expr) \ >> ^~~~ >> >> >> >> On 12/08/2017 10:54 AM, Hans Wennborg via cfe-commits wrote: >>> >>> Author: hans >>> Date: Fri Dec 8 08:54:08 2017 >>> New Revision: 320162 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=320162&view=rev >>> Log: >>> Revert "Unify implementation of our two different flavours of >>> -Wtautological-compare." >>> >>>> Unify implementation of our two different flavours of >>>> -Wtautological-compare. >>>> >>>> In so doing, fix a handful of remaining bugs where we would report false >>>> positives or false negatives if we promote a signed value to an unsigned >>>> type >>>> for the comparison. >>> >>> >>> This caused a new warning in Chromium: >>> >>> ../../base/trace_event/trace_log.cc:1545:29: error: comparison of >>> constant 64 >>> with expression of type 'unsigned int' is always true >>> [-Werror,-Wtautological-constant-out-of-range-compare] >>> DCHECK(handle.event_index < TraceBufferChunk::kTraceBufferChunkSize); >>> ~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>> >>> The 'unsigned int' is really a 6-bit bitfield, which is why it's always >>> less than 64. >>> >>> I thought we didn't use to warn (with out-of-range-compare) when >>> comparing >>> against the boundaries of a type? >>> >>> Modified: >>> cfe/trunk/lib/Sema/SemaChecking.cpp >>> cfe/trunk/test/Sema/tautological-constant-compare.c >>> cfe/trunk/test/Sema/tautological-constant-enum-compare.c >>> cfe/trunk/test/SemaCXX/compare.cpp >>> >>> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=320162&r1=320161&r2=320162&view=diff >>> >>> ============================================================================== >>> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) >>> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Dec 8 08:54:08 2017 >>> @@ -8662,113 +8662,54 @@ static bool isKnownToHaveUnsignedValue(E >>> } >>> namespace { >>> -/// The promoted range of values of a type. In general this has the >>> -/// following structure: >>> -/// >>> -/// |-----------| . . . |-----------| >>> -/// ^ ^ ^ ^ >>> -/// Min HoleMin HoleMax Max >>> -/// >>> -/// ... where there is only a hole if a signed type is promoted to >>> unsigned >>> -/// (in which case Min and Max are the smallest and largest >>> representable >>> -/// values). >>> -struct PromotedRange { >>> - // Min, or HoleMax if there is a hole. >>> - llvm::APSInt PromotedMin; >>> - // Max, or HoleMin if there is a hole. >>> - llvm::APSInt PromotedMax; >>> - >>> - PromotedRange(IntRange R, unsigned BitWidth, bool Unsigned) { >>> - if (R.Width == 0) >>> - PromotedMin = PromotedMax = llvm::APSInt(BitWidth, Unsigned); >>> - else { >>> - PromotedMin = llvm::APSInt::getMinValue(R.Width, R.NonNegative) >>> - .extOrTrunc(BitWidth); >>> - PromotedMin.setIsUnsigned(Unsigned); >>> - >>> - PromotedMax = llvm::APSInt::getMaxValue(R.Width, R.NonNegative) >>> - .extOrTrunc(BitWidth); >>> - PromotedMax.setIsUnsigned(Unsigned); >>> - } >>> - } >>> - // Determine whether this range is contiguous (has no hole). >>> - bool isContiguous() const { return PromotedMin <= PromotedMax; } >>> +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 }; >>> +}; >>> - // Where a constant value is within the range. >>> - enum ComparisonResult { >>> - LT = 0x1, >>> - LE = 0x2, >>> - GT = 0x4, >>> - GE = 0x8, >>> - EQ = 0x10, >>> - NE = 0x20, >>> - InRangeFlag = 0x40, >>> - >>> - Less = LE | LT | NE, >>> - Min = LE | InRangeFlag, >>> - InRange = InRangeFlag, >>> - Max = GE | InRangeFlag, >>> - Greater = GE | GT | NE, >>> +} // namespace >>> - OnlyValue = LE | GE | EQ | InRangeFlag, >>> - InHole = NE >>> - }; >>> +/// 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). >>> +/// The Value is the evaluation of Constant >>> +static llvm::Optional<LimitType> IsTypeLimit(Sema &S, Expr *Constant, >>> + Expr *Other, >>> + const llvm::APSInt &Value) >>> { >>> + if (IsEnumConstOrFromMacro(S, Constant)) >>> + return llvm::Optional<LimitType>(); >>> - ComparisonResult compare(const llvm::APSInt &Value) const { >>> - assert(Value.getBitWidth() == PromotedMin.getBitWidth() && >>> - Value.isUnsigned() == PromotedMin.isUnsigned()); >>> - if (!isContiguous()) { >>> - assert(Value.isUnsigned() && "discontiguous range for signed >>> compare"); >>> - if (Value.isMinValue()) return Min; >>> - if (Value.isMaxValue()) return Max; >>> - if (Value >= PromotedMin) return InRange; >>> - if (Value <= PromotedMax) return InRange; >>> - return InHole; >>> - } >>> + if (isKnownToHaveUnsignedValue(Other) && Value == 0) >>> + return LimitType::Min; >>> - switch (llvm::APSInt::compareValues(Value, PromotedMin)) { >>> - case -1: return Less; >>> - case 0: return PromotedMin == PromotedMax ? OnlyValue : Min; >>> - case 1: >>> - switch (llvm::APSInt::compareValues(Value, PromotedMax)) { >>> - case -1: return InRange; >>> - case 0: return Max; >>> - case 1: return Greater; >>> - } >>> - } >>> + // TODO: Investigate using GetExprRange() to get tighter bounds >>> + // on the bit ranges. >>> + QualType OtherT = Other->IgnoreParenImpCasts()->getType(); >>> + if (const auto *AT = OtherT->getAs<AtomicType>()) >>> + OtherT = AT->getValueType(); >>> - llvm_unreachable("impossible compare result"); >>> - } >>> + IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT); >>> + if (Other->isKnownToHaveBooleanValue()) >>> + OtherRange = IntRange::forBoolType(); >>> - static llvm::Optional<bool> constantValue(BinaryOperatorKind Op, >>> - ComparisonResult R, >>> - bool ConstantOnRHS) { >>> - ComparisonResult TrueFlag, FalseFlag; >>> - if (Op == BO_EQ) { >>> - TrueFlag = EQ; >>> - FalseFlag = NE; >>> - } else if (Op == BO_NE) { >>> - TrueFlag = NE; >>> - FalseFlag = EQ; >>> - } else { >>> - if ((Op == BO_LT || Op == BO_GE) ^ ConstantOnRHS) { >>> - TrueFlag = LT; >>> - FalseFlag = GE; >>> - } else { >>> - TrueFlag = GT; >>> - FalseFlag = LE; >>> - } >>> - if (Op == BO_GE || Op == BO_LE) >>> - std::swap(TrueFlag, FalseFlag); >>> - } >>> - if (R & TrueFlag) >>> - return true; >>> - if (R & FalseFlag) >>> - return false; >>> - return llvm::None; >>> - } >>> -}; >>> + // 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>(); >>> + >>> + if (llvm::APSInt::isSameValue( >>> + llvm::APSInt::getMaxValue(OtherRange.Width, >>> OtherRange.NonNegative), >>> + Value)) >>> + return LimitType::Max; >>> + >>> + if (llvm::APSInt::isSameValue( >>> + llvm::APSInt::getMinValue(OtherRange.Width, >>> OtherRange.NonNegative), >>> + Value)) >>> + return LimitType::Min; >>> + >>> + return llvm::None; >>> } >>> static bool HasEnumType(Expr *E) { >>> @@ -8806,47 +8747,29 @@ static bool CheckTautologicalComparison( >>> if (S.inTemplateInstantiation() || !E->isRelationalOp()) >>> return false; >>> - if (IsEnumConstOrFromMacro(S, Constant)) >>> - return false; >>> - >>> - Expr *OriginalOther = Other; >>> + BinaryOperatorKind Op = E->getOpcode(); >>> - Constant = Constant->IgnoreParenImpCasts(); >>> - Other = Other->IgnoreParenImpCasts(); >>> - >>> - // TODO: Investigate using GetExprRange() to get tighter bounds >>> - // on the bit ranges. >>> - QualType OtherT = Other->getType(); >>> - if (const auto *AT = OtherT->getAs<AtomicType>()) >>> - OtherT = AT->getValueType(); >>> - IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT); >>> - >>> - // Whether we're treating Other as being a bool because of the form of >>> - // expression despite it having another type (typically 'int' in C). >>> - bool OtherIsBooleanDespiteType = >>> - !OtherT->isBooleanType() && Other->isKnownToHaveBooleanValue(); >>> - if (OtherIsBooleanDespiteType) >>> - OtherRange = IntRange::forBoolType(); >>> - >>> - if (FieldDecl *Bitfield = Other->getSourceBitField()) >>> - if (!Bitfield->getBitWidth()->isValueDependent()) >>> - OtherRange.Width = >>> - std::min(Bitfield->getBitWidthValue(S.Context), >>> OtherRange.Width); >>> - >>> - // Check whether the constant value can be represented in OtherRange. >>> Bail >>> - // out if so; this isn't an out-of-range comparison. >>> - PromotedRange OtherPromotedRange(OtherRange, Value.getBitWidth(), >>> - Value.isUnsigned()); >>> - >>> - auto Cmp = OtherPromotedRange.compare(Value); >>> - if (Cmp != PromotedRange::Min && Cmp != PromotedRange::Max && >>> - Cmp != PromotedRange::OnlyValue) >>> + QualType OType = Other->IgnoreParenImpCasts()->getType(); >>> + if (!OType->isIntegerType()) >>> return false; >>> - auto Result = PromotedRange::constantValue(E->getOpcode(), Cmp, >>> RhsConstant); >>> - if (!Result) >>> + // Determine which limit (min/max) the constant is, if either. >>> + llvm::Optional<LimitType> ValueType = IsTypeLimit(S, Constant, Other, >>> Value); >>> + if (!ValueType) >>> return false; >>> + bool ConstIsLowerBound = (Op == BO_LT || Op == BO_LE) ^ RhsConstant; >>> + bool ResultWhenConstEqualsOther = (Op == BO_LE || Op == BO_GE); >>> + if (ValueType != LimitType::Both) { >>> + bool ResultWhenConstNeOther = >>> + ConstIsLowerBound ^ (ValueType == LimitType::Max); >>> + if (ResultWhenConstEqualsOther != ResultWhenConstNeOther) >>> + return false; // The comparison is not tautological. >>> + } else if (ResultWhenConstEqualsOther == ConstIsLowerBound) >>> + return false; // The comparison is not tautological. >>> + >>> + const bool Result = ResultWhenConstEqualsOther; >>> + >>> // Should be enough for uint128 (39 decimal digits) >>> SmallString<64> PrettySourceValue; >>> llvm::raw_svector_ostream OS(PrettySourceValue); >>> @@ -8859,20 +8782,20 @@ static bool CheckTautologicalComparison( >>> S.DiagRuntimeBehavior( >>> E->getOperatorLoc(), E, >>> S.PDiag(diag::warn_tautological_bool_compare) >>> - << OS.str() << classifyConstantValue(Constant) >>> - << OtherT << !OtherT->isBooleanType() << *Result >>> + << OS.str() << >>> classifyConstantValue(Constant->IgnoreParenImpCasts()) >>> + << OType << !OType->isBooleanType() << Result >>> << E->getLHS()->getSourceRange() << >>> E->getRHS()->getSourceRange()); >>> return true; >>> } >>> - unsigned Diag = (isKnownToHaveUnsignedValue(OriginalOther) && Value >>> == 0) >>> - ? (HasEnumType(OriginalOther) >>> + unsigned Diag = (isKnownToHaveUnsignedValue(Other) && Value == 0) >>> + ? (HasEnumType(Other) >>> ? >>> diag::warn_unsigned_enum_always_true_comparison >>> : >>> diag::warn_unsigned_always_true_comparison) >>> : diag::warn_tautological_constant_compare; >>> S.Diag(E->getOperatorLoc(), Diag) >>> - << RhsConstant << OtherT << E->getOpcodeStr() << OS.str() << >>> *Result >>> + << RhsConstant << OType << E->getOpcodeStr() << OS.str() << Result >>> << E->getLHS()->getSourceRange() << >>> E->getRHS()->getSourceRange(); >>> return true; >>> @@ -8894,6 +8817,7 @@ static bool DiagnoseOutOfRangeComparison >>> QualType OtherT = Other->getType(); >>> if (const auto *AT = OtherT->getAs<AtomicType>()) >>> OtherT = AT->getValueType(); >>> + >>> IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT); >>> // Whether we're treating Other as being a bool because of the form >>> of >>> @@ -8903,25 +8827,91 @@ static bool DiagnoseOutOfRangeComparison >>> if (OtherIsBooleanDespiteType) >>> OtherRange = IntRange::forBoolType(); >>> - if (FieldDecl *Bitfield = Other->getSourceBitField()) >>> - if (!Bitfield->getBitWidth()->isValueDependent()) >>> - OtherRange.Width = >>> - std::min(Bitfield->getBitWidthValue(S.Context), >>> OtherRange.Width); >>> + unsigned OtherWidth = OtherRange.Width; >>> + >>> + BinaryOperatorKind op = E->getOpcode(); >>> + bool IsTrue = true; >>> // Check whether the constant value can be represented in >>> OtherRange. Bail >>> // out if so; this isn't an out-of-range comparison. >>> - PromotedRange OtherPromotedRange(OtherRange, Value.getBitWidth(), >>> - Value.isUnsigned()); >>> - auto Cmp = OtherPromotedRange.compare(Value); >>> - >>> - // If Value is in the range of possible Other values, this comparison >>> is not >>> - // tautological. >>> - if (Cmp & PromotedRange::InRangeFlag) >>> - return false; >>> + { >>> + QualType ConstantT = Constant->getType(); >>> + QualType CommonT = E->getLHS()->getType(); >>> - auto IsTrue = PromotedRange::constantValue(E->getOpcode(), Cmp, >>> RhsConstant); >>> - if (!IsTrue) >>> - return false; >>> + if (S.Context.hasSameUnqualifiedType(OtherT, ConstantT) && >>> + !OtherIsBooleanDespiteType) >>> + return false; >>> + assert((OtherT->isIntegerType() && ConstantT->isIntegerType()) && >>> + "comparison with non-integer type"); >>> + >>> + bool ConstantSigned = ConstantT->isSignedIntegerType(); >>> + bool CommonSigned = CommonT->isSignedIntegerType(); >>> + >>> + bool EqualityOnly = false; >>> + >>> + if (CommonSigned) { >>> + // The common type is signed, therefore no signed to unsigned >>> conversion. >>> + if (!OtherRange.NonNegative) { >>> + // Check that the constant is representable in type OtherT. >>> + if (ConstantSigned) { >>> + if (OtherWidth >= Value.getMinSignedBits()) >>> + return false; >>> + } else { // !ConstantSigned >>> + if (OtherWidth >= Value.getActiveBits() + 1) >>> + return false; >>> + } >>> + } else { // !OtherSigned >>> + // Check that the constant is representable in type >>> OtherT. >>> + // Negative values are out of range. >>> + if (ConstantSigned) { >>> + if (Value.isNonNegative() && OtherWidth >= >>> Value.getActiveBits()) >>> + return false; >>> + } else { // !ConstantSigned >>> + if (OtherWidth >= Value.getActiveBits()) >>> + return false; >>> + } >>> + } >>> + } else { // !CommonSigned >>> + if (OtherRange.NonNegative) { >>> + if (OtherWidth >= Value.getActiveBits()) >>> + return false; >>> + } else { // OtherSigned >>> + assert(!ConstantSigned && >>> + "Two signed types converted to unsigned types."); >>> + // Check to see if the constant is representable in OtherT. >>> + if (OtherWidth > Value.getActiveBits()) >>> + return false; >>> + // Check to see if the constant is equivalent to a negative >>> value >>> + // cast to CommonT. >>> + if (S.Context.getIntWidth(ConstantT) == >>> + S.Context.getIntWidth(CommonT) && >>> + Value.isNegative() && Value.getMinSignedBits() <= >>> OtherWidth) >>> + return false; >>> + // The constant value rests between values that OtherT can >>> represent >>> + // after conversion. Relational comparison still works, but >>> equality >>> + // comparisons will be tautological. >>> + EqualityOnly = true; >>> + } >>> + } >>> + >>> + bool PositiveConstant = !ConstantSigned || Value.isNonNegative(); >>> + >>> + if (op == BO_EQ || op == BO_NE) { >>> + IsTrue = op == BO_NE; >>> + } else if (EqualityOnly) { >>> + return false; >>> + } else if (RhsConstant) { >>> + if (op == BO_GT || op == BO_GE) >>> + IsTrue = !PositiveConstant; >>> + else // op == BO_LT || op == BO_LE >>> + IsTrue = PositiveConstant; >>> + } else { >>> + if (op == BO_LT || op == BO_LE) >>> + IsTrue = !PositiveConstant; >>> + else // op == BO_GT || op == BO_GE >>> + IsTrue = PositiveConstant; >>> + } >>> + } >>> // If this is a comparison to an enum constant, include that >>> // constant in the diagnostic. >>> @@ -8940,7 +8930,7 @@ static bool DiagnoseOutOfRangeComparison >>> E->getOperatorLoc(), E, >>> S.PDiag(diag::warn_out_of_range_compare) >>> << OS.str() << classifyConstantValue(Constant) >>> - << OtherT << OtherIsBooleanDespiteType << *IsTrue >>> + << OtherT << OtherIsBooleanDespiteType << IsTrue >>> << E->getLHS()->getSourceRange() << >>> E->getRHS()->getSourceRange()); >>> return true; >>> >>> Modified: cfe/trunk/test/Sema/tautological-constant-compare.c >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/tautological-constant-compare.c?rev=320162&r1=320161&r2=320162&view=diff >>> >>> ============================================================================== >>> --- cfe/trunk/test/Sema/tautological-constant-compare.c (original) >>> +++ cfe/trunk/test/Sema/tautological-constant-compare.c Fri Dec 8 >>> 08:54:08 2017 >>> @@ -94,17 +94,15 @@ int main() >>> if (-32768 >= s) >>> return 0; >>> - // Note: both sides are promoted to unsigned long prior to the >>> comparison, so >>> - // it is perfectly possible for a short to compare greater than >>> 32767UL. >>> if (s == 32767UL) >>> return 0; >>> if (s != 32767UL) >>> return 0; >>> if (s < 32767UL) >>> return 0; >>> - if (s <= 32767UL) >>> + if (s <= 32767UL) // expected-warning {{comparison 'short' <= 32767 is >>> always true}} >>> return 0; >>> - if (s > 32767UL) >>> + if (s > 32767UL) // expected-warning {{comparison 'short' > 32767 is >>> always false}} >>> return 0; >>> if (s >= 32767UL) >>> return 0; >>> @@ -113,66 +111,13 @@ int main() >>> return 0; >>> if (32767UL != s) >>> return 0; >>> - if (32767UL < s) >>> + if (32767UL < s) // expected-warning {{comparison 32767 < 'short' is >>> always false}} >>> return 0; >>> if (32767UL <= s) >>> return 0; >>> if (32767UL > s) >>> return 0; >>> - if (32767UL >= s) >>> - return 0; >>> - >>> - if (s == 0UL) >>> - return 0; >>> - if (s != 0UL) >>> - return 0; >>> - if (s < 0UL) // expected-warning {{comparison of unsigned expression < >>> 0 is always false}} >>> - return 0; >>> - if (s <= 0UL) >>> - return 0; >>> - if (s > 0UL) >>> - return 0; >>> - if (s >= 0UL) // expected-warning {{comparison of unsigned expression >>> >= 0 is always true}} >>> - return 0; >>> - >>> - if (0UL == s) >>> - return 0; >>> - if (0UL != s) >>> - return 0; >>> - if (0UL < s) >>> - return 0; >>> - if (0UL <= s) // expected-warning {{comparison of 0 <= unsigned >>> expression is always true}} >>> - return 0; >>> - if (0UL > s) // expected-warning {{comparison of 0 > unsigned >>> expression is always false}} >>> - return 0; >>> - if (0UL >= s) >>> - return 0; >>> - >>> - enum { ULONG_MAX = (2UL * (unsigned long)__LONG_MAX__ + 1UL) }; >>> - if (s == 2UL * (unsigned long)__LONG_MAX__ + 1UL) >>> - return 0; >>> - if (s != 2UL * (unsigned long)__LONG_MAX__ + 1UL) >>> - return 0; >>> - if (s < 2UL * (unsigned long)__LONG_MAX__ + 1UL) >>> - return 0; >>> - if (s <= 2UL * (unsigned long)__LONG_MAX__ + 1UL) // >>> expected-warning-re {{comparison 'short' <= {{.*}} is always true}} >>> - return 0; >>> - if (s > 2UL * (unsigned long)__LONG_MAX__ + 1UL) // >>> expected-warning-re {{comparison 'short' > {{.*}} is always false}} >>> - return 0; >>> - if (s >= 2UL * (unsigned long)__LONG_MAX__ + 1UL) >>> - return 0; >>> - >>> - if (2UL * (unsigned long)__LONG_MAX__ + 1UL == s) >>> - return 0; >>> - if (2UL * (unsigned long)__LONG_MAX__ + 1UL != s) >>> - return 0; >>> - if (2UL * (unsigned long)__LONG_MAX__ + 1UL < s) // >>> expected-warning-re {{comparison {{.*}} < 'short' is always false}} >>> - return 0; >>> - if (2UL * (unsigned long)__LONG_MAX__ + 1UL <= s) >>> - return 0; >>> - if (2UL * (unsigned long)__LONG_MAX__ + 1UL > s) >>> - return 0; >>> - if (2UL * (unsigned long)__LONG_MAX__ + 1UL >= s) // >>> expected-warning-re {{comparison {{.*}} >= 'short' is always true}} >>> + if (32767UL >= s) // expected-warning {{comparison 32767 >= 'short' is >>> always true}} >>> return 0; >>> // FIXME: assumes two's complement >>> @@ -336,6 +281,8 @@ int main() >>> if (0 >= s) >>> return 0; >>> + // However the comparison with 0U would warn >>> + >>> unsigned short us = value(); >>> #ifdef TEST >>> >>> Modified: cfe/trunk/test/Sema/tautological-constant-enum-compare.c >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/tautological-constant-enum-compare.c?rev=320162&r1=320161&r2=320162&view=diff >>> >>> ============================================================================== >>> --- cfe/trunk/test/Sema/tautological-constant-enum-compare.c (original) >>> +++ cfe/trunk/test/Sema/tautological-constant-enum-compare.c Fri Dec 8 >>> 08:54:08 2017 >>> @@ -9,96 +9,78 @@ int main() { >>> #ifdef SILENCE >>> // expected-no-diagnostics >>> -#else >>> - // If we promote to unsigned, it doesn't matter whether the enum's >>> underlying >>> - // type was signed. >>> - if (a < 0U) // expected-warning {{comparison of unsigned enum >>> expression < 0 is always false}} >>> - return 0; >>> - if (0U >= a) >>> - return 0; >>> - if (a > 0U) >>> - return 0; >>> - if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum >>> expression is always true}} >>> - return 0; >>> - if (a <= 0U) >>> - return 0; >>> - if (0U > a) // expected-warning {{comparison of 0 > unsigned enum >>> expression is always false}} >>> - return 0; >>> - if (a >= 0U) // expected-warning {{comparison of unsigned enum >>> expression >= 0 is always true}} >>> - return 0; >>> - if (0U < a) >>> - return 0; >>> +#endif >>> - if (a < 4294967295U) >>> +#ifdef UNSIGNED >>> +#ifndef SILENCE >>> + if (a < 0) // expected-warning {{comparison of unsigned enum >>> expression < 0 is always false}} >>> return 0; >>> - if (4294967295U >= a) // expected-warning {{comparison 4294967295 >= >>> 'enum A' is always true}} >>> + if (0 >= a) >>> return 0; >>> - if (a > 4294967295U) // expected-warning {{comparison 'enum A' > >>> 4294967295 is always false}} >>> + if (a > 0) >>> return 0; >>> - if (4294967295U <= a) >>> + if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum >>> expression is always true}} >>> return 0; >>> - if (a <= 4294967295U) // expected-warning {{comparison 'enum A' <= >>> 4294967295 is always true}} >>> + if (a <= 0) >>> return 0; >>> - if (4294967295U > a) >>> + if (0 > a) // expected-warning {{comparison of 0 > unsigned enum >>> expression is always false}} >>> return 0; >>> - if (a >= 4294967295U) >>> + if (a >= 0) // expected-warning {{comparison of unsigned enum >>> expression >= 0 is always true}} >>> return 0; >>> - if (4294967295U < a) // expected-warning {{comparison 4294967295 < >>> 'enum A' is always false}} >>> + if (0 < a) >>> return 0; >>> - if (a < 2147483647U) >>> + if (a < 0U) // expected-warning {{comparison of unsigned enum >>> expression < 0 is always false}} >>> return 0; >>> - if (2147483647U >= a) >>> + if (0U >= a) >>> return 0; >>> - if (a > 2147483647U) >>> + if (a > 0U) >>> return 0; >>> - if (2147483647U <= a) >>> + if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum >>> expression is always true}} >>> return 0; >>> - if (a <= 2147483647U) >>> + if (a <= 0U) >>> return 0; >>> - if (2147483647U > a) >>> + if (0U > a) // expected-warning {{comparison of 0 > unsigned enum >>> expression is always false}} >>> return 0; >>> - if (a >= 2147483647U) >>> + if (a >= 0U) // expected-warning {{comparison of unsigned enum >>> expression >= 0 is always true}} >>> return 0; >>> - if (2147483647U < a) >>> + if (0U < a) >>> return 0; >>> -#endif >>> -#if defined(UNSIGNED) && !defined(SILENCE) >>> - if (a < 0) // expected-warning {{comparison of unsigned enum >>> expression < 0 is always false}} >>> + if (a < 4294967295) >>> return 0; >>> - if (0 >= a) >>> + if (4294967295 >= a) // expected-warning {{comparison 4294967295 >= >>> 'enum A' is always true}} >>> return 0; >>> - if (a > 0) >>> + if (a > 4294967295) // expected-warning {{comparison 'enum A' > >>> 4294967295 is always false}} >>> return 0; >>> - if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum >>> expression is always true}} >>> + if (4294967295 <= a) >>> return 0; >>> - if (a <= 0) >>> + if (a <= 4294967295) // expected-warning {{comparison 'enum A' <= >>> 4294967295 is always true}} >>> return 0; >>> - if (0 > a) // expected-warning {{comparison of 0 > unsigned enum >>> expression is always false}} >>> + if (4294967295 > a) >>> return 0; >>> - if (a >= 0) // expected-warning {{comparison of unsigned enum >>> expression >= 0 is always true}} >>> + if (a >= 4294967295) >>> return 0; >>> - if (0 < a) >>> + if (4294967295 < a) // expected-warning {{comparison 4294967295 < >>> 'enum A' is always false}} >>> return 0; >>> - if (a < 4294967295) >>> + if (a < 4294967295U) >>> return 0; >>> - if (4294967295 >= a) // expected-warning {{comparison 4294967295 >= >>> 'enum A' is always true}} >>> + if (4294967295U >= a) // expected-warning {{comparison 4294967295 >= >>> 'enum A' is always true}} >>> return 0; >>> - if (a > 4294967295) // expected-warning {{comparison 'enum A' > >>> 4294967295 is always false}} >>> + if (a > 4294967295U) // expected-warning {{comparison 'enum A' > >>> 4294967295 is always false}} >>> return 0; >>> - if (4294967295 <= a) >>> + if (4294967295U <= a) >>> return 0; >>> - if (a <= 4294967295) // expected-warning {{comparison 'enum A' <= >>> 4294967295 is always true}} >>> + if (a <= 4294967295U) // expected-warning {{comparison 'enum A' <= >>> 4294967295 is always true}} >>> return 0; >>> - if (4294967295 > a) >>> + if (4294967295U > a) >>> return 0; >>> - if (a >= 4294967295) >>> + if (a >= 4294967295U) >>> return 0; >>> - if (4294967295 < a) // expected-warning {{comparison 4294967295 < >>> 'enum A' is always false}} >>> + if (4294967295U < a) // expected-warning {{comparison 4294967295 < >>> 'enum A' is always false}} >>> return 0; >>> -#else // SIGNED || SILENCE >>> +#else // SILENCE >>> if (a < 0) >>> return 0; >>> if (0 >= a) >>> @@ -116,24 +98,23 @@ int main() { >>> if (0 < a) >>> return 0; >>> -#ifndef SILENCE >>> - if (a < 4294967295) // expected-warning {{comparison of constant >>> 4294967295 with expression of type 'enum A' is always true}} >>> + if (a < 0U) >>> return 0; >>> - if (4294967295 >= a) // expected-warning {{comparison of constant >>> 4294967295 with expression of type 'enum A' is always true}} >>> + if (0U >= a) >>> return 0; >>> - if (a > 4294967295) // expected-warning {{comparison of constant >>> 4294967295 with expression of type 'enum A' is always false}} >>> + if (a > 0U) >>> return 0; >>> - if (4294967295 <= a) // expected-warning {{comparison of constant >>> 4294967295 with expression of type 'enum A' is always false}} >>> + if (0U <= a) >>> return 0; >>> - if (a <= 4294967295) // expected-warning {{comparison of constant >>> 4294967295 with expression of type 'enum A' is always true}} >>> + if (a <= 0U) >>> return 0; >>> - if (4294967295 > a) // expected-warning {{comparison of constant >>> 4294967295 with expression of type 'enum A' is always true}} >>> + if (0U > a) >>> return 0; >>> - if (a >= 4294967295) // expected-warning {{comparison of constant >>> 4294967295 with expression of type 'enum A' is always false}} >>> + if (a >= 0U) >>> return 0; >>> - if (4294967295 < a) // expected-warning {{comparison of constant >>> 4294967295 with expression of type 'enum A' is always false}} >>> + if (0U < a) >>> return 0; >>> -#else >>> + >>> if (a < 4294967295) >>> return 0; >>> if (4294967295 >= a) >>> @@ -150,10 +131,26 @@ int main() { >>> return 0; >>> if (4294967295 < a) >>> return 0; >>> -#endif >>> -#endif >>> -#if defined(SIGNED) && !defined(SILENCE) >>> + if (a < 4294967295U) >>> + return 0; >>> + if (4294967295U >= a) >>> + return 0; >>> + if (a > 4294967295U) >>> + return 0; >>> + if (4294967295U <= a) >>> + return 0; >>> + if (a <= 4294967295U) >>> + return 0; >>> + if (4294967295U > a) >>> + return 0; >>> + if (a >= 4294967295U) >>> + return 0; >>> + if (4294967295U < a) >>> + return 0; >>> +#endif >>> +#elif defined(SIGNED) >>> +#ifndef SILENCE >>> if (a < -2147483648) // expected-warning {{comparison 'enum A' < >>> -2147483648 is always false}} >>> return 0; >>> if (-2147483648 >= a) >>> @@ -187,25 +184,24 @@ int main() { >>> return 0; >>> if (2147483647 < a) // expected-warning {{comparison 2147483647 < >>> 'enum A' is always false}} >>> return 0; >>> -#elif defined(UNSIGNED) && !defined(SILENCE) >>> -#ifndef SILENCE >>> - if (a < -2147483648) // expected-warning {{comparison of constant >>> -2147483648 with expression of type 'enum A' is always false}} >>> + >>> + if (a < 2147483647U) >>> return 0; >>> - if (-2147483648 >= a) // expected-warning {{comparison of constant >>> -2147483648 with expression of type 'enum A' is always false}} >>> + if (2147483647U >= a) // expected-warning {{comparison 2147483647 >= >>> 'enum A' is always true}} >>> return 0; >>> - if (a > -2147483648) // expected-warning {{comparison of constant >>> -2147483648 with expression of type 'enum A' is always true}} >>> + if (a > 2147483647U) // expected-warning {{comparison 'enum A' > >>> 2147483647 is always false}} >>> return 0; >>> - if (-2147483648 <= a) // expected-warning {{comparison of constant >>> -2147483648 with expression of type 'enum A' is always true}} >>> + if (2147483647U <= a) >>> return 0; >>> - if (a <= -2147483648) // expected-warning {{comparison of constant >>> -2147483648 with expression of type 'enum A' is always false}} >>> + if (a <= 2147483647U) // expected-warning {{comparison 'enum A' <= >>> 2147483647 is always true}} >>> return 0; >>> - if (-2147483648 > a) // expected-warning {{comparison of constant >>> -2147483648 with expression of type 'enum A' is always false}} >>> + if (2147483647U > a) >>> return 0; >>> - if (a >= -2147483648) // expected-warning {{comparison of constant >>> -2147483648 with expression of type 'enum A' is always true}} >>> + if (a >= 2147483647U) >>> return 0; >>> - if (-2147483648 < a) // expected-warning {{comparison of constant >>> -2147483648 with expression of type 'enum A' is always true}} >>> + if (2147483647U < a) // expected-warning {{comparison 2147483647 < >>> 'enum A' is always false}} >>> return 0; >>> -#else >>> +#else // SILENCE >>> if (a < -2147483648) >>> return 0; >>> if (-2147483648 >= a) >>> @@ -222,7 +218,6 @@ int main() { >>> return 0; >>> if (-2147483648 < a) >>> return 0; >>> -#endif >>> if (a < 2147483647) >>> return 0; >>> @@ -240,6 +235,24 @@ int main() { >>> return 0; >>> if (2147483647 < a) >>> return 0; >>> + >>> + if (a < 2147483647U) >>> + return 0; >>> + if (2147483647U >= a) >>> + return 0; >>> + if (a > 2147483647U) >>> + return 0; >>> + if (2147483647U <= a) >>> + return 0; >>> + if (a <= 2147483647U) >>> + return 0; >>> + if (2147483647U > a) >>> + return 0; >>> + if (a >= 2147483647U) >>> + return 0; >>> + if (2147483647U < a) >>> + return 0; >>> +#endif >>> #endif >>> return 1; >>> >>> Modified: cfe/trunk/test/SemaCXX/compare.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/compare.cpp?rev=320162&r1=320161&r2=320162&view=diff >>> >>> ============================================================================== >>> --- cfe/trunk/test/SemaCXX/compare.cpp (original) >>> +++ cfe/trunk/test/SemaCXX/compare.cpp Fri Dec 8 08:54:08 2017 >>> @@ -245,8 +245,8 @@ void test4(short s) { >>> // unsigned. >>> const unsigned B = -1; >>> void (s < B); // expected-warning{{comparison of integers of >>> different signs: 'short' and 'const unsigned int'}} >>> - void (s > B); // expected-warning{{comparison 'short' > 4294967295 is >>> always false}} >>> - void (s <= B); // expected-warning{{comparison 'short' <= 4294967295 >>> is always true}} >>> + void (s > B); // expected-warning{{comparison of integers of different >>> signs: 'short' and 'const unsigned int'}} >>> + void (s <= B); // expected-warning{{comparison of integers of >>> different signs: 'short' and 'const unsigned int'}} >>> void (s >= B); // expected-warning{{comparison of integers of >>> different signs: 'short' and 'const unsigned int'}} >>> void (s == B); // expected-warning{{comparison of integers of >>> different signs: 'short' and 'const unsigned int'}} >>> void (s != B); // expected-warning{{comparison of integers of >>> different signs: 'short' and 'const unsigned int'}} >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> cfe-commits@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>> >> >> >> -- >> >> -Bill Seurer >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits