On Thu, Aug 21, 2014 at 02:34:54PM -0400, Jason Merrill wrote:
> On 08/21/2014 11:41 AM, Marek Polacek wrote:
> >+ current.lhs_type = cp_lexer_next_token_is (parser->lexer, CPP_NOT)
> >+ ? TRUTH_NOT_EXPR : ERROR_MARK;
> ...
> >+ rhs_type = cp_lexer_next_token_is (parser->lexer, CPP_NOT)
> >+ ? TRUTH_NOT_EXPR : ERROR_MARK;
>
> Again, this indentation needs parens to survive Emacs.
Ah, since I don't use Emacs, I thought that the parens are necessary
only if the first operand is e.g. a comparison. Fixed both.
> >+ /* If the LHS was !CST, we have true/false now. Convert it
> >+ to integer type, otherwise we wouldn't warn. */
> >+ if (TREE_CODE (current.lhs) == INTEGER_CST)
> >+ lhs = convert (integer_type_node, current.lhs);
>
> Hmm, why are we checking for BOOLEAN_TYPE on the lhs, again? It seems to me
> that
>
> bool b;
> if (!b == 1)
>
> is also probably an error.
I was checking for BOOLEAN_TYPE on the lhs because that's what clang
seems to be doing. Yet I agree with you, meaning that I can just drop
the convert line, and the lhs check as well. I added a new test for
that and updated docs to that effect. Note that I also added an EXPR_P
check, because without that, we might ICE on an invalid code. So
hopefully this version is better than the others ;).
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2014-08-22 Marek Polacek <pola...@redhat.com>
PR c++/62199
* doc/invoke.texi: Update -Wlogical-not-parentheses description.
c-family/
* c-common.c (warn_logical_not_parentheses): Don't check whether
the LHS is a boolean.
cp/
* parser.c (cp_parser_binary_expression): Check each LHS if it's
preceded with logical not. Handle logical not of constants.
testsuite/
* c-c++-common/pr62199.c: New test.
* c-c++-common/pr62199-2.c: New test.
* g++.dg/warn/Wparentheses-25.C: Drop XFAILs.
diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index c5eb2a7..58e5944 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -1727,7 +1727,8 @@ warn_logical_operator (location_t location, enum
tree_code code, tree type,
/* Warn about logical not used on the left hand side operand of a comparison.
This function assumes that the LHS is inside of TRUTH_NOT_EXPR.
- Do not warn if the LHS or RHS is of a boolean or a vector type. */
+ Do not warn if the LHS is of a vector type, or if RHS is of a boolean or
+ a vector type. */
void
warn_logical_not_parentheses (location_t location, enum tree_code code,
@@ -1738,8 +1739,7 @@ warn_logical_not_parentheses (location_t location, enum
tree_code code,
if (TREE_TYPE (lhs) == NULL_TREE
|| TREE_TYPE (rhs) == NULL_TREE)
;
- else if (TREE_CODE (TREE_TYPE (lhs)) == BOOLEAN_TYPE
- || TREE_CODE (TREE_TYPE (rhs)) == BOOLEAN_TYPE
+ else if (TREE_CODE (TREE_TYPE (rhs)) == BOOLEAN_TYPE
|| VECTOR_TYPE_P (TREE_TYPE (lhs))
|| VECTOR_TYPE_P (TREE_TYPE (rhs)))
return;
diff --git gcc/cp/parser.c gcc/cp/parser.c
index 9053bfa..7a2cab5 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -8020,13 +8020,12 @@ cp_parser_binary_expression (cp_parser* parser, bool
cast_p,
enum tree_code rhs_type;
enum cp_parser_prec new_prec, lookahead_prec;
tree overload;
- bool parenthesized_not_lhs_warn
- = cp_lexer_next_token_is (parser->lexer, CPP_NOT);
/* Parse the first expression. */
+ current.lhs_type = (cp_lexer_next_token_is (parser->lexer, CPP_NOT))
+ ? TRUTH_NOT_EXPR : ERROR_MARK;
current.lhs = cp_parser_cast_expression (parser, /*address_p=*/false,
cast_p, decltype_p, pidk);
- current.lhs_type = ERROR_MARK;
current.prec = prec;
if (cp_parser_error_occurred (parser))
@@ -8081,8 +8080,9 @@ cp_parser_binary_expression (cp_parser* parser, bool
cast_p,
/* Extract another operand. It may be the RHS of this expression
or the LHS of a new, higher priority expression. */
+ rhs_type = (cp_lexer_next_token_is (parser->lexer, CPP_NOT))
+ ? TRUTH_NOT_EXPR : ERROR_MARK;
rhs = cp_parser_simple_cast_expression (parser);
- rhs_type = ERROR_MARK;
/* Get another operator token. Look up its precedence to avoid
building a useless (immediately popped) stack entry for common
@@ -8125,9 +8125,15 @@ cp_parser_binary_expression (cp_parser* parser, bool
cast_p,
c_inhibit_evaluation_warnings -= current.lhs == truthvalue_true_node;
if (warn_logical_not_paren
- && parenthesized_not_lhs_warn)
- warn_logical_not_parentheses (current.loc, current.tree_type,
- TREE_OPERAND (current.lhs, 0), rhs);
+ && current.lhs_type == TRUTH_NOT_EXPR)
+ {
+ if (TREE_CODE (current.lhs) == INTEGER_CST)
+ warn_logical_not_parentheses (current.loc, current.tree_type,
+ current.lhs, rhs);
+ else if (EXPR_P (current.lhs))
+ warn_logical_not_parentheses (current.loc, current.tree_type,
+ TREE_OPERAND (current.lhs, 0), rhs);
+ }
overload = NULL;
/* ??? Currently we pass lhs_type == ERROR_MARK and rhs_type ==
diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index f8499bc..df2b722 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -4776,8 +4776,9 @@ bit-wise operator is likely to be expected.
@opindex Wlogical-not-parentheses
@opindex Wno-logical-not-parentheses
Warn about logical not used on the left hand side operand of a comparison.
-This option does not warn if the LHS or RHS operand is of a boolean or
-a vector type. Its purpose is to detect suspicious code like the following:
+This option does not warn if the LHS is of a vector type, or if the RHS
+operand is of a boolean or a vector type. Its purpose is to detect suspicious
+code like the following:
@smallexample
int a;
@dots{}
diff --git gcc/testsuite/c-c++-common/pr62199-2.c
gcc/testsuite/c-c++-common/pr62199-2.c
index e69de29..7647f16 100644
--- gcc/testsuite/c-c++-common/pr62199-2.c
+++ gcc/testsuite/c-c++-common/pr62199-2.c
@@ -0,0 +1,20 @@
+/* PR c++/62199 */
+/* { dg-do compile } */
+/* { dg-options "-Wlogical-not-parentheses" } */
+
+#ifndef __cplusplus
+# define bool _Bool
+#endif
+
+bool r;
+
+void
+foo (bool b)
+{
+ r = !b == 1; /* { dg-warning "logical not is only applied to the left hand
side of comparison" } */
+ r = !b != 1; /* { dg-warning "logical not is only applied to the left hand
side of comparison" } */
+ r = !b > 1; /* { dg-warning "logical not is only applied to the left hand
side of comparison" } */
+ r = !b >= 1; /* { dg-warning "logical not is only applied to the left hand
side of comparison" } */
+ r = !b < 1; /* { dg-warning "logical not is only applied to the left hand
side of comparison" } */
+ r = !b <= 1; /* { dg-warning "logical not is only applied to the left hand
side of comparison" } */
+}
diff --git gcc/testsuite/c-c++-common/pr62199.c
gcc/testsuite/c-c++-common/pr62199.c
index e69de29..51078c8 100644
--- gcc/testsuite/c-c++-common/pr62199.c
+++ gcc/testsuite/c-c++-common/pr62199.c
@@ -0,0 +1,22 @@
+/* PR c++/62199 */
+/* { dg-do compile } */
+/* { dg-options "-Wlogical-not-parentheses" } */
+
+int r;
+void
+foo (int a)
+{
+ r = a > 0 || !a >= 2; /* { dg-warning "19:logical not is only applied to the
left hand side of comparison" } */
+ r = !a || a == 10;
+ r = !a && !a < 4; /* { dg-warning "16:logical not is only applied to the
left hand side of comparison" } */
+ r = !a > 0 && a < 6; /* { dg-warning "10:logical not is only applied to the
left hand side of comparison" } */
+ r = a + (!a < 12); /* { dg-warning "15:logical not is only applied to the
left hand side of comparison" } */
+ r = a == 7 || !a < 12; /* { dg-warning "20:logical not is only applied to
the left hand side of comparison" } */
+ r = (a == 7 * a > 0) || !a < 2; /* { dg-warning "30:logical not is only
applied to the left hand side of comparison" } */
+ r = (1 > !a) || (!42 > a); /* { dg-warning "24:logical not is only applied
to the left hand side of comparison" } */
+ r = (!5 > a); /* { dg-warning "11:logical not is only applied to the left
hand side of comparison" } */
+ r = (!0 > a); /* { dg-warning "11:logical not is only applied to the left
hand side of comparison" } */
+ r = (!-5 > a); /* { dg-warning "12:logical not is only applied to the left
hand side of comparison" } */
+ r = (!(5 + 3) > a); /* { dg-warning "17:logical not is only applied to the
left hand side of comparison" } */
+ r = (!(5 - a) > a); /* { dg-warning "17:logical not is only applied to the
left hand side of comparison" } */
+}
diff --git gcc/testsuite/g++.dg/warn/Wparentheses-25.C
gcc/testsuite/g++.dg/warn/Wparentheses-25.C
index ab00c25..d9951a4 100644
--- gcc/testsuite/g++.dg/warn/Wparentheses-25.C
+++ gcc/testsuite/g++.dg/warn/Wparentheses-25.C
@@ -8,7 +8,7 @@ int foo (int);
int
bar (int a, int b, int c)
{
- foo (!a & b); /* { dg-warning "parentheses" "correct warning" { xfail *-*-*
} } */
+ foo (!a & b); /* { dg-warning "parentheses" "correct warning" } */
foo (!a & (b < c));
foo (!a & (b > c));
foo (!a & (b == c));
@@ -20,7 +20,7 @@ bar (int a, int b, int c)
foo (!a & !b);
foo (!(a & b));
foo ((!a) & b);
- foo (!a & 2); /* { dg-warning "parentheses" "correct warning" { xfail *-*-*
} } */
+ foo (!a & 2); /* { dg-warning "parentheses" "correct warning" } */
foo (!a & (2 < c));
foo (!a & (2 > c));
foo (!a & (2 == c));
@@ -32,7 +32,7 @@ bar (int a, int b, int c)
foo (!a & !2);
foo (!(a & 2));
foo ((!a) & 2);
- foo (!1 & 2); /* { dg-warning "parentheses" "correct warning" { xfail *-*-*
} } */
+ foo (!1 & 2); /* { dg-warning "parentheses" "correct warning" } */
foo (!1 & (2 < c));
foo (!1 & (2 > c));
foo (!1 & (2 == c));
@@ -44,7 +44,7 @@ bar (int a, int b, int c)
foo (!1 & !2);
foo (!(1 & 2));
- foo (!a | b); /* { dg-warning "parentheses" "correct warning" { xfail *-*-*
} } */
+ foo (!a | b); /* { dg-warning "parentheses" "correct warning" } */
foo (!a | (b < c));
foo (!a | (b > c));
foo (!a | (b == c));
@@ -56,7 +56,7 @@ bar (int a, int b, int c)
foo (!a | !b);
foo (!(a | b));
foo ((!a) | b);
- foo (!a | 2); /* { dg-warning "parentheses" "correct warning" { xfail *-*-*
} } */
+ foo (!a | 2); /* { dg-warning "parentheses" "correct warning" } */
foo (!a | (2 < c));
foo (!a | (2 > c));
foo (!a | (2 == c));
@@ -68,7 +68,7 @@ bar (int a, int b, int c)
foo (!a | !2);
foo (!(a | 2));
foo ((!a) | 2);
- foo (!1 | 2); /* { dg-warning "parentheses" "correct warning" { xfail *-*-*
} } */
+ foo (!1 | 2); /* { dg-warning "parentheses" "correct warning" } */
foo (!1 | (2 < c));
foo (!1 | (2 > c));
foo (!1 | (2 == c));
@@ -159,55 +159,55 @@ bar (int a, int b, int c)
int
baz (int a, int b, int c)
{
- foo (!a & (b << c));/* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a & (b >> c));/* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a & (b + c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a & (b - c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a & (b = c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a & ~b); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a & (b & c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a & (b | c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a & 2); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a & (2 << c));/* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a & (2 >> c));/* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a & (2 + c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a & (2 - c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a & (c = 2)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a & ~2); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a & (2 & c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a & (2 | c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!1 & (2 << c));/* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!1 & (2 >> c));/* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!1 & (2 + c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!1 & (2 - c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!1 & (c = 2)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!1 & ~2); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!1 & (2 & c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!1 & (2 | c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a | (b << c));/* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a | (b >> c));/* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a | (b + c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a | (b - c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a | (b = c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a | ~b); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a | (b & c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a | (b | c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a | (2 << c));/* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a | (2 >> c));/* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a | (2 + c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a | (2 - c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a | (c = 2)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a | ~2); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a | (2 & c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!a | (2 | c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!1 | (2 << c));/* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!1 | (2 >> c));/* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!1 | (2 + c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!1 | (2 - c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!1 | (c = 2)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!1 | ~2); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!1 | (2 & c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
- foo (!1 | (2 | c)); /* { dg-warning "parentheses" "correct warning" { xfail
*-*-* } } */
+ foo (!a & (b << c));/* { dg-warning "parentheses" "correct warning" } */
+ foo (!a & (b >> c));/* { dg-warning "parentheses" "correct warning" } */
+ foo (!a & (b + c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a & (b - c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a & (b = c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a & ~b); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a & (b & c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a & (b | c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a & 2); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a & (2 << c));/* { dg-warning "parentheses" "correct warning" } */
+ foo (!a & (2 >> c));/* { dg-warning "parentheses" "correct warning" } */
+ foo (!a & (2 + c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a & (2 - c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a & (c = 2)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a & ~2); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a & (2 & c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a & (2 | c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!1 & (2 << c));/* { dg-warning "parentheses" "correct warning" } */
+ foo (!1 & (2 >> c));/* { dg-warning "parentheses" "correct warning" } */
+ foo (!1 & (2 + c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!1 & (2 - c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!1 & (c = 2)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!1 & ~2); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!1 & (2 & c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!1 & (2 | c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a | (b << c));/* { dg-warning "parentheses" "correct warning" } */
+ foo (!a | (b >> c));/* { dg-warning "parentheses" "correct warning" } */
+ foo (!a | (b + c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a | (b - c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a | (b = c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a | ~b); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a | (b & c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a | (b | c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a | (2 << c));/* { dg-warning "parentheses" "correct warning" } */
+ foo (!a | (2 >> c));/* { dg-warning "parentheses" "correct warning" } */
+ foo (!a | (2 + c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a | (2 - c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a | (c = 2)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a | ~2); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a | (2 & c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!a | (2 | c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!1 | (2 << c));/* { dg-warning "parentheses" "correct warning" } */
+ foo (!1 | (2 >> c));/* { dg-warning "parentheses" "correct warning" } */
+ foo (!1 | (2 + c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!1 | (2 - c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!1 | (c = 2)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!1 | ~2); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!1 | (2 & c)); /* { dg-warning "parentheses" "correct warning" } */
+ foo (!1 | (2 | c)); /* { dg-warning "parentheses" "correct warning" } */
foo ((b << c) & !a);
foo ((b >> c) & !a);
foo ((b + c) & !a);
Marek
