On Thu, 3 Jan 2019 at 15:40, David Malcolm wrote:
>
> On Thu, 2019-01-03 at 15:59 +0100, Daniel Marjamäki wrote:
> > Hello!
> >
> > I have used GCC for decades and would like to contribute a little. :-
> > )
>
> Hi, and welcome!
>
> > I would like to see if I can improve the syntax errors.
> >
> > Here is one example code:
> >
> >     int x = 3) + 0;
> >
> > I have created this ugly experimental patch:
> >
> > --- a/gcc/c/c-parser.c
> > +++ b/gcc/c/c-parser.c
> > @@ -2228,7 +2228,10 @@ c_parser_declaration_or_fndef (c_parser
> > *parser, bool fndef_ok,
> >             }
> >           else
> >             {
> > -             c_parser_error (parser, "expected %<,%> or %<;%>");
> > +             if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN))
> > +               c_parser_error (parser, "extraneous )");
> > +             else
> > +               c_parser_error (parser, "expected %<,%> or %<;%>");
> >               c_parser_skip_to_end_of_block_or_statement (parser);
> >               return;
> >             }
> >
> > Before my patch:
> > test1.c:3:12: error: expected ‘,’ or ‘;’ before ‘)’ token
> >
> > After my patch:
> > test1.c:3:12: error: extraneous ) before ‘)’ token
>
> > That is not perfect neither.
>
> Thanks for trying it out.
>
> A minor nit: the ")" is a source code construct and thus should be
> quoted in the message, by wrapping it in %< and %> like in the existing
> code:
>
>   c_parser_error (parser, "extraneous %<)%>");
>
> or by using %qs to quote a const const *:
>
>   c_parser_error (parser, "extraneous %qs", ")");
>
>
> (FWIW, the word "extraneous" seems a bit "jargony" to me, how about
> "stray %qs token"?  (giving: "error: stray ')' token")   I'm not sure
> though)

Maybe "unmatched"?

> > I have 2 questions..
> >  * can somebody give me a hint how I improve the error message so it
> > does not say "before ) token"?
>
> The before ')' token is being supplied due to the use of
> c_parser_error, which calls c_parse_error, which adds a
>   "before SOMETHING"
> suffix to the message before calling into the diagnostic subsystem.
> You could use:
>
>   error_at (token->location, "some message");
>
> to go directly to the underlying diagnostic API to avoid getting the
> "before SOMETHING" suffix.
>
> >  * how do I run the tests?
>
> You might want to look at this guide I put together:
>   https://dmalcolm.fedorapeople.org/gcc/newbies-guide/index.html
> which among other things has some notes on running tests (and writing
> new ones), and on stepping through gcc in a debugger.
>
> > Basically I want that when there is a unmatched extra ) or } or ]
> > then
> > it should just say "extraneous .." instead of "expected ',' or ';'.
> > Adding a ',' or ';' in the example code will not fix the syntax
> > error.
>
> I wonder how much we want to special-case this.  Are you thinking about
> the case where there's a stray symbol in the code (perhaps due to a
> stray keypress, or unfinished manual edits)?   Perhaps we could have a
> predicate for determining if a token can never make sense in the given
> context, and have something like:
>
> if (c_parser_next_token_is_meaningless_p (parser))
>   complain_about_stray_token (parser);
> else
>   ...
>
> or somesuch (we use a "_p" suffix for predicates).

Right, why emit a special diagnostic for 3) but not 3]?

Reply via email to