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: analysis crash analyzing new uses of go1.23 range iterators #67237

Closed
adonovan opened this issue May 7, 2024 · 4 comments
Closed
Assignees
Labels
gopls/analysis Issues related to running analysis in gopls
Milestone

Comments

@adonovan
Copy link
Member

adonovan commented May 7, 2024

@ianlancetaylor reports that gopls crashes in the buildir analyzer (which is staticcheck's fork of go/ssa and buildssa) when it encounters a range-over-func statement in new code he is trying to merge the to the std slices package. The ir package doesn't yet support range-over-func. Nor does go/ssa (but see https://go.dev/cl/555075).

As a matter of priority, we need to disable ssa- and ir-based analyzers on packages that use range-over-func. Then we need to fix these packages before go1.23.

@adonovan adonovan self-assigned this May 7, 2024
@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 May 7, 2024
@adonovan adonovan added gopls/analysis Issues related to running analysis in gopls and removed gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. labels May 7, 2024
@gopherbot gopherbot added this to the Unreleased milestone May 7, 2024
@adonovan adonovan modified the milestones: Unreleased, Go1.23, gopls/v0.16.0 May 7, 2024
@dominikh
Copy link
Member

dominikh commented May 7, 2024

Relevant Staticcheck issue: dominikh/go-tools#1494

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/583778 mentions this issue: gopls/internal/analysis: disable ssa/ir analyzers on range-over-func

gopherbot pushed a commit to golang/tools that referenced this issue May 8, 2024
This change disables analyzers that cannot yet safely process
go1.23 range-over-func statements, including buildssa and buildir.
(This is done by poking in an additional Analyzer.Requires edge
on a new temporary analyzer that fails when it sees a range-over-func.)

We plan to revert this change when ssa and ir support the new feature,
but this CL will unblock uses of it in the standard library which
would otherwise cause gopls' tests to crash.

Updates golang/go#67237
Updates dominikh/go-tools#1494

Change-Id: Ibed2a88da94fb84234b4410b6bc7562a493287ce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/583778
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
gopherbot pushed a commit to golang/tools that referenced this issue May 8, 2024
This is a short-term stopgap to stop SSA-based tools from
panicking in the SSA builder when it encounters a go1.23
range-over-func statement. A principled fix will follow
soon in CL 555075.

Updates golang/go#67237

Change-Id: I26f3b4ac15f8ae24aa5e93a188a6e8b70acc43f3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/584295
Reviewed-by: Tim King <taking@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/584295 mentions this issue: go/ssa: compile range-over-func to panic

@adonovan
Copy link
Member Author

This was fixed by the two CLs above. Reenabling the suppressed features is tracked by #67262.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls/analysis Issues related to running analysis in gopls
Projects
None yet
Development

No branches or pull requests

3 participants