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