[cfe-commits] r86128 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/Sema.h lib/Sema/SemaExpr.cpp lib/Sema/SemaExprCXX.cpp test/Sema/compare.c test/Sema/conditional-expr.c test/SemaCXX/compare.cpp test/SemaCXX/conditional-expr.cpp
John McCall
rjmccall at apple.com
Thu Nov 5 01:23:40 PST 2009
Author: rjmccall
Date: Thu Nov 5 03:23:39 2009
New Revision: 86128
URL: http://llvm.org/viewvc/llvm-project?rev=86128&view=rev
Log:
Implement the conditional-operator part of -Wsign-compare. Turn
DiagnoseSignCompare into Sema::CheckSignCompare and call it from more places.
Add some enumerator tests. These seem to expose some oddities in the
types we're converting C++ enumerators to; in particular, they're converting
to unsigned before int, which seems to contradict 4.5 [conv.prom] p2.
Note to self: stop baiting Doug in my commit messages.
Added:
cfe/trunk/test/SemaCXX/compare.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/Sema.h
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/test/Sema/compare.c
cfe/trunk/test/Sema/conditional-expr.c
cfe/trunk/test/SemaCXX/conditional-expr.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=86128&r1=86127&r2=86128&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Nov 5 03:23:39 2009
@@ -1540,6 +1540,9 @@
def warn_mixed_sign_comparison : Warning<
"comparison of integers of different signs: %0 and %1">,
InGroup<DiagGroup<"sign-compare">>;
+def warn_mixed_sign_conditional : Warning<
+ "operands of ? are 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">;
Modified: cfe/trunk/lib/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=86128&r1=86127&r2=86128&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/Sema.h (original)
+++ cfe/trunk/lib/Sema/Sema.h Thu Nov 5 03:23:39 2009
@@ -1609,6 +1609,9 @@
void DiagnoseSentinelCalls(NamedDecl *D, SourceLocation Loc,
Expr **Args, unsigned NumArgs);
+ void CheckSignCompare(Expr *LHS, Expr *RHS, SourceLocation Loc,
+ const PartialDiagnostic &PD);
+
virtual ExpressionEvaluationContext
PushExpressionEvaluationContext(ExpressionEvaluationContext NewContext);
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=86128&r1=86127&r2=86128&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Nov 5 03:23:39 2009
@@ -3340,6 +3340,8 @@
if (getLangOptions().CPlusPlus)
return CXXCheckConditionalOperands(Cond, LHS, RHS, QuestionLoc);
+ CheckSignCompare(LHS, RHS, QuestionLoc, diag::warn_mixed_sign_conditional);
+
UsualUnaryConversions(Cond);
UsualUnaryConversions(LHS);
UsualUnaryConversions(RHS);
@@ -4428,8 +4430,8 @@
}
/// Implements -Wsign-compare.
-static void DiagnoseSignCompare(Sema &S, Expr *lex, Expr *rex,
- BinaryOperator::Opcode Opc, SourceLocation OpLoc) {
+void Sema::CheckSignCompare(Expr *lex, Expr *rex, SourceLocation OpLoc,
+ const PartialDiagnostic &PD) {
QualType lt = lex->getType(), rt = rex->getType();
// Only warn if both operands are integral.
@@ -4450,14 +4452,14 @@
// 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)) {
+ if (signedOperand->isIntegerConstantExpr(value, Context)) {
assert(value.isSigned() && "result of signed expression not signed");
if (value.isNonNegative())
return;
}
- S.Diag(OpLoc, diag::warn_mixed_sign_comparison)
+ Diag(OpLoc, PD)
<< lex->getType() << rex->getType()
<< lex->getSourceRange() << rex->getSourceRange();
}
@@ -4470,7 +4472,7 @@
if (lex->getType()->isVectorType() || rex->getType()->isVectorType())
return CheckVectorCompareOperands(lex, rex, Loc, isRelational);
- DiagnoseSignCompare(*this, lex, rex, Opc, Loc);
+ CheckSignCompare(lex, rex, Loc, diag::warn_mixed_sign_comparison);
// C99 6.5.8p3 / C99 6.5.9p4
if (lex->getType()->isArithmeticType() && rex->getType()->isArithmeticType())
Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=86128&r1=86127&r2=86128&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Thu Nov 5 03:23:39 2009
@@ -1603,6 +1603,8 @@
if (LHS->isTypeDependent() || RHS->isTypeDependent())
return Context.DependentTy;
+ CheckSignCompare(LHS, RHS, QuestionLoc, diag::warn_mixed_sign_conditional);
+
// C++0x 5.16p2
// If either the second or the third operand has type (cv) void, ...
QualType LTy = LHS->getType();
Modified: cfe/trunk/test/Sema/compare.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/compare.c?rev=86128&r1=86127&r2=86128&view=diff
==============================================================================
--- cfe/trunk/test/Sema/compare.c (original)
+++ cfe/trunk/test/Sema/compare.c Thu Nov 5 03:23:39 2009
@@ -13,6 +13,18 @@
((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}}
+
+ enum Enum {B};
+ return (a == B) +
+ ((int)a == B) +
+ ((short)a == B) +
+ (a == (unsigned int) B) + // expected-warning {{comparison of integers of different signs}}
+ (a == (unsigned short) B); // expected-warning {{comparison of integers of different signs}}
+
+ // Should be able to prove all of these are non-negative.
+ return (b == (long) B) +
+ (b == (int) B) +
+ (b == (short) B);
}
int equal(char *a, const char *b) {
Modified: cfe/trunk/test/Sema/conditional-expr.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/conditional-expr.c?rev=86128&r1=86127&r2=86128&view=diff
==============================================================================
--- cfe/trunk/test/Sema/conditional-expr.c (original)
+++ cfe/trunk/test/Sema/conditional-expr.c Thu Nov 5 03:23:39 2009
@@ -34,6 +34,25 @@
typedef void *asdf;
*(0 ? (asdf) 0 : &x) = 10;
+
+ unsigned long test0 = 5;
+ test0 = test0 ? (long) test0 : test0; // expected-warning {{operands of ? are integers of different signs}}
+ test0 = test0 ? (int) test0 : test0; // expected-warning {{operands of ? are integers of different signs}}
+ test0 = test0 ? (short) test0 : test0; // expected-warning {{operands of ? are integers of different signs}}
+ test0 = test0 ? test0 : (long) test0; // expected-warning {{operands of ? are integers of different signs}}
+ test0 = test0 ? test0 : (int) test0; // expected-warning {{operands of ? are integers of different signs}}
+ test0 = test0 ? test0 : (short) test0; // expected-warning {{operands of ? are integers of different signs}}
+ test0 = test0 ? test0 : (long) 10;
+ test0 = test0 ? test0 : (int) 10;
+ test0 = test0 ? test0 : (short) 10;
+ test0 = test0 ? (long) 10 : test0;
+ test0 = test0 ? (int) 10 : test0;
+ test0 = test0 ? (short) 10 : test0;
+
+ enum Enum { EVal };
+ test0 = test0 ? EVal : test0;
+ test0 = test0 ? EVal : (int) test0; // okay: EVal is an int
+ test0 = test0 ? (unsigned) EVal : (int) test0; // expected-warning {{operands of ? are integers of different signs}}
}
int Postgresql() {
Added: cfe/trunk/test/SemaCXX/compare.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/compare.cpp?rev=86128&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/compare.cpp (added)
+++ cfe/trunk/test/SemaCXX/compare.cpp Thu Nov 5 03:23:39 2009
@@ -0,0 +1,15 @@
+// RUN: clang-cc -fsyntax-only -pedantic -verify %s
+
+int test0(long a, unsigned long b) {
+ enum Enum {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}}
+
+ // Should be able to prove all of these are non-negative.
+ return (b == (long) B) +
+ (b == (int) B) +
+ (b == (short) B);
+}
Modified: cfe/trunk/test/SemaCXX/conditional-expr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/conditional-expr.cpp?rev=86128&r1=86127&r2=86128&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/conditional-expr.cpp (original)
+++ cfe/trunk/test/SemaCXX/conditional-expr.cpp Thu Nov 5 03:23:39 2009
@@ -156,8 +156,8 @@
i1 = i1 ? i1 : ir1;
int *pi1 = i1 ? &i1 : 0;
pi1 = i1 ? 0 : &i1;
- i1 = i1 ? i1 : EVal;
- i1 = i1 ? EVal : i1;
+ i1 = i1 ? i1 : EVal; // expected-warning {{operands of ? are integers of different signs}} ??
+ i1 = i1 ? EVal : i1; // expected-warning {{operands of ? are integers of different signs}} ??
d1 = i1 ? 'c' : 4.0;
d1 = i1 ? 4.0 : 'c';
Base *pb = i1 ? (Base*)0 : (Derived*)0;
@@ -177,6 +177,24 @@
(void)&(i1 ? flds.b1 : flds.i1); // expected-error {{address of bit-field requested}}
(void)&(i1 ? flds.i1 : flds.b1); // expected-error {{address of bit-field requested}}
+
+ unsigned long test0 = 5;
+ test0 = test0 ? (long) test0 : test0; // expected-warning {{operands of ? are integers of different signs}}
+ test0 = test0 ? (int) test0 : test0; // expected-warning {{operands of ? are integers of different signs}}
+ test0 = test0 ? (short) test0 : test0; // expected-warning {{operands of ? are integers of different signs}}
+ test0 = test0 ? test0 : (long) test0; // expected-warning {{operands of ? are integers of different signs}}
+ test0 = test0 ? test0 : (int) test0; // expected-warning {{operands of ? are integers of different signs}}
+ test0 = test0 ? test0 : (short) test0; // expected-warning {{operands of ? are integers of different signs}}
+ test0 = test0 ? test0 : (long) 10;
+ test0 = test0 ? test0 : (int) 10;
+ test0 = test0 ? test0 : (short) 10;
+ test0 = test0 ? (long) 10 : test0;
+ test0 = test0 ? (int) 10 : test0;
+ test0 = test0 ? (short) 10 : test0;
+
+ test0 = test0 ? EVal : test0;
+ test0 = test0 ? EVal : (int) test0; // expected-warning {{operands of ? are integers of different signs}}
+
// Note the thing that this does not test: since DR446, various situations
// *must* create a separate temporary copy of class objects. This can only
// be properly tested at runtime, though.
More information about the cfe-commits
mailing list