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

x/exp/typeparams: a new module with a transitional API for tools #50447

Closed
findleyr opened this issue Jan 5, 2022 · 18 comments
Closed

x/exp/typeparams: a new module with a transitional API for tools #50447

findleyr opened this issue Jan 5, 2022 · 18 comments

Comments

@findleyr
Copy link
Contributor

findleyr commented Jan 5, 2022

Background

With Go 1.18, we are adding a large number of new APIs in go/* packages such as go/ast and go/types to represent the new languages features introduced with generics (see proposals #47781 and #47916). Those proposals describe the new types and functions, but do not comprehensively discuss their semantics nor offer guidance on how to update existing tools. Recently, we've updated many tools in x/tools, such as gopls and vet analyzers, and it would be great to share what we learned from that work with the larger community, to help others update their tools.

There were two notably difficult aspects of updating our tools to support generics:

  1. We need to handle the new syntax and types while continuing to build at older Go versions.
  2. For analyzers in particular, we often needed to compute a representation of the structural restrictions on a type parameter's type set. For example, this is necessary in the printf analyzer to report a diagnostic if any members of the type set to not match the corresponding printf verb. go/types does not yet expose an API for this, because at the time we weren't sure what the correct interface for a type set should be.

Our solutions to both of those problems are contained in the x/tools/internal/typeparams package. This package exposes a transitional API, which at Go 1.18 is just a proxy for the corresponding go/* APIs, and at earlier Go versions is a stub. For example, it exports a typeparams.Union type, which is an alias for go/types.Union at Go 1.18 but a placeholder at Go 1.17. Furthermore the typeparams.StructuralTerms function computes the 'structural' terms of a type parameter, i.e. the normalized list of structural restrictions, after reducing unions and intersections.

Proposal

I propose that we to add a new module to x/exp, golang.org/x/exp/typeparams, which contains the functionality currently exposed by x/tools/internal/typeparams (modulo improved documentation and other superficial improvements). By being publicly importable, this module would provide the same functionality to other tools as x/tools/internal/typeparams has provided x/tools. Furthermore, this module could serve as a home (or entrypoint) for more detailed documentation on how to update tools to support generics. As an initial pass, it could include guidance in its package documentation or in an associated README.

I considered instead suggesting that tools simply copy x/tools/internal/typeparams, but particularly with the typeparams.StructuralTerms API it is possible that we will release bug-fixes, so we should make updating easy.

As for why a new module, and why x/exp, consider the following:

  • This package will be small and have no dependencies.
  • At some point we may want to tag a final version and delete it. This could possibly occur upon the release of Go 1.21 (two versions after 1.19, which will probably also contain API additions).

Putting this code in a new module in x/exp identifies it as experimental/transient, and means that we won't break builds depending on x/tools for other reasons, if/when it is deleted.

Caveats

If we add x/exp/typeparams, it makes sense to use it to replace the existing usage of x/tools/internal/typeparams. In that case, x/exp/typeparams would not only be required by x/tools, it would be vendored into cmd by way of cmd/vet. It might not be a good precedent to include code from x/exp in the go distribution, though I'm not aware of a practical problem with this. We could alternatively create a new submodule of x/tools. Since this is disallowed, we'll have to keep using x/tools/internal/typeparams, and sync it with x/exp via a script.

Aside: suggestions for improved APIs are certainly welcome, but I'd like to keep the initial focus of the discussion here about whether or not to create such a module, and where to put it. If this proposal is accepted, we can shift discussion to the details.

@gopherbot gopherbot added this to the Proposal milestone Jan 5, 2022
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Jan 5, 2022
@rsc
Copy link
Contributor

rsc commented Jan 5, 2022

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc moved this from Incoming to Active in Proposals (old) Jan 5, 2022
@timothy-king
Copy link
Contributor

It may be worth emphasizing that without this package [or something similar] it is challenging to develop tools that deal with the types package that can be compiled with pre-Go 1.18 compilers and also supports generics.

If we add x/exp/typeparams, it makes sense to use it to replace the existing usage of x/tools/internal/typeparams. In that case, x/exp/typeparams would not only be required by x/tools, it would be vendored into cmd by way of cmd/vet.

A way to avoid the vendoring from x/exp into cmd is to have another vendored copy in something like x/tools. Which x/tools/internal/typeparams already is. Maintaining both x/tools/internal/typeparams and a x/exp/typeparams is clearly not ideal, but neither is vendoring x/exp into cmd.

Furthermore the typeparams.StructuralTerms function computes the 'structural' terms of a type parameter, i.e. the normalized list of structural restrictions, after reducing unions and intersections.

We may need a permanent home for the hard to implement utilities like this. x/tools/go/types comes to mind. Might be good to resolve this now? Not sure if this is feasible now though.

@rsc
Copy link
Contributor

rsc commented Jan 5, 2022

Indeed. Right now tip.golang.org/pkg/constraints/?m=old is failing for precisely this reason.
(It is built against Go 1.16 because that's what App Engine provides.)

@rsc
Copy link
Contributor

rsc commented Jan 5, 2022

It is disallowed to have any x repo, nor the main repo, depend on x/exp.
Please leave things as they are and just make a copy in x/exp.
It's fine if there is a script to update the x/exp copy.

@findleyr
Copy link
Contributor Author

findleyr commented Jan 5, 2022

It is disallowed to have any x repo, nor the main repo, depend on x/exp.
Please leave things as they are and just make a copy in x/exp.
It's fine if there is a script to update the x/exp copy.

SGTM. It is better for us to maintain two copies than have every tool to maintain its own copy...

@findleyr
Copy link
Contributor Author

findleyr commented Jan 5, 2022

@timothy-king

We may need a permanent home for the hard to implement utilities like this. x/tools/go/types comes to mind. Might be good to resolve this now? Not sure if this is feasible now though.

I think this should eventually be a part of go/types itself, perhaps in 1.19. We just didn't want to commit to an API before we understood the subtleties and applications of type sets (and FWIW I'm still glad we made this decision).

@gopherbot
Copy link

Change https://golang.org/cl/377834 mentions this issue: internal/typeparams/example: start adding a guide to generics for tools

@mvdan
Copy link
Member

mvdan commented Jan 12, 2022

Out of curiosity, would this be a v0 or a v1+ module? "tag a final version and delete it" makes me think that we may not need to reach a v1.0.0.

@rsc
Copy link
Contributor

rsc commented Jan 12, 2022

We have no v1 in x repos yet (working on it) so I don't think we'd start with this one.

@rsc rsc moved this from Active to Likely Accept in Proposals (old) Jan 12, 2022
@rsc
Copy link
Contributor

rsc commented Jan 12, 2022

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc rsc moved this from Likely Accept to Accepted in Proposals (old) Jan 19, 2022
@rsc
Copy link
Contributor

rsc commented Jan 19, 2022

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: x/exp/typeparams: a new module with a transitional API for tools x/exp/typeparams: a new module with a transitional API for tools Jan 19, 2022
@rsc rsc modified the milestones: Proposal, Backlog Jan 19, 2022
gopherbot pushed a commit to golang/tools that referenced this issue Jan 22, 2022
This CL begins adding a guide for the new APIs introduced with Go 1.18
to support writing tools that understand generic Go code.

For now I've added a summary of the new APIs, an initial example, and
some discussion of the typeparams package. Subsequent CLs will add more
examples, and polish.

Updates golang/go#50447

Change-Id: I4ed8d7a2f43e748374d14f3f515673d69ab2d5a0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/377834
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Trust: Dominik Honnef <dominik@honnef.co>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/380554 mentions this issue: internal/typeparams: add a helper to return the origin method

gopherbot pushed a commit to golang/tools that referenced this issue Feb 3, 2022
With instantiated types, method objects are no longer unique: they may
be instantiations of methods with generic receiver. However, some
use-cases require finding the canonical method representing the method
in the source. For these use-cases, provide an OriginMethod helper.

For golang/go#50447

Change-Id: I6f8af3fb5c5eeefb11f8f3bdba54cd6692ca389f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/380554
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Tim King <taking@google.com>
@findleyr
Copy link
Contributor Author

findleyr commented Feb 4, 2022

This was blocked by beta2 and some outstanding concerns about predicates on generic types (#50887), with both of those hopefully resolved, I'll work on implementing this tomorrow. I'll put together a CL for the new module, and leave it open for at least a week to invite comments.

@gopherbot
Copy link

Change https://golang.org/cl/383375 mentions this issue: typeparams: a new module with a transitional API for generics in tools

@findleyr
Copy link
Contributor Author

findleyr commented Feb 9, 2022

Hi! Please add comments on https://golang.org/cl/383375 if you have opinions on the API for this new module. Even naming suggestions are appreciated.

I am pretty content with the current API, though one likely upcoming change is the name StructuralTerms: the word structural is being revisited in the spec this week, and we'll likely want to align with whatever terminology emerges there.

gopherbot pushed a commit to golang/exp that referenced this issue Feb 15, 2022
Add a new module for use by tool authors that need operate on generic
code, while still building at older Go versions.

This module API proxies the Go 1.18 API for type parameters, providing
stubs at earlier Go versions. It also contains some common helpers to
supplement the go/types API.

For golang/go#50447

Change-Id: I97feb834eb2da646620f8377904520b511871915
Reviewed-on: https://go-review.googlesource.com/c/exp/+/383375
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Trust: Dominik Honnef <dominik@honnef.co>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@findleyr
Copy link
Contributor Author

I've submitted the initial CL for this module: https://pkg.go.dev/golang.org/x/exp/typeparams

I didn't get much feedback on the CL, so decided to add a caveat that the API is still experimental. Please try to use it in your projects, and let me know if you have opinions. I will remove the caveat in around a week, at which point we will not break the API (though we may still deprecate things).

Meanwhile, I am lagging behind on documentation, unfortunately. With 1.18 issues more or less resolved at this point, it is my highest priority.

@gopherbot
Copy link

Change https://go.dev/cl/388274 mentions this issue: exp/typeparams: add a guide to the new APIs

gopherbot pushed a commit to golang/exp that referenced this issue Mar 3, 2022
Add a rough draft of a guide to the new go/ast and go/types APIs
introduced with Go 1.18, including explanation of how the
x/exp/typeparams package helps bridge gaps.

For golang/go#50447

Change-Id: I2a5f9a5f0801a71466a4faa3e42efca75b3c7d3c
Reviewed-on: https://go-review.googlesource.com/c/exp/+/388274
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
@gopherbot
Copy link

Change https://go.dev/cl/391674 mentions this issue: exp/typeparams: remove caveat around usage

@golang golang locked and limited conversation to collaborators Mar 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

No branches or pull requests

6 participants
@rsc @timothy-king @mvdan @gopherbot @findleyr and others