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

Hans Wennborg via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 8 13:40:00 PST 2017


For what it's worth, Chromium's bitfield warning is easy to fix. The
"-1 vs enum" comparisons are more plentiful and less straightforward.
I'm guessing it's the same for the self-host.

On Fri, Dec 8, 2017 at 1:33 PM, Richard Smith via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
> I'm going to reland without the changes to bit-field handling. If we want
> those changes (which I think we do, based on the bugs it found in selfhost),
> that'll need to be done more carefully, and I don't want to get the
> refactoring and bugfixes here tangled up with that.
>
> On 8 December 2017 at 13:27, Bill Seurer via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
>>
>> It also caused all the sanitizer builds to fail.  For example:
>>
>> http://lab.llvm.org:8011/builders/sanitizer-ppc64le-linux/builds/3654
>>
>>
>> /home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm/utils/TableGen/X86RecognizableInstr.cpp:789:21:
>> error: comparison of constant -1 with expression of type
>> 'llvm::X86Disassembler::OpcodeType' is always true
>> [-Werror,-Wtautological-constant-out-of-range-compare]
>>   assert(opcodeType != (OpcodeType)-1 &&
>>          ~~~~~~~~~~ ^  ~~~~~~~~~~~~~~
>> /usr/include/assert.h:86:5: note: expanded from macro 'assert'
>>   ((expr)                                                               \
>>     ^~~~
>> 1 error generated.
>> utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/build.make:1022: recipe for
>> target
>> 'utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/X86RecognizableInstr.cpp.o'
>> failed
>> make[3]: ***
>> [utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/X86RecognizableInstr.cpp.o]
>> Error 1
>>
>>
>>
>> And there are lots more warnings (which are flagged as errors for the
>> sanitizer builds) when I run a build by hand.  For example:
>>
>> [4001/4008] Building CXX object
>> tools/clang/tools/libclang/CMakeFiles/libclang.dir/CIndex.cpp.o
>> In file included from
>> /home/seurer/llvm/llvm-test2/tools/clang/tools/libclang/CIndex.cpp:23:
>> In file included from
>> /home/seurer/llvm/llvm-test2/tools/clang/tools/libclang/CursorVisitor.h:16:
>> In file included from
>> /home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/DeclVisitor.h:19:
>> In file included from
>> /home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/DeclCXX.h:21:
>> In file included from
>> /home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/Attr.h:19:
>> /home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/Expr.h:2745:17:
>> warning: comparison of constant -1 with expression of type 'const
>> clang::CastKind' is always true
>> [-Wtautological-constant-out-of-range-compare]
>>     assert(kind != CK_Invalid && "creating cast with invalid cast kind");
>>            ~~~~ ^  ~~~~~~~~~~
>> /usr/include/assert.h:89:5: note: expanded from macro 'assert'
>>   ((expr)                                                               \
>>     ^~~~
>>
>>
>>
>> On 12/08/2017 10:54 AM, Hans Wennborg via cfe-commits wrote:
>>>
>>> Author: hans
>>> Date: Fri Dec  8 08:54:08 2017
>>> New Revision: 320162
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=320162&view=rev
>>> Log:
>>> Revert "Unify implementation of our two different flavours of
>>> -Wtautological-compare."
>>>
>>>> Unify implementation of our two different flavours of
>>>> -Wtautological-compare.
>>>>
>>>> In so doing, fix a handful of remaining bugs where we would report false
>>>> positives or false negatives if we promote a signed value to an unsigned
>>>> type
>>>> for the comparison.
>>>
>>>
>>> This caused a new warning in Chromium:
>>>
>>> ../../base/trace_event/trace_log.cc:1545:29: error: comparison of
>>> constant 64
>>> with expression of type 'unsigned int' is always true
>>> [-Werror,-Wtautological-constant-out-of-range-compare]
>>>    DCHECK(handle.event_index < TraceBufferChunk::kTraceBufferChunkSize);
>>>           ~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> The 'unsigned int' is really a 6-bit bitfield, which is why it's always
>>> less than 64.
>>>
>>> I thought we didn't use to warn (with out-of-range-compare) when
>>> comparing
>>> against the boundaries of a type?
>>>
>>> Modified:
>>>      cfe/trunk/lib/Sema/SemaChecking.cpp
>>>      cfe/trunk/test/Sema/tautological-constant-compare.c
>>>      cfe/trunk/test/Sema/tautological-constant-enum-compare.c
>>>      cfe/trunk/test/SemaCXX/compare.cpp
>>>
>>> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=320162&r1=320161&r2=320162&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Dec  8 08:54:08 2017
>>> @@ -8662,113 +8662,54 @@ static bool isKnownToHaveUnsignedValue(E
>>>   }
>>>     namespace {
>>> -/// The promoted range of values of a type. In general this has the
>>> -/// following structure:
>>> -///
>>> -///     |-----------| . . . |-----------|
>>> -///     ^           ^       ^           ^
>>> -///    Min       HoleMin  HoleMax      Max
>>> -///
>>> -/// ... where there is only a hole if a signed type is promoted to
>>> unsigned
>>> -/// (in which case Min and Max are the smallest and largest
>>> representable
>>> -/// values).
>>> -struct PromotedRange {
>>> -  // Min, or HoleMax if there is a hole.
>>> -  llvm::APSInt PromotedMin;
>>> -  // Max, or HoleMin if there is a hole.
>>> -  llvm::APSInt PromotedMax;
>>> -
>>> -  PromotedRange(IntRange R, unsigned BitWidth, bool Unsigned) {
>>> -    if (R.Width == 0)
>>> -      PromotedMin = PromotedMax = llvm::APSInt(BitWidth, Unsigned);
>>> -    else {
>>> -      PromotedMin = llvm::APSInt::getMinValue(R.Width, R.NonNegative)
>>> -                        .extOrTrunc(BitWidth);
>>> -      PromotedMin.setIsUnsigned(Unsigned);
>>> -
>>> -      PromotedMax = llvm::APSInt::getMaxValue(R.Width, R.NonNegative)
>>> -                        .extOrTrunc(BitWidth);
>>> -      PromotedMax.setIsUnsigned(Unsigned);
>>> -    }
>>> -  }
>>>   -  // Determine whether this range is contiguous (has no hole).
>>> -  bool isContiguous() const { return PromotedMin <= PromotedMax; }
>>> +enum class LimitType {
>>> +  Max = 1U << 0U,  // e.g. 32767 for short
>>> +  Min = 1U << 1U,  // e.g. -32768 for short
>>> +  Both = Max | Min // When the value is both the Min and the Max limit
>>> at the
>>> +                   // same time; e.g. in C++, A::a in enum A { a = 0 };
>>> +};
>>>   -  // Where a constant value is within the range.
>>> -  enum ComparisonResult {
>>> -    LT = 0x1,
>>> -    LE = 0x2,
>>> -    GT = 0x4,
>>> -    GE = 0x8,
>>> -    EQ = 0x10,
>>> -    NE = 0x20,
>>> -    InRangeFlag = 0x40,
>>> -
>>> -    Less = LE | LT | NE,
>>> -    Min = LE | InRangeFlag,
>>> -    InRange = InRangeFlag,
>>> -    Max = GE | InRangeFlag,
>>> -    Greater = GE | GT | NE,
>>> +} // namespace
>>>   -    OnlyValue = LE | GE | EQ | InRangeFlag,
>>> -    InHole = NE
>>> -  };
>>> +/// Checks whether Expr 'Constant' may be the
>>> +/// std::numeric_limits<>::max() or std::numeric_limits<>::min()
>>> +/// of the Expr 'Other'. If true, then returns the limit type (min or
>>> max).
>>> +/// The Value is the evaluation of Constant
>>> +static llvm::Optional<LimitType> IsTypeLimit(Sema &S, Expr *Constant,
>>> +                                             Expr *Other,
>>> +                                             const llvm::APSInt &Value)
>>> {
>>> +  if (IsEnumConstOrFromMacro(S, Constant))
>>> +    return llvm::Optional<LimitType>();
>>>   -  ComparisonResult compare(const llvm::APSInt &Value) const {
>>> -    assert(Value.getBitWidth() == PromotedMin.getBitWidth() &&
>>> -           Value.isUnsigned() == PromotedMin.isUnsigned());
>>> -    if (!isContiguous()) {
>>> -      assert(Value.isUnsigned() && "discontiguous range for signed
>>> compare");
>>> -      if (Value.isMinValue()) return Min;
>>> -      if (Value.isMaxValue()) return Max;
>>> -      if (Value >= PromotedMin) return InRange;
>>> -      if (Value <= PromotedMax) return InRange;
>>> -      return InHole;
>>> -    }
>>> +  if (isKnownToHaveUnsignedValue(Other) && Value == 0)
>>> +    return LimitType::Min;
>>>   -    switch (llvm::APSInt::compareValues(Value, PromotedMin)) {
>>> -    case -1: return Less;
>>> -    case 0: return PromotedMin == PromotedMax ? OnlyValue : Min;
>>> -    case 1:
>>> -      switch (llvm::APSInt::compareValues(Value, PromotedMax)) {
>>> -      case -1: return InRange;
>>> -      case 0: return Max;
>>> -      case 1: return Greater;
>>> -      }
>>> -    }
>>> +  // TODO: Investigate using GetExprRange() to get tighter bounds
>>> +  // on the bit ranges.
>>> +  QualType OtherT = Other->IgnoreParenImpCasts()->getType();
>>> +  if (const auto *AT = OtherT->getAs<AtomicType>())
>>> +    OtherT = AT->getValueType();
>>>   -    llvm_unreachable("impossible compare result");
>>> -  }
>>> +  IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
>>> +  if (Other->isKnownToHaveBooleanValue())
>>> +    OtherRange = IntRange::forBoolType();
>>>   -  static llvm::Optional<bool> constantValue(BinaryOperatorKind Op,
>>> -                                            ComparisonResult R,
>>> -                                            bool ConstantOnRHS) {
>>> -    ComparisonResult TrueFlag, FalseFlag;
>>> -    if (Op == BO_EQ) {
>>> -      TrueFlag = EQ;
>>> -      FalseFlag = NE;
>>> -    } else if (Op == BO_NE) {
>>> -      TrueFlag = NE;
>>> -      FalseFlag = EQ;
>>> -    } else {
>>> -      if ((Op == BO_LT || Op == BO_GE) ^ ConstantOnRHS) {
>>> -        TrueFlag = LT;
>>> -        FalseFlag = GE;
>>> -      } else {
>>> -        TrueFlag = GT;
>>> -        FalseFlag = LE;
>>> -      }
>>> -      if (Op == BO_GE || Op == BO_LE)
>>> -        std::swap(TrueFlag, FalseFlag);
>>> -    }
>>> -    if (R & TrueFlag)
>>> -      return true;
>>> -    if (R & FalseFlag)
>>> -      return false;
>>> -    return llvm::None;
>>> -  }
>>> -};
>>> +  // Special-case for C++ for enum with one enumerator with value of 0.
>>> +  if (OtherRange.Width == 0)
>>> +    return Value == 0 ? LimitType::Both : llvm::Optional<LimitType>();
>>> +
>>> +  if (llvm::APSInt::isSameValue(
>>> +          llvm::APSInt::getMaxValue(OtherRange.Width,
>>> OtherRange.NonNegative),
>>> +          Value))
>>> +    return LimitType::Max;
>>> +
>>> +  if (llvm::APSInt::isSameValue(
>>> +          llvm::APSInt::getMinValue(OtherRange.Width,
>>> OtherRange.NonNegative),
>>> +          Value))
>>> +    return LimitType::Min;
>>> +
>>> +  return llvm::None;
>>>   }
>>>     static bool HasEnumType(Expr *E) {
>>> @@ -8806,47 +8747,29 @@ static bool CheckTautologicalComparison(
>>>     if (S.inTemplateInstantiation() || !E->isRelationalOp())
>>>       return false;
>>>   -  if (IsEnumConstOrFromMacro(S, Constant))
>>> -    return false;
>>> -
>>> -  Expr *OriginalOther = Other;
>>> +  BinaryOperatorKind Op = E->getOpcode();
>>>   -  Constant = Constant->IgnoreParenImpCasts();
>>> -  Other = Other->IgnoreParenImpCasts();
>>> -
>>> -  // TODO: Investigate using GetExprRange() to get tighter bounds
>>> -  // on the bit ranges.
>>> -  QualType OtherT = Other->getType();
>>> -  if (const auto *AT = OtherT->getAs<AtomicType>())
>>> -    OtherT = AT->getValueType();
>>> -  IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
>>> -
>>> -  // Whether we're treating Other as being a bool because of the form of
>>> -  // expression despite it having another type (typically 'int' in C).
>>> -  bool OtherIsBooleanDespiteType =
>>> -      !OtherT->isBooleanType() && Other->isKnownToHaveBooleanValue();
>>> -  if (OtherIsBooleanDespiteType)
>>> -    OtherRange = IntRange::forBoolType();
>>> -
>>> -  if (FieldDecl *Bitfield = Other->getSourceBitField())
>>> -    if (!Bitfield->getBitWidth()->isValueDependent())
>>> -      OtherRange.Width =
>>> -          std::min(Bitfield->getBitWidthValue(S.Context),
>>> OtherRange.Width);
>>> -
>>> -  // Check whether the constant value can be represented in OtherRange.
>>> Bail
>>> -  // out if so; this isn't an out-of-range comparison.
>>> -  PromotedRange OtherPromotedRange(OtherRange, Value.getBitWidth(),
>>> -                                   Value.isUnsigned());
>>> -
>>> -  auto Cmp = OtherPromotedRange.compare(Value);
>>> -  if (Cmp != PromotedRange::Min && Cmp != PromotedRange::Max &&
>>> -      Cmp != PromotedRange::OnlyValue)
>>> +  QualType OType = Other->IgnoreParenImpCasts()->getType();
>>> +  if (!OType->isIntegerType())
>>>       return false;
>>>   -  auto Result = PromotedRange::constantValue(E->getOpcode(), Cmp,
>>> RhsConstant);
>>> -  if (!Result)
>>> +  // Determine which limit (min/max) the constant is, if either.
>>> +  llvm::Optional<LimitType> ValueType = IsTypeLimit(S, Constant, Other,
>>> Value);
>>> +  if (!ValueType)
>>>       return false;
>>>   +  bool ConstIsLowerBound = (Op == BO_LT || Op == BO_LE) ^ RhsConstant;
>>> +  bool ResultWhenConstEqualsOther = (Op == BO_LE || Op == BO_GE);
>>> +  if (ValueType != LimitType::Both) {
>>> +    bool ResultWhenConstNeOther =
>>> +        ConstIsLowerBound ^ (ValueType == LimitType::Max);
>>> +    if (ResultWhenConstEqualsOther != ResultWhenConstNeOther)
>>> +      return false; // The comparison is not tautological.
>>> +  } else if (ResultWhenConstEqualsOther == ConstIsLowerBound)
>>> +    return false; // The comparison is not tautological.
>>> +
>>> +  const bool Result = ResultWhenConstEqualsOther;
>>> +
>>>     // Should be enough for uint128 (39 decimal digits)
>>>     SmallString<64> PrettySourceValue;
>>>     llvm::raw_svector_ostream OS(PrettySourceValue);
>>> @@ -8859,20 +8782,20 @@ static bool CheckTautologicalComparison(
>>>       S.DiagRuntimeBehavior(
>>>         E->getOperatorLoc(), E,
>>>         S.PDiag(diag::warn_tautological_bool_compare)
>>> -          << OS.str() << classifyConstantValue(Constant)
>>> -          << OtherT << !OtherT->isBooleanType() << *Result
>>> +          << OS.str() <<
>>> classifyConstantValue(Constant->IgnoreParenImpCasts())
>>> +          << OType << !OType->isBooleanType() << Result
>>>             << E->getLHS()->getSourceRange() <<
>>> E->getRHS()->getSourceRange());
>>>       return true;
>>>     }
>>>   -  unsigned Diag = (isKnownToHaveUnsignedValue(OriginalOther) && Value
>>> == 0)
>>> -                      ? (HasEnumType(OriginalOther)
>>> +  unsigned Diag = (isKnownToHaveUnsignedValue(Other) && Value == 0)
>>> +                      ? (HasEnumType(Other)
>>>                                ?
>>> diag::warn_unsigned_enum_always_true_comparison
>>>                                :
>>> diag::warn_unsigned_always_true_comparison)
>>>                         : diag::warn_tautological_constant_compare;
>>>       S.Diag(E->getOperatorLoc(), Diag)
>>> -      << RhsConstant << OtherT << E->getOpcodeStr() << OS.str() <<
>>> *Result
>>> +      << RhsConstant << OType << E->getOpcodeStr() << OS.str() << Result
>>>         << E->getLHS()->getSourceRange() <<
>>> E->getRHS()->getSourceRange();
>>>       return true;
>>> @@ -8894,6 +8817,7 @@ static bool DiagnoseOutOfRangeComparison
>>>     QualType OtherT = Other->getType();
>>>     if (const auto *AT = OtherT->getAs<AtomicType>())
>>>       OtherT = AT->getValueType();
>>> +
>>>     IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
>>>       // Whether we're treating Other as being a bool because of the form
>>> of
>>> @@ -8903,25 +8827,91 @@ static bool DiagnoseOutOfRangeComparison
>>>     if (OtherIsBooleanDespiteType)
>>>       OtherRange = IntRange::forBoolType();
>>>   -  if (FieldDecl *Bitfield = Other->getSourceBitField())
>>> -    if (!Bitfield->getBitWidth()->isValueDependent())
>>> -      OtherRange.Width =
>>> -          std::min(Bitfield->getBitWidthValue(S.Context),
>>> OtherRange.Width);
>>> +  unsigned OtherWidth = OtherRange.Width;
>>> +
>>> +  BinaryOperatorKind op = E->getOpcode();
>>> +  bool IsTrue = true;
>>>       // Check whether the constant value can be represented in
>>> OtherRange. Bail
>>>     // out if so; this isn't an out-of-range comparison.
>>> -  PromotedRange OtherPromotedRange(OtherRange, Value.getBitWidth(),
>>> -                                   Value.isUnsigned());
>>> -  auto Cmp = OtherPromotedRange.compare(Value);
>>> -
>>> -  // If Value is in the range of possible Other values, this comparison
>>> is not
>>> -  // tautological.
>>> -  if (Cmp & PromotedRange::InRangeFlag)
>>> -    return false;
>>> +  {
>>> +    QualType ConstantT = Constant->getType();
>>> +    QualType CommonT = E->getLHS()->getType();
>>>   -  auto IsTrue = PromotedRange::constantValue(E->getOpcode(), Cmp,
>>> RhsConstant);
>>> -  if (!IsTrue)
>>> -    return false;
>>> +    if (S.Context.hasSameUnqualifiedType(OtherT, ConstantT) &&
>>> +        !OtherIsBooleanDespiteType)
>>> +      return false;
>>> +    assert((OtherT->isIntegerType() && ConstantT->isIntegerType()) &&
>>> +           "comparison with non-integer type");
>>> +
>>> +    bool ConstantSigned = ConstantT->isSignedIntegerType();
>>> +    bool CommonSigned = CommonT->isSignedIntegerType();
>>> +
>>> +    bool EqualityOnly = false;
>>> +
>>> +    if (CommonSigned) {
>>> +      // The common type is signed, therefore no signed to unsigned
>>> conversion.
>>> +      if (!OtherRange.NonNegative) {
>>> +        // Check that the constant is representable in type OtherT.
>>> +        if (ConstantSigned) {
>>> +          if (OtherWidth >= Value.getMinSignedBits())
>>> +            return false;
>>> +        } else { // !ConstantSigned
>>> +          if (OtherWidth >= Value.getActiveBits() + 1)
>>> +            return false;
>>> +        }
>>> +      } else { // !OtherSigned
>>> +               // Check that the constant is representable in type
>>> OtherT.
>>> +        // Negative values are out of range.
>>> +        if (ConstantSigned) {
>>> +          if (Value.isNonNegative() && OtherWidth >=
>>> Value.getActiveBits())
>>> +            return false;
>>> +        } else { // !ConstantSigned
>>> +          if (OtherWidth >= Value.getActiveBits())
>>> +            return false;
>>> +        }
>>> +      }
>>> +    } else { // !CommonSigned
>>> +      if (OtherRange.NonNegative) {
>>> +        if (OtherWidth >= Value.getActiveBits())
>>> +          return false;
>>> +      } else { // OtherSigned
>>> +        assert(!ConstantSigned &&
>>> +               "Two signed types converted to unsigned types.");
>>> +        // Check to see if the constant is representable in OtherT.
>>> +        if (OtherWidth > Value.getActiveBits())
>>> +          return false;
>>> +        // Check to see if the constant is equivalent to a negative
>>> value
>>> +        // cast to CommonT.
>>> +        if (S.Context.getIntWidth(ConstantT) ==
>>> +                S.Context.getIntWidth(CommonT) &&
>>> +            Value.isNegative() && Value.getMinSignedBits() <=
>>> OtherWidth)
>>> +          return false;
>>> +        // The constant value rests between values that OtherT can
>>> represent
>>> +        // after conversion.  Relational comparison still works, but
>>> equality
>>> +        // comparisons will be tautological.
>>> +        EqualityOnly = true;
>>> +      }
>>> +    }
>>> +
>>> +    bool PositiveConstant = !ConstantSigned || Value.isNonNegative();
>>> +
>>> +    if (op == BO_EQ || op == BO_NE) {
>>> +      IsTrue = op == BO_NE;
>>> +    } else if (EqualityOnly) {
>>> +      return false;
>>> +    } else if (RhsConstant) {
>>> +      if (op == BO_GT || op == BO_GE)
>>> +        IsTrue = !PositiveConstant;
>>> +      else // op == BO_LT || op == BO_LE
>>> +        IsTrue = PositiveConstant;
>>> +    } else {
>>> +      if (op == BO_LT || op == BO_LE)
>>> +        IsTrue = !PositiveConstant;
>>> +      else // op == BO_GT || op == BO_GE
>>> +        IsTrue = PositiveConstant;
>>> +    }
>>> +  }
>>>       // If this is a comparison to an enum constant, include that
>>>     // constant in the diagnostic.
>>> @@ -8940,7 +8930,7 @@ static bool DiagnoseOutOfRangeComparison
>>>       E->getOperatorLoc(), E,
>>>       S.PDiag(diag::warn_out_of_range_compare)
>>>           << OS.str() << classifyConstantValue(Constant)
>>> -        << OtherT << OtherIsBooleanDespiteType << *IsTrue
>>> +        << OtherT << OtherIsBooleanDespiteType << IsTrue
>>>           << E->getLHS()->getSourceRange() <<
>>> E->getRHS()->getSourceRange());
>>>        return true;
>>>
>>> Modified: cfe/trunk/test/Sema/tautological-constant-compare.c
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/tautological-constant-compare.c?rev=320162&r1=320161&r2=320162&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/test/Sema/tautological-constant-compare.c (original)
>>> +++ cfe/trunk/test/Sema/tautological-constant-compare.c Fri Dec  8
>>> 08:54:08 2017
>>> @@ -94,17 +94,15 @@ int main()
>>>     if (-32768 >= s)
>>>         return 0;
>>>   -  // Note: both sides are promoted to unsigned long prior to the
>>> comparison, so
>>> -  // it is perfectly possible for a short to compare greater than
>>> 32767UL.
>>>     if (s == 32767UL)
>>>         return 0;
>>>     if (s != 32767UL)
>>>         return 0;
>>>     if (s < 32767UL)
>>>         return 0;
>>> -  if (s <= 32767UL)
>>> +  if (s <= 32767UL) // expected-warning {{comparison 'short' <= 32767 is
>>> always true}}
>>>         return 0;
>>> -  if (s > 32767UL)
>>> +  if (s > 32767UL) // expected-warning {{comparison 'short' > 32767 is
>>> always false}}
>>>         return 0;
>>>     if (s >= 32767UL)
>>>         return 0;
>>> @@ -113,66 +111,13 @@ int main()
>>>         return 0;
>>>     if (32767UL != s)
>>>         return 0;
>>> -  if (32767UL < s)
>>> +  if (32767UL < s) // expected-warning {{comparison 32767 < 'short' is
>>> always false}}
>>>         return 0;
>>>     if (32767UL <= s)
>>>         return 0;
>>>     if (32767UL > s)
>>>         return 0;
>>> -  if (32767UL >= s)
>>> -      return 0;
>>> -
>>> -  if (s == 0UL)
>>> -      return 0;
>>> -  if (s != 0UL)
>>> -      return 0;
>>> -  if (s < 0UL) // expected-warning {{comparison of unsigned expression <
>>> 0 is always false}}
>>> -      return 0;
>>> -  if (s <= 0UL)
>>> -      return 0;
>>> -  if (s > 0UL)
>>> -      return 0;
>>> -  if (s >= 0UL) // expected-warning {{comparison of unsigned expression
>>> >= 0 is always true}}
>>> -      return 0;
>>> -
>>> -  if (0UL == s)
>>> -      return 0;
>>> -  if (0UL != s)
>>> -      return 0;
>>> -  if (0UL < s)
>>> -      return 0;
>>> -  if (0UL <= s) // expected-warning {{comparison of 0 <= unsigned
>>> expression is always true}}
>>> -      return 0;
>>> -  if (0UL > s) // expected-warning {{comparison of 0 > unsigned
>>> expression is always false}}
>>> -      return 0;
>>> -  if (0UL >= s)
>>> -      return 0;
>>> -
>>> -  enum { ULONG_MAX = (2UL * (unsigned long)__LONG_MAX__ + 1UL) };
>>> -  if (s == 2UL * (unsigned long)__LONG_MAX__ + 1UL)
>>> -      return 0;
>>> -  if (s != 2UL * (unsigned long)__LONG_MAX__ + 1UL)
>>> -      return 0;
>>> -  if (s < 2UL * (unsigned long)__LONG_MAX__ + 1UL)
>>> -      return 0;
>>> -  if (s <= 2UL * (unsigned long)__LONG_MAX__ + 1UL) //
>>> expected-warning-re {{comparison 'short' <= {{.*}} is always true}}
>>> -      return 0;
>>> -  if (s > 2UL * (unsigned long)__LONG_MAX__ + 1UL) //
>>> expected-warning-re {{comparison 'short' > {{.*}} is always false}}
>>> -      return 0;
>>> -  if (s >= 2UL * (unsigned long)__LONG_MAX__ + 1UL)
>>> -      return 0;
>>> -
>>> -  if (2UL * (unsigned long)__LONG_MAX__ + 1UL == s)
>>> -      return 0;
>>> -  if (2UL * (unsigned long)__LONG_MAX__ + 1UL != s)
>>> -      return 0;
>>> -  if (2UL * (unsigned long)__LONG_MAX__ + 1UL < s) //
>>> expected-warning-re {{comparison {{.*}} < 'short' is always false}}
>>> -      return 0;
>>> -  if (2UL * (unsigned long)__LONG_MAX__ + 1UL <= s)
>>> -      return 0;
>>> -  if (2UL * (unsigned long)__LONG_MAX__ + 1UL > s)
>>> -      return 0;
>>> -  if (2UL * (unsigned long)__LONG_MAX__ + 1UL >= s) //
>>> expected-warning-re {{comparison {{.*}} >= 'short' is always true}}
>>> +  if (32767UL >= s) // expected-warning {{comparison 32767 >= 'short' is
>>> always true}}
>>>         return 0;
>>>       // FIXME: assumes two's complement
>>> @@ -336,6 +281,8 @@ int main()
>>>     if (0 >= s)
>>>       return 0;
>>>   +  // However the comparison with 0U would warn
>>> +
>>>     unsigned short us = value();
>>>     #ifdef TEST
>>>
>>> Modified: cfe/trunk/test/Sema/tautological-constant-enum-compare.c
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/tautological-constant-enum-compare.c?rev=320162&r1=320161&r2=320162&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/test/Sema/tautological-constant-enum-compare.c (original)
>>> +++ cfe/trunk/test/Sema/tautological-constant-enum-compare.c Fri Dec  8
>>> 08:54:08 2017
>>> @@ -9,96 +9,78 @@ int main() {
>>>     #ifdef SILENCE
>>>     // expected-no-diagnostics
>>> -#else
>>> -  // If we promote to unsigned, it doesn't matter whether the enum's
>>> underlying
>>> -  // type was signed.
>>> -  if (a < 0U) // expected-warning {{comparison of unsigned enum
>>> expression < 0 is always false}}
>>> -    return 0;
>>> -  if (0U >= a)
>>> -    return 0;
>>> -  if (a > 0U)
>>> -    return 0;
>>> -  if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum
>>> expression is always true}}
>>> -    return 0;
>>> -  if (a <= 0U)
>>> -    return 0;
>>> -  if (0U > a) // expected-warning {{comparison of 0 > unsigned enum
>>> expression is always false}}
>>> -    return 0;
>>> -  if (a >= 0U) // expected-warning {{comparison of unsigned enum
>>> expression >= 0 is always true}}
>>> -    return 0;
>>> -  if (0U < a)
>>> -    return 0;
>>> +#endif
>>>   -  if (a < 4294967295U)
>>> +#ifdef UNSIGNED
>>> +#ifndef SILENCE
>>> +  if (a < 0) // expected-warning {{comparison of unsigned enum
>>> expression < 0 is always false}}
>>>       return 0;
>>> -  if (4294967295U >= a) // expected-warning {{comparison 4294967295 >=
>>> 'enum A' is always true}}
>>> +  if (0 >= a)
>>>       return 0;
>>> -  if (a > 4294967295U) // expected-warning {{comparison 'enum A' >
>>> 4294967295 is always false}}
>>> +  if (a > 0)
>>>       return 0;
>>> -  if (4294967295U <= a)
>>> +  if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum
>>> expression is always true}}
>>>       return 0;
>>> -  if (a <= 4294967295U) // expected-warning {{comparison 'enum A' <=
>>> 4294967295 is always true}}
>>> +  if (a <= 0)
>>>       return 0;
>>> -  if (4294967295U > a)
>>> +  if (0 > a) // expected-warning {{comparison of 0 > unsigned enum
>>> expression is always false}}
>>>       return 0;
>>> -  if (a >= 4294967295U)
>>> +  if (a >= 0) // expected-warning {{comparison of unsigned enum
>>> expression >= 0 is always true}}
>>>       return 0;
>>> -  if (4294967295U < a) // expected-warning {{comparison 4294967295 <
>>> 'enum A' is always false}}
>>> +  if (0 < a)
>>>       return 0;
>>>   -  if (a < 2147483647U)
>>> +  if (a < 0U) // expected-warning {{comparison of unsigned enum
>>> expression < 0 is always false}}
>>>       return 0;
>>> -  if (2147483647U >= a)
>>> +  if (0U >= a)
>>>       return 0;
>>> -  if (a > 2147483647U)
>>> +  if (a > 0U)
>>>       return 0;
>>> -  if (2147483647U <= a)
>>> +  if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum
>>> expression is always true}}
>>>       return 0;
>>> -  if (a <= 2147483647U)
>>> +  if (a <= 0U)
>>>       return 0;
>>> -  if (2147483647U > a)
>>> +  if (0U > a) // expected-warning {{comparison of 0 > unsigned enum
>>> expression is always false}}
>>>       return 0;
>>> -  if (a >= 2147483647U)
>>> +  if (a >= 0U) // expected-warning {{comparison of unsigned enum
>>> expression >= 0 is always true}}
>>>       return 0;
>>> -  if (2147483647U < a)
>>> +  if (0U < a)
>>>       return 0;
>>> -#endif
>>>   -#if defined(UNSIGNED) && !defined(SILENCE)
>>> -  if (a < 0) // expected-warning {{comparison of unsigned enum
>>> expression < 0 is always false}}
>>> +  if (a < 4294967295)
>>>       return 0;
>>> -  if (0 >= a)
>>> +  if (4294967295 >= a) // expected-warning {{comparison 4294967295 >=
>>> 'enum A' is always true}}
>>>       return 0;
>>> -  if (a > 0)
>>> +  if (a > 4294967295) // expected-warning {{comparison 'enum A' >
>>> 4294967295 is always false}}
>>>       return 0;
>>> -  if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum
>>> expression is always true}}
>>> +  if (4294967295 <= a)
>>>       return 0;
>>> -  if (a <= 0)
>>> +  if (a <= 4294967295) // expected-warning {{comparison 'enum A' <=
>>> 4294967295 is always true}}
>>>       return 0;
>>> -  if (0 > a) // expected-warning {{comparison of 0 > unsigned enum
>>> expression is always false}}
>>> +  if (4294967295 > a)
>>>       return 0;
>>> -  if (a >= 0) // expected-warning {{comparison of unsigned enum
>>> expression >= 0 is always true}}
>>> +  if (a >= 4294967295)
>>>       return 0;
>>> -  if (0 < a)
>>> +  if (4294967295 < a) // expected-warning {{comparison 4294967295 <
>>> 'enum A' is always false}}
>>>       return 0;
>>>   -  if (a < 4294967295)
>>> +  if (a < 4294967295U)
>>>       return 0;
>>> -  if (4294967295 >= a) // expected-warning {{comparison 4294967295 >=
>>> 'enum A' is always true}}
>>> +  if (4294967295U >= a) // expected-warning {{comparison 4294967295 >=
>>> 'enum A' is always true}}
>>>       return 0;
>>> -  if (a > 4294967295) // expected-warning {{comparison 'enum A' >
>>> 4294967295 is always false}}
>>> +  if (a > 4294967295U) // expected-warning {{comparison 'enum A' >
>>> 4294967295 is always false}}
>>>       return 0;
>>> -  if (4294967295 <= a)
>>> +  if (4294967295U <= a)
>>>       return 0;
>>> -  if (a <= 4294967295) // expected-warning {{comparison 'enum A' <=
>>> 4294967295 is always true}}
>>> +  if (a <= 4294967295U) // expected-warning {{comparison 'enum A' <=
>>> 4294967295 is always true}}
>>>       return 0;
>>> -  if (4294967295 > a)
>>> +  if (4294967295U > a)
>>>       return 0;
>>> -  if (a >= 4294967295)
>>> +  if (a >= 4294967295U)
>>>       return 0;
>>> -  if (4294967295 < a) // expected-warning {{comparison 4294967295 <
>>> 'enum A' is always false}}
>>> +  if (4294967295U < a) // expected-warning {{comparison 4294967295 <
>>> 'enum A' is always false}}
>>>       return 0;
>>> -#else // SIGNED || SILENCE
>>> +#else // SILENCE
>>>     if (a < 0)
>>>       return 0;
>>>     if (0 >= a)
>>> @@ -116,24 +98,23 @@ int main() {
>>>     if (0 < a)
>>>       return 0;
>>>   -#ifndef SILENCE
>>> -  if (a < 4294967295) // expected-warning {{comparison of constant
>>> 4294967295 with expression of type 'enum A' is always true}}
>>> +  if (a < 0U)
>>>       return 0;
>>> -  if (4294967295 >= a) // expected-warning {{comparison of constant
>>> 4294967295 with expression of type 'enum A' is always true}}
>>> +  if (0U >= a)
>>>       return 0;
>>> -  if (a > 4294967295) // expected-warning {{comparison of constant
>>> 4294967295 with expression of type 'enum A' is always false}}
>>> +  if (a > 0U)
>>>       return 0;
>>> -  if (4294967295 <= a) // expected-warning {{comparison of constant
>>> 4294967295 with expression of type 'enum A' is always false}}
>>> +  if (0U <= a)
>>>       return 0;
>>> -  if (a <= 4294967295) // expected-warning {{comparison of constant
>>> 4294967295 with expression of type 'enum A' is always true}}
>>> +  if (a <= 0U)
>>>       return 0;
>>> -  if (4294967295 > a) // expected-warning {{comparison of constant
>>> 4294967295 with expression of type 'enum A' is always true}}
>>> +  if (0U > a)
>>>       return 0;
>>> -  if (a >= 4294967295) // expected-warning {{comparison of constant
>>> 4294967295 with expression of type 'enum A' is always false}}
>>> +  if (a >= 0U)
>>>       return 0;
>>> -  if (4294967295 < a) // expected-warning {{comparison of constant
>>> 4294967295 with expression of type 'enum A' is always false}}
>>> +  if (0U < a)
>>>       return 0;
>>> -#else
>>> +
>>>     if (a < 4294967295)
>>>       return 0;
>>>     if (4294967295 >= a)
>>> @@ -150,10 +131,26 @@ int main() {
>>>       return 0;
>>>     if (4294967295 < a)
>>>       return 0;
>>> -#endif
>>> -#endif
>>>   -#if defined(SIGNED) && !defined(SILENCE)
>>> +  if (a < 4294967295U)
>>> +    return 0;
>>> +  if (4294967295U >= a)
>>> +    return 0;
>>> +  if (a > 4294967295U)
>>> +    return 0;
>>> +  if (4294967295U <= a)
>>> +    return 0;
>>> +  if (a <= 4294967295U)
>>> +    return 0;
>>> +  if (4294967295U > a)
>>> +    return 0;
>>> +  if (a >= 4294967295U)
>>> +    return 0;
>>> +  if (4294967295U < a)
>>> +    return 0;
>>> +#endif
>>> +#elif defined(SIGNED)
>>> +#ifndef SILENCE
>>>     if (a < -2147483648) // expected-warning {{comparison 'enum A' <
>>> -2147483648 is always false}}
>>>       return 0;
>>>     if (-2147483648 >= a)
>>> @@ -187,25 +184,24 @@ int main() {
>>>       return 0;
>>>     if (2147483647 < a) // expected-warning {{comparison 2147483647 <
>>> 'enum A' is always false}}
>>>       return 0;
>>> -#elif defined(UNSIGNED) && !defined(SILENCE)
>>> -#ifndef SILENCE
>>> -  if (a < -2147483648) // expected-warning {{comparison of constant
>>> -2147483648 with expression of type 'enum A' is always false}}
>>> +
>>> +  if (a < 2147483647U)
>>>       return 0;
>>> -  if (-2147483648 >= a) // expected-warning {{comparison of constant
>>> -2147483648 with expression of type 'enum A' is always false}}
>>> +  if (2147483647U >= a) // expected-warning {{comparison 2147483647 >=
>>> 'enum A' is always true}}
>>>       return 0;
>>> -  if (a > -2147483648) // expected-warning {{comparison of constant
>>> -2147483648 with expression of type 'enum A' is always true}}
>>> +  if (a > 2147483647U) // expected-warning {{comparison 'enum A' >
>>> 2147483647 is always false}}
>>>       return 0;
>>> -  if (-2147483648 <= a) // expected-warning {{comparison of constant
>>> -2147483648 with expression of type 'enum A' is always true}}
>>> +  if (2147483647U <= a)
>>>       return 0;
>>> -  if (a <= -2147483648) // expected-warning {{comparison of constant
>>> -2147483648 with expression of type 'enum A' is always false}}
>>> +  if (a <= 2147483647U) // expected-warning {{comparison 'enum A' <=
>>> 2147483647 is always true}}
>>>       return 0;
>>> -  if (-2147483648 > a) // expected-warning {{comparison of constant
>>> -2147483648 with expression of type 'enum A' is always false}}
>>> +  if (2147483647U > a)
>>>       return 0;
>>> -  if (a >= -2147483648) // expected-warning {{comparison of constant
>>> -2147483648 with expression of type 'enum A' is always true}}
>>> +  if (a >= 2147483647U)
>>>       return 0;
>>> -  if (-2147483648 < a) // expected-warning {{comparison of constant
>>> -2147483648 with expression of type 'enum A' is always true}}
>>> +  if (2147483647U < a) // expected-warning {{comparison 2147483647 <
>>> 'enum A' is always false}}
>>>       return 0;
>>> -#else
>>> +#else // SILENCE
>>>     if (a < -2147483648)
>>>       return 0;
>>>     if (-2147483648 >= a)
>>> @@ -222,7 +218,6 @@ int main() {
>>>       return 0;
>>>     if (-2147483648 < a)
>>>       return 0;
>>> -#endif
>>>       if (a < 2147483647)
>>>       return 0;
>>> @@ -240,6 +235,24 @@ int main() {
>>>       return 0;
>>>     if (2147483647 < a)
>>>       return 0;
>>> +
>>> +  if (a < 2147483647U)
>>> +    return 0;
>>> +  if (2147483647U >= a)
>>> +    return 0;
>>> +  if (a > 2147483647U)
>>> +    return 0;
>>> +  if (2147483647U <= a)
>>> +    return 0;
>>> +  if (a <= 2147483647U)
>>> +    return 0;
>>> +  if (2147483647U > a)
>>> +    return 0;
>>> +  if (a >= 2147483647U)
>>> +    return 0;
>>> +  if (2147483647U < a)
>>> +    return 0;
>>> +#endif
>>>   #endif
>>>       return 1;
>>>
>>> Modified: cfe/trunk/test/SemaCXX/compare.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/compare.cpp?rev=320162&r1=320161&r2=320162&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/test/SemaCXX/compare.cpp (original)
>>> +++ cfe/trunk/test/SemaCXX/compare.cpp Fri Dec  8 08:54:08 2017
>>> @@ -245,8 +245,8 @@ void test4(short s) {
>>>     // unsigned.
>>>     const unsigned B = -1;
>>>     void (s < B); // expected-warning{{comparison of integers of
>>> different signs: 'short' and 'const unsigned int'}}
>>> -  void (s > B); // expected-warning{{comparison 'short' > 4294967295 is
>>> always false}}
>>> -  void (s <= B); // expected-warning{{comparison 'short' <= 4294967295
>>> is always true}}
>>> +  void (s > B); // expected-warning{{comparison of integers of different
>>> signs: 'short' and 'const unsigned int'}}
>>> +  void (s <= B); // expected-warning{{comparison of integers of
>>> different signs: 'short' and 'const unsigned int'}}
>>>     void (s >= B); // expected-warning{{comparison of integers of
>>> different signs: 'short' and 'const unsigned int'}}
>>>     void (s == B); // expected-warning{{comparison of integers of
>>> different signs: 'short' and 'const unsigned int'}}
>>>     void (s != B); // expected-warning{{comparison of integers of
>>> different signs: 'short' and 'const unsigned int'}}
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>> --
>>
>> -Bill Seurer
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>


More information about the cfe-commits mailing list