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]?