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

cmd/go2go: underscore type names cause panic #39743

Closed
rogpeppe opened this issue Jun 21, 2020 · 5 comments
Closed

cmd/go2go: underscore type names cause panic #39743

rogpeppe opened this issue Jun 21, 2020 · 5 comments
Assignees
Milestone

Comments

@rogpeppe
Copy link
Contributor

@rogpeppe rogpeppe commented Jun 21, 2020

commit d872bbc

https://go2goplay.golang.org/p/z4dUhE98Co0

The following program panics:

package main

func main(){ }

type Foo(type T) struct{}

func (f Foo(_)) X() {
}

The panic message is:

panic: prog.go2:7:13: no type found for *ast.Ident _

The proposal specifically suggests that _ is an OK name for type parameters, so this should be valid.

@rogpeppe
Copy link
Contributor Author

@rogpeppe rogpeppe commented Jun 21, 2020

commit d872bbc

https://go2goplay.golang.org/p/7tn_5lE9y4k

Here's a different panic, perhaps with the same underlying cause:

package main

func main() {}

type S(type T) struct {}

func (*S(_)) Foo() { }

func (*S(T)) Bar(s *S(T)) {
	s.Foo()
}

This panics with the following stack trace:

panic: internal error: receiver type parameter inference failed [recovered]
	panic: internal error: receiver type parameter inference failed [recovered]
	panic: internal error: receiver type parameter inference failed

goroutine 1 [running]:
go/types.(*Checker).handleBailout(0xc00007c5a0, 0xc0000c1be0)
	/usr/local/go-faketime/src/go/types/check.go:251 +0x98
panic(0x642580, 0x6d2a40)
	/usr/local/go-faketime/src/runtime/panic.go:969 +0x175
go/types.(*Checker).stmt.func1(0xc00007c5a0, 0xc0000559a0)
	/usr/local/go-faketime/src/go/types/stmt.go:304 +0x85
panic(0x642580, 0x6d2a40)
	/usr/local/go-faketime/src/runtime/panic.go:975 +0x3e9
go/types.(*Checker).selector(0xc00007c5a0, 0xc000072980, 0xc00000c520)
	/usr/local/go-faketime/src/go/types/call.go:534 +0x25b7
go/types.(*Checker).exprInternal(0xc00007c5a0, 0xc000072980, 0x6dada0, 0xc00000c520, 0x0, 0x0, 0x0)
	/usr/local/go-faketime/src/go/types/expr.go:1308 +0x29cc
go/types.(*Checker).rawExpr(0xc00007c5a0, 0xc000072980, 0x6dada0, 0xc00000c520, 0x0, 0x0, 0x0)
	/usr/local/go-faketime/src/go/types/expr.go:1021 +0xc5
go/types.(*Checker).exprOrType(0xc00007c5a0, 0xc000072980, 0x6dada0, 0xc00000c520)
	/usr/local/go-faketime/src/go/types/expr.go:1705 +0x55
go/types.(*Checker).call(0xc00007c5a0, 0xc000072980, 0xc000072500, 0x0)
	/usr/local/go-faketime/src/go/types/call.go:17 +0x69
go/types.(*Checker).exprInternal(0xc00007c5a0, 0xc000072980, 0x6da620, 0xc000072500, 0x0, 0x0, 0x38)
	/usr/local/go-faketime/src/go/types/expr.go:1557 +0x1dd0
go/types.(*Checker).rawExpr(0xc00007c5a0, 0xc000072980, 0x6da620, 0xc000072500, 0x0, 0x0, 0x0)
	/usr/local/go-faketime/src/go/types/expr.go:1021 +0xc5
go/types.(*Checker).stmt(0xc00007c5a0, 0x0, 0x6da8a0, 0xc00001c5f0)
	/usr/local/go-faketime/src/go/types/stmt.go:330 +0x3e9b
go/types.(*Checker).stmtList(0xc00007c5a0, 0x0, 0xc00001c600, 0x1, 0x1)
	/usr/local/go-faketime/src/go/types/stmt.go:125 +0xd1
go/types.(*Checker).funcBody(0xc00007c5a0, 0xc00004a5a0, 0xc00001a418, 0x3, 0xc00004a720, 0xc000010e10, 0x0, 0x0)
	/usr/local/go-faketime/src/go/types/stmt.go:42 +0x257
go/types.(*Checker).funcDecl.func1()
	/usr/local/go-faketime/src/go/types/decl.go:786 +0x67
go/types.(*Checker).processDelayed(0xc00007c5a0, 0x0)
	/usr/local/go-faketime/src/go/types/check.go:315 +0x3e
go/types.(*Checker).checkFiles(0xc00007c5a0, 0xc00003dcb8, 0x1, 0x1, 0x0, 0x0)
	/usr/local/go-faketime/src/go/types/check.go:283 +0x145
go/types.(*Checker).Files(...)
	/usr/local/go-faketime/src/go/types/check.go:256
go/types.(*Config).Check(0xc000072540, 0xc00001a3e0, 0x4, 0xc000072240, 0xc00003dcb8, 0x1, 0x1, 0xc000055360, 0x0, 0x4b6d2f, ...)
	/usr/local/go-faketime/src/go/types/api.go:387 +0x188
go/go2go.RewriteBuffer(0xc0000553b0, 0x7ffe5398cdd8, 0x1e, 0xc0000b8000, 0x75, 0x275, 0x0, 0xc000010a50, 0xc000010a20, 0xc0000109f0, ...)
	/usr/local/go-faketime/src/go/go2go/go2go.go:127 +0x24f
main.translateFile(0xc0000553b0, 0x7ffe5398cdd8, 0x1e)
	/usr/local/go-faketime/src/cmd/go2go/translate.go:26 +0xa9
main.main()
	/usr/local/go-faketime/src/cmd/go2go/main.go:64 +0x2ea
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jun 22, 2020

@griesemer The problem here is that the translation tool doesn't know that

func (f Foo(_)) X() {}

is the declaration of a method of a parameterized type. The tool tries to figure that out by asking for the info.TypeOf the receiver type, in this case Foo(_), and then checking whether that type has any parameters. However, in this case, presumably because of the use of _, Foo(_) has no type, and so the test fails.

Any thoughts on the best way to determine, based only on the AST, that this method declaration is for a parameterized type? Thanks.

@ianlancetaylor ianlancetaylor added this to the Unreleased milestone Jun 22, 2020
@ianlancetaylor ianlancetaylor self-assigned this Jun 22, 2020
@griesemer
Copy link
Contributor

@griesemer griesemer commented Jun 22, 2020

A receiver is both a declaration of the (local) type parameter names and, for the type checker to work uniformly, an instantiation of the receiver's base type with those parameter names. That instantiation would fail if we were using _'s because a _ is never declared and so it cannot be found. So the type-checker renames _ in receivers to n_ where n stands for the type parameter index, starting with 0. For instance, for a type T with 5 type parameters, the local receiver type parameter names for this method m

func (recv T(P, _, _, R, _)) m() { ... }

get renamed to

func (recv T(P, 1_, 2_, R, 4_)) m() { ... }

Hope that helps.

@gopherbot
Copy link

@gopherbot gopherbot commented Jun 22, 2020

Change https://golang.org/cl/239382 mentions this issue: [dev.go2go] go/go2go: don't permit underscore as type argument

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jun 22, 2020

The translation tool is trying to look up the type given an ast.Ident, so knowing that there is a different ast.Ident that would work doesn't help much. And even if we get past that, there are other steps that fail. So I just changed the translation tool to reject attempts to instantiate a type with _.

This will now give an error message on the dev.go2go branch.

gopherbot pushed a commit that referenced this issue Jun 22, 2020
The go/types package doesn't give us an easy way to get the type,
so just reject it.

Fixes #39743

Change-Id: I5404c10baede0fd2cf67980b06fbebd214a50dff
Reviewed-on: https://go-review.googlesource.com/c/go/+/239382
Reviewed-by: Ian Lance Taylor <iant@golang.org>
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
4 participants
You can’t perform that action at this time.