-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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: escape analysis oddity #7213
Comments
OK. for the current interface implementation the interface value r couldn't be leaked, but the actual value in r could be leaked. So yes, Thing should be able to allocate on the stack. so the conclusion is that for interface values contained in structs, if it's not leaked thru. other means, just calling interface method will never leak the interface? this seems easy enough to implement in the compiler. |
I think this is because of the interface{}. The compiler doesn't know what io.Reader is: It could be a pointer or a value. Either way r is tied to Thing, and it has to fit on the stack as well, whether that be a pointer or a value. At compile time the compiler doesn't know which. For some reason this seems to make a difference (not sure why): func (t Thing) Read() error { _, err := t.r.Read(nil) return err } # command-line-arguments ./test.go:9: leaking param: t I observed something similar with issue #7661. I am still unsure why bytes.Buffer is always heap allocated. |
FWIW: This is the output under go1.7rc2 $ go build -gcflags '-m -l' leak.go
# command-line-arguments
./leak.go:9: leaking param content: t
./leak.go:16: Use t does not escape While t is detected as leaking, it does not escape. |
/cc @davecheney @minux @dvyukov to the update from @freeformz's comment here #7213 (comment) |
Yeah, seems like this minimal repro is no longer a repro, yet the &json.Decoder literal from json.NewDecoder still escapes (even though json.NewDecoder is inlined). (Go 1.8.) |
The original minimized repro case has been fixed (since Go 1.7, evidently), so I'm going to close this as fixed. |
The text was updated successfully, but these errors were encountered: