r320211 - Unify implementation of our two different flavours of -Wtautological-compare,

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 8 14:57:11 PST 2017


Author: rsmith
Date: Fri Dec  8 14:57:11 2017
New Revision: 320211

URL: http://llvm.org/viewvc/llvm-project?rev=320211&view=rev
Log:
Unify implementation of our two different flavours of -Wtautological-compare,
and fold together into a single function.

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 re-commits r320122 and r320124, minus two changes:

 * Comparisons between a constant and a non-constant expression of enumeration
   type never warn, not even if the constant is out of range. We should be
   warning about the creation of such a constant, not about its use.

 * We do not use more precise bit-widths for comparisons against bit-fields.
   The more precise diagnostics probably are the right thing, but we should
   consider moving them under their own warning flag.

Other than the refactoring, this patch should only change the behavior for the
buggy cases (where the warnings didn't take into account that promotion from
signed to unsigned can leave a range of inaccessible values in the middle of
the promoted 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=320211&r1=320210&r2=320211&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Dec  8 14:57:11 2017
@@ -8662,54 +8662,119 @@ 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 if (R.Width >= BitWidth && !Unsigned) {
+      // Promotion made the type *narrower*. This happens when promoting
+      // a < 32-bit unsigned / <= 32-bit signed bit-field to 'signed int'.
+      // Treat all values of 'signed int' as being in range for now.
+      PromotedMin = llvm::APSInt::getMinValue(BitWidth, Unsigned);
+      PromotedMax = llvm::APSInt::getMaxValue(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);
+    }
+  }
 
-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 };
-};
-
-} // namespace
+  // Determine whether this range is contiguous (has no hole).
+  bool isContiguous() const { return PromotedMin <= PromotedMax; }
 
-/// 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>();
+  // 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,
 
-  if (isKnownToHaveUnsignedValue(Other) && Value == 0)
-    return LimitType::Min;
+    OnlyValue = LE | GE | EQ | InRangeFlag,
+    InHole = NE
+  };
 
-  // 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();
+  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;
+    }
 
-  IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
-  if (Other->isKnownToHaveBooleanValue())
-    OtherRange = IntRange::forBoolType();
+    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;
+      }
+    }
 
-  // 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;
+    llvm_unreachable("impossible compare result");
+  }
 
-  return llvm::None;
+  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;
+  }
+};
 }
 
 static bool HasEnumType(Expr *E) {
@@ -8742,82 +8807,30 @@ static bool CheckTautologicalComparison(
                                         Expr *Constant, Expr *Other,
                                         const llvm::APSInt &Value,
                                         bool RhsConstant) {
-  // Disable warning in template instantiations
-  // and only analyze <, >, <= and >= operations.
-  if (S.inTemplateInstantiation() || !E->isRelationalOp())
-    return false;
-
-  BinaryOperatorKind Op = E->getOpcode();
-
-  QualType OType = Other->IgnoreParenImpCasts()->getType();
-  if (!OType->isIntegerType())
-    return false;
-
-  // 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);
-  OS << Value;
-
-  // FIXME: We use a somewhat different formatting for the cases involving
-  // boolean values for historical reasons. We should pick a consistent way
-  // of presenting these diagnostics.
-  if (Other->isKnownToHaveBooleanValue()) {
-    S.DiagRuntimeBehavior(
-      E->getOperatorLoc(), E,
-      S.PDiag(diag::warn_tautological_bool_compare)
-          << OS.str() << classifyConstantValue(Constant->IgnoreParenImpCasts())
-          << OType << !OType->isBooleanType() << Result
-          << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange());
-    return true;
-  }
-
-  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 << OType << E->getOpcodeStr() << OS.str() << Result
-      << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
-
-  return true;
-}
-
-static bool DiagnoseOutOfRangeComparison(Sema &S, BinaryOperator *E,
-                                         Expr *Constant, Expr *Other,
-                                         const llvm::APSInt &Value,
-                                         bool RhsConstant) {
-  // Disable warning in template instantiations.
   if (S.inTemplateInstantiation())
     return false;
 
+  Expr *OriginalOther = Other;
+
   Constant = Constant->IgnoreParenImpCasts();
   Other = Other->IgnoreParenImpCasts();
 
+  // Suppress warnings on tautological comparisons between values of the same
+  // enumeration type. There are only two ways we could warn on this:
+  //  - If the constant is outside the range of representable values of
+  //    the enumeration. In such a case, we should warn about the cast
+  //    to enumeration type, not about the comparison.
+  //  - If the constant is the maximum / minimum in-range value. For an
+  //    enumeratin type, such comparisons can be meaningful and useful.
+  if (Constant->getType()->isEnumeralType() &&
+      S.Context.hasSameUnqualifiedType(Constant->getType(), Other->getType()))
+    return false;
+
   // 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
@@ -8827,91 +8840,24 @@ static bool DiagnoseOutOfRangeComparison
   if (OtherIsBooleanDespiteType)
     OtherRange = IntRange::forBoolType();
 
-  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.
-  {
-    QualType ConstantT = Constant->getType();
-    QualType CommonT = E->getLHS()->getType();
-
-    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();
+  // Determine the promoted range of the other type and see if a comparison of
+  // the constant against that range is tautological.
+  PromotedRange OtherPromotedRange(OtherRange, Value.getBitWidth(),
+                                   Value.isUnsigned());
+  auto Cmp = OtherPromotedRange.compare(Value);
+  auto Result = PromotedRange::constantValue(E->getOpcode(), Cmp, RhsConstant);
+  if (!Result)
+    return false;
 
-    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;
-    }
-  }
+  // Suppress the diagnostic for an in-range comparison if the constant comes
+  // from a macro or enumerator. We don't want to diagnose
+  //
+  //   some_long_value <= INT_MAX
+  //
+  // when sizeof(int) == sizeof(long).
+  bool InRange = Cmp & PromotedRange::InRangeFlag;
+  if (InRange && IsEnumConstOrFromMacro(S, Constant))
+    return false;
 
   // If this is a comparison to an enum constant, include that
   // constant in the diagnostic.
@@ -8919,6 +8865,7 @@ static bool DiagnoseOutOfRangeComparison
   if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(Constant))
     ED = dyn_cast<EnumConstantDecl>(DR->getDecl());
 
