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: esc.go fails to mark anonymous receiver parameters as non-escaping #24305

Open
mdempsky opened this issue Mar 7, 2018 · 6 comments
Labels
Milestone

Comments

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Mar 7, 2018

package p

type T struct { x int }

//go:noinline
func (*T) M() {}  

//go:noinline
func (_ *T) N() {}

func f() {
        var t T
        t.M()
        t.N()
        (*T).M(&t)
        (*T).N(&t)
}

Compiling the above code with "go tool compile -m" outputs:

a.go:13:3: t escapes to heap
a.go:12:6: moved to heap: t
a.go:14:3: t escapes to heap
a.go:15:9: &t escapes to heap
a.go:16:9: &t escapes to heap

This doesn't happen if the receiver parameters are given a proper name, or if the receiver parameters are turned into normal parameters (i.e., changing the methods into functions).

The direct calls (t.M() and t.N()) are because (*EscState).esctag's unnamed parameter loop only touches fn.Type.Params(), not fn.Type.Recvs().

The indirect calls ((*T).M(&t) and (*T).N(&t)) appear to be because esc.go just doesn't look for esc tags for methods called as functions.

@mdempsky mdempsky added the NeedsFix label Mar 7, 2018
@mdempsky mdempsky added this to the Go1.11 milestone Mar 7, 2018
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Mar 7, 2018

Change https://golang.org/cl/99335 mentions this issue: cmd/compile: mark anonymous receiver parameters as non-escaping

gopherbot pushed a commit that referenced this issue Mar 8, 2018
This was already done for normal parameters, and the same logic
applies for receiver parameters too.

Updates #24305.

Change-Id: Ia2a46f68d14e8fb62004ff0da1db0f065a95a1b7
Reviewed-on: https://go-review.googlesource.com/99335
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@dr2chase

This comment has been minimized.

Copy link
Contributor

@dr2chase dr2chase commented Jun 15, 2018

This looks fixed. Any problems with closing?

@dr2chase dr2chase closed this Jun 22, 2018
@mdempsky mdempsky reopened this Jun 22, 2018
@mdempsky

This comment has been minimized.

Copy link
Member Author

@mdempsky mdempsky commented Jun 22, 2018

This is half fixed. (*T).M(&t) and (*T).N(&t) still escape.

@dr2chase

This comment has been minimized.

Copy link
Contributor

@dr2chase dr2chase commented Jun 25, 2018

Still for 1.11, or do we put off to 1.12?

@dr2chase

This comment has been minimized.

Copy link
Contributor

@dr2chase dr2chase commented Jun 25, 2018

Doesn't this have the obvious user workaround of applying the local purely local rewrite of (*T).M(&t) into (&t).M(), which can actually be written t.M()?

I'd like to postpone this to unplanned.

@mdempsky mdempsky modified the milestones: Go1.11, Unplanned Jun 25, 2018
@mdempsky

This comment has been minimized.

Copy link
Member Author

@mdempsky mdempsky commented Apr 16, 2019

Still an issue with newescape.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.