Improve -Wtautological-constant-out-of-range-compare by taking into account

type conversion between integers.  This allows the warning to be more accurate.

Also, turned the warning off in an analyzer test.  The relavent test cases
are covered by the tests in Sema.


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

1 files changed, 73 insertions, 15 deletions

diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 692a210ef3..7fd28b6191 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -4328,38 +4328,96 @@ static void DiagnoseOutOfRangeComparison(Sema &S, BinaryOperator *E,
                                          Expr *Constant, Expr *Other,
                                          llvm::APSInt Value,
                                          bool RhsConstant) {
+  // 0 values are handled later by CheckTrivialUnsignedComparison().
+  if (Value == 0)
+    return;
+
   BinaryOperatorKind op = E->getOpcode();
   QualType OtherT = Other->getType();
   QualType ConstantT = Constant->getType();
+  QualType CommonT = E->getLHS()->getType();
   if (S.Context.hasSameUnqualifiedType(OtherT, ConstantT))
     return;
   assert((OtherT->isIntegerType() && ConstantT->isIntegerType())
          && "comparison with non-integer type");
-  // FIXME. handle cases for signedness to catch (signed char)N == 200
+
+  bool ConstantSigned = ConstantT->isSignedIntegerType();
+  bool OtherSigned = OtherT->isSignedIntegerType();
+  bool CommonSigned = CommonT->isSignedIntegerType();
+
+  bool EqualityOnly = false;
+
+  // TODO: Investigate using GetExprRange() to get tighter bounds on
+  // on the bit ranges.
   IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
-  IntRange LitRange = GetValueRange(S.Context, Value, Value.getBitWidth());
-  if (OtherRange.Width >= LitRange.Width)
-    return;
+  unsigned OtherWidth = OtherRange.Width;
+  
+  if (CommonSigned) {
+    // The common type is signed, therefore no signed to unsigned conversion.
+    if (OtherSigned) {
+      // Check that the constant is representable in type OtherT.
+      if (ConstantSigned) {
+        if (OtherWidth >= Value.getMinSignedBits())
+          return;
+      } else { // !ConstantSigned
+        if (OtherWidth >= Value.getActiveBits() + 1)
+          return;
+      }
+    } 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;
+      } else { // !ConstantSigned
+        if (OtherWidth >= Value.getActiveBits())
+          return;
+      }
+    }
+  } else {  // !CommonSigned
+    if (!OtherSigned) {
+      if (OtherWidth >= Value.getActiveBits())
+        return;
+    } else if (OtherSigned && !ConstantSigned) {
+      // Check to see if the constant is representable in OtherT.
+      if (OtherWidth > Value.getActiveBits())
+        return;
+      // 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;
+      // The constant value rests between values that OtherT can represent after
+      // conversion.  Relational comparison still works, but equality
+      // comparisons will be tautological.
+      EqualityOnly = true;
+    } else { // OtherSigned && ConstantSigned
+      assert(0 && "Two signed types converted to unsigned types.");
+    }
+  }
+
+  bool PositiveConstant = !ConstantSigned || Value.isNonNegative();
+
   bool IsTrue = true;
-  if (op == BO_EQ)
-    IsTrue = false;
-  else if (op == BO_NE)
-    IsTrue = true;
-  else if (RhsConstant) {
+  if (op == BO_EQ || op == BO_NE) {
+    IsTrue = op == BO_NE;
+  } else if (EqualityOnly) {
+    return;
+  } else if (RhsConstant) {
     if (op == BO_GT || op == BO_GE)
-      IsTrue = !LitRange.NonNegative;
+      IsTrue = !PositiveConstant;
     else // op == BO_LT || op == BO_LE
-      IsTrue = LitRange.NonNegative;
+      IsTrue = PositiveConstant;
   } else {
     if (op == BO_LT || op == BO_LE)
-      IsTrue = !LitRange.NonNegative;
+      IsTrue = !PositiveConstant;
     else // op == BO_GT || op == BO_GE
-      IsTrue = LitRange.NonNegative;
+      IsTrue = PositiveConstant;
   }
   SmallString<16> PrettySourceValue(Value.toString(10));
   S.Diag(E->getOperatorLoc(), diag::warn_out_of_range_compare)
-  << PrettySourceValue << OtherT << IsTrue
-  << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
+      << PrettySourceValue << OtherT << IsTrue
+      << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
 }
 
 /// Analyze the operands of the given comparison.  Implements the