Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go/parser: more robust error handling #11377

Closed
rsc opened this issue Jun 24, 2015 · 3 comments
Closed

go/parser: more robust error handling #11377

rsc opened this issue Jun 24, 2015 · 3 comments
Assignees
Milestone

Comments

@rsc
Copy link
Contributor

@rsc rsc commented Jun 24, 2015

I'm going to try to start filing bugs against go/parser when I notice things that trigger less than ideal error messages in the course of ordinary usage (via goimports, go test, and so on), because I think there are some simple cases that can be improved. I expect most of the initial ones will be about getting too many errors. Here's the first one.

package p

func bad() {
    if f()) {
        return
    }
}

func F1() {}
func F2() {}
func F3() {}
func F4() {}
func F5() {}
func F6() {}
func F7() {}
func F8() {}
func F9() {}
func F10() {}

This produces:

$ gofmt /tmp/xx.go
/tmp/xx.go:4:8: expected '{', found ')'
/tmp/xx.go:9:6: expected '(', found 'IDENT' F1
/tmp/xx.go:10:6: expected '(', found 'IDENT' F2
/tmp/xx.go:11:6: expected '(', found 'IDENT' F3
/tmp/xx.go:12:6: expected '(', found 'IDENT' F4
/tmp/xx.go:13:6: expected '(', found 'IDENT' F5
/tmp/xx.go:14:6: expected '(', found 'IDENT' F6
/tmp/xx.go:15:6: expected '(', found 'IDENT' F7
/tmp/xx.go:16:6: expected '(', found 'IDENT' F8
/tmp/xx.go:17:6: expected '(', found 'IDENT' F9
/tmp/xx.go:18:6: expected '(', found 'IDENT' F10
$ 

This is a reduced version of a real case I had, where an extra paren in a function near the top of a long source file triggered many many messages. In this program, the braces are still balanced, so there should be a good signal for the parser to recover by the end of the function (and ideally the end of the enclosing block, although here they are the same).

This is a case where the yacc grammar's error recovery is actually doing better:

$ go tool compile /tmp/xx.go
/tmp/xx.go:4: syntax error: unexpected ), expecting {
/tmp/xx.go:7: syntax error: unexpected }
$

Also the IDENT in the go/parser error is probably not helpful to the user. It's an internal detail.

@griesemer
Copy link
Contributor

@griesemer griesemer commented Jan 11, 2018

FWIW, the new syntax parser and thus cmd/compile produce exactly one error in this case:

x.go:6:11: syntax error: unexpected ), expecting semicolon or newline
@gopherbot
Copy link

@gopherbot gopherbot commented Jan 11, 2018

Change https://golang.org/cl/87495 mentions this issue: go/parser: more robust error handling for 'if' headers

@griesemer griesemer modified the milestones: Unplanned, Go1.11 Jan 12, 2018
@gopherbot
Copy link

@gopherbot gopherbot commented Jan 17, 2018

Change https://golang.org/cl/87900 mentions this issue: go/parser: improved error message for unexpected literals

@gopherbot gopherbot closed this in c5f3a8b Feb 12, 2018
gopherbot pushed a commit that referenced this issue Feb 12, 2018
R=go1.11

This is a follow up for #11377 which reported that an error like

/tmp/xx.go:9:6: expected '(', found 'IDENT' F1

shouldn't print 'IDENT', as it's just an internal detail.
The relevant change wasn't made in the original fix, so here it is.

For #11377.

Change-Id: Ib76957d86b88e3e63646fbe4abf03a3b9d045139
Reviewed-on: https://go-review.googlesource.com/87900
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
@golang golang locked and limited conversation to collaborators Feb 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.