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 14:09:06 PST 2017
Selfhost failures are ultimately caused by:
https://github.com/llvm-mirror/llvm/blob/master/utils/TableGen/X86RecognizableInstr.cpp#L709
https://github.com/llvm-mirror/clang/blob/master/include/clang/AST/OperationKinds.h#L26
These are genuine bugs: assigning -1 into an enumeration with no negative
enumerators results in undefined behavior. We should probably have a
warning for the casts here rather than only detecting the problem under
-Wtautological-compare, but I would not be surprised to find that Clang
miscompiles itself with -fstrict-enums due to the above two issues.
On 8 December 2017 at 13:40, Hans Wennborg via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> 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 at 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 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/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 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
> >
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
> _______________________________________________
> 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/2e1a29bf/attachment-0001.html>
More information about the cfe-commits
mailing list