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.

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

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 0d3913448f3e8cbe2874dce63206b54aff05a3d8..76b9aecdac802da6fb927b9a30c4f20c7735ec25 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -8374,19 +8374,16 @@ static void diagnoseLogicalNotOnLHSofComparison(Sema &S, ExprResult &LHS,
                                                 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)

diff --git a/test/Sema/atomic-compare.c b/test/Sema/atomic-compare.c
index 01eb8200472560fe24e0bffe8736e9cd92628ae1..9e7555d57f5aa5c003f79408fa5f3d17068a057d 100644 (file)
--- a/test/Sema/atomic-compare.c
+++ b/test/Sema/atomic-compare.c
@@ -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

diff --git a/test/Sema/bool-compare.c b/test/Sema/bool-compare.c
index 3f1f286a2a381d24f259f439197f201c1d0562fb..923ff4208e8be1d116814a2a362441967b7fbdfd 100644 (file)
--- a/test/Sema/bool-compare.c
+++ b/test/Sema/bool-compare.c
@@ -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) {

diff --git a/test/Sema/warn-logical-not-compare.c b/test/Sema/warn-logical-not-compare.c
new file mode 100644 (file)
index 0000000..b67845e
--- /dev/null
+++ b/test/Sema/warn-logical-not-compare.c
@@ -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@-1 {{logical not is only applied to the left hand side of this comparison}}
+  // expected-note@-2 {{add parentheses after the '!' to evaluate the comparison first}}
+  // expected-note@-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@-1 {{logical not is only applied to the left hand side of this comparison}}
+  // expected-note@-2 {{add parentheses after the '!' to evaluate the comparison first}}
+  // expected-note@-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@-1 {{logical not is only applied to the left hand side of this comparison}}
+  // expected-note@-2 {{add parentheses after the '!' to evaluate the comparison first}}
+  // expected-note@-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@-1 {{logical not is only applied to the left hand side of this comparison}}
+  // expected-note@-2 {{add parentheses after the '!' to evaluate the comparison first}}
+  // expected-note@-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@-1 {{logical not is only applied to the left hand side of this comparison}}
+  // expected-note@-2 {{add parentheses after the '!' to evaluate the comparison first}}
+  // expected-note@-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@-1 {{logical not is only applied to the left hand side of this comparison}}
+  // expected-note@-2 {{add parentheses after the '!' to evaluate the comparison first}}
+  // expected-note@-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@-1 {{logical not is only applied to the left hand side of this comparison}}
+  // expected-note@-2 {{add parentheses after the '!' to evaluate the comparison first}}
+  // expected-note@-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@-1 {{logical not is only applied to the left hand side of this comparison}}
+  // expected-note@-2 {{add parentheses after the '!' to evaluate the comparison first}}
+  // expected-note@-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@-1 {{logical not is only applied to the left hand side of this comparison}}
+  // expected-note@-2 {{add parentheses after the '!' to evaluate the comparison first}}
+  // expected-note@-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@-1 {{logical not is only applied to the left hand side of this comparison}}
+  // expected-note@-2 {{add parentheses after the '!' to evaluate the comparison first}}
+  // expected-note@-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@-1 {{logical not is only applied to the left hand side of this comparison}}
+  // expected-note@-2 {{add parentheses after the '!' to evaluate the comparison first}}
+  // expected-note@-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@-1 {{logical not is only applied to the left hand side of this comparison}}
+  // expected-note@-2 {{add parentheses after the '!' to evaluate the comparison first}}
+  // expected-note@-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;
+}