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@-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; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits