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

[generics] gopls crash on applying formatting #1551

Closed
OneOfOne opened this issue Jun 8, 2021 · 2 comments
Closed

[generics] gopls crash on applying formatting #1551

OneOfOne opened this issue Jun 8, 2021 · 2 comments
Assignees
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@OneOfOne
Copy link
Contributor

OneOfOne commented Jun 8, 2021

Using Go's dev.typeparams branch and gopls built with -tags=typeparams -gcflags=all=-G=3, it works fairly well, but it randomly crashes with this:

I didn't dig much deeper into it but I figured I should open a bug report here to keep track.

I'll try to create a minimal repro and share it once I figure out how to trigger it.

❯ go version
go version devel go1.17-a9de78ac88 2021-06-08 17:03:39 +0000 linux/amd64
❯ git describe
gopls/v0.1.7-2226-g4e58f8f0
panic: Apply: unexpected node type *ast.ListExpr [recovered]
	panic: Apply: unexpected node type *ast.ListExpr

goroutine 1246 [running]:
golang.org/x/tools/go/ast/astutil.Apply.func1()
	/home/oneofone/code/go/src/golang.org/x/tools/go/ast/astutil/rewrite.go:47 +0x89
panic({0xc0cd00, 0xc002974a30})
	/usr/src/go1.18/src/runtime/panic.go:1037 +0x215
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc00454be60, {0xe7a1b8, 0xc00463c750}, {0xd18575, 0x33}, 0xe8bd58, {0xe7a258, 0xc004628d08})
	/home/oneofone/code/go/src/golang.org/x/tools/go/ast/astutil/rewrite.go:440 +0x26c7
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc00454be60, {0xe7a438, 0xc004628d20}, {0xe60b49, 0x1}, 0x1, {0xe7a1b8, 0xc00463c750})
	/home/oneofone/code/go/src/golang.org/x/tools/go/ast/astutil/rewrite.go:252 +0x255d
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc00454be60, {0xe79b28, 0xc00463c780}, {0xd168a5, 0x0}, 0x0, {0xe7a438, 0xc004628d20})
	/home/oneofone/code/go/src/golang.org/x/tools/go/ast/astutil/rewrite.go:269 +0xc45
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc00454be60, {0xe79f88, 0xc0015a9d40}, {0xd175e3, 0x5}, 0xc00464a230, {0xe79b28, 0xc00463c780})
	/home/oneofone/code/go/src/golang.org/x/tools/go/ast/astutil/rewrite.go:285 +0x12fb
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc00454be60, {0xe79fb0, 0xc00463c7b0}, {0xd173e3, 0x0}, 0xea7590, {0xe79f88, 0xc0015a9d40})
	/home/oneofone/code/go/src/golang.org/x/tools/go/ast/astutil/rewrite.go:221 +0x1645
golang.org/x/tools/go/ast/astutil.(*application).applyList(0xc00454be60, {0xe79fb0, 0xc00463c7b0}, {0xd173e3, 0x4})
	/home/oneofone/code/go/src/golang.org/x/tools/go/ast/astutil/rewrite.go:473 +0xae
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc00454be60, {0xe7a488, 0xc004628d50}, {0xd19e97, 0x0}, 0x20, {0xe79fb0, 0xc00463c7b0})
	/home/oneofone/code/go/src/golang.org/x/tools/go/ast/astutil/rewrite.go:226 +0x1ba5
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc00454be60, {0xe7a528, 0xc0015a9b80}, {0xd175e3, 0x20}, 0xc00464a228, {0xe7a488, 0xc004628d50})
	/home/oneofone/code/go/src/golang.org/x/tools/go/ast/astutil/rewrite.go:288 +0xd25
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc00454be60, {0xe7a0a0, 0xc0015a9e40}, {0xd1893a, 0xc0015a9e40}, 0xbfe5e0, {0xe7a528, 0xc0015a9b80})
	/home/oneofone/code/go/src/golang.org/x/tools/go/ast/astutil/rewrite.go:403 +0xf68
golang.org/x/tools/go/ast/astutil.(*application).applyList(0xc00454be60, {0xe7a0a0, 0xc0015a9e40}, {0xd1893a, 0x5})
	/home/oneofone/code/go/src/golang.org/x/tools/go/ast/astutil/rewrite.go:473 +0xae
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc00454be60, {0xe79fd8, 0xc00105dc00}, {0xd18327, 0xc00105dc00}, 0xe1, {0xe7a0a0, 0xc0015a9e40})
	/home/oneofone/code/go/src/golang.org/x/tools/go/ast/astutil/rewrite.go:411 +0x2255
golang.org/x/tools/go/ast/astutil.(*application).applyList(0xc00454be60, {0xe79fd8, 0xc00105dc00}, {0xd18327, 0x5})
	/home/oneofone/code/go/src/golang.org/x/tools/go/ast/astutil/rewrite.go:473 +0xae
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc00454be60, {0xe7b388, 0xc002974a10}, {0xd17463, 0x10}, 0x7fbd0309a108, {0xe79fd8, 0xc00105dc00})
	/home/oneofone/code/go/src/golang.org/x/tools/go/ast/astutil/rewrite.go:424 +0xbc5
