Improve syntax error

Improve syntax error

[Daniel Marjamäki]

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

Re: Improve syntax error

[Segher Boessenkool]

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

Re: Improve syntax error

[Daniel Marjamäki]

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

Re: Improve syntax error

[Daniel Marjamäki]

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

Re: Improve syntax error

[Daniel Marjamäki]

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

Re: Improve syntax error

[Joseph Myers]

[-- 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

Re: Improve syntax error

[Segher Boessenkool]

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