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
Closed

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

alandonovan opened this issue Jun 27, 2018 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@alandonovan
Copy link
Contributor

alandonovan 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
Copy link
Member

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

@griesemer
Copy link
Contributor

@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
Copy link
Contributor Author

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

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 28, 2018
@bcmills bcmills added this to the Go1.12 milestone Jun 28, 2018
@griesemer
Copy link
Contributor

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.

@golang golang locked and limited conversation to collaborators Oct 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants