x/tools/gopls: types.Func.typ=nil panic in missingMethod with gopls@master built with go@master

[timothy-king]

gopls version: v0.0.0-20230404161500-4d205d81b5a0 (devel go1.21-ac571a388dc Mon Apr 3 20:17:02 2023 0000)
gopls flags:
update flags: proxy
extension version: 0.38.0
go version: devel 1.21-ac571a388dc
environment: Visual Studio Code darwin
initialization error: undefined
issue timestamp: Thu, 20 Apr 2023 19:39:00 GMT
restart history:
Wed, 19 Apr 2023 21:24:47 GMT: activation (enabled: true)

ATTENTION: PLEASE PROVIDE THE DETAILS REQUESTED BELOW.

Describe what you observed.

panic: interface conversion: types.Type is nil, not *types.Signature

goroutine 2426 [running]:
go/types.(*Checker).handleBailout(0xc0030341e0, 0xc002ded5c8)
	  check.go:299  0x88
panic({0x100983da0%3F, 0xc00336b020%3F})
	  panic.go:913  0x21f
go/types.(*Checker).funcString(0xc0030341e0, 0xc0010e8cc0, 0x0)
	  lookup.go:483  0x1df
go/types.(*Checker).missingMethod(0xc0030341e0, {0x100d03458%3F, 0xc003ad0ec0}, {0x100d03430, 0xc000706cb0}, 0x1, 0x100bf7d00, 0xc00303dfa0)
	  lookup.go:432  0x46b
go/types.(*Checker).implements(0xc0030341e0, {0x100d03458%3F, 0xc003ad0ec0}, {0x100d03430%3F, 0xc000706cb0}, 0x0, 0xc00303dfa0)
	  instantiate.go:246  0x55d
go/types.(*operand).assignableTo(0xc0027d3a00, 0xc0030341e0, {0x100d03430%3F, 0xc000706cb0%3F}, 0xc002debfa0)
	  operand.go:285  0x89c
go/types.(*Checker).assignment(0xc0030341e0, 0xc0027d3a00, {0x100d03430, 0xc000706cb0}, {0x100aab8c9, 0x16})
	  assignments.go:89  0x7cc
go/types.(*Checker).indexedElts(0x100d034a8%3F, {0xc003ad07a0, 0x1, 0xc002dec488%3F}, {0x100d03430, 0xc000706cb0}, 0xffffffffffffffff)
	  index.go:453  0x14d
go/types.(*Checker).exprInternal(0xc0030341e0, {0x0%3F, 0x0%3F}, 0xc0027d3800, {0x100d059a0, 0xc0027d2180%3F}, {0x0%3F, 0x0%3F})
	  expr.go:1520  0xacd
go/types.(*Checker).rawExpr(0xc0030341e0, {0x0, 0x0}, 0xc0027d3800, {0x100d059a0%3F, 0xc0027d2180%3F}, {0x0%3F, 0x0%3F}, 0x0)
	  expr.go:1254  0x1ac
go/types.(*Checker).expr(0xc0030341e0%3F, {0x0%3F, 0x0%3F}, 0xc00256b800%3F, {0x100d059a0%3F, 0xc0027d2180%3F})
	  expr.go:1787  0x45
go/types.(*Checker).exprInternal(0xc0030341e0, {0x0%3F, 0x0%3F}, 0xc0027d3800, {0x100d059a0, 0xc0027d2200%3F}, {0x0%3F, 0x0%3F})
	  expr.go:1435  0x2459
go/types.(*Checker).rawExpr(0xc0030341e0, {0x0, 0x0}, 0xc0027d3800, {0x100d059a0%3F, 0xc0027d2200%3F}, {0x0%3F, 0x0%3F}, 0x0)
	  expr.go:1254  0x1ac
go/types.(*Checker).expr(0x20%3F, {0x0%3F, 0x0%3F}, 0x129295c88%3F, {0x100d059a0%3F, 0xc0027d2200%3F})
	  expr.go:1787  0x45
go/types.(*Checker).unary(0xc0030341e0, 0xc0027d3800, 0xc002a187c0)
	  expr.go:163  0x45
go/types.(*Checker).exprInternal(0xc0030341e0, {0x0, 0x0}, 0xc0027d3800, {0x100d06030, 0xc002a187c0%3F}, {0x0%3F, 0x0%3F})
	  expr.go:1680  0x1605
go/types.(*Checker).rawExpr(0xc0030341e0, {0x0, 0x0}, 0xc0027d3800, {0x100d06030%3F, 0xc002a187c0%3F}, {0x0%3F, 0x0%3F}, 0x0)
	  expr.go:1254  0x1ac
go/types.(*Checker).expr(0x10000700b%3F, {0x0%3F, 0x0%3F}, 0x0%3F, {0x100d06030%3F, 0xc002a187c0%3F})
	  expr.go:1787  0x45
go/types.(*Checker).varDecl(0x10096d220%3F, 0xc0010e8a20, {0xc00086e758%3F, 0x1, 0x1}, {0x0, 0x0}, {0x100d06030, 0xc002a187c0})
	  decl.go:513  0x16c
go/types.(*Checker).objDecl(0xc0030341e0, {0x100d10198, 0xc0010e8a20}, 0x1%3F)
	  decl.go:194  0x95e
go/types.(*Checker).packageObjects(0xc0030341e0)
	  resolver.go:662  0x465
go/types.(*Checker).checkFiles(0xc0030341e0, {0xc002a19d00, 0x3, 0x4})
	  check.go:329  0x149
go/types.(*Checker).Files(...)
	  check.go:304
golang.org/x/tools/gopls/internal/lsp/cache.doTypeCheck({0x100d070a8, 0xc003539800}, 0xc0028c81e0, {{0xc00071a000, 0x3b}, {0xc00071a000, 0x3b}, {0xc000bfdec0, 0xc}, {0xc00375d2f0, ...}, ...})
	  check.go:1101  0x745
golang.org/x/tools/gopls/internal/lsp/cache.typeCheckImpl({0x100d070a8, 0xc003539650}, 0x1000100ad%3F, {{0xc00071a000, 0x3b}, {0xc00071a000, 0x3b}, {0xc000bfdec0, 0xc}, {0xc00375d2f0, ...}, ...})
	  check.go:968  0x205
golang.org/x/tools/gopls/internal/lsp/cache.(*typeCheckBatch).checkPackage(0xc002deded8%3F, {0x100d070a8, 0xc0028d2300}, 0xc0034c5420)
	  check.go:648  0x225
golang.org/x/tools/gopls/internal/lsp/cache.(*typeCheckBatch).handleSyntaxPackage(0xc0028c81e0, {0x100d070a8, 0xc0028d2300}, 0xc000cbb3e0%3F, {0xc00071a000, 0x3b})
	  check.go:532  0x425
golang.org/x/tools/gopls/internal/lsp/cache.(*snapshot).forEachPackageInternal.func2()
	  check.go:407  0x2b
golang.org/x/sync/errgroup.(*Group).Go.func1()
	  errgroup.go:75  0x5b
created by golang.org/x/sync/errgroup.(*Group).Go in goroutine 1998
	  errgroup.go:72  0x96
[Error - 12:38:45 PM] 

OPTIONAL: If you would like to share more information, you can attach your complete gopls logs.

NOTE: THESE MAY CONTAIN SENSITIVE INFORMATION ABOUT YOUR CODEBASE.
DO NOT SHARE LOGS IF YOU ARE WORKING IN A PRIVATE REPOSITORY.

<OPTIONAL: ATTACH LOGS HERE>

[findleyr]

Thanks @timothy-king!

This may be a type-checker regression, or related to the fact that we import invalid packages from export data. I'll investigate.

CC @adonovan @griesemer for awareness.

I'll transfer to the Go issue tracker.

[adonovan]

I wonder if this is dup of #59371, which was just fixed.

What exact version of GOROOT are you using? I can't relate the stack trace to the source. Specifically:

go/types.(*Checker).assignment(0xc0030341e0, 0xc0027d3a00, {0x100d03430, 0xc000706cb0}, {0x100aab8c9, 0x16})
	  assignments.go:89  0x7cc

this frame currently contains a plain return statement (in a function that doesn't use defer), and rolling back recent commits don't make it line up either:

goroot$ git co master
goroot$ nl -ba ./src/go/types/assignments.go | grep -w 89
    89			return
goroot$ git log ./src/go/types/assignments.go | grep ^commit | head -n 2
commit 522eace4ca496626bcb3b65f9dcb6e8db2673fed
commit 969ab34e4629bdda410c8468d4f45a08e4fec9f8
goroot$ git co 522eace4ca496626bcb3b65f9dcb6e8db2673fed^
goroot$ nl -ba ./src/go/types/assignments.go | grep -w 89
    89			return
goroot$ git co 969ab34e4629bdda410c8468d4f45a08e4fec9f8^
goroot$ nl -ba ./src/go/types/assignments.go | grep -w 89
    89		}
goroot$ 

[EDIT: never mind, I see that it's go@ ac571a3, and it does line up]

[adonovan]

The panic is caused by Func.typ=nil in funcString when missingMethod tries to print the wrongName reason for the failed assignment to a nontrivial interface type. The syntax must be something like var _ = MyStruct{field: I{x}} where I is an interface type and x has a method of the wrong name or receiver pointerness. But that doesn't explain why the type of the I.f or x.f method is nil, nor which one it is. (Perhaps the program counter might answer the "which" question.) I don't think it can be due to a cycle because a type decl can't depend on a var decl.

[griesemer]

wrongName can only happen in one case where a function f is found with a folded-case name that matches.

[adonovan]

wrongName can only happen in one case where a function f is found with a folded-case name that matches.

Or a receiver type that lacks a pointer, I think.

[findleyr]

From looking at this, I think go/types.Checker cannot produce a Func without a Signature type. All Funcs are produced via NewFunc, which is only called from Checker.interfaceType (with a provably non-nil signature), or from collectObjects (which will be provably given a non-nil signature by Checker.FuncDecl).

I could be mistaken, but I think the Func must be created by the importer.

[findleyr]

I could be mistaken, but I think the Func must be created by the importer.

Hah, well I should have looked at the importer before making this assertion. I must be mistaken: it is even easier to see that the indexed importer cannot produce a Func without a signature (NewFunc is only called with the result of importReader.signature). Now I'm very confused.

[findleyr]

@griesemer and I both have no ideas how to proceed; let's move this to the v0.13.0 milestone, as I don't think we'll be able to figure it out without more data.

[findleyr]

We had two google-internal reproducers of this crash, not using gopls, albeit with slightly different call stacks (but still in initVar). @griesemer and @adonovan I CCed you on those internal issues.

Therefore, I think this is a go/types regression related to the recent refactoring of assignability. Moving into the go1.21 milestone.

[findleyr]

Tentatively marking this as a release blocker. There was a new internal google toolchain release to our internal infrastructure yesterday, and two crashes overnight suggests to me that this is a frequent enough crash pattern to block the release.

[findleyr]

Oh. This shouldn't have been this complicated. missingMethod logic was refactored in https://go.dev/cl/472637 and subsequent.

It looks like there is a way to miss the objDecl call, here:
https://cs.opensource.google/go/go/+/master:src/go/types/lookup.go;l=391;drc=804d786a30c664df63ef1e0c95d44772063afae1

Note that in the wrongName case, we skip the objDecl call.

Now to construct a reproducer.

[findleyr]

Simple repro:

type T struct{}
type I interface{M()}
var _ I = T{}
func (T) m()

[gopherbot]

Change https://go.dev/cl/494615 mentions this issue: go/types, types2: be sure to type-check wrong methods in missingMethod