From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 130753 invoked by alias); 8 Jan 2019 20:29:14 -0000 Mailing-List: contact gcc-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-owner@gcc.gnu.org Received: (qmail 130744 invoked by uid 89); 8 Jan 2019 20:29:13 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,MISSING_HEADERS,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=consume, Consume, recovered, segher@kernel.crashing.org X-HELO: mail-io1-f54.google.com Received: from mail-io1-f54.google.com (HELO mail-io1-f54.google.com) (209.85.166.54) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 08 Jan 2019 20:29:09 +0000 Received: by mail-io1-f54.google.com with SMTP id b23so4199062ios.10 for ; Tue, 08 Jan 2019 12:29:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:cc :content-transfer-encoding; bh=qmr+Ige4UJBj1TeynnjHN4DWGcd74B6BpSl732TPhXk=; b=AhMMVnCPt7wx3XU96osKBkr8jnavQy71CopXLM9hqGogGnKudTwftJLVnE/xmP6Qhq kLmpHvibxNxhdAU6IHw4unh/Tmg18oWR6n/sO/y7T9P9l/u2B+i1EKLr6LDrP637FZe/ JlymOMqGGQsEcviKaCMhCEEhx5fjqm9Zpv4Xd7uxIF4i3Blmj/QBGFKpxLjf7ZPalRjf N+yCc5lBO/MpNeEKSOZpbi0CFsmanJSVQ7a9WslbUEYaOyZ9+YYPHWn1KHl/hmZINgyn NPXsYDh/hSFgdyBTFnIC5APnLyX4E9WOE5L29LJyYiZDpXI237GPRYgalEbgo6kj7EPO oJ6w== MIME-Version: 1.0 References: <20190105085023.GJ14180@gate.crashing.org> In-Reply-To: From: =?UTF-8?Q?Daniel_Marjam=C3=A4ki?= Date: Tue, 08 Jan 2019 20:29:00 -0000 Message-ID: Subject: Re: Improve syntax error Cc: gcc@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2019-01/txt/msg00036.txt.bz2 Ping Den l=C3=B6r 5 jan. 2019 kl 20:44 skrev Daniel Marjam=C3=A4ki : > > 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 *pa= rser) > 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 !=3D CPP_EOF); > gcc_assert (!parser->in_pragma || parser->tokens[0].type !=3D CPP_PRAG= MA_EOL); > gcc_assert (parser->error || parser->tokens[0].type !=3D 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 =3D parser->tokens[0].location; > if (parser->tokens !=3D &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 =3D=3D 0; > + return false; > +} > + > +static void complain_about_unmatched_token (c_parser *parser) > +{ > + c_token *token =3D 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 definit= ion > 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/unma= tched.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 =3D 0)+3; /* { dg-error "unmatched" } */ > +} > + > +void f2() { > + int b =3D (1]+3; /* { dg-error "expected" } */ > +} > + > +void f3() { > + int b =3D 1]+3; /* { dg-error "unmatched" } */ > +} > + > +void f4() { > + int c =3D (1))+3; /* { dg-error "unmatched" } */ > +} > + > > Den l=C3=B6r 5 jan. 2019 kl 18:02 skrev Daniel Marjam=C3=A4ki > : > > > > 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 =3D 3) + 0; > > > > Writing a ) or , or ; will not fix the syntax error. You have to > > remove the ) or add a ( somewhere. > > > > > > > > Den l=C3=B6r 5 jan. 2019 kl 09:50 skrev Segher Boessenkool > > : > > > > > > Hi Daniel, > > > > > > Some mostly boring comments: > > > > > > On Fri, Jan 04, 2019 at 09:25:10PM +0100, Daniel Marjam=C3=A4ki 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 war= n 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_pars= er *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