On Mon, May 10, 2021 at 04:11:39PM +0200, Marcel Vollweiler wrote:
> @@ -15660,37 +15665,54 @@ c_parser_omp_clause_map (c_parser *parser, tree
> list)
> if (!parens.require_open (parser))
> return list;
>
> - if (c_parser_next_token_is (parser, CPP_NAME))
> + int always = 0;
> + int close = 0;
> + int pos = 1;
> + while (c_parser_peek_nth_token_raw (parser, pos)->type == CPP_NAME)
Nice, totally missed that Joseph has added this.
> {
> - c_token *tok = c_parser_peek_token (parser);
> + c_token *tok = c_parser_peek_nth_token_raw (parser, pos);
> const char *p = IDENTIFIER_POINTER (tok->value);
> - always_id_kind = tok->id_kind;
> - always_loc = tok->location;
> - always_id = tok->value;
> if (strcmp ("always", p) == 0)
> {
> - c_token *sectok = c_parser_peek_2nd_token (parser);
> - if (sectok->type == CPP_COMMA)
> + if (always)
> {
> - c_parser_consume_token (parser);
> - c_parser_consume_token (parser);
> - always = 2;
> + c_parser_error (parser, "expected modifier %<always%> only once");
The usual wording would be
"too many %<always%> modifiers"
> + parens.skip_until_found_close (parser);
> + return list;
> + }
> +
> + always_id_kind = tok->id_kind;
> + always_loc = tok->location;
> + always_id = tok->value;
But you don't need any of the always_{id_kind,loc,id} variables anymore,
so they should be removed and everything that touches them too.
> +
> + always++;
> + }
> + else if (strcmp ("close", p) == 0)
> + {
> + if (close)
> + {
> + c_parser_error (parser, "expected modifier %<close%> only once");
Similarly.
> + parens.skip_until_found_close (parser);
> + return list;
> }
> - else if (sectok->type == CPP_NAME)
> +
> + close++;
> + }
> + else if (c_parser_peek_nth_token_raw (parser, pos + 1)->type ==
> CPP_COLON)
IMHO you should at least check that tok->type == CPP_NAME before
checking pos + 1 token's type, you don't want to skip over CPP_EOF,
CPP_PRAGMA_EOF, or even CPP_CLOSE_PAREN etc.
Perhaps by adding
if (tok->type != CPP_NAME)
break;
right after c_token *tok = c_parser_peek_nth_token_raw (parser, pos); ?
> + {
> + for (int i = 1; i < pos; ++i)
> {
> - p = IDENTIFIER_POINTER (sectok->value);
> - if (strcmp ("alloc", p) == 0
> - || strcmp ("to", p) == 0
> - || strcmp ("from", p) == 0
> - || strcmp ("tofrom", p) == 0
> - || strcmp ("release", p) == 0
> - || strcmp ("delete", p) == 0)
> - {
> - c_parser_consume_token (parser);
> - always = 1;
> - }
> + c_parser_peek_token(parser);
Formatting, space before (
> + c_parser_consume_token (parser);
> }
> + break;
And, IMHO something should clear always and close (btw, might be better
to use close_modifier as variable name and for consistency always_modifier)
unless we reach the CPP_COLON case.
Because we don't want
map (always, close)
to imply
map (always, close, tofrom: always, close)
but
map (tofrom: always, close)
and my reading of your changes suggests that we actually use the
*_ALWAYS* kinds in that case.
> + cp_parser_error (parser,
> + "expected modifier %<always%> only once");
See above.
> + cp_parser_skip_to_closing_parenthesis (parser,
> + /*recovering=*/true,
> + /*or_comma=*/false,
> + /*consume_paren=*/true);
> + return list;
> + }
> +
> + always = true;
> + }
> + else if (strcmp ("close", p) == 0)
> + {
> + if (close)
> + {
> + cp_parser_error (parser,
> + "expected modifier %<close%> only once");
Likewise.
> + else if (cp_lexer_peek_nth_token (parser->lexer, pos + 1)->type
> + == CPP_COLON)
> + {
> + for (int i = 1; i < pos; ++i)
> + cp_lexer_consume_token (parser->lexer);
> + break;
> + }
> + else
> + break;
> +
> + if (cp_lexer_peek_nth_token (parser->lexer, pos + 1)->type ==
> CPP_COMMA)
> + pos++;
> + pos++;
> }
Again, I don't see anything that would clear always/close if it didn't reach
the CPP_COLON case.
And it should be covered in the testcase.
Jakub
