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

cmd/compile: incorrect position information when -iexport=false #26085

Closed
alandonovan opened this issue Jun 27, 2018 · 4 comments

Comments

Projects
None yet
4 participants
@alandonovan
Copy link
Contributor

commented Jun 27, 2018

If you patch the -iexport flag to default to false (cmd/compile/internal/gc/main.go:248) and run make.bash, it builds the standard library using old bexport format. The files it produces contain incorrect position information for indirectly referenced packages: every object appears to be defined at the point at which it is imported.

You can observe this using the gcexportdata tool, though you'll need its -package flag (https://go-review.googlesource.com/c/tools/+/121195).

$ go build -o gcexportdata golang.org/x/tools/go/gcexportdata/main.go

First, the expected result:
$ ./gcexportdata ~/goroot/pkg/linux_amd64/go/token.a | grep FileSet.*Iterate
$GOROOT/src/go/token/position.go:392:1: method (*FileSet) Iterate(f func(*File) bool)

But when obtaining go/token.FileSet from the go/ast.a file, the position differs:
$ ./gcexportdata -package go/token ~/goroot/pkg/linux_amd64/go/ast.a | grep FileSet.*Iterate
$GOROOT/src/go/ast/ast.go:11:1: method (*FileSet) Iterate(f func(*File) bool)

One resolution would be to delete bexport (and the copy of it in x/tools).

[Google internal issue b/37534272]

@mdempsky

This comment has been minimized.

Copy link
Member

commented Jun 27, 2018

To be clear, this is an existing issue, not a regression introduced by the refactoring work done for iexport, right?

@griesemer

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2018

@mdempsky I believe that's correct (pre-existing issue). I'm not sure how much work (if any) we should put into this given that we want to deprecate the old export format after 1.11 is out. But perhaps we need to keep supporting the old format due to products that don't move with the Go release cycle.

@alandonovan

This comment has been minimized.

Copy link
Contributor Author

commented Jun 27, 2018

Yes, pre-existing. If it's one-line fix, let's fix it, otherwise we can wait.

@bcmills bcmills added this to the Go1.12 milestone Jun 28, 2018

@griesemer

This comment has been minimized.

Copy link
Contributor

commented Oct 22, 2018

The old binary export format is gone and so is the -export flag. There may be issues with the go/types based binary export format writer (in x/tools), but that would be a separate issue. Closing.

@griesemer griesemer closed this Oct 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.