improve syntax errors

[Martin Sebor]

On 1/3/19 8:40 AM, 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", ")");

I would suggest the quoted form since it means less work for message
translantors.  (Both forms are used in GCC messages but I think it
would be worth adopting the quoted form.)

> (FWIW, the word "extraneous" seems a bit "jargony" to me, how about
> "stray %qs token"?  (giving: "error: stray ')' token")   I'm not sure
> though)

A helpful rule of thumb I like to use is checking which form is
prevalent in gcc/po/gcc.pot.  In this case, there's just one
message with the word "extraneous" in all of GCC and 7 that
use "stray."  There are also three messages that use the word
"unmatched", and for variety, five that use "unterminated" (not
all of them are interchangeable).

Martin

>> 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).
> 
> It might make sense to emit a fix-it hint suggesting the removal of the
> stray token.
> 
> Much of these ideas could probably apply to the C++ frontend as well
> (annoyingly, not much of this code is shared between C and C++).
> 
>> Best regards,
>> Daniel Marjamäki
> 
> Hope this is helpful, and welcome again.
> 
> Dave
>