r320162 - Revert "Unify implementation of our two different flavours of -Wtautological-compare."

Bill Seurer via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 8 13:27:18 PST 2017


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



More information about the cfe-commits mailing list