From 45aa4557fe4210034e85f4a807ff637a9dd146d6 Mon Sep 17 00:00:00 2001 From: John McCall Date: Thu, 5 Nov 2009 00:40:04 +0000 Subject: [PATCH] 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 --- include/clang/Basic/DiagnosticSemaKinds.td | 3 ++ lib/Sema/SemaExpr.cpp | 37 ++++++++++++++++++++++ test/Parser/if-scope-c90.c | 2 +- test/Parser/if-scope-c99.c | 2 +- test/Sema/compare.c | 8 +++++ 5 files changed, 50 insertions(+), 2 deletions(-) diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 1ce7dbc949..aae4d0e6a2 100644 --- 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>; 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 ac7cced2ea..ea8504549a 100644 --- 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 fdc75e9f10..53987dccbc 100644 --- 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 37cd0e15ab..b4cb51ca8c 100644 --- 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 87131bb621..50b40e459c 100644 --- 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; } -- 2.40.0