This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 2/4] c/c++, asm: Use nicer error for duplicate asm qualifiers
On Tue, Dec 11, 2018 at 10:35:00AM -0500, David Malcolm wrote:
> On Mon, 2018-12-10 at 22:47 +0000, Segher Boessenkool wrote:
>
> [...]
>
> > diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
> > index 121a91c..652e53c 100644
> > --- a/gcc/c/c-parser.c
> > +++ b/gcc/c/c-parser.c
> > @@ -6360,41 +6360,54 @@ c_parser_for_statement (c_parser *parser,
> > bool ivdep, unsigned short unroll,
> > static tree
> > c_parser_asm_statement (c_parser *parser)
> > {
> > - tree quals, str, outputs, inputs, clobbers, labels, ret;
> > - bool simple, is_volatile, is_inline, is_goto;
> > + tree str, outputs, inputs, clobbers, labels, ret;
> > + bool simple;
> > location_t asm_loc = c_parser_peek_token (parser)->location;
> > int section, nsections;
> >
> > gcc_assert (c_parser_next_token_is_keyword (parser, RID_ASM));
> > c_parser_consume_token (parser);
> >
> > - quals = NULL_TREE;
> > - is_volatile = false;
> > - is_inline = false;
> > - is_goto = false;
> > + /* Handle the asm-qualifier-list. */
> > + location_t volatile_loc = UNKNOWN_LOCATION;
> > + location_t inline_loc = UNKNOWN_LOCATION;
> > + location_t goto_loc = UNKNOWN_LOCATION;
> > for (;;)
> > {
> > - switch (c_parser_peek_token (parser)->keyword)
> > + c_token *token = c_parser_peek_token (parser);
> > + location_t loc = token->location;
> > + switch (token->keyword)
> > {
> > case RID_VOLATILE:
> > - if (is_volatile)
> > - break;
> > - is_volatile = true;
> > - quals = c_parser_peek_token (parser)->value;
> > + if (volatile_loc)
> > + {
> > + error_at (loc, "duplicate asm qualifier %qE", token-
> > >value);
> > + inform (volatile_loc, "first seen here");
> > + }
>
> Thanks for the improvements.
>
> Is there test coverage for these errors and notes?
Yes, in this same patch. The error, that is; I have no idea how to test
the note, and that belongs in a different test anyhow. Dejagnu ignores
notes normally.
> A diagnostic nit (new with gcc 9): please add an:
> auto_diagnostic_group d;
> to the start of the guarded block, so that the "error" and "note" are
> known to be related.
How would that work for
asm volatile goto volatile goto ("uh-oh" :::: lab); lab :;
There are two groups, overlapping, but not nested. I suppose something
could be done with new() but that is too ugly for words. Is there a
procedural interface, too? Something that does not depend on C++ magic.
Segher