* Improve syntax error @ 2019-01-04 20:25 Daniel Marjamäki 2019-01-05 8:50 ` Segher Boessenkool 0 siblings, 1 reply; 7+ messages in thread From: Daniel Marjamäki @ 2019-01-04 20:25 UTC (permalink / raw) To: gcc Hello! I just chose the word "unmatched" for now.. let me know if "stray" or some other word is better. :-) I have a work in progress patch that as far as I can tell works. I wonder if you see some problems with my design. What do I need to do..? Basically I have chosen to add a nesting_depth to the c_parser struct. That is updated in the consume_token. I first tried to add such variable in c_parser_declaration_or_fndef() but that was much more difficult. I wanted to warn about unmatched }. It is however more complicated than I thought. For two reasons: The first reason is the hard problem, but maybe we can ignore this now also: void f() { } // <- looking at the indentation, it seems preferable to warn about this } The second reason is easier.. I just don't want to get into too deep water too quickly. To warn about that also I can't just have a nesting_depth anymore. I need to have a stack that track the nesting. I have experimented with a linked list like this and made it work: struct nesting { cpp_ttype type; struct nesting *next; }; but well here I run into memory management and I also wonder if you like the design decision to put a linked list in the c_parser struct. Here is my current patch... diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index 972b629c092..eabc5ffa15e 100644 --- a/gcc/c/c-parser.c +++ b/gcc/c/c-parser.c @@ -171,6 +171,8 @@ struct GTY(()) c_parser { /* How many look-ahead tokens are available (0 - 4, or more if parsing from pre-lexed tokens). */ unsigned int tokens_avail; + /* nesting depth in expression (parentheses / squares) */ + unsigned int nesting_depth; /* True if a syntax error is being recovered from; false otherwise. c_parser_error sets this flag. It should clear this flag when enough tokens have been consumed to recover from the error. */ @@ -763,6 +765,22 @@ c_parser_next_tokens_start_declaration (c_parser *parser) return false; } +/* Nesting start token */ + +static bool c_parser_is_nesting_start (c_parser *parser) +{ + return c_parser_next_token_is (parser, CPP_OPEN_PAREN) || + c_parser_next_token_is (parser, CPP_OPEN_SQUARE); +} + +/* Nesting end token */ + +static bool c_parser_is_nesting_end (c_parser *parser) +{ + return c_parser_next_token_is (parser, CPP_CLOSE_PAREN) || + c_parser_next_token_is (parser, CPP_CLOSE_SQUARE); +} + /* Consume the next token from PARSER. */ void @@ -772,6 +790,10 @@ c_parser_consume_token (c_parser *parser) gcc_assert (parser->tokens[0].type != CPP_EOF); gcc_assert (!parser->in_pragma || parser->tokens[0].type != CPP_PRAGMA_EOL); gcc_assert (parser->error || parser->tokens[0].type != CPP_PRAGMA); + if (c_parser_is_nesting_start (parser)) + parser->nesting_depth++; + else if (parser->nesting_depth > 0 && c_parser_is_nesting_end (parser)) + parser->nesting_depth--; parser->last_token_location = parser->tokens[0].location; if (parser->tokens != &parser->tokens_buf[0]) parser->tokens++; @@ -1673,6 +1695,20 @@ add_debug_begin_stmt (location_t loc) add_stmt (stmt); } +static bool c_parser_unmatched_p (c_parser *parser) +{ + if (c_parser_is_nesting_end (parser)) + return parser->nesting_depth == 0; + return false; +} + +static void complain_about_unmatched_token (c_parser *parser) +{ + c_token *token = c_parser_peek_token(parser); + error_at (token->location, "unmatched %<%s%>", + cpp_type2name(token->type, token->flags)); +} + /* Parse a declaration or function definition (C90 6.5, 6.7.1, C99 6.7, 6.9.1, C11 6.7, 6.9.1). If FNDEF_OK is true, a function definition is accepted; otherwise (old-style parameter declarations) only other @@ -2228,7 +2264,10 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, } else { - c_parser_error (parser, "expected %<,%> or %<;%>"); + if (c_parser_unmatched_p (parser)) + complain_about_unmatched_token (parser); + else + c_parser_error (parser, "expected %<,%> or %<;%>"); c_parser_skip_to_end_of_block_or_statement (parser); return; } diff --git a/gcc/testsuite/gcc.dg/unmatched.c b/gcc/testsuite/gcc.dg/unmatched.c new file mode 100644 index 00000000000..bd458a01109 --- /dev/null +++ b/gcc/testsuite/gcc.dg/unmatched.c @@ -0,0 +1,19 @@ + +/* { dg-do compile } */ + +void f1() { + int a = 0)+3; /* { dg-error "unmatched" } */ +} + +void f2() { + int b = (1]+3; /* { dg-error "expected" } */ +} + +void f3() { + int b = 1]+3; /* { dg-error "unmatched" } */ +} + +void f4() { + int c = (1))+3; /* { dg-error "unmatched" } */ +} + Best regards, Daniel Marjamäki ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Improve syntax error 2019-01-04 20:25 Improve syntax error Daniel Marjamäki @ 2019-01-05 8:50 ` Segher Boessenkool 2019-01-05 17:02 ` Daniel Marjamäki 0 siblings, 1 reply; 7+ messages in thread From: Segher Boessenkool @ 2019-01-05 8:50 UTC (permalink / raw) To: Daniel Marjamäki; +Cc: gcc Hi Daniel, Some mostly boring comments: On Fri, Jan 04, 2019 at 09:25:10PM +0100, Daniel Marjamäki wrote: > The first reason is the hard problem, but maybe we can ignore this now also: > > void f() > { > } // <- looking at the indentation, it seems preferable to warn about this > } I think the indentation warnings should catch that? > diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c > index 972b629c092..eabc5ffa15e 100644 > --- a/gcc/c/c-parser.c > +++ b/gcc/c/c-parser.c > @@ -171,6 +171,8 @@ struct GTY(()) c_parser { > /* How many look-ahead tokens are available (0 - 4, or > more if parsing from pre-lexed tokens). */ > unsigned int tokens_avail; > + /* nesting depth in expression (parentheses / squares) */ Start sentences with a capital, and end with full stop space space. I realise this isn't a full sentence, but the comment right above does this as well ;-) > @@ -763,6 +765,22 @@ c_parser_next_tokens_start_declaration (c_parser *parser) > return false; > } > > +/* Nesting start token */ > + > +static bool c_parser_is_nesting_start (c_parser *parser) > +{ > + return c_parser_next_token_is (parser, CPP_OPEN_PAREN) || > + c_parser_next_token_is (parser, CPP_OPEN_SQUARE); Indents should use tabs for every leading eight spaces. > @@ -2228,7 +2264,10 @@ c_parser_declaration_or_fndef (c_parser > *parser, bool fndef_ok, > } > else > { > - c_parser_error (parser, "expected %<,%> or %<;%>"); > + if (c_parser_unmatched_p (parser)) > + complain_about_unmatched_token (parser); Should this say something like "expected ) or , or ;"? > + else > + c_parser_error (parser, "expected %<,%> or %<;%>"); > c_parser_skip_to_end_of_block_or_statement (parser); > return; > } Segher ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Improve syntax error 2019-01-05 8:50 ` Segher Boessenkool @ 2019-01-05 17:02 ` Daniel Marjamäki 2019-01-05 19:44 ` Daniel Marjamäki 2019-01-10 10:39 ` Segher Boessenkool 0 siblings, 2 replies; 7+ messages in thread From: Daniel Marjamäki @ 2019-01-05 17:02 UTC (permalink / raw) To: Segher Boessenkool; +Cc: gcc Thanks! I will take care of the indentation and fix the comment. > I think the indentation warnings should catch that? I get this: void f() { } } // <- error: expected identifier or '(' before '}' token I ran with -Wall -Wextra -pedantic and did not see a indentation warning. Am I missing some indentation warning? The error message I get is a little misplaced. I think it's fine to warn about that } but it could also say in the error message that the problem is probably the previous } > Should this say something like "expected ) or , or ;"? No none of those suggestions will solve the error. Look at this code: int x = 3) + 0; Writing a ) or , or ; will not fix the syntax error. You have to remove the ) or add a ( somewhere. Den lör 5 jan. 2019 kl 09:50 skrev Segher Boessenkool <segher@kernel.crashing.org>: > > Hi Daniel, > > Some mostly boring comments: > > On Fri, Jan 04, 2019 at 09:25:10PM +0100, Daniel Marjamäki wrote: > > The first reason is the hard problem, but maybe we can ignore this now also: > > > > void f() > > { > > } // <- looking at the indentation, it seems preferable to warn about this > > } > > I think the indentation warnings should catch that? > > > diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c > > index 972b629c092..eabc5ffa15e 100644 > > --- a/gcc/c/c-parser.c > > +++ b/gcc/c/c-parser.c > > @@ -171,6 +171,8 @@ struct GTY(()) c_parser { > > /* How many look-ahead tokens are available (0 - 4, or > > more if parsing from pre-lexed tokens). */ > > unsigned int tokens_avail; > > + /* nesting depth in expression (parentheses / squares) */ > > Start sentences with a capital, and end with full stop space space. I > realise this isn't a full sentence, but the comment right above does this > as well ;-) > > > @@ -763,6 +765,22 @@ c_parser_next_tokens_start_declaration (c_parser *parser) > > return false; > > } > > > > +/* Nesting start token */ > > + > > +static bool c_parser_is_nesting_start (c_parser *parser) > > +{ > > + return c_parser_next_token_is (parser, CPP_OPEN_PAREN) || > > + c_parser_next_token_is (parser, CPP_OPEN_SQUARE); > > Indents should use tabs for every leading eight spaces. > > > @@ -2228,7 +2264,10 @@ c_parser_declaration_or_fndef (c_parser > > *parser, bool fndef_ok, > > } > > else > > { > > - c_parser_error (parser, "expected %<,%> or %<;%>"); > > + if (c_parser_unmatched_p (parser)) > > + complain_about_unmatched_token (parser); > > Should this say something like "expected ) or , or ;"? > > > + else > > + c_parser_error (parser, "expected %<,%> or %<;%>"); > > c_parser_skip_to_end_of_block_or_statement (parser); > > return; > > } > > > Segher ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Improve syntax error 2019-01-05 17:02 ` Daniel Marjamäki @ 2019-01-05 19:44 ` Daniel Marjamäki 2019-01-08 20:29 ` Daniel Marjamäki 2019-01-10 10:39 ` Segher Boessenkool 1 sibling, 1 reply; 7+ messages in thread From: Daniel Marjamäki @ 2019-01-05 19:44 UTC (permalink / raw) To: Segher Boessenkool; +Cc: gcc Here is a new patch with fixed comments and indentation.... diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index 972b629c092..294ff34fe55 100644 --- a/gcc/c/c-parser.c +++ b/gcc/c/c-parser.c @@ -171,6 +171,8 @@ struct GTY(()) c_parser { /* How many look-ahead tokens are available (0 - 4, or more if parsing from pre-lexed tokens). */ unsigned int tokens_avail; + /* Nesting depth in expression (parentheses / squares). */ + unsigned int nesting_depth; /* True if a syntax error is being recovered from; false otherwise. c_parser_error sets this flag. It should clear this flag when enough tokens have been consumed to recover from the error. */ @@ -763,6 +765,22 @@ c_parser_next_tokens_start_declaration (c_parser *parser) return false; } +/* Nesting start token. */ + +static bool c_parser_is_nesting_start (c_parser *parser) +{ + return c_parser_next_token_is (parser, CPP_OPEN_PAREN) || + c_parser_next_token_is (parser, CPP_OPEN_SQUARE); +} + +/* Nesting end token. */ + +static bool c_parser_is_nesting_end (c_parser *parser) +{ + return c_parser_next_token_is (parser, CPP_CLOSE_PAREN) || + c_parser_next_token_is (parser, CPP_CLOSE_SQUARE); +} + /* Consume the next token from PARSER. */ void @@ -772,6 +790,10 @@ c_parser_consume_token (c_parser *parser) gcc_assert (parser->tokens[0].type != CPP_EOF); gcc_assert (!parser->in_pragma || parser->tokens[0].type != CPP_PRAGMA_EOL); gcc_assert (parser->error || parser->tokens[0].type != CPP_PRAGMA); + if (c_parser_is_nesting_start (parser)) + parser->nesting_depth++; + else if (parser->nesting_depth > 0 && c_parser_is_nesting_end (parser)) + parser->nesting_depth--; parser->last_token_location = parser->tokens[0].location; if (parser->tokens != &parser->tokens_buf[0]) parser->tokens++; @@ -1673,6 +1695,20 @@ add_debug_begin_stmt (location_t loc) add_stmt (stmt); } +static bool c_parser_unmatched_p (c_parser *parser) +{ + if (c_parser_is_nesting_end (parser)) + return parser->nesting_depth == 0; + return false; +} + +static void complain_about_unmatched_token (c_parser *parser) +{ + c_token *token = c_parser_peek_token (parser); + error_at (token->location, "unmatched %qs", + cpp_type2name (token->type, token->flags)); +} + /* Parse a declaration or function definition (C90 6.5, 6.7.1, C99 6.7, 6.9.1, C11 6.7, 6.9.1). If FNDEF_OK is true, a function definition is accepted; otherwise (old-style parameter declarations) only other @@ -2228,7 +2264,10 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, } else { - c_parser_error (parser, "expected %<,%> or %<;%>"); + if (c_parser_unmatched_p (parser)) + complain_about_unmatched_token (parser); + else + c_parser_error (parser, "expected %<,%> or %<;%>"); c_parser_skip_to_end_of_block_or_statement (parser); return; } diff --git a/gcc/testsuite/gcc.dg/unmatched.c b/gcc/testsuite/gcc.dg/unmatched.c new file mode 100644 index 00000000000..bd458a01109 --- /dev/null +++ b/gcc/testsuite/gcc.dg/unmatched.c @@ -0,0 +1,19 @@ + +/* { dg-do compile } */ + +void f1() { + int a = 0)+3; /* { dg-error "unmatched" } */ +} + +void f2() { + int b = (1]+3; /* { dg-error "expected" } */ +} + +void f3() { + int b = 1]+3; /* { dg-error "unmatched" } */ +} + +void f4() { + int c = (1))+3; /* { dg-error "unmatched" } */ +} + Den lör 5 jan. 2019 kl 18:02 skrev Daniel Marjamäki <daniel.marjamaki@gmail.com>: > > Thanks! > > I will take care of the indentation and fix the comment. > > > I think the indentation warnings should catch that? > > I get this: > > void f() > { > } > } // <- error: expected identifier or '(' before '}' token > > I ran with -Wall -Wextra -pedantic and did not see a indentation > warning. Am I missing some indentation warning? The error message I > get is a little misplaced. I think it's fine to warn about that } but > it could also say in the error message that the problem is probably > the previous } > > > Should this say something like "expected ) or , or ;"? > > No none of those suggestions will solve the error. > > Look at this code: > > int x = 3) + 0; > > Writing a ) or , or ; will not fix the syntax error. You have to > remove the ) or add a ( somewhere. > > > > Den lör 5 jan. 2019 kl 09:50 skrev Segher Boessenkool > <segher@kernel.crashing.org>: > > > > Hi Daniel, > > > > Some mostly boring comments: > > > > On Fri, Jan 04, 2019 at 09:25:10PM +0100, Daniel Marjamäki wrote: > > > The first reason is the hard problem, but maybe we can ignore this now also: > > > > > > void f() > > > { > > > } // <- looking at the indentation, it seems preferable to warn about this > > > } > > > > I think the indentation warnings should catch that? > > > > > diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c > > > index 972b629c092..eabc5ffa15e 100644 > > > --- a/gcc/c/c-parser.c > > > +++ b/gcc/c/c-parser.c > > > @@ -171,6 +171,8 @@ struct GTY(()) c_parser { > > > /* How many look-ahead tokens are available (0 - 4, or > > > more if parsing from pre-lexed tokens). */ > > > unsigned int tokens_avail; > > > + /* nesting depth in expression (parentheses / squares) */ > > > > Start sentences with a capital, and end with full stop space space. I > > realise this isn't a full sentence, but the comment right above does this > > as well ;-) > > > > > @@ -763,6 +765,22 @@ c_parser_next_tokens_start_declaration (c_parser *parser) > > > return false; > > > } > > > > > > +/* Nesting start token */ > > > + > > > +static bool c_parser_is_nesting_start (c_parser *parser) > > > +{ > > > + return c_parser_next_token_is (parser, CPP_OPEN_PAREN) || > > > + c_parser_next_token_is (parser, CPP_OPEN_SQUARE); > > > > Indents should use tabs for every leading eight spaces. > > > > > @@ -2228,7 +2264,10 @@ c_parser_declaration_or_fndef (c_parser > > > *parser, bool fndef_ok, > > > } > > > else > > > { > > > - c_parser_error (parser, "expected %<,%> or %<;%>"); > > > + if (c_parser_unmatched_p (parser)) > > > + complain_about_unmatched_token (parser); > > > > Should this say something like "expected ) or , or ;"? > > > > > + else > > > + c_parser_error (parser, "expected %<,%> or %<;%>"); > > > c_parser_skip_to_end_of_block_or_statement (parser); > > > return; > > > } > > > > > > Segher ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Improve syntax error 2019-01-05 19:44 ` Daniel Marjamäki @ 2019-01-08 20:29 ` Daniel Marjamäki 2019-01-08 20:44 ` Joseph Myers 0 siblings, 1 reply; 7+ messages in thread From: Daniel Marjamäki @ 2019-01-08 20:29 UTC (permalink / raw) Cc: gcc Ping Den lör 5 jan. 2019 kl 20:44 skrev Daniel Marjamäki <daniel.marjamaki@gmail.com>: > > Here is a new patch with fixed comments and indentation.... > > diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c > index 972b629c092..294ff34fe55 100644 > --- a/gcc/c/c-parser.c > +++ b/gcc/c/c-parser.c > @@ -171,6 +171,8 @@ struct GTY(()) c_parser { > /* How many look-ahead tokens are available (0 - 4, or > more if parsing from pre-lexed tokens). */ > unsigned int tokens_avail; > + /* Nesting depth in expression (parentheses / squares). */ > + unsigned int nesting_depth; > /* True if a syntax error is being recovered from; false otherwise. > c_parser_error sets this flag. It should clear this flag when > enough tokens have been consumed to recover from the error. */ > @@ -763,6 +765,22 @@ c_parser_next_tokens_start_declaration (c_parser *parser) > return false; > } > > +/* Nesting start token. */ > + > +static bool c_parser_is_nesting_start (c_parser *parser) > +{ > + return c_parser_next_token_is (parser, CPP_OPEN_PAREN) || > + c_parser_next_token_is (parser, CPP_OPEN_SQUARE); > +} > + > +/* Nesting end token. */ > + > +static bool c_parser_is_nesting_end (c_parser *parser) > +{ > + return c_parser_next_token_is (parser, CPP_CLOSE_PAREN) || > + c_parser_next_token_is (parser, CPP_CLOSE_SQUARE); > +} > + > /* Consume the next token from PARSER. */ > > void > @@ -772,6 +790,10 @@ c_parser_consume_token (c_parser *parser) > gcc_assert (parser->tokens[0].type != CPP_EOF); > gcc_assert (!parser->in_pragma || parser->tokens[0].type != CPP_PRAGMA_EOL); > gcc_assert (parser->error || parser->tokens[0].type != CPP_PRAGMA); > + if (c_parser_is_nesting_start (parser)) > + parser->nesting_depth++; > + else if (parser->nesting_depth > 0 && c_parser_is_nesting_end (parser)) > + parser->nesting_depth--; > parser->last_token_location = parser->tokens[0].location; > if (parser->tokens != &parser->tokens_buf[0]) > parser->tokens++; > @@ -1673,6 +1695,20 @@ add_debug_begin_stmt (location_t loc) > add_stmt (stmt); > } > > +static bool c_parser_unmatched_p (c_parser *parser) > +{ > + if (c_parser_is_nesting_end (parser)) > + return parser->nesting_depth == 0; > + return false; > +} > + > +static void complain_about_unmatched_token (c_parser *parser) > +{ > + c_token *token = c_parser_peek_token (parser); > + error_at (token->location, "unmatched %qs", > + cpp_type2name (token->type, token->flags)); > +} > + > /* Parse a declaration or function definition (C90 6.5, 6.7.1, C99 > 6.7, 6.9.1, C11 6.7, 6.9.1). If FNDEF_OK is true, a function definition > is accepted; otherwise (old-style parameter declarations) only other > @@ -2228,7 +2264,10 @@ c_parser_declaration_or_fndef (c_parser > *parser, bool fndef_ok, > } > else > { > - c_parser_error (parser, "expected %<,%> or %<;%>"); > + if (c_parser_unmatched_p (parser)) > + complain_about_unmatched_token (parser); > + else > + c_parser_error (parser, "expected %<,%> or %<;%>"); > c_parser_skip_to_end_of_block_or_statement (parser); > return; > } > diff --git a/gcc/testsuite/gcc.dg/unmatched.c b/gcc/testsuite/gcc.dg/unmatched.c > new file mode 100644 > index 00000000000..bd458a01109 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/unmatched.c > @@ -0,0 +1,19 @@ > + > +/* { dg-do compile } */ > + > +void f1() { > + int a = 0)+3; /* { dg-error "unmatched" } */ > +} > + > +void f2() { > + int b = (1]+3; /* { dg-error "expected" } */ > +} > + > +void f3() { > + int b = 1]+3; /* { dg-error "unmatched" } */ > +} > + > +void f4() { > + int c = (1))+3; /* { dg-error "unmatched" } */ > +} > + > > Den lör 5 jan. 2019 kl 18:02 skrev Daniel Marjamäki > <daniel.marjamaki@gmail.com>: > > > > Thanks! > > > > I will take care of the indentation and fix the comment. > > > > > I think the indentation warnings should catch that? > > > > I get this: > > > > void f() > > { > > } > > } // <- error: expected identifier or '(' before '}' token > > > > I ran with -Wall -Wextra -pedantic and did not see a indentation > > warning. Am I missing some indentation warning? The error message I > > get is a little misplaced. I think it's fine to warn about that } but > > it could also say in the error message that the problem is probably > > the previous } > > > > > Should this say something like "expected ) or , or ;"? > > > > No none of those suggestions will solve the error. > > > > Look at this code: > > > > int x = 3) + 0; > > > > Writing a ) or , or ; will not fix the syntax error. You have to > > remove the ) or add a ( somewhere. > > > > > > > > Den lör 5 jan. 2019 kl 09:50 skrev Segher Boessenkool > > <segher@kernel.crashing.org>: > > > > > > Hi Daniel, > > > > > > Some mostly boring comments: > > > > > > On Fri, Jan 04, 2019 at 09:25:10PM +0100, Daniel Marjamäki wrote: > > > > The first reason is the hard problem, but maybe we can ignore this now also: > > > > > > > > void f() > > > > { > > > > } // <- looking at the indentation, it seems preferable to warn about this > > > > } > > > > > > I think the indentation warnings should catch that? > > > > > > > diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c > > > > index 972b629c092..eabc5ffa15e 100644 > > > > --- a/gcc/c/c-parser.c > > > > +++ b/gcc/c/c-parser.c > > > > @@ -171,6 +171,8 @@ struct GTY(()) c_parser { > > > > /* How many look-ahead tokens are available (0 - 4, or > > > > more if parsing from pre-lexed tokens). */ > > > > unsigned int tokens_avail; > > > > + /* nesting depth in expression (parentheses / squares) */ > > > > > > Start sentences with a capital, and end with full stop space space. I > > > realise this isn't a full sentence, but the comment right above does this > > > as well ;-) > > > > > > > @@ -763,6 +765,22 @@ c_parser_next_tokens_start_declaration (c_parser *parser) > > > > return false; > > > > } > > > > > > > > +/* Nesting start token */ > > > > + > > > > +static bool c_parser_is_nesting_start (c_parser *parser) > > > > +{ > > > > + return c_parser_next_token_is (parser, CPP_OPEN_PAREN) || > > > > + c_parser_next_token_is (parser, CPP_OPEN_SQUARE); > > > > > > Indents should use tabs for every leading eight spaces. > > > > > > > @@ -2228,7 +2264,10 @@ c_parser_declaration_or_fndef (c_parser > > > > *parser, bool fndef_ok, > > > > } > > > > else > > > > { > > > > - c_parser_error (parser, "expected %<,%> or %<;%>"); > > > > + if (c_parser_unmatched_p (parser)) > > > > + complain_about_unmatched_token (parser); > > > > > > Should this say something like "expected ) or , or ;"? > > > > > > > + else > > > > + c_parser_error (parser, "expected %<,%> or %<;%>"); > > > > c_parser_skip_to_end_of_block_or_statement (parser); > > > > return; > > > > } > > > > > > > > > Segher ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Improve syntax error 2019-01-08 20:29 ` Daniel Marjamäki @ 2019-01-08 20:44 ` Joseph Myers 0 siblings, 0 replies; 7+ messages in thread From: Joseph Myers @ 2019-01-08 20:44 UTC (permalink / raw) To: Daniel Marjamäki; +Cc: gcc [-- Attachment #1: Type: text/plain, Size: 1425 bytes --] On Tue, 8 Jan 2019, Daniel Marjamäki wrote: > Ping New features should be submitted in development stage 1; we're in regression-fixes-only mode at present so new features will not receive attention until probably April or May. When submitting a revised patch, please make sure each patch submission email is fully self-contained, with a full explanation of that version of the patch suitable for a git-style commit message (in addition to any description of changes relative to a previous patch version, which would not go in such a commit message). Rather than picking just one place there might be an unmatched close-parenthesis token, it would seem better to me to make the general logic printing errors for any unexpected token use different wording in the case where the unexpected token is an unmatched parenthesis (and make sure such wording is consistent between C and C++). I.e., it would be c_parser_error_richloc / C++ equivalents that would change. Note that c_parser_error_richloc has existing smarts to give a different diagnostic in the case of version control conflict markers in a source file, which is a strong indication it's the right place for other smarts relating to better diagnostics for particular unexpected tokens. Please note GNU Coding Standards formatting (for example, split lines before binary operators, not after them). -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Improve syntax error 2019-01-05 17:02 ` Daniel Marjamäki 2019-01-05 19:44 ` Daniel Marjamäki @ 2019-01-10 10:39 ` Segher Boessenkool 1 sibling, 0 replies; 7+ messages in thread From: Segher Boessenkool @ 2019-01-10 10:39 UTC (permalink / raw) To: Daniel Marjamäki; +Cc: gcc On Sat, Jan 05, 2019 at 06:02:08PM +0100, Daniel Marjamäki wrote: > > I think the indentation warnings should catch that? > > I get this: > > void f() > { > } > } // <- error: expected identifier or '(' before '}' token > > I ran with -Wall -Wextra -pedantic and did not see a indentation > warning. Am I missing some indentation warning? The error message I > get is a little misplaced. I think it's fine to warn about that } but > it could also say in the error message that the problem is probably > the previous } I opened https://gcc.gnu.org/PR88790 . > > > Should this say something like "expected ) or , or ;"? > > No none of those suggestions will solve the error. > > Look at this code: > > int x = 3) + 0; > > Writing a ) or , or ; will not fix the syntax error. You have to > remove the ) or add a ( somewhere. Yeah, I wasn't quite awake when I wrote that, apparently :-) Segher ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-01-10 10:39 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-01-04 20:25 Improve syntax error Daniel Marjamäki 2019-01-05 8:50 ` Segher Boessenkool 2019-01-05 17:02 ` Daniel Marjamäki 2019-01-05 19:44 ` Daniel Marjamäki 2019-01-08 20:29 ` Daniel Marjamäki 2019-01-08 20:44 ` Joseph Myers 2019-01-10 10:39 ` Segher Boessenkool
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).