For now, I've undone the change that caused us to warn (ever!) on comparisons between two expressions of enumeration type, and also the change to use more precise bit-width information for bit-field comparisons, and re-committed as r320211.
The bugs exposed by self-host have been fixed in r320212 and r320206. On 8 December 2017 at 14:09, Richard Smith <rich...@metafoo.co.uk> wrote: > 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@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@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/X86Recognizab >> leInstr.cpp.o' >> >> failed >> >> make[3]: *** >> >> [utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/X86Recognizab >> leInstr.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/Curs >> orVisitor.h:16: >> >> In file included from >> >> /home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/D >> eclVisitor.h:19: >> >> In file included from >> >> /home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/D >> eclCXX.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/E >> xpr.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::kTraceBuffer >> ChunkSize); >> >>> ~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> ~~~~~~~~~ >> >>> >> >>> 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->isKnownToHaveBooleanVal >> ue(); >> >>> - 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/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 <(214)%20748-3648>) // expected-warning >> {{comparison 'enum A' < >> >>> -2147483648 <(214)%20748-3648> is always false}} >> >>> return 0; >> >>> if (-2147483648 <(214)%20748-3648> >= a) >> >>> @@ -187,25 +184,24 @@ int main() { >> >>> return 0; >> >>> if (2147483647 <(214)%20748-3647> < a) // expected-warning >> {{comparison 2147483647 <(214)%20748-3647> < >> >>> 'enum A' is always false}} >> >>> return 0; >> >>> -#elif defined(UNSIGNED) && !defined(SILENCE) >> >>> -#ifndef SILENCE >> >>> - if (a < -2147483648 <(214)%20748-3648>) // expected-warning >> {{comparison of constant >> >>> -2147483648 <(214)%20748-3648> with expression of type 'enum A' is >> always false}} >> >>> + >> >>> + if (a < 2147483647U) >> >>> return 0; >> >>> - if (-2147483648 <(214)%20748-3648> >= a) // expected-warning >> {{comparison of constant >> >>> -2147483648 <(214)%20748-3648> with expression of type 'enum A' is >> always false}} >> >>> + if (2147483647U >= a) // expected-warning {{comparison 2147483647 >> <(214)%20748-3647> >= >> >>> 'enum A' is always true}} >> >>> return 0; >> >>> - if (a > -2147483648 <(214)%20748-3648>) // expected-warning >> {{comparison of constant >> >>> -2147483648 <(214)%20748-3648> with expression of type 'enum A' is >> always true}} >> >>> + if (a > 2147483647U) // expected-warning {{comparison 'enum A' > >> >>> 2147483647 <(214)%20748-3647> is always false}} >> >>> return 0; >> >>> - if (-2147483648 <(214)%20748-3648> <= a) // expected-warning >> {{comparison of constant >> >>> -2147483648 <(214)%20748-3648> with expression of type 'enum A' is >> always true}} >> >>> + if (2147483647U <= a) >> >>> return 0; >> >>> - if (a <= -2147483648 <(214)%20748-3648>) // expected-warning >> {{comparison of constant >> >>> -2147483648 <(214)%20748-3648> with expression of type 'enum A' is >> always false}} >> >>> + if (a <= 2147483647U) // expected-warning {{comparison 'enum A' <= >> >>> 2147483647 <(214)%20748-3647> is always true}} >> >>> return 0; >> >>> - if (-2147483648 <(214)%20748-3648> > a) // expected-warning >> {{comparison of constant >> >>> -2147483648 <(214)%20748-3648> with expression of type 'enum A' is >> always false}} >> >>> + if (2147483647U > a) >> >>> return 0; >> >>> - if (a >= -2147483648 <(214)%20748-3648>) // expected-warning >> {{comparison of constant >> >>> -2147483648 <(214)%20748-3648> with expression of type 'enum A' is >> always true}} >> >>> + if (a >= 2147483647U) >> >>> return 0; >> >>> - if (-2147483648 <(214)%20748-3648> < a) // expected-warning >> {{comparison of constant >> >>> -2147483648 <(214)%20748-3648> with expression of type 'enum A' is >> always true}} >> >>> + if (2147483647U < a) // expected-warning {{comparison 2147483647 >> <(214)%20748-3647> < >> >>> 'enum A' is always false}} >> >>> return 0; >> >>> -#else >> >>> +#else // SILENCE >> >>> if (a < -2147483648 <(214)%20748-3648>) >> >>> return 0; >> >>> if (-2147483648 <(214)%20748-3648> >= a) >> >>> @@ -222,7 +218,6 @@ int main() { >> >>> return 0; >> >>> if (-2147483648 <(214)%20748-3648> < a) >> >>> return 0; >> >>> -#endif >> >>> if (a < 2147483647 <(214)%20748-3647>) >> >>> return 0; >> >>> @@ -240,6 +235,24 @@ int main() { >> >>> return 0; >> >>> if (2147483647 <(214)%20748-3647> < 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 >> > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
