cmd/compile: generic method values at package scope pin their receiver value

[mdempsky]

https://go.dev/play/p/EO98OQn3OIr

If the generic method value x.M appears at package scope, then x is spilled into a global variable and never gets garbage collected, even if the method value itself is garbage collected.

This happens because for generic method values, stencil.go desugar the method value into a function literal that captures the receiver value after evaluating it. However, the compiler backend (particularly escape analysis) doesn't currently handle package-scope function literals with closure variables. This causes a problem for generic method values that appear in package-scope initialization expressions.

The solution currently taken by stencil.go (and that I've duplicated in unified IR's shaped-based stenciling) is to spill the receiver value to a global variable instead. However, this is suboptimal because it prevents garbage collection.

Strictly speaking, this isn't a correctness issue, because runtime.SetFinalizer warns that variables allocated during package initialization may never be finalized. But it's something that we can do better about.

/cc @golang/compiler

[cherrymui]

Maybe it could rewrite to something like

var m func()
func init() { m = New[int]().M }

So it will not be a "package-scoped closure"?

[mdempsky]

So the natural time to do the rewrite would be during IR construction, which currently happens before initialization ordering. That rewrite would unfortunately affect when m is initialized.

But along those lines, I think rewriting to this would work without affecting initialization ordering:

var m = func(r *T[int]) func() { return r.M }(New[int]())

And then that could get further rewritten into:

var m = func(r *T[int]) func() { return func() { T[int].M(r, &.dict.T[int]) } }(New[int]())

Thanks for the suggestion.

[gopherbot]

Change https://go.dev/cl/422198 mentions this issue: test: add test for package-scope method value GC

[mdempsky]

https://go-review.googlesource.com/c/go/+/423074 tested a non-generic version of the test, and it still flaked on arm with "never GC'd": https://storage.googleapis.com/go-build-log/019cddda/linux-arm-aws_8b1e8c2b.log

[gopherbot]

Change https://go.dev/cl/423115 mentions this issue: test: make issue54343.go robust against the tiny allocator

[mdempsky]

This will be fixed in 1.20 with GOEXPERIMENT=unified, and there's a regress test.