Implement -Wsign-compare, or at least the actual comparison part of it.

Conditional operands are next.

Fixes part of rdar://problem/7289584.

llvm-svn: 86083

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -1537,6 +1537,9 @@ def err_typecheck_vector_comparison : Error<
 def err_typecheck_assign_const : Error<"read-only variable is not assignable">;
 def err_stmtexpr_file_scope : Error<
   "statement expression not allowed at file scope">;
+def warn_mixed_sign_comparison : Warning<
+  "comparison of integers of different signs: %0 and %1">,
+  InGroup<DiagGroup<"sign-compare">>;
 
 def err_invalid_this_use : Error<
   "invalid use of 'this' outside of a nonstatic member function">;

clang/lib/Sema/SemaExpr.cpp

@@ -4427,6 +4427,41 @@ QualType Sema::CheckShiftOperands(Expr *&lex, Expr *&rex, SourceLocation Loc,
   return LHSTy;
 }
 
+/// Implements -Wsign-compare.
+static void DiagnoseSignCompare(Sema &S, Expr *lex, Expr *rex,
+                                BinaryOperator::Opcode Opc, SourceLocation OpLoc) {
+  QualType lt = lex->getType(), rt = rex->getType();
+
+  // Only warn if both operands are integral.
+  if (!lt->isIntegerType() || !rt->isIntegerType())
+    return;
+
+  // The rule is that the signed operand becomes unsigned, so isolate the
+  // signed operand.
+  Expr *signedOperand;
+  if (lt->isSignedIntegerType()) {
+    if (rt->isSignedIntegerType()) return;
+    signedOperand = lex;
+  } else {
+    if (!rt->isSignedIntegerType()) return;
+    signedOperand = rex;
+  }
+
+  // If the value is a non-negative integer constant, then the
+  // signed->unsigned conversion won't change it.
+  llvm::APSInt value;
+  if (signedOperand->isIntegerConstantExpr(value, S.Context)) {
+    assert(value.isSigned() && "result of signed expression not signed");
+
+    if (value.isNonNegative())
+      return;
+  }
+
+  S.Diag(OpLoc, diag::warn_mixed_sign_comparison)
+    << lex->getType() << rex->getType()
+    << lex->getSourceRange() << rex->getSourceRange();
+}
+
 // C99 6.5.8, C++ [expr.rel]
 QualType Sema::CheckCompareOperands(Expr *&lex, Expr *&rex, SourceLocation Loc,
                                     unsigned OpaqueOpc, bool isRelational) {
@@ -4435,6 +4470,8 @@ QualType Sema::CheckCompareOperands(Expr *&lex, Expr *&rex, SourceLocation Loc,
   if (lex->getType()->isVectorType() || rex->getType()->isVectorType())
     return CheckVectorCompareOperands(lex, rex, Loc, isRelational);
 
+  DiagnoseSignCompare(*this, lex, rex, Opc, Loc);
+
   // C99 6.5.8p3 / C99 6.5.9p4
   if (lex->getType()->isArithmeticType() && rex->getType()->isArithmeticType())
     UsualArithmeticConversions(lex, rex);

clang/test/Parser/if-scope-c90.c

@@ -2,7 +2,7 @@
 
 int f (int z)
 { 
-   if (z > sizeof (enum {a, b}))
+  if (z > (int) sizeof (enum {a, b}))
       return a;
    return b;
 } 

clang/test/Parser/if-scope-c99.c

@@ -2,7 +2,7 @@
 
 int f (int z)
 { 
-   if (z > sizeof (enum {a, b}))
+   if (z > (int) sizeof (enum {a, b}))
       return a;
    return b; // expected-error{{use of undeclared identifier}}
 }

clang/test/Sema/compare.c

@@ -7,6 +7,14 @@ int test(char *C) { // nothing here should warn.
   return C != 1;  // expected-warning {{comparison between pointer and integer ('char *' and 'int')}}
 }
 
+int ints(long a, unsigned long b) {
+  return (a == b) +        // expected-warning {{comparison of integers of different signs}}
+         ((int)a == b) +   // expected-warning {{comparison of integers of different signs}}
+         ((short)a == b) + // expected-warning {{comparison of integers of different signs}}
+         (a == (unsigned int) b) +  // expected-warning {{comparison of integers of different signs}}
+         (a == (unsigned short) b); // expected-warning {{comparison of integers of different signs}}
+}
+
 int equal(char *a, const char *b) {
     return a == b;
 }