Today I was playing with my -Wlogical-not-parentheses warning and unfortunately I discovered two bugs: 1) if we have an expression consisting of more binary subexpression, such as "n > 5 || !n < 10", we don't warn for the second subexpr, because the code looked for "!" only on the very first LHS. Fixed by introducing a temporary flag that says whether an expression has "!" - and if this expr turns out to be a LHS, warn. 2) ICE with e.g. "!42 > n". The result of the LHS is a constant, so we can't use TREE_OPERAND on it.
The C FE happens to be fine and doesn't need any tweaks. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2014-08-20 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. diff --git gcc/cp/parser.c gcc/cp/parser.c index 9053bfa..3b02349 100644 --- gcc/cp/parser.c +++ gcc/cp/parser.c @@ -8034,6 +8034,7 @@ cp_parser_binary_expression (cp_parser* parser, bool cast_p, for (;;) { + bool not_expr_save; /* Get an operator token. */ token = cp_lexer_peek_token (parser->lexer); @@ -8071,6 +8072,7 @@ cp_parser_binary_expression (cp_parser* parser, bool cast_p, /* We used the operator token. */ cp_lexer_consume_token (parser->lexer); + not_expr_save = cp_lexer_next_token_is (parser->lexer, CPP_NOT); /* For "false && x" or "true || x", x will never be executed; disable warnings while evaluating it. */ @@ -8101,6 +8103,7 @@ cp_parser_binary_expression (cp_parser* parser, bool cast_p, current.lhs_type = rhs_type; current.prec = new_prec; new_prec = lookahead_prec; + parenthesized_not_lhs_warn = not_expr_save; goto get_rhs; pop: @@ -8126,8 +8129,17 @@ cp_parser_binary_expression (cp_parser* parser, bool cast_p, if (warn_logical_not_paren && parenthesized_not_lhs_warn) - warn_logical_not_parentheses (current.loc, current.tree_type, - TREE_OPERAND (current.lhs, 0), rhs); + { + 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..3191151 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" } */ +} Marek