Revert r320124 "Fold together the in-range and out-of-range portions of -Wtautological-compare."

This broke 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 63.

Did this use to fall under the "in-range" case before? I thought we
didn't use to warn when comparing against the boundaries of a type.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@320133 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Sema/SemaChecking.cpp

@@ -8801,7 +8801,12 @@ static bool CheckTautologicalComparison(Sema &S, BinaryOperator *E,
                                         Expr *Constant, Expr *Other,
                                         const llvm::APSInt &Value,
                                         bool RhsConstant) {
-  if (S.inTemplateInstantiation())
+  // Disable warning in template instantiations
+  // and only analyze <, >, <= and >= operations.
+  if (S.inTemplateInstantiation() || !E->isRelationalOp())
+    return false;
+
+  if (IsEnumConstOrFromMacro(S, Constant))
     return false;
 
   Expr *OriginalOther = Other;
@@ -8828,23 +8833,94 @@ static bool CheckTautologicalComparison(Sema &S, BinaryOperator *E,
       OtherRange.Width =
           std::min(Bitfield->getBitWidthValue(S.Context), OtherRange.Width);
 
-  // Determine the promoted range of the other type and see if a comparison of
-  // the constant against that range is tautological.
+  // 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)
+    return false;
+
   auto Result = PromotedRange::constantValue(E->getOpcode(), Cmp, RhsConstant);
   if (!Result)
     return false;
 
-  // 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))
+  // 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)
+          << OtherT << !OtherT->isBooleanType() << *Result
+          << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange());
+    return true;
+  }
+
+  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;
+}
+
+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;
+
+  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 Value is in the range of possible Other values, this comparison is not
+  // tautological.
+  if (Cmp & PromotedRange::InRangeFlag)
+    return false;
+
+  auto IsTrue = PromotedRange::constantValue(E->getOpcode(), Cmp, RhsConstant);
+  if (!IsTrue)
     return false;
 
   // If this is a comparison to an enum constant, include that
@@ -8853,7 +8929,6 @@ static bool CheckTautologicalComparison(Sema &S, BinaryOperator *E,
   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)
@@ -8861,30 +8936,14 @@ static bool CheckTautologicalComparison(Sema &S, BinaryOperator *E,
   else
     OS << Value;
 
-  // 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();
-  }
+  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());
 
-  return true;
+   return true;
 }
 
 /// Analyze the operands of the given comparison.  Implements the
@@ -8934,8 +8993,12 @@ static void AnalyzeComparison(Sema &S, BinaryOperator *E) {
 
       // 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);
     }
   }