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/compile, go/types: ICE due to recursive unification inside func literals #57155

Closed
findleyr opened this issue Dec 8, 2022 · 1 comment
Closed
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge release-blocker
Milestone

Comments

@findleyr
Copy link
Member

findleyr commented Dec 8, 2022

The following valid Go code causes the type checker to panic, at 1.19 and tip. At 1.18 it merely causes a confusing compilation error (because we cautiously didn't panic at the unification depth limit).

func f[P *Q, Q any](P, Q) {
	func() {
		_ = f[P]
	}()
}

https://go.dev/play/p/9-OrhazCB7t?v=gotip

The reason is that function literal type-checking clobbers the environment used to detect self-recursive instantiation in https://go.dev/cl/385494.

This seems like a panic that would not be that hard to encounter, though it is clearly not a burning fire since it has been present since the initial launch of generics.

I think we should probably fix for 1.20, and perhaps backport.

CC @griesemer

@findleyr findleyr self-assigned this Dec 8, 2022
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Dec 8, 2022
@findleyr findleyr added this to the Go1.20 milestone Dec 8, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/456236 mentions this issue: go/types, types2: always rename type parameters during inference

@golang golang locked and limited conversation to collaborators Dec 8, 2023
mateusz834 pushed a commit to mateusz834/tgoast that referenced this issue Dec 31, 2024
Type inference uses a trick of "renaming" type parameters in the type
parameter list to avoid cycles during unification. This separates the
identity of type parameters from type arguments. When this trick was
introduced in CL 385494, we restricted its application to scenarios
where inference is truly self-recursive: the type parameter list being
inferred was the same as the type parameter list of the outer function
declaration. Unfortunately, the heuristic used to determine
self-recursiveness was flawed: type-checking function literals clobbers
the type-checker environment, losing information about the outer
signature.

We could fix this by introducing yet more state into the type-checker
(e.g. a 'declSig' field that would hold the signature of the active
function declaration), but it is simpler to just avoid this optimization
and always perform type parameter renaming. We can always optimize
later.

This CL removes the check for true self-recursion, always performing the
renaming.

Fixes golang/go#57155

Change-Id: I34c7617005c1f0ccfe2192da0e5ed104be6b92c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/456236
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge release-blocker
Projects
None yet
Development

No branches or pull requests

2 participants