r245507 - Fix -Wlogical-not-parentheses to work better with C code.
Richard Trieu via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 19 14:33:54 PDT 2015
Author: rtrieu
Date: Wed Aug 19 16:33:54 2015
New Revision: 245507
URL: http://llvm.org/viewvc/llvm-project?rev=245507&view=rev
Log:
Fix -Wlogical-not-parentheses to work better with C code.
Remove the assumption of a Boolean type by checking if an expression is known
to have a boolean value. Disable warning in two other tests.
Added:
cfe/trunk/test/Sema/warn-logical-not-compare.c
Modified:
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/atomic-compare.c
cfe/trunk/test/Sema/bool-compare.c
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=245507&r1=245506&r2=245507&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Aug 19 16:33:54 2015
@@ -8374,19 +8374,16 @@ static void diagnoseLogicalNotOnLHSofCom
ExprResult &RHS,
SourceLocation Loc,
unsigned OpaqueOpc) {
- // This checking requires bools.
- if (!S.getLangOpts().Bool) return;
-
// Check that left hand side is !something.
UnaryOperator *UO = dyn_cast<UnaryOperator>(LHS.get()->IgnoreImpCasts());
if (!UO || UO->getOpcode() != UO_LNot) return;
// Only check if the right hand side is non-bool arithmetic type.
- if (RHS.get()->getType()->isBooleanType()) return;
+ if (RHS.get()->isKnownToHaveBooleanValue()) return;
// Make sure that the something in !something is not bool.
Expr *SubExpr = UO->getSubExpr()->IgnoreImpCasts();
- if (SubExpr->getType()->isBooleanType()) return;
+ if (SubExpr->isKnownToHaveBooleanValue()) return;
// Emit warning.
S.Diag(UO->getOperatorLoc(), diag::warn_logical_not_on_lhs_of_comparison)
Modified: cfe/trunk/test/Sema/atomic-compare.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/atomic-compare.c?rev=245507&r1=245506&r2=245507&view=diff
==============================================================================
--- cfe/trunk/test/Sema/atomic-compare.c (original)
+++ cfe/trunk/test/Sema/atomic-compare.c Wed Aug 19 16:33:54 2015
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -verify -fsyntax-only
+// RUN: %clang_cc1 %s -verify -fsyntax-only -Wno-logical-not-parentheses
void f(_Atomic(int) a, _Atomic(int) b) {
if (a > b) {} // no warning
Modified: cfe/trunk/test/Sema/bool-compare.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/bool-compare.c?rev=245507&r1=245506&r2=245507&view=diff
==============================================================================
--- cfe/trunk/test/Sema/bool-compare.c (original)
+++ cfe/trunk/test/Sema/bool-compare.c Wed Aug 19 16:33:54 2015
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s -Wno-logical-not-parentheses
void f(int x, int y, int z) {
Added: cfe/trunk/test/Sema/warn-logical-not-compare.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-logical-not-compare.c?rev=245507&view=auto
==============================================================================
--- cfe/trunk/test/Sema/warn-logical-not-compare.c (added)
+++ cfe/trunk/test/Sema/warn-logical-not-compare.c Wed Aug 19 16:33:54 2015
@@ -0,0 +1,204 @@
+// RUN: %clang_cc1 -fsyntax-only -Wlogical-not-parentheses -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wlogical-not-parentheses -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+int getInt();
+
+int test1(int i1, int i2) {
+ int ret;
+
+ ret = !i1 == i2;
+ // expected-warning at -1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note at -2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note at -3 {{add parentheses around left hand side expression to silence this warning}}
+ // CHECK: warn-logical-not-compare.c:[[line:[0-9]*]]:9: warning
+ // CHECK: to evaluate the comparison first
+ // CHECK: fix-it:"{{.*}}":{[[line]]:10-[[line]]:10}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:18-[[line]]:18}:")"
+ // CHECK: to silence this warning
+ // CHECK: fix-it:"{{.*}}":{[[line]]:9-[[line]]:9}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:12-[[line]]:12}:")"
+
+ ret = !i1 != i2;
+ //expected-warning at -1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note at -2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note at -3 {{add parentheses around left hand side expression to silence this warning}}
+ // CHECK: warn-logical-not-compare.c:[[line:[0-9]*]]:9: warning
+ // CHECK: to evaluate the comparison first
+ // CHECK: fix-it:"{{.*}}":{[[line]]:10-[[line]]:10}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:18-[[line]]:18}:")"
+ // CHECK: to silence this warning
+ // CHECK: fix-it:"{{.*}}":{[[line]]:9-[[line]]:9}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:12-[[line]]:12}:")"
+
+ ret = !i1 < i2;
+ //expected-warning at -1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note at -2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note at -3 {{add parentheses around left hand side expression to silence this warning}}
+ // CHECK: warn-logical-not-compare.c:[[line:[0-9]*]]:9: warning
+ // CHECK: to evaluate the comparison first
+ // CHECK: fix-it:"{{.*}}":{[[line]]:10-[[line]]:10}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:17-[[line]]:17}:")"
+ // CHECK: to silence this warning
+ // CHECK: fix-it:"{{.*}}":{[[line]]:9-[[line]]:9}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:12-[[line]]:12}:")"
+
+ ret = !i1 > i2;
+ //expected-warning at -1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note at -2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note at -3 {{add parentheses around left hand side expression to silence this warning}}
+ // CHECK: warn-logical-not-compare.c:[[line:[0-9]*]]:9: warning
+ // CHECK: to evaluate the comparison first
+ // CHECK: fix-it:"{{.*}}":{[[line]]:10-[[line]]:10}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:17-[[line]]:17}:")"
+ // CHECK: to silence this warning
+ // CHECK: fix-it:"{{.*}}":{[[line]]:9-[[line]]:9}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:12-[[line]]:12}:")"
+
+ ret = !i1 <= i2;
+ //expected-warning at -1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note at -2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note at -3 {{add parentheses around left hand side expression to silence this warning}}
+ // CHECK: warn-logical-not-compare.c:[[line:[0-9]*]]:9: warning
+ // CHECK: to evaluate the comparison first
+ // CHECK: fix-it:"{{.*}}":{[[line]]:10-[[line]]:10}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:18-[[line]]:18}:")"
+ // CHECK: to silence this warning
+ // CHECK: fix-it:"{{.*}}":{[[line]]:9-[[line]]:9}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:12-[[line]]:12}:")"
+
+ ret = !i1 >= i2;
+ //expected-warning at -1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note at -2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note at -3 {{add parentheses around left hand side expression to silence this warning}}
+ // CHECK: warn-logical-not-compare.c:[[line:[0-9]*]]:9: warning
+ // CHECK: to evaluate the comparison first
+ // CHECK: fix-it:"{{.*}}":{[[line]]:10-[[line]]:10}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:18-[[line]]:18}:")"
+ // CHECK: to silence this warning
+ // CHECK: fix-it:"{{.*}}":{[[line]]:9-[[line]]:9}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:12-[[line]]:12}:")"
+
+ ret = i1 == i2;
+ ret = i1 != i2;
+ ret = i1 < i2;
+ ret = i1 > i2;
+ ret = i1 <= i2;
+ ret = i1 >= i2;
+
+ // Warning silenced by parens.
+ ret = (!i1) == i2;
+ ret = (!i1) != i2;
+ ret = (!i1) < i2;
+ ret = (!i1) > i2;
+ ret = (!i1) <= i2;
+ ret = (!i1) >= i2;
+
+ ret = !getInt() == i1;
+ // expected-warning at -1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note at -2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note at -3 {{add parentheses around left hand side expression to silence this warning}}
+ // CHECK: warn-logical-not-compare.c:[[line:[0-9]*]]:9: warning
+ // CHECK: to evaluate the comparison first
+ // CHECK: fix-it:"{{.*}}":{[[line]]:10-[[line]]:10}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:24-[[line]]:24}:")"
+ // CHECK: to silence this warning
+ // CHECK: fix-it:"{{.*}}":{[[line]]:9-[[line]]:9}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:18-[[line]]:18}:")"
+
+ ret = (!getInt()) == i1;
+ return ret;
+}
+
+enum E {e1, e2};
+enum E getE();
+
+int test2 (enum E e) {
+ int ret;
+ ret = e == e1;
+ ret = e == getE();
+ ret = getE() == e1;
+ ret = getE() == getE();
+
+ ret = !e == e1;
+ // expected-warning at -1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note at -2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note at -3 {{add parentheses around left hand side expression to silence this warning}}
+ // CHECK: warn-logical-not-compare.c:[[line:[0-9]*]]:9: warning
+ // CHECK: to evaluate the comparison first
+ // CHECK: fix-it:"{{.*}}":{[[line]]:10-[[line]]:10}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:17-[[line]]:17}:")"
+ // CHECK: to silence this warning
+ // CHECK: fix-it:"{{.*}}":{[[line]]:9-[[line]]:9}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:11-[[line]]:11}:")"
+
+ ret = !e == getE();
+ // expected-warning at -1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note at -2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note at -3 {{add parentheses around left hand side expression to silence this warning}}
+ // CHECK: warn-logical-not-compare.c:[[line:[0-9]*]]:9: warning
+ // CHECK: to evaluate the comparison first
+ // CHECK: fix-it:"{{.*}}":{[[line]]:10-[[line]]:10}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:21-[[line]]:21}:")"
+ // CHECK: to silence this warning
+ // CHECK: fix-it:"{{.*}}":{[[line]]:9-[[line]]:9}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:11-[[line]]:11}:")"
+
+ ret = !getE() == e1;
+ // expected-warning at -1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note at -2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note at -3 {{add parentheses around left hand side expression to silence this warning}}
+ // CHECK: warn-logical-not-compare.c:[[line:[0-9]*]]:9: warning
+ // CHECK: to evaluate the comparison first
+ // CHECK: fix-it:"{{.*}}":{[[line]]:10-[[line]]:10}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:22-[[line]]:22}:")"
+ // CHECK: to silence this warning
+ // CHECK: fix-it:"{{.*}}":{[[line]]:9-[[line]]:9}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:16-[[line]]:16}:")"
+
+ ret = !getE() == getE();
+ // expected-warning at -1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note at -2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note at -3 {{add parentheses around left hand side expression to silence this warning}}
+ // CHECK: warn-logical-not-compare.c:[[line:[0-9]*]]:9: warning
+ // CHECK: to evaluate the comparison first
+ // CHECK: fix-it:"{{.*}}":{[[line]]:10-[[line]]:10}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:26-[[line]]:26}:")"
+ // CHECK: to silence this warning
+ // CHECK: fix-it:"{{.*}}":{[[line]]:9-[[line]]:9}:"("
+ // CHECK: fix-it:"{{.*}}":{[[line]]:16-[[line]]:16}:")"
+
+ ret = !(e == e1);
+ ret = !(e == getE());
+ ret = !(getE() == e1);
+ ret = !(getE() == getE());
+
+ ret = (!e) == e1;
+ ret = (!e) == getE();
+ ret = (!getE()) == e1;
+ ret = (!getE()) == getE();
+
+ return ret;
+}
+
+int PR16673(int x) {
+ int ret;
+ // Make sure we don't emit a fixit for the left paren, but not the right paren.
+#define X(x) x
+ ret = X(!x == 1 && 1);
+ // expected-warning at -1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note at -2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note at -3 {{add parentheses around left hand side expression to silence this warning}}
+ // CHECK: warn-logical-not-compare.c:[[line:[0-9]*]]:11: warning
+ // CHECK: to evaluate the comparison first
+ // CHECK-NOT: fix-it
+ // CHECK: to silence this warning
+ // CHECK-NOT: fix-it
+ return ret;
+}
+
+int compare_pointers(int* a, int* b) {
+ int ret;
+ ret = !!a == !!b;
+ ret = !!a != !!b;
+ return ret;
+}
More information about the cfe-commits
mailing list