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: [type go2] add a flag to support the new bracket syntax #40316

Closed
OneOfOne opened this issue Jul 20, 2020 · 4 comments
Closed

x/tools/gopls: [type go2] add a flag to support the new bracket syntax #40316

OneOfOne opened this issue Jul 20, 2020 · 4 comments
Labels
Milestone

Comments

@OneOfOne
Copy link
Contributor

@OneOfOne OneOfOne commented Jul 20, 2020

With the new bracket syntax probably being the default for the go2go branch (893c5ec17),
it'd be nice if there was a way to toggle it in gopls.

Right now

package main

type X[type T] struct {
	x T
}

func main() {
	var x X[int]
	x.x = 10
}

Crashes with:

crash log
panic: ast.Walk: unexpected node type *ast.InstantiatedType

goroutine 212 [running]:
go/ast.Walk(0xebe1e0, 0xc00051e1c0, 0x7f9269ed8400, 0xc000432200)
	/usr/src/go2/src/go/ast/walk.go:372 +0x3046
go/ast.Walk(0xebe1e0, 0xc00051e1c0, 0xec2540, 0xc0000da910)
	/usr/src/go2/src/go/ast/walk.go:311 +0x5b7
go/ast.Walk(0xebe1e0, 0xc00051e1c0, 0xec1dc0, 0xc000432240)
	/usr/src/go2/src/go/ast/walk.go:339 +0xe7b
go/ast.Walk(0xebe1e0, 0xc00051e1c0, 0xec1ac0, 0xc00033a720)
	/usr/src/go2/src/go/ast/walk.go:191 +0x2bf7
go/ast.walkStmtList(0xebe1e0, 0xc00051e1c0, 0xc00033a760, 0x1, 0x1)
	/usr/src/go2/src/go/ast/walk.go:32 +0x9e
go/ast.Walk(0xebe1e0, 0xc00051e1c0, 0xec1780, 0xc0002675c0)
	/usr/src/go2/src/go/ast/walk.go:229 +0x1905
go/ast.Walk(0xebe1e0, 0xc00051e1c0, 0xec1d00, 0xc000267620)
	/usr/src/go2/src/go/ast/walk.go:352 +0xcdd
go/ast.walkDeclList(0xebe1e0, 0xc00051e1c0, 0xc00051e180, 0x2, 0x2)
	/usr/src/go2/src/go/ast/walk.go:38 +0x9e
go/ast.Walk(0xebe1e0, 0xc00051e1c0, 0xec1c80, 0xc0003ba3f0)
	/usr/src/go2/src/go/ast/walk.go:361 +0x23c6
go/ast.Inspect(...)
	/usr/src/go2/src/go/ast/walk.go:393
golang.org/x/tools/internal/lsp/cache.walkASTWithParent(0xec1c80, 0xc0003ba3f0, 0xc0000daa00)
	/home/oneofone/code/go/src/golang.org/x/tools/internal/lsp/cache/parse.go:423 +0x9e
golang.org/x/tools/internal/lsp/cache.fixAST(0xed0360, 0xc000267050, 0xec1c80, 0xc0003ba3f0, 0xc0002c1080, 0xc0002aa280, 0x4a, 0x50, 0x0)
	/home/oneofone/code/go/src/golang.org/x/tools/internal/lsp/cache/parse.go:369 +0x105
golang.org/x/tools/internal/lsp/cache.parseGo(0xed0360, 0xc000267050, 0xc0002bb6c0, 0xed3fa0, 0xc000414180, 0x2, 0x0)
	/home/oneofone/code/go/src/golang.org/x/tools/internal/lsp/cache/parse.go:285 +0x74b
golang.org/x/tools/internal/lsp/cache.(*Cache).parseGoHandle.func1(0xed02a0, 0xc0002ba540, 0x0, 0x1)
	/home/oneofone/code/go/src/golang.org/x/tools/internal/lsp/cache/parse.go:69 +0x59
golang.org/x/tools/internal/memoize.(*Handle).run.func1(0xed02a0, 0xc0002ba540, 0xc0004759b0, 0xc0000ce1e0)
	/home/oneofone/code/go/src/golang.org/x/tools/internal/memoize/memoize.go:237 +0x6b
created by golang.org/x/tools/internal/memoize.(*Handle).run
	/home/oneofone/code/go/src/golang.org/x/tools/internal/memoize/memoize.go:231 +0x11a

However go tool go2go -brackets translate main.go2 works fine.

@gopherbot gopherbot added this to the Unreleased milestone Jul 20, 2020
@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Jul 20, 2020

Thanks for the report! I think the right fix would be to add a case for *ast.InstantiatedType to go/ast.Walk.
@griesemer, if that sounds OK, I'll send out a CL for that.

Edit: Actually, it was such a simple fix that I mailed the CL.

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 20, 2020

Change https://golang.org/cl/243717 mentions this issue: [dev.go2go] go/ast: add *ast.InstantiatedType case to go/ast.walk

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 20, 2020

Change https://golang.org/cl/243719 mentions this issue: [dev.go2go] go/ast: add *ast.InstantiatedType case to go/ast.Walk

gopherbot pushed a commit that referenced this issue Jul 20, 2020
This is necessary for continued support for the prototype in gopls.

Fixes #40316

Change-Id: Iafab2e78cfdfd7415d2e2419b8ed1abe6e6216c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/243717
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
@OneOfOne
Copy link
Contributor Author

@OneOfOne OneOfOne commented Jul 21, 2020

I can confirm that fixes the issue.

@OneOfOne OneOfOne closed this Jul 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.