r320162 - Revert "Unify implementation of our two different flavours of -Wtautological-compare."
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Fri Dec 8 13:33:18 PST 2017
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 at 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/CMa
> keFiles/libclang.dir/CIndex.cpp.o
> In file included from /home/seurer/llvm/llvm-test2/t
> ools/clang/tools/libclang/CIndex.cpp:23:
> In file included from /home/seurer/llvm/llvm-test2/t
> ools/clang/tools/libclang/CursorVisitor.h:16:
> In file included from /home/seurer/llvm/llvm-test2/t
> ools/clang/include/clang/AST/DeclVisitor.h:19:
> In file included from /home/seurer/llvm/llvm-test2/t
> ools/clang/include/clang/AST/DeclCXX.h:21:
> In file included from /home/seurer/llvm/llvm-test2/t
> ools/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-o
> f-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/SemaC
>> hecking.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_alway
>> s_true_comparison
>> : diag::warn_unsigned_always_tru
>> e_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/taut
>> ological-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/taut
>> ological-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 at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
>
> --
>
> -Bill Seurer
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171208/e3e126b9/attachment-0001.html>
More information about the cfe-commits
mailing list