Warn about comparing an unsigned expression with 0 in tautological ways.

Patch by mikem!

llvm-svn: 98279

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -1857,6 +1857,12 @@ def warn_mixed_sign_comparison : Warning<
 def warn_mixed_sign_conditional : Warning<
   "operands of ? are integers of different signs: %0 and %1">,
   InGroup<DiagGroup<"sign-compare">>, DefaultIgnore;
+def warn_lunsigned_always_true_comparison : Warning<
+  "comparison of unsigned expression %0 is always %1">,
+  InGroup<DiagGroup<"sign-compare">>, DefaultIgnore;
+def warn_runsigned_always_true_comparison : Warning<
+  "comparison of %0 unsigned expression is always %1">,
+  InGroup<DiagGroup<"sign-compare">>, DefaultIgnore;
 
 def err_invalid_this_use : Error<
   "invalid use of 'this' outside of a nonstatic member function">;

clang/lib/Sema/Sema.h

@@ -4245,8 +4245,7 @@ private:
                             SourceLocation ReturnLoc);
   void CheckFloatComparison(SourceLocation loc, Expr* lex, Expr* rex);
   void CheckSignCompare(Expr *LHS, Expr *RHS, SourceLocation Loc,
-                        const PartialDiagnostic &PD,
-                        bool Equality = false);
+                        const BinaryOperator::Opcode* BinOpc = 0);
   void CheckImplicitConversion(Expr *E, QualType Target);
 };
 

clang/lib/Sema/SemaChecking.cpp

@@ -2011,10 +2011,9 @@ bool IsSameFloatAfterCast(const APValue &value,
 /// \param lex the left-hand expression
 /// \param rex the right-hand expression
 /// \param OpLoc the location of the joining operator
-/// \param Equality whether this is an "equality-like" join, which
-///   suppresses the warning in some cases
+/// \param BinOpc binary opcode or 0
 void Sema::CheckSignCompare(Expr *lex, Expr *rex, SourceLocation OpLoc,
-                            const PartialDiagnostic &PD, bool Equality) {
+                            const BinaryOperator::Opcode* BinOpc) {
   // Don't warn if we're in an unevaluated context.
   if (ExprEvalContexts.back().Context == Unevaluated)
     return;
@@ -2075,17 +2074,51 @@ void Sema::CheckSignCompare(Expr *lex, Expr *rex, SourceLocation OpLoc,
 
   // If the signed operand is non-negative, then the signed->unsigned
   // conversion won't change it.
-  if (signedRange.NonNegative)
+  if (signedRange.NonNegative) {
+    // Emit warnings for comparisons of unsigned to integer constant 0.
+    //   always false: x < 0  (or 0 > x)
+    //   always true:  x >= 0 (or 0 <= x)
+    llvm::APSInt X;
+    if (BinOpc && signedOperand->isIntegerConstantExpr(X, Context) && X == 0) {
+      if (signedOperand != lex) {
+        if (*BinOpc == BinaryOperator::LT) {
+          Diag(OpLoc, diag::warn_lunsigned_always_true_comparison)
+            << "< 0" << "false"
+            << lex->getSourceRange() << rex->getSourceRange();
+        }
+        else if (*BinOpc == BinaryOperator::GE) {
+          Diag(OpLoc, diag::warn_lunsigned_always_true_comparison)
+            << ">= 0" << "true"
+            << lex->getSourceRange() << rex->getSourceRange();
+        }
+      }
+      else {
+        if (*BinOpc == BinaryOperator::GT) {
+          Diag(OpLoc, diag::warn_runsigned_always_true_comparison)
+            << "0 >" << "false" 
+            << lex->getSourceRange() << rex->getSourceRange();
+        } 
+        else if (*BinOpc == BinaryOperator::LE) {
+          Diag(OpLoc, diag::warn_runsigned_always_true_comparison)
+            << "0 <=" << "true" 
+            << lex->getSourceRange() << rex->getSourceRange();
+        }
+      }
+    }
     return;
+  }
 
   // For (in)equality comparisons, if the unsigned operand is a
   // constant which cannot collide with a overflowed signed operand,
   // then reinterpreting the signed operand as unsigned will not
   // change the result of the comparison.
-  if (Equality && unsignedRange.Width < unsignedWidth)
+  if (BinOpc &&
+      (*BinOpc == BinaryOperator::EQ || *BinOpc == BinaryOperator::NE) &&
+      unsignedRange.Width < unsignedWidth)
     return;
 
-  Diag(OpLoc, PD)
+  Diag(OpLoc, BinOpc ? diag::warn_mixed_sign_comparison
+                     : diag::warn_mixed_sign_conditional)
     << lt << rt << lex->getSourceRange() << rex->getSourceRange();
 }
 

clang/lib/Sema/SemaExpr.cpp

@@ -4105,7 +4105,7 @@ QualType Sema::CheckConditionalOperands(Expr *&Cond, Expr *&LHS, Expr *&RHS,
   if (getLangOptions().CPlusPlus)
     return CXXCheckConditionalOperands(Cond, LHS, RHS, QuestionLoc);
 
-  CheckSignCompare(LHS, RHS, QuestionLoc, diag::warn_mixed_sign_conditional);
+  CheckSignCompare(LHS, RHS, QuestionLoc);
 
   UsualUnaryConversions(Cond);
   UsualUnaryConversions(LHS);
@@ -5282,8 +5282,7 @@ QualType Sema::CheckCompareOperands(Expr *&lex, Expr *&rex, SourceLocation Loc,
   if (lex->getType()->isVectorType() || rex->getType()->isVectorType())
     return CheckVectorCompareOperands(lex, rex, Loc, isRelational);
 
-  CheckSignCompare(lex, rex, Loc, diag::warn_mixed_sign_comparison,
-                   (Opc == BinaryOperator::EQ || Opc == BinaryOperator::NE));
+  CheckSignCompare(lex, rex, Loc, &Opc);
 
   // C99 6.5.8p3 / C99 6.5.9p4
   if (lex->getType()->isArithmeticType() && rex->getType()->isArithmeticType())

clang/lib/Sema/SemaExprCXX.cpp

@@ -2085,7 +2085,7 @@ QualType Sema::CXXCheckConditionalOperands(Expr *&Cond, Expr *&LHS, Expr *&RHS,
   if (LHS->isTypeDependent() || RHS->isTypeDependent())
     return Context.DependentTy;
 
-  CheckSignCompare(LHS, RHS, QuestionLoc, diag::warn_mixed_sign_conditional);
+  CheckSignCompare(LHS, RHS, QuestionLoc);
 
   // C++0x 5.16p2
   //   If either the second or the third operand has type (cv) void, ...

clang/test/Sema/compare.c

@@ -274,3 +274,11 @@ void test4() {
   if (value < (unsigned long) &ptr4) // expected-warning {{comparison of integers of different signs}}
     return;
 }
+
+// PR4807
+int test5(unsigned int x) {
+  return (x < 0) // expected-warning {{comparison of unsigned expression < 0 is always false}}
+    && (0 > x)   // expected-warning {{comparison of 0 > unsigned expression is always false}}
+    && (x >= 0)  // expected-warning {{comparison of unsigned expression >= 0 is always true}}
+    && (0 <= x); // expected-warning {{comparison of 0 <= unsigned expression is always true}}
+}