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: cleanup the extra "func" in closure name #55980

Open
cuonglm opened this issue Oct 1, 2022 · 3 comments
Open

cmd/compile: cleanup the extra "func" in closure name #55980

cuonglm opened this issue Oct 1, 2022 · 3 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@cuonglm
Copy link
Member

cuonglm commented Oct 1, 2022

Coming here from https://go-review.googlesource.com/c/go/+/437216/2..3/test/closure3.dir/main.go#b255.

Currently, the compiler names the closures base on the outer function:

package main

func main() {
	a := 2
	if r := func(x int) int {
		b := 3
		return func(y int) int {
			c := 5
			return func(z int) int {
				return a*x + b*y + c*z
			}(10)
		}(100)
	}(1000); r != 2350 {
		panic("r != 2350")
	}
}

For:

return func(z int) int {
	return a*x + b*y + c*z
}(10)

Unified IR generates main.func1.func2, while the old frontend generates main.func1.2. The discrepancy happens when calling ir.NameClosure. The old frontend use the immediate outer function (return func(y int) int { ... }) as outer function, while Unified IR use the outer most (func main() { ... }).

@mdempsky think that the func prefix may not be necessary, and we can remove it.

@cuonglm cuonglm added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 1, 2022
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Oct 1, 2022
@dmitshur dmitshur added this to the Backlog milestone Oct 1, 2022
@thediveo
Copy link

thediveo commented Oct 1, 2022

If I'm not mistaken then the IR names generation might break goroutine leak checkers, such as https://github.com/uber-go/goleak and gleak from https://github.com/onsi/gomega; these today use filter lists of function names to filter out known "good" go routines.

@mdempsky
Copy link
Member

mdempsky commented Oct 1, 2022

@thediveo Thanks, that's an interesting use case to keep in mind.

@mdempsky
Copy link
Member

mdempsky commented Oct 1, 2022

Note that even today we're inconsistent about naming of function literals when inlining is enabled.

Here's an example where the same function literal is assigned 3 different names due to to inlining: https://go.dev/play/p/_nvmEZGSmRb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Projects
Status: Todo
Development

No branches or pull requests

5 participants