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/tools/gopls: handle go1.22's go/types.Alias #64581

Open
adonovan opened this issue Dec 6, 2023 · 7 comments
Open

x/tools/gopls: handle go1.22's go/types.Alias #64581

adonovan opened this issue Dec 6, 2023 · 7 comments
Assignees
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@adonovan
Copy link
Member

adonovan commented Dec 6, 2023

This issue tracks the adoption in gopls of go1.22's new types.Alias type. We will need to audit every place that type-switches or type-asserts a types.Type to ensure that it works correctly with the new type. Initially the type checker will not produce instances of these types unless GODEBUG=gotypesalias=1 is set.

Things to resolve:

Related:

@gri @findleyr

@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Dec 6, 2023
@gopherbot gopherbot added this to the Unreleased milestone Dec 6, 2023
@findleyr findleyr modified the milestones: Unreleased, gopls/v0.16.0 Dec 6, 2023
@gopherbot
Copy link

Change https://go.dev/cl/547876 mentions this issue: gopls: updates for go1.22 types.Alias

@adonovan
Copy link
Member Author

adonovan commented Dec 7, 2023

I tested gopls using tip GOROOT after hardwiring types.Checker.enablealias to true. (We can't use GODEBUG=gotypesalias=1 to control it yet because this environment variable would be inherited by 'go list' and the Go compiler, which doesn't yet have support for writing out Alias types, so it crashes.)

I added rudimentary support to the gcimporter package used by gopls for reading and writing aliases, and sprinkled lots of Unalias operations throughout the tree, and was able to get all but three tests (of vulncheck and staticheck) to pass. So I think it's in good shape for the 1.22 release, though we have a lot of work to do after the release.

@gopherbot
Copy link

Change https://go.dev/cl/574717 mentions this issue: internal/gcimporter: support materialized aliases

@gopherbot
Copy link

Change https://go.dev/cl/574737 mentions this issue: go/types: changes to support materialized aliases

gopherbot pushed a commit to golang/tools that referenced this issue Apr 2, 2024
This CL adds support for preserving materialized aliases
while types transit export data (indexed or unified).

(This CL depends on CL 574737 to the compiler and
go/types.)

Updates golang/go#65294
Updates golang/go#64581

Change-Id: I1a0a08357e4f6a480ba6250fbea327922e455873
Reviewed-on: https://go-review.googlesource.com/c/tools/+/574717
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopherbot pushed a commit that referenced this issue Apr 5, 2024
This CL changes the compiler's type import/export logic
to create and preserve materialized Alias types
when GODEBUG=gotypesaliases=1.

In conjunction with CL 574717, it allows the x/tools
tests to pass with GODEBUG=gotypesaliases=1.

Updates #65294
Updates #64581
Fixes #66550

Change-Id: I70b9279f4e0ae7a1f95ad153c4e6909a878915a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/574737
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/577576 mentions this issue: internal/apidiff: support materialized aliases

@adonovan adonovan self-assigned this Apr 12, 2024
@gopherbot
Copy link

Change https://go.dev/cl/579756 mentions this issue: gopls/internal/test/integration/misc: reenable staticcheck test

gopherbot pushed a commit to golang/tools that referenced this issue Apr 19, 2024
This test reenables the staticcheck test, which now passes
with gotypesalias={,0,1}. (Staticcheck may yet need work, but
that isn't evident to our tests.)

Updates golang/go#64581

Change-Id: I1c8acca2c6b7d16e551a5c8a68be564254713d0d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/579756
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/580455 mentions this issue: gopls: cleanups related to gotypesalias

gopherbot pushed a commit to golang/tools that referenced this issue Apr 23, 2024
The predicate that determines whether the type checker
creates types.Alias nodes is complex: it depends on an
environment variable (GODEBUG) that is somewhat tricky
to parse correctly, since it is a rightmost-wins list.

Critically, however, its default value is a function of
the go directive in the application's go.mod file, which
is inaccessible to logic in x/tools (per-file build tags
can detect the toolchain version, but not the go.mod version).
Equally critically, the current effective value of the
gotypesalias variable changes when os.Setenv(GODEBUG) is
called, which happens in tests, and there is no way to
detect those events; therefore any attempt to cache the
value is wrong.

This change exposes a simplified version of the Enabled
function from aliases, which always computes the ground
truth by invoking the type checker, regardless of cost.
It also adds an 'enabled' parameter to NewAlias so that
callers can amortize this cost.

Also, various minor cleanups related to aliases.

Updates golang/go#64581

Change-Id: If926edefb8e1c1f63c17e4fad0a808e27bac6d5b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/580455
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

3 participants