Improve syntax error

Tue Jan 8 20:29:00 GMT 2019

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