code review 4803047: go/parser: report illegal label declarations at ':' rat...

go/parser: report illegal label declarations at ':' rather than guessing the start

Also:
- Add parser.SpuriousError flag. If set, the parser reports all (including
  spurious) errors rather then at most one error per line.
- Add -e flag to gofmt and gotype: If set, gofmt and gotype report all
  (including spurious) errors rather than at most one error per line.
- Updated the respective documentation.

Fixes issue 2088.

[gri, 2011-07-21 20:30:56]

Hello rsc@golang.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[rsc, 2011-07-22 02:00:33]

The label fix looks fine.

The -e flag seems not useful.
Most users won't even know it exists, unless
you want the meme to propagate that
'if gofmt errors don't make sense, use gofmt -e'.
I used gofmt as an example.  This comes up
in any tool that uses go/parser; when it happened
to me it was gotest.  Goinstall will have the same
issue.  I don't think every tool should have -e.

Instead of -e it seems worth the effort to just
do the right thing.  Of course that requires
figuring out what the right thing is, which is
probably non-trivial, but it will help many more
users.  How can we make this just work?

Russ

[rsc, 2011-07-22 02:30:19]

LGTM

Just read your comments on issue 2088.
If the % error comes out first then the 1-per-line
is still okay, and as long as -e is only for
debugging, lgtm.

One of the nicer things that yacc does is let
you write a rule like

stmt: error ';'

meaning that if you find a syntax error while
trying to parse a statement, after printing the
error, don't immediately try to keep parsing.
Instead, just eat tokens until you get to a ';'
and then use that combination - whatever led
to the syntax error plus the discarded tokens
plus the ';' - as a stmt and keep going.
In practice it means that once you get a syntax
error you don't get another one until a ;.
I wonder if a heuristic like that would work
better than suppressing all but the first thing
printed on each line.

Russ

[gri, 2011-07-22 16:46:48]

On Thu, Jul 21, 2011 at 7:30 PM, Russ Cox <rsc@golang.org> wrote:
> LGTM
>
> Just read your comments on issue 2088.
> If the % error comes out first then the 1-per-line
> is still okay, and as long as -e is only for
> debugging, lgtm.

The idea is definitively for the % error to come first, which it now
does. The -e flag is similar to the -e flag for 6g. It should not be
needed in general.

>
> One of the nicer things that yacc does is let
> you write a rule like
>
> stmt: error ';'
>
> meaning that if you find a syntax error while
> trying to parse a statement, after printing the
> error, don't immediately try to keep parsing.
> Instead, just eat tokens until you get to a ';'
> and then use that combination - whatever led
> to the syntax error plus the discarded tokens
> plus the ';' - as a stmt and keep going.
> In practice it means that once you get a syntax
> error you don't get another one until a ;.
> I wonder if a heuristic like that would work
> better than suppressing all but the first thing
> printed on each line.

One can something along the same lines here as well. N. Wirth
describes a similar heuristic for recursive descent parsers where one
has a list of "stop symbols" to which to read after a syntax error,
for more graceful recovery. In Go this is particularly easy because
all statements end in ";". I just took a cheap route for now (one
error per line) as it works reasonably well. I'll play with it some
time.

- Robert


>
> Russ
>

[gri, 2011-07-22 16:55:42]

*** Submitted as http://code.google.com/p/go/source/detail?r=e6713d09fa34 ***

go/parser: report illegal label declarations at ':' rather than guessing the
start

Also:
- Add parser.SpuriousError flag. If set, the parser reports all (including
  spurious) errors rather then at most one error per line.
- Add -e flag to gofmt and gotype: If set, gofmt and gotype report all
  (including spurious) errors rather than at most one error per line.
- Updated the respective documentation.

Fixes issue 2088.

R=rsc
CC=golang-dev
http://codereview.appspot.com/4803047