Hello,
could somebody please review and commit the atached patch for pr12463? Thank
you.
--
Lubos Lunak
[email protected]
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 1523563..784b6b2 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -7322,48 +7322,55 @@ QualType Sema::CheckCompareOperands(ExprResult &LHS, ExprResult &RHS,
diagnoseLogicalNotOnLHSofComparison(*this, LHS, RHS, Loc, OpaqueOpc);
if (!LHSType->hasFloatingRepresentation() &&
- !(LHSType->isBlockPointerType() && IsRelational) &&
- !LHS.get()->getLocStart().isMacroID() &&
- !RHS.get()->getLocStart().isMacroID()) {
+ !(LHSType->isBlockPointerType() && IsRelational)) {
// For non-floating point types, check for self-comparisons of the form
// x == x, x != x, x < x, etc. These always evaluate to a constant, and
// often indicate logic errors in the program.
//
- // NOTE: Don't warn about comparison expressions resulting from macro
- // expansion. Also don't warn about comparisons which are only self
+ // NOTE: Don't warn about self-comparison expressions resulting from macro
+ // expansion, unless they don't make sense at all.
+ // Also don't warn about comparisons which are only self
// comparisons within a template specialization. The warnings should catch
// obvious cases in the definition of the template anyways. The idea is to
// warn when the typed comparison operator will always evaluate to the same
// result.
- if (DeclRefExpr* DRL = dyn_cast<DeclRefExpr>(LHSStripped)) {
+ bool InMacro = LHS.get()->getLocStart().isMacroID() ||
+ RHS.get()->getLocStart().isMacroID();
+ if (DeclRefExpr *DRL = dyn_cast<DeclRefExpr>(LHSStripped)) {
if (DeclRefExpr* DRR = dyn_cast<DeclRefExpr>(RHSStripped)) {
+ if (isa<CastExpr>(LHSStripped))
+ LHSStripped = LHSStripped->IgnoreParenCasts();
if (DRL->getDecl() == DRR->getDecl() &&
- !IsWithinTemplateSpecialization(DRL->getDecl())) {
- DiagRuntimeBehavior(Loc, 0, PDiag(diag::warn_comparison_always)
- << 0 // self-
- << (Opc == BO_EQ
- || Opc == BO_LE
- || Opc == BO_GE));
+ !IsWithinTemplateSpecialization(DRL->getDecl()) && !InMacro &&
+ !isa<StringLiteral>(LHSStripped) &&
+ !isa<ObjCEncodeExpr>(LHSStripped)) {
+ DiagRuntimeBehavior(
+ Loc, 0, PDiag(diag::warn_comparison_always)
+ << 0 // self-
+ << (Opc == BO_EQ || Opc == BO_LE || Opc == BO_GE));
} else if (LHSType->isArrayType() && RHSType->isArrayType() &&
!DRL->getDecl()->getType()->isReferenceType() &&
!DRR->getDecl()->getType()->isReferenceType()) {
- // what is it always going to eval to?
- char always_evals_to;
switch(Opc) {
case BO_EQ: // e.g. array1 == array2
- always_evals_to = 0; // false
+ if (!InMacro)
+ DiagRuntimeBehavior(Loc, 0, PDiag(diag::warn_comparison_always)
+ << 1 // array
+ << 0 /* evaluates to false */);
break;
case BO_NE: // e.g. array1 != array2
- always_evals_to = 1; // true
+ if (!InMacro)
+ DiagRuntimeBehavior(Loc, 0, PDiag(diag::warn_comparison_always)
+ << 1 // array
+ << 1 /* evaluates to true */);
break;
- default:
- // best we can say is 'a constant'
- always_evals_to = 2; // e.g. array1 <= array2
+ default: // e.g. array1 <= array2
+ // best we can say is 'a constant'
+ DiagRuntimeBehavior(Loc, 0, PDiag(diag::warn_comparison_always)
+ << 1 // array
+ << 2 /* evaluates to constant */);
break;
}
- DiagRuntimeBehavior(Loc, 0, PDiag(diag::warn_comparison_always)
- << 1 // array
- << always_evals_to);
}
}
}
diff --git a/test/Sema/exprs.c b/test/Sema/exprs.c
index 2fb17e4..03d3e7a 100644
--- a/test/Sema/exprs.c
+++ b/test/Sema/exprs.c
@@ -125,6 +125,12 @@ int test12b(const char *X) {
return sizeof(X == "foo"); // no-warning
}
+// PR12463
+#define FOO_LITERAL "foo"
+int test12c(const char *X) {
+ return X == FOO_LITERAL; // expected-warning {{comparison against a string literal is unspecified (use strncmp instead)}}
+}
+
// rdar://6719156
void test13(
void (^P)()) { // expected-error {{blocks support disabled - compile with -fblocks}}
diff --git a/test/Sema/self-comparison.c b/test/Sema/self-comparison.c
index edb3a6a..8679a3f 100644
--- a/test/Sema/self-comparison.c
+++ b/test/Sema/self-comparison.c
@@ -72,6 +72,13 @@ int array_comparisons() {
return array1 <= array2; // expected-warning{{array comparison always evaluates to a constant}}
return array1 > array2; // expected-warning{{array comparison always evaluates to a constant}}
return array1 >= array2; // expected-warning{{array comparison always evaluates to a constant}}
+ // Issue a warning for this even if macro expansion is involved (PR12463)
+#define ARRAY1 array1
+#define ARRAY2 array2
+ return ARRAY1 < ARRAY2; // expected-warning{{array comparison always evaluates to a constant}}
+ return ARRAY1 <= ARRAY2; // expected-warning{{array comparison always evaluates to a constant}}
+ return ARRAY1 > ARRAY2; // expected-warning{{array comparison always evaluates to a constant}}
+ return ARRAY1 >= ARRAY2; // expected-warning{{array comparison always evaluates to a constant}}
}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
