Ping. On Wed, Aug 07, 2019 at 04:05:53PM -0400, Marek Polacek wrote: > When implementing -Wcomma-subscript I failed to realize that a comma in > a template-argument-list shouldn't be warned about. > > But we can't simply ignore any commas inside < ... > because the following > needs to be caught: > > a[b < c, b > c]; > > This patch from Jakub fixes it by moving the warning to cp_parser_expression > where we can better detect top-level commas (and avoid saving tokens). > > I've extended the patch to revert the cp_parser_skip_to_closing_square_bracket > changes I made in r274121 -- they are no longer needed. > > Apologies for the thinko. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2019-08-07 Jakub Jelinek <ja...@redhat.com> > Marek Polacek <pola...@redhat.com> > > PR c++/91391 - bogus -Wcomma-subscript warning. > * parser.c (cp_parser_postfix_open_square_expression): Don't warn about > a deprecated comma here. Pass warn_comma_subscript down to > cp_parser_expression. > (cp_parser_expression): New bool parameter. Warn about uses of a comma > operator within a subscripting expression. > (cp_parser_skip_to_closing_square_bracket): Revert to pre-r274121 state. > (cp_parser_skip_to_closing_square_bracket_1): Remove. > > * g++.dg/cpp2a/comma5.C: New test. > > diff --git gcc/cp/parser.c gcc/cp/parser.c > index 14b724095c4..eccc3749fd0 100644 > --- gcc/cp/parser.c > +++ gcc/cp/parser.c > @@ -2102,7 +2102,7 @@ static cp_expr cp_parser_assignment_expression > static enum tree_code cp_parser_assignment_operator_opt > (cp_parser *); > static cp_expr cp_parser_expression > - (cp_parser *, cp_id_kind * = NULL, bool = false, bool = false); > + (cp_parser *, cp_id_kind * = NULL, bool = false, bool = false, bool = > false); > static cp_expr cp_parser_constant_expression > (cp_parser *, bool = false, bool * = NULL, bool = false); > static cp_expr cp_parser_builtin_offsetof > @@ -2669,8 +2669,6 @@ static bool cp_parser_init_statement_p > (cp_parser *); > static bool cp_parser_skip_to_closing_square_bracket > (cp_parser *); > -static int cp_parser_skip_to_closing_square_bracket_1 > - (cp_parser *, enum cpp_ttype); > > /* Concept-related syntactic transformations */ > > @@ -7524,33 +7522,9 @@ cp_parser_postfix_open_square_expression (cp_parser > *parser, > index = cp_parser_braced_list (parser, &expr_nonconst_p); > } > else > - { > - /* [depr.comma.subscript]: A comma expression appearing as > - the expr-or-braced-init-list of a subscripting expression > - is deprecated. A parenthesized comma expression is not > - deprecated. */ > - if (warn_comma_subscript) > - { > - /* Save tokens so that we can put them back. */ > - cp_lexer_save_tokens (parser->lexer); > - > - /* Look for ',' that is not nested in () or {}. */ > - if (cp_parser_skip_to_closing_square_bracket_1 (parser, > - CPP_COMMA) == -1) > - { > - auto_diagnostic_group d; > - warning_at (cp_lexer_peek_token (parser->lexer)->location, > - OPT_Wcomma_subscript, > - "top-level comma expression in array subscript " > - "is deprecated"); > - } > - > - /* Roll back the tokens we skipped. */ > - cp_lexer_rollback_tokens (parser->lexer); > - } > - > - index = cp_parser_expression (parser); > - } > + index = cp_parser_expression (parser, NULL, /*cast_p=*/false, > + /*decltype_p=*/false, > + /*warn_comma_p=*/warn_comma_subscript); > } > > parser->greater_than_is_operator_p = saved_greater_than_is_operator_p; > @@ -9932,12 +9906,13 @@ cp_parser_assignment_operator_opt (cp_parser* parser) > CAST_P is true if this expression is the target of a cast. > DECLTYPE_P is true if this expression is the immediate operand of > decltype, > except possibly parenthesized or on the RHS of a comma (N3276). > + WARN_COMMA_P is true if a comma should be diagnosed. > > Returns a representation of the expression. */ > > static cp_expr > cp_parser_expression (cp_parser* parser, cp_id_kind * pidk, > - bool cast_p, bool decltype_p) > + bool cast_p, bool decltype_p, bool warn_comma_p) > { > cp_expr expression = NULL_TREE; > location_t loc = UNKNOWN_LOCATION; > @@ -9984,6 +9959,17 @@ cp_parser_expression (cp_parser* parser, cp_id_kind * > pidk, > break; > /* Consume the `,'. */ > loc = cp_lexer_peek_token (parser->lexer)->location; > + if (warn_comma_p) > + { > + /* [depr.comma.subscript]: A comma expression appearing as > + the expr-or-braced-init-list of a subscripting expression > + is deprecated. A parenthesized comma expression is not > + deprecated. */ > + warning_at (loc, OPT_Wcomma_subscript, > + "top-level comma expression in array subscript " > + "is deprecated"); > + warn_comma_p = false; > + } > cp_lexer_consume_token (parser->lexer); > /* A comma operator cannot appear in a constant-expression. */ > if (cp_parser_non_integral_constant_expression (parser, NIC_COMMA)) > @@ -22888,25 +22874,16 @@ cp_parser_braced_list (cp_parser* parser, bool* > non_constant_p) > } > > /* Consume tokens up to, and including, the next non-nested closing `]'. > - Returns 1 iff we found a closing `]'. Returns -1 if OR_TTYPE is not > - CPP_EOF and we found an unnested token of that type. */ > + Returns true iff we found a closing `]'. */ > > -static int > -cp_parser_skip_to_closing_square_bracket_1 (cp_parser *parser, > - enum cpp_ttype or_ttype) > +static bool > +cp_parser_skip_to_closing_square_bracket (cp_parser *parser) > { > unsigned square_depth = 0; > - unsigned paren_depth = 0; > - unsigned brace_depth = 0; > > while (true) > { > - cp_token *token = cp_lexer_peek_token (parser->lexer); > - > - /* Have we found what we're looking for before the closing square? */ > - if (token->type == or_ttype && or_ttype != CPP_EOF > - && brace_depth == 0 && paren_depth == 0 && square_depth == 0) > - return -1; > + cp_token * token = cp_lexer_peek_token (parser->lexer); > > switch (token->type) > { > @@ -22916,38 +22893,20 @@ cp_parser_skip_to_closing_square_bracket_1 > (cp_parser *parser, > /* FALLTHRU */ > case CPP_EOF: > /* If we've run out of tokens, then there is no closing `]'. */ > - return 0; > + return false; > > case CPP_OPEN_SQUARE: > ++square_depth; > break; > > case CPP_CLOSE_SQUARE: > - if (square_depth-- == 0) > + if (!square_depth--) > { > cp_lexer_consume_token (parser->lexer); > - return 1; > + return true; > } > break; > > - case CPP_OPEN_BRACE: > - ++brace_depth; > - break; > - > - case CPP_CLOSE_BRACE: > - if (brace_depth-- == 0) > - return 0; > - break; > - > - case CPP_OPEN_PAREN: > - ++paren_depth; > - break; > - > - case CPP_CLOSE_PAREN: > - if (paren_depth-- == 0) > - return 0; > - break; > - > default: > break; > } > @@ -22957,15 +22916,6 @@ cp_parser_skip_to_closing_square_bracket_1 > (cp_parser *parser, > } > } > > -/* Consume tokens up to, and including, the next non-nested closing `]'. > - Returns true iff we found a closing `]'. */ > - > -static bool > -cp_parser_skip_to_closing_square_bracket (cp_parser *parser) > -{ > - return cp_parser_skip_to_closing_square_bracket_1 (parser, CPP_EOF) == 1; > -} > - > /* Return true if we are looking at an array-designator, false otherwise. */ > > static bool > diff --git gcc/testsuite/g++.dg/cpp2a/comma5.C > gcc/testsuite/g++.dg/cpp2a/comma5.C > new file mode 100644 > index 00000000000..68d19c09ccf > --- /dev/null > +++ gcc/testsuite/g++.dg/cpp2a/comma5.C > @@ -0,0 +1,21 @@ > +// PR c++/91391 - bogus -Wcomma-subscript warning. > +// { dg-do compile { target c++2a } } > + > +template<typename T, typename U> > +int foo(T t, U u) { return t + u; } > + > +void > +fn (int *a, int b, int c) > +{ > + a[foo<int, int>(1, 2)]; > + a[foo<int, int>(1, 2), foo<int, int>(3, 4)]; // { dg-warning "24:top-level > comma expression in array subscript is deprecated" } > + > + a[b < c, b < c]; // { dg-warning "top-level comma expression in array > subscript is deprecated" } > + a[b < c, b > c]; // { dg-warning "top-level comma expression in array > subscript is deprecated" } > + a[b > c, b > c]; // { dg-warning "top-level comma expression in array > subscript is deprecated" } > + a[b > c, b < c]; // { dg-warning "top-level comma expression in array > subscript is deprecated" } > + a[(b < c, b < c)]; > + a[(b < c, b > c)]; > + a[b << c, b << c]; // { dg-warning "top-level comma expression in array > subscript is deprecated" } > + a[(b << c, b << c)]; > +}
Marek