Hi!
On Wed, Dec 05, 2018 at 04:47:37PM -0500, Jason Merrill wrote:
> On 12/2/18 11:38 AM, Segher Boessenkool wrote:
> >PR55681 observes that currently only one qualifier is allowed for
> >inline asm, so that e.g. "volatile asm" is allowed, "const asm" is also
> >okay (with a warning), but "const volatile asm" gives an error. Also
> >"goto" has to be last.
> >
> >This patch changes things so that only "asm-qualifiers" are allowed,
> >that is "volatile" and "goto", in any combination, in any order, but
> >without repetitions.
> >
> >
> >2018-12-02 Segher Boessenkool <seg...@kernel.crashing.org>
> >
> > PR inline-asm/55681
> > * doc/extend.texi (Basic Asm): Update grammar.
> > (Extended Asm): Update grammar.
> >
> >gcc/c/
> > PR inline-asm/55681
> > * c-parser.c (c_parser_for_statement): Update grammar. Allow any
> > combination of volatile and goto, in any order, without repetitions.
> >
> >gcc/cp/
> > PR inline-asm/55681
> > * parser.c (cp_parser_using_directive): Update grammar. Allow any
> > combination of volatile and goto, in any order, without repetitions.
>
> You don't actually change cp_parser_using_directive, despite what diff
> says: you're changing cp_parser_asm_definition.
I trust diff too much, sigh.
> >+ for (bool done = false; !done ; )
> >+ switch (cp_lexer_peek_token (parser->lexer)->keyword)
> >+ {
> >+ case RID_VOLATILE:
> >+ if (!volatile_p)
> >+ {
> >+ /* Remember that we saw the `volatile' keyword. */
> >+ volatile_p = true;
> >+ /* Consume the token. */
> >+ cp_lexer_consume_token (parser->lexer);
> >+ }
> >+ else
> >+ done = true;
> >+ break;
> >+ case RID_GOTO:
> >+ if (!goto_p && parser->in_function_body)
> >+ {
> >+ /* Remember that we saw the `goto' keyword. */
> >+ goto_p = true;
> >+ /* Consume the token. */
> >+ cp_lexer_consume_token (parser->lexer);
> >+ }
> >+ else
> >+ done = true;
> >+ break;
> >+ default:
> >+ done = true;
> >+ }
>
> An arguably simpler alternative to using the 'done' variable would be to
> 'break' out of the loop after the switch, and have the consume_token
> cases explicitly 'continue'.
Yeah, that is neater, continue only deals with the loop. Nice.
> We also might remember the old tokens and give a more helpful error
> message in the case of duplicate keywords.
>
> But I won't insist on either of these, the C++ changes are OK as-is.
I'll commit it like this then, and work on the improvements afterwards
(they also apply to the C frontend).
Thanks for the review!
Segher
