* C++ PATCH for c++/91391 - bogus -Wcomma-subscript warning
@ 2019-08-07 20:37 Marek Polacek
2019-08-14 13:19 ` Marek Polacek
0 siblings, 1 reply; 3+ messages in thread
From: Marek Polacek @ 2019-08-07 20:37 UTC (permalink / raw)
To: GCC Patches, Jakub Jelinek, Jason Merrill
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 <jakub@redhat.com>
Marek Polacek <polacek@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)];
+}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: C++ PATCH for c++/91391 - bogus -Wcomma-subscript warning
2019-08-07 20:37 C++ PATCH for c++/91391 - bogus -Wcomma-subscript warning Marek Polacek
@ 2019-08-14 13:19 ` Marek Polacek
2019-08-14 15:06 ` Jason Merrill
0 siblings, 1 reply; 3+ messages in thread
From: Marek Polacek @ 2019-08-14 13:19 UTC (permalink / raw)
To: GCC Patches, Jakub Jelinek, Jason Merrill
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 <jakub@redhat.com>
> Marek Polacek <polacek@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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: C++ PATCH for c++/91391 - bogus -Wcomma-subscript warning
2019-08-14 13:19 ` Marek Polacek
@ 2019-08-14 15:06 ` Jason Merrill
0 siblings, 0 replies; 3+ messages in thread
From: Jason Merrill @ 2019-08-14 15:06 UTC (permalink / raw)
To: Marek Polacek, GCC Patches, Jakub Jelinek
On 8/14/19 9:15 AM, Marek Polacek wrote:
> 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?
OK.
Jason
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-08-14 14:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-07 20:37 C++ PATCH for c++/91391 - bogus -Wcomma-subscript warning Marek Polacek
2019-08-14 13:19 ` Marek Polacek
2019-08-14 15:06 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).