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

proposal: go/types: add Import, Export functions #69491

Open
adonovan opened this issue Sep 16, 2024 · 4 comments
Open

proposal: go/types: add Import, Export functions #69491

adonovan opened this issue Sep 16, 2024 · 4 comments
Labels
Milestone

Comments

@adonovan
Copy link
Member

adonovan commented Sep 16, 2024

[Status: WORK IN PROGRESS. Do not review yet.]

Background: The package golang.org/x/tools/go/gcexportdata provides two functions, Write and Read. As described in #68898, they serve two purposes:

  1. a serialization mechanism for types.Package values. They assume that writer and reader are compiled from the same source code; there is no protocol stability.
  2. a mechanism to read export data written by last "tip + 2" releases of the Go compiler.

These two purposes are quite independent, and the actual protocols they use are unrelated. (The first uses "indexed" form, the second "unified".) Much of the challenge of maintaining go/types and x/tools/go/gcexportdata comes from version skew between std and x/tools: witness the number of delicate changes to both made recently to support materialized Alias types, and then the addition of type parameters to aliases. In this instance, the skew is unnecessary as the codec can be tightly coupled go/types and needn't support any other versions.

Proposal: We propose to add the read and write functions to go/types itself:

package types // "go/types"

// Export returns a serialized form of pkg that can be read by the same version of [Import].
// The encoding is not specified and may change at any time.
func Export(fset *token.FileSet, pkg *Package, opts ExportOptions) ([]byte, error)

type ExportOptions struct {
    Deep bool // encode information about dependencies too
}

// Import returns the package described by data,
// which must have been produced by the same version of [Export].
// It calls importDeps at most once to obtain all needed direct dependencies (perhaps in parallel).
func Import(fset *token.FileSet, data []byte, importDeps importDepsFunc, path string, opts ImportOptions) (*Package, error)

type ImportOptions struct { /* for future use */ }

type importDepsFunc = func(packagePaths []string) ([]*types.Package, error)

This API is a synthesis of gcexportdata and internal/gcimporter.{Im,Ex}portShallow, which are used by gopls for "shallow" encodings.

Open questions:

  • The implementation of Export and Import depend on objectpath. How would this work in go/types?
  • Assuming the legacy uses of Read for files produced by Write (not cmd/compile) would be supported by (eventually) delegating to types.Import, that would break the implicit promise that newer versions of Read can consume files produced by older versions of Write, which is not something that types.Import,Export would guarantee. How disruptive is that likely to be? I suspect that no existing tools rely on this implicit promise because we often temporarily break it during the course of normal development (e.g. adding generics, type aliases) over many CLs.

@griesemer @findleyr @timothy-king

@znkr (for shallow export data)

@gopherbot gopherbot added this to the Proposal milestone Sep 16, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/623638 mentions this issue: go/gcexportdata: document 2 releases + tip policy

@findleyr
Copy link
Member

findleyr commented Nov 4, 2024

See also #52163.

The export data format has a versioning header precisely for this reason. Presumably if the version has not changed, go/types.Import should be able to read the export data produced by an older version of go/types. On the other hand, if the version changes, we'll need to snapshot the logic of the older version in x/tools. Therefore, x/tools will have already interpretted the version header before it calls into go/types. Is you expectation that the bytes passed to go/types will be stripped of this header?

gopherbot pushed a commit to golang/tools that referenced this issue Nov 4, 2024
And update the package documentation.

Fixes golang/go#68898
Updates golang/go#69491

Change-Id: Ib01ef9ca4dfa204410c7b3e8d9439ddfee2d76f5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/623638
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
@adonovan
Copy link
Member Author

adonovan commented Nov 4, 2024

Is your expectation that the bytes passed to go/types will be stripped of this header?

No, for better or worse the existing gcexportdata.Read function must continue to support files produced both by the compiler (2 releases + tip) and by gcexportdata.Write (which will become a wrapper around types.Export), so we would still need some way to detect the format. I propose that we pick a new version byte (in addition to [vcdiu]) that means "the private version namespace of types.{Export.Import}", and use this to dispatch to Import. Import and Export can then do whatever they want with the payload, including having a second version field that we bump as often as we like (e.g. on every change), so that we can report the error "Import called on a file produced by a different source version of Export".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Hold
Development

No branches or pull requests

4 participants