On Wed, Aug 20, 2014 at 05:02:24PM -0400, Jason Merrill wrote: > On 08/20/2014 04:02 PM, Marek Polacek wrote: > >On Wed, Aug 20, 2014 at 02:36:21PM -0400, Jason Merrill wrote: > >>Could we set current.lhs_type to TRUTH_NOT_EXPR when we see a ! rather than > >>track nots in two separate local variables? > > > >Good point. So like the following? > > I was thinking to do away with parenthesized_not_lhs_warn as well, so > instead of > > > bool parenthesized_not_lhs_warn > > = cp_lexer_next_token_is (parser->lexer, CPP_NOT); > > > > /* Parse the first expression. */ > > current.lhs = cp_parser_cast_expression (parser, /*address_p=*/false, > > cast_p, decltype_p, pidk); > > current.lhs_type = ERROR_MARK; > > we would set current.lhs_type to TRUTH_NOT_EXPR here if the first token is > CPP_NOT, and > > > /* Extract another operand. It may be the RHS of this expression > > or the LHS of a new, higher priority expression. */ > > rhs = cp_parser_simple_cast_expression (parser); > > rhs_type = ERROR_MARK; > > here we would do the same thing for rhs_type. > > > cp_lexer_consume_token (parser->lexer); > >+ if (cp_lexer_next_token_is (parser->lexer, CPP_NOT)) > >+ current.lhs_type = TRUTH_NOT_EXPR; > ... > > current.lhs = rhs; > >+ parenthesized_not_lhs_warn = current.lhs_type == TRUTH_NOT_EXPR; > > current.lhs_type = rhs_type; > > and then you don't need any changes in these places. > > > if (warn_logical_not_paren > > && parenthesized_not_lhs_warn) > > And here you check current.lhs_type.
Oh, I see now. Thanks, this is much better. This change has an interesting effect on Wparentheses-25.C test; it made it XPASS - I believe we now generate the correct warnings, the same as C FE. So I got rid of all those xfails in that test and it now passes cleanly. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2014-08-21 Marek Polacek <pola...@redhat.com> PR c++/62199 * parser.c (cp_parser_binary_expression): Check each LHS if it's preceded with logical not. Handle logical not of constants. * c-c++-common/pr62199.c: New test. * g++.dg/warn/Wparentheses-25.C: Remove XFAILs. diff --git gcc/cp/parser.c gcc/cp/parser.c index 9053bfa..ddcbab9 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,18 @@ 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) + { + tree lhs; + /* 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); + else + lhs = TREE_OPERAND (current.lhs, 0); + warn_logical_not_parentheses (current.loc, current.tree_type, + lhs, rhs); + } overload = NULL; /* ??? Currently we pass lhs_type == ERROR_MARK and rhs_type == 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