golang.org/x/tools/go/ast/astutil.Apply({0xe79fd8, 0xc00105dc00}, 0xc0029749f0, 0xc002974a00)
	/home/oneofone/code/go/src/golang.org/x/tools/go/ast/astutil/rewrite.go:52 +0x166
mvdan.cc/gofumpt/format.File(0xc0015a9940, 0xc00105dc00, {{0x0, 0xc02ea0}, 0xe8})
	/home/oneofone/code/go/pkg/mod/mvdan.cc/gofumpt@v0.1.1/format/format.go:95 +0x245
mvdan.cc/gofumpt/format.Source({0xc004621980, 0x18a1, 0x1910}, {{0x0, 0xc000250c78}, 0xc8})
	/home/oneofone/code/go/pkg/mod/mvdan.cc/gofumpt@v0.1.1/format/format.go:55 +0xb9
golang.org/x/tools/gopls/internal/hooks.Options.func1({0xe67f60, 0xc004621980}, {0xc004621980, 0xc4aee0, 0xc001b2fc80})
	/home/oneofone/code/go/src/golang.org/x/tools/gopls/internal/hooks/hooks.go:26 +0x2e
golang.org/x/tools/internal/lsp/source.Format({0xe7f718, 0xc001c7a480}, {0xea7ff8, 0xc000b79100}, {0xe8b518, 0xc00410aa20})
	/home/oneofone/code/go/src/golang.org/x/tools/internal/lsp/source/format.go:60 +0x3b8
golang.org/x/tools/internal/lsp.(*Server).formatting(0xc0045d4538, {0xe7f718, 0xc001c7a480}, 0xc0045d4510)
	/home/oneofone/code/go/src/golang.org/x/tools/internal/lsp/format.go:25 +0x1d6
golang.org/x/tools/internal/lsp.(*Server).Formatting(0xc000589260, {0xe7f718, 0xc001c7a480}, 0xbef5e0)
	/home/oneofone/code/go/src/golang.org/x/tools/internal/lsp/server_gen.go:124 +0x25
golang.org/x/tools/internal/lsp/protocol.serverDispatch({0xe7f718, 0xc001c7a480}, {0xeac500, 0xc0001b22a0}, 0xc004603b00, {0xe7f980, 0xc001c7a400})
	/home/oneofone/code/go/src/golang.org/x/tools/internal/lsp/protocol/tsserver.go:469 +0x1b25
golang.org/x/tools/internal/lsp/protocol.ServerHandler.func1({0xe7f718, 0xc001c7a480}, 0xc004603b00, {0xe7f980, 0xc001c7a400})
	/home/oneofone/code/go/src/golang.org/x/tools/internal/lsp/protocol/protocol.go:154 +0x90
golang.org/x/tools/internal/lsp/lsprpc.handshaker.func1({0xe7f718, 0xc001c7a480}, 0xc004603b00, {0xe7f980, 0xc001c7a400})
	/home/oneofone/code/go/src/golang.org/x/tools/internal/lsp/lsprpc/lsprpc.go:684 +0xa7d
golang.org/x/tools/internal/jsonrpc2.MustReplyHandler.func1({0xe7f718, 0xc001c7a480}, 0xc001e851d0, {0xe7f980, 0xc001c7a400})
	/home/oneofone/code/go/src/golang.org/x/tools/internal/jsonrpc2/handler.go:35 +0xf6
golang.org/x/tools/internal/jsonrpc2.AsyncHandler.func1.2()
	/home/oneofone/code/go/src/golang.org/x/tools/internal/jsonrpc2/handler.go:103 +0xa3
created by golang.org/x/tools/internal/jsonrpc2.AsyncHandler.func1
	/home/oneofone/code/go/src/golang.org/x/tools/internal/jsonrpc2/handler.go:100 +0x20f
@gopherbot gopherbot added this to the Untriaged milestone Jun 8, 2021
@findleyr
Copy link
Contributor

findleyr commented Jun 8, 2021

Thanks for the report. The ListExpr node was added to hold expression lists in type instantiation, and it's the first time a new node type has been added in at least 10 years. I'm working on writing up a proper proposal for the changes to go/ast, and this type of panic is one of our primary concerns, though it may be unavoidable.

I'll fix this particular panic in x/tools.

CC @griesemer

@findleyr findleyr added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 8, 2021
@findleyr findleyr self-assigned this Jun 8, 2021
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/326131 mentions this issue: go/ast/astutil: fix panic when rewriting multi-argument type instances

@golang golang locked and limited conversation to collaborators Jun 9, 2022
CarlosRosuero pushed a commit to CarlosRosuero/typeparams that referenced this issue Jun 13, 2023
Use the typeparams helper package to fix AST rewriting in go/ast/astutil
when encountering the new ListExpr type.

This is liable to break in the future when the go/ast API changes, but
at least it will be easy to find by virtue of using internal/typeparams.

Fixes golang/vscode-go#1551

Change-Id: Id34bbcdede9024ed9818bef6d73a19e993dd76a8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/326131
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants