-
Notifications
You must be signed in to change notification settings - Fork 17.5k
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: stop marking arguments live for entire function call #15843
Comments
Here's the plan:
I'm signing myself up to do #2 and #3. I will put #2 behind a flag of some sort and have it default to the new 1.8 behavior. We should probably think more about how to do #1 robustly. |
I don't think we need to go out of our way for step 1. It was already in the Go 1.7 release notes, and we'd put it in the Go 1.8 release notes again, probably a bit more prominently. I think even a flag is pretty generous. |
Same about the flag, it just gives people a way to continue to use subtly On Wed, 31 Aug 2016, 08:33 Brad Fitzpatrick notifications@github.com
|
CL https://golang.org/cl/28310 mentions this issue. |
We're dropping this behavior in favor of runtime.KeepAlive. Implement runtime.KeepAlive as an intrinsic. Update #15843 Change-Id: Ib60225bd30d6770ece1c3c7d1339a06aa25b1cbc Reviewed-on: https://go-review.googlesource.com/28310 Run-TryBot: Keith Randall <khr@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
For Go 1.8 we probably need to stop marking args live for the entire function call. It just doesn't fit with the sophisticated analyses being done by the SSA back end. Programmers will have runtime.KeepAlive already in Go 1.7, so people doing Go 1.8 beta testing will be able to update their code in a (Go 1.7-)backwards compatible way. And maybe we can find some way to help people find problems faster. It's going to be quite a band-aid to rip off.
But see my recent comment in #15277 for the pain we're in if we don't do this.
/cc @randall77 @aclements @RLH @dr2chase
The text was updated successfully, but these errors were encountered: