I'm going to reland without the changes to bit-field handling. If we want those changes (which I think we do, based on the bugs it found in selfhost), that'll need to be done more carefully, and I don't want to get the refactoring and bugfixes here tangled up with that.
On 8 December 2017 at 13:27, Bill Seurer via cfe-commits < cfe-commits@lists.llvm.org> wrote: > It also caused all the sanitizer builds to fail. For example: > > http://lab.llvm.org:8011/builders/sanitizer-ppc64le-linux/builds/3654 > > /home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/ > llvm/utils/TableGen/X86RecognizableInstr.cpp:789:21: error: comparison of > constant -1 with expression of type 'llvm::X86Disassembler::OpcodeType' > is always true [-Werror,-Wtautological-constant-out-of-range-compare] > assert(opcodeType != (OpcodeType)-1 && > ~~~~~~~~~~ ^ ~~~~~~~~~~~~~~ > /usr/include/assert.h:86:5: note: expanded from macro 'assert' > ((expr) \ > ^~~~ > 1 error generated. > utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/build.make:1022: recipe for > target > 'utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/X86RecognizableInstr.cpp.o' > failed > make[3]: *** > [utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/X86RecognizableInstr.cpp.o] > Error 1 > > > > And there are lots more warnings (which are flagged as errors for the > sanitizer builds) when I run a build by hand. For example: > > [4001/4008] Building CXX object tools/clang/tools/libclang/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@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
