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: misleading error location #7458

Closed
bradfitz opened this issue Mar 4, 2014 · 8 comments
Closed

go/parser: misleading error location #7458

bradfitz opened this issue Mar 4, 2014 · 8 comments
Assignees
Milestone

Comments

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Mar 4, 2014

This is a simplified example of something that just wasted too much of my debugging
time, tracking down a typo of mine.

http://play.golang.org/p/H2DW718LM5

package main

func main{
    go func() {
        x(fn func() error {
            return nil
        })
    }()
}

In that example, the call to x should just be x(func() error { ... }), not bogusly
trying to name the function literal passed to x.

But instead of showing an error on that line (line 5), it instead says:

$ gofmt x.go
x.go:3:10: expected '(', found '{'
x.go:4:2: expected type, found 'go'
x.go:5:4: expected ')', found '('
x.go:6:4: expected declaration, found 'return'

I would expect the first error to be about line 5, ideally.

In this example it's bad but not terrible, but in the larger program the error messages
actively misled my debugging.
@griesemer
Copy link
Contributor

@griesemer griesemer commented Mar 4, 2014

Comment 1:

The first error is that you are missing the ()'s of the main function, which is properly
messaged at the right place. Once you fix this, the next error is correctly on line 5 as
desired.
Please comment (object) or close as invalid.

Status changed to WaitingForReply.

@bradfitz
Copy link
Contributor Author

@bradfitz bradfitz commented Mar 4, 2014

Comment 2:

Gah, can't type. :/
Sorry, more like this: http://play.golang.org/p/oQdKp7KQ1P
I would expect the gofmt error on line 52, not 51 (the go call)

Status changed to New.

@griesemer
Copy link
Contributor

@griesemer griesemer commented Mar 4, 2014

Comment 3:

I'm getting an import error suggesting that syntactically this is correct... - please
update once more :-)
@griesemer
Copy link
Contributor

@griesemer griesemer commented Mar 4, 2014

Comment 4:

Never mind, I see the problem.
@griesemer
Copy link
Contributor

@griesemer griesemer commented Mar 4, 2014

Comment 5:

Even after commenting out the imports: http://play.golang.org/p/nkpjCROcTK
I get the error message on line 52 as desired. It could be better perhaps. Is that what
you are complaining about?

Status changed to WaitingForReply.

@bradfitz
Copy link
Contributor Author

@bradfitz bradfitz commented Mar 4, 2014

Comment 6:

You don't have to hit "RUN" in the playground.  Just hit "Format" or run gofmt.
I expect the error message to be about the line containing BOGUS_NAME_HERE, not the line
before it.

Status changed to New.

@griesemer
Copy link
Contributor

@griesemer griesemer commented Mar 4, 2014

Comment 7:

One problem is that the playground (format) only reports the first error here. If
gofmt'ed explicitly, the error messages are:
x.go:51:6: expected function/method call
x.go:52:70: expected ')', found 'func'
x.go:62:5: expected statement, found ')'
x.go:72:3: expected '}', found 'EOF'
The 2nd error is what you'd expect (more or less). Error messages are reported in sorted
order, so even though the 2nd error is actually the first error encountered during
parsing, the error on line 51 comes first (and is caused by the 2nd and its follow-up
errors).
The parser could be smarter about what to report (if anything) for line 51 in this case.

Labels changed: added release-go1.3maybe.

Status changed to Accepted.

@griesemer
Copy link
Contributor

@griesemer griesemer commented Mar 4, 2014

Comment 8:

This issue was closed by revision 1624c73.

Status changed to Fixed.

@bradfitz bradfitz added fixed labels Mar 4, 2014
@rsc rsc added this to the Go1.3 milestone Apr 14, 2015
@rsc rsc removed the release-go1.3maybe label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
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
4 participants
You can’t perform that action at this time.