+  // Should be enough for uint128 (39 decimal digits)
   SmallString<64> PrettySourceValue;
   llvm::raw_svector_ostream OS(PrettySourceValue);
   if (ED)
@@ -8926,14 +8873,30 @@ static bool DiagnoseOutOfRangeComparison
   else
     OS << Value;
 
-  S.DiagRuntimeBehavior(
-    E->getOperatorLoc(), E,
-    S.PDiag(diag::warn_out_of_range_compare)
-        << OS.str() << classifyConstantValue(Constant)
-        << OtherT << OtherIsBooleanDespiteType << IsTrue
-        << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange());
+  // FIXME: We use a somewhat different formatting for the in-range cases and
+  // cases involving boolean values for historical reasons. We should pick a
+  // consistent way of presenting these diagnostics.
+  if (!InRange || Other->isKnownToHaveBooleanValue()) {
+    S.DiagRuntimeBehavior(
+      E->getOperatorLoc(), E,
+      S.PDiag(!InRange ? diag::warn_out_of_range_compare
+                       : diag::warn_tautological_bool_compare)
+          << OS.str() << classifyConstantValue(Constant)
+          << OtherT << OtherIsBooleanDespiteType << *Result
+          << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange());
+  } else {
+    unsigned Diag = (isKnownToHaveUnsignedValue(OriginalOther) && Value == 0)
+                        ? (HasEnumType(OriginalOther)
+                               ? 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
+        << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
+  }
 
-   return true;
+  return true;
 }
 
 /// Analyze the operands of the given comparison.  Implements the
@@ -8983,12 +8946,8 @@ static void AnalyzeComparison(Sema &S, B
 
       // Check whether an integer constant comparison results in a value
       // of 'true' or 'false'.
-
       if (CheckTautologicalComparison(S, E, Const, Other, Value, RhsConstant))
         return AnalyzeImpConvsInComparison(S, E);
-
-      if (DiagnoseOutOfRangeComparison(S, E, Const, Other, Value, RhsConstant))
-        return AnalyzeImpConvsInComparison(S, E);
     }
   }
 

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=320211&r1=320210&r2=320211&view=diff
==============================================================================
--- cfe/trunk/test/Sema/tautological-constant-compare.c (original)
+++ cfe/trunk/test/Sema/tautological-constant-compare.c Fri Dec  8 14:57:11 2017
@@ -94,15 +94,17 @@ 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) // expected-warning {{comparison 'short' <= 32767 is always true}}
+  if (s <= 32767UL)
       return 0;
-  if (s > 32767UL) // expected-warning {{comparison 'short' > 32767 is always false}}
+  if (s > 32767UL)
       return 0;
   if (s >= 32767UL)
       return 0;
@@ -111,13 +113,66 @@ int main()
       return 0;
   if (32767UL != s)
       return 0;
-  if (32767UL < s) // expected-warning {{comparison 32767 < 'short' is always false}}
+  if (32767UL < s)
       return 0;
   if (32767UL <= s)
       return 0;
   if (32767UL > s)
       return 0;
-  if (32767UL >= s) // expected-warning {{comparison 32767 >= 'short' is always true}}
+  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}}
       return 0;
 
   // FIXME: assumes two's complement
@@ -281,8 +336,6 @@ int main()
   if (0 >= s)
     return 0;
 
-  // However the comparison with 0U would warn
-
   unsigned short us = value();
 
 #ifdef TEST
@@ -510,5 +563,23 @@ int main()
   if (maybe >= e)
       return 0;
 
+  // For the time being, use the declared type of bit-fields rather than their
+  // length when determining whether a value is in-range.
+  // FIXME: Reconsider this.
+  struct A {
+    int a : 3;
+    unsigned b : 3;
+    long c : 3;
+    unsigned long d : 3;
+  } a;
+  if (a.a < 3) {}
+  if (a.a < 4) {}
+  if (a.b < 7) {}
+  if (a.b < 8) {}
+  if (a.c < 3) {}
+  if (a.c < 4) {}
+  if (a.d < 7) {}
+  if (a.d < 8) {}
+
   return 1;
 }

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=320211&r1=320210&r2=320211&view=diff
==============================================================================
--- cfe/trunk/test/Sema/tautological-constant-enum-compare.c (original)
+++ cfe/trunk/test/Sema/tautological-constant-enum-compare.c Fri Dec  8 14:57:11 2017
@@ -9,78 +9,96 @@ int main() {
 
 #ifdef SILENCE
   // expected-no-diagnostics
-#endif
+#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;
 
-#ifdef UNSIGNED
-#ifndef SILENCE
-  if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
+  if (a < 4294967295U)
     return 0;
-  if (0 >= a)
+  if (4294967295U >= a) // expected-warning {{comparison 4294967295 >= 'enum A' is always true}}
     return 0;
-  if (a > 0)
+  if (a > 4294967295U) // 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 (4294967295U <= a)
     return 0;
-  if (a <= 0)
+  if (a <= 4294967295U) // 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 (4294967295U > a)
     return 0;
-  if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+  if (a >= 4294967295U)
     return 0;
-  if (0 < a)
+  if (4294967295U < a) // expected-warning {{comparison 4294967295 < 'enum A' is always false}}
     return 0;
 
-  if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
+  if (a < 2147483647U)
     return 0;
-  if (0U >= a)
+  if (2147483647U >= a)
     return 0;
-  if (a > 0U)
+  if (a > 2147483647U)
     return 0;
-  if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+  if (2147483647U <= a)
     return 0;
-  if (a <= 0U)
+  if (a <= 2147483647U)
     return 0;
-  if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+  if (2147483647U > a)
     return 0;
-  if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+  if (a >= 2147483647U)
     return 0;
-  if (0U < a)
+  if (2147483647U < a)
     return 0;
+#endif
 
-  if (a < 4294967295)
+#if defined(UNSIGNED) && !defined(SILENCE)
+  if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
     return 0;
-  if (4294967295 >= a) // expected-warning {{comparison 4294967295 >= 'enum A' is always true}}
+  if (0 >= a)
     return 0;
-  if (a > 4294967295) // expected-warning {{comparison 'enum A' > 4294967295 is always false}}
+  if (a > 0)
     return 0;
-  if (4294967295 <= a)
+  if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
     return 0;
-  if (a <= 4294967295) // expected-warning {{comparison 'enum A' <= 4294967295 is always true}}
+  if (a <= 0)
     return 0;
-  if (4294967295 > a)
+  if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
     return 0;
-  if (a >= 4294967295)
+  if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
     return 0;
-  if (4294967295 < a) // expected-warning {{comparison 4294967295 < 'enum A' is always false}}
+  if (0 < a)
     return 0;
 
-  if (a < 4294967295U)
+  if (a < 4294967295)
     return 0;
-  if (4294967295U >= a) // expected-warning {{comparison 4294967295 >= 'enum A' is always true}}
+  if (4294967295 >= a) // expected-warning {{comparison 4294967295 >= 'enum A' is always true}}
     return 0;
-  if (a > 4294967295U) // expected-warning {{comparison 'enum A' > 4294967295 is always false}}
+  if (a > 4294967295) // expected-warning {{comparison 'enum A' > 4294967295 is always false}}
     return 0;
-  if (4294967295U <= a)
+  if (4294967295 <= a)
     return 0;
-  if (a <= 4294967295U) // expected-warning {{comparison 'enum A' <= 4294967295 is always true}}
+  if (a <= 4294967295) // expected-warning {{comparison 'enum A' <= 4294967295 is always true}}
     return 0;
-  if (4294967295U > a)
+  if (4294967295 > a)
     return 0;
-  if (a >= 4294967295U)
+  if (a >= 4294967295)
     return 0;
-  if (4294967295U < a) // expected-warning {{comparison 4294967295 < 'enum A' is always false}}
+  if (4294967295 < a) // expected-warning {{comparison 4294967295 < 'enum A' is always false}}
     return 0;
-#else // SILENCE
+#else // SIGNED || SILENCE
   if (a < 0)
     return 0;
   if (0 >= a)
@@ -98,23 +116,24 @@ int main() {
   if (0 < a)
     return 0;
 
-  if (a < 0U)
+#ifndef SILENCE
+  if (a < 4294967295) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always true}}
     return 0;
-  if (0U >= a)
+  if (4294967295 >= a) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always true}}
     return 0;
-  if (a > 0U)
+  if (a > 4294967295) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always false}}
     return 0;
-  if (0U <= a)
+  if (4294967295 <= a) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always false}}
     return 0;
-  if (a <= 0U)
+  if (a <= 4294967295) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always true}}
     return 0;
-  if (0U > a)
+  if (4294967295 > a) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always true}}
     return 0;
-  if (a >= 0U)
+  if (a >= 4294967295) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always false}}
     return 0;
-  if (0U < a)
+  if (4294967295 < a) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always false}}
     return 0;
-
+#else
   if (a < 4294967295)
     return 0;
   if (4294967295 >= a)
@@ -131,26 +150,10 @@ int main() {
     return 0;
   if (4294967295 < 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;
-  if (a >= 4294967295U)
-    return 0;
-  if (4294967295U < a)
-    return 0;
 #endif
-#elif defined(SIGNED)
-#ifndef SILENCE
+#endif
+
+#if defined(SIGNED) && !defined(SILENCE)
   if (a < -2147483648) // expected-warning {{comparison 'enum A' < -2147483648 is always false}}
     return 0;
   if (-2147483648 >= a)
@@ -184,24 +187,25 @@ int main() {
     return 0;
   if (2147483647 < a) // expected-warning {{comparison 2147483647 < 'enum A' is always false}}
     return 0;
-
-  if (a < 2147483647U)
+#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}}
     return 0;
-  if (2147483647U >= a) // expected-warning {{comparison 2147483647 >= 'enum A' is always true}}
+  if (-2147483648 >= a) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always false}}
     return 0;
-  if (a > 2147483647U) // expected-warning {{comparison 'enum A' > 2147483647 is always false}}
+  if (a > -2147483648) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always true}}
     return 0;
-  if (2147483647U <= a)
+  if (-2147483648 <= a) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always true}}
     return 0;
-  if (a <= 2147483647U) // expected-warning {{comparison 'enum A' <= 2147483647 is always true}}
+  if (a <= -2147483648) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always false}}
     return 0;
-  if (2147483647U > a)
+  if (-2147483648 > a) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always false}}
     return 0;
-  if (a >= 2147483647U)
+  if (a >= -2147483648) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always true}}
     return 0;
-  if (2147483647U < a) // expected-warning {{comparison 2147483647 < 'enum A' is always false}}
+  if (-2147483648 < a) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always true}}
     return 0;
-#else // SILENCE
+#else
   if (a < -2147483648)
     return 0;
   if (-2147483648 >= a)
@@ -218,6 +222,7 @@ int main() {
     return 0;
   if (-2147483648 < a)
     return 0;
+#endif
 
   if (a < 2147483647)
     return 0;
@@ -235,24 +240,6 @@ 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=320211&r1=320210&r2=320211&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/compare.cpp (original)
+++ cfe/trunk/test/SemaCXX/compare.cpp Fri Dec  8 14:57:11 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 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 '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'}}
@@ -423,3 +423,19 @@ namespace templates {
     testx<B>();
   }
 }
+
+namespace tautological_enum {
+  enum E { a, b, c } e;
+
+  // FIXME: We should warn about constructing this out-of-range numeration value.
+  const E invalid = (E)-1;
+  // ... but we should not warn about comparing against it.
+  bool x = e == invalid;
+
+  // We should not warn about relational comparisons for enumerators, even if
+  // they're tautological.
+  bool y = e >= a && e <= b;
+  const E first_in_range = a;
+  const E last_in_range = b;
+  bool z = e >= first_in_range && e <= last_in_range;
+}




More information about the cfe-commits mailing list