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

Conditional operands are next.

Fixes part of rdar://problem/7289584.

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

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 1ce7dbc9497b04a8092b1d7fa36244276575c66f..aae4d0e6a2c0acabfcd7625b867d6056b2bea122 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/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">;

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index ac7cced2eaeff278c582fecd0b19c697521f274b..ea8504549aae21d409474f978bb9b65e514e1406 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/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);

diff --git a/test/Parser/if-scope-c90.c b/test/Parser/if-scope-c90.c
index fdc75e9f10b0fcb527690b0bf19ca8513b9b2115..53987dccbc37f9e9657fc9984aba72798da6f900 100644 (file)
--- a/test/Parser/if-scope-c90.c
+++ b/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;
 } 

diff --git a/test/Parser/if-scope-c99.c b/test/Parser/if-scope-c99.c
index 37cd0e15ab8e499a4cce194997ac145182ce0d18..b4cb51ca8c4f195f963ba292c2b833d61112f575 100644 (file)
--- a/test/Parser/if-scope-c99.c
+++ b/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}}
 }

diff --git a/test/Sema/compare.c b/test/Sema/compare.c
index 87131bb62183ae87d81b59194a9b1e1e9b0e901d..50b40e459c39e783dbf7cffb684b1e5fb994a197 100644 (file)
--- a/test/Sema/compare.c
+++ b/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;
 }