x/tools/go/gcexportdata: support export using indexed format

[alandonovan]

We need to port the reader of indexed import data ($GOROOT/src/cmd/compile/internal/gc/iexport.go) to produce go/types data structures, analogous to what we do for the $$B format, which is deprecated.

@stamblerre

[stamblerre]

It seems like gcexportdata.Read is already reading indexed export data as of golang/tools@9a70f1f.

@alandonovan or @griesemer: is there anything else that needs to be done here?

[alandonovan]

I corrected the issue title to mean "write", not "read".

[stamblerre]

Thanks - I'll start working on this now.

[gopherbot]

Change https://golang.org/cl/156901 mentions this issue: go/internal/gcimporter: write export data for go/types

[griesemer]

@stamblerre Is this finished, i.e., can we close this issue?

[stamblerre]

I think the final step here would be to have the gcexportdata.Write function actually write the indexed export data format (https://github.com/golang/tools/blob/master/go/gcexportdata/gcexportdata.go#L102). I will send out a CL to that effect ASAP.

[gopherbot]

Change https://golang.org/cl/198742 mentions this issue: go/gcexportdata: use IExportData when writing gcexportdata

[jba]

https://golang.org/cl/198742 introduced a compatible change, but a breaking one for some programs. It used to be that Read returned the same package that was written if path was garbage. Now it returns something else. For example, this program (which the playground won't currently run) writes the export data for "cloud.google.com/go/bigquery", then reads it back in with path set to nonsense (the filename). It prints "bufio".

I think Read should return an error if it can't find path in the export data, or at least it should return the equivalent of imports[path]. That is also a compatible breaking change, but one that would have caught my bug much earlier.

[stamblerre]

@jba: I just mailed CL 201097, with which you should be able to import the package with the filename again. Does it fix the problem for you?

[gopherbot]

Change https://golang.org/cl/201097 mentions this issue: go/internal/gcimporter: use empty string for the top-level package path

[jba]

I worked around it a different way. (I saved the path with the export data.)

It sounds like you're keeping the previous behavior, where Read returns the written package for certain values of path. That is useful behavior, but it doesn't match the spec of Read.

Should we update Read's doc to describe it? Maybe "If path is empty, Read returns the same package that was passed to Write"? In which case I still think you should return an error if path is neither empty nor a path found in the file.

[stamblerre]

Ended up adding documentation to IExportData (https://go-review.googlesource.com/c/tools/+/201097/6/go/internal/gcimporter/iexport.go), since it's really a behavior of the export data writer. The data that gets read should always have the import path that is provided by the caller, which is the way it worked in the past. It was my misunderstanding that changed the behavior, though it was never documented that the top-level package's path should be dropped. I think adding that documentation should be sufficient.