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: return statement does not set result parameters before executing deferred function returned by a literal function #32175

Closed
sandan opened this issue May 21, 2019 · 10 comments
Labels
Milestone

Comments

@sandan
Copy link

@sandan sandan commented May 21, 2019

What version of Go are you using (go version)?

$ go version
go version go1.12.4 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/mark/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"

What did you do?

See the playground example: https://play.golang.org/p/SptuCsFa3uw
I have two defer statements in their own functions. They both call anonymous functions that return a function to execute at the end of the function (when the result variable has been assigned the value in the return statement).

The functions differ in how the result variable is assigned it's value. One does it implicitly by returning the expression. The other does a bare return after modifying the named return variable.

What did you expect to see?

I expected to see the anonymous function (that prints "on exit") print the result variable with a value of 16 in the not_working() function.

What did you see instead?

That anonymous function printed 0 for the result return variable (the initial value of the result name return variable).

@sandan sandan changed the title bug: defer on-entry and on-semantics with anonymous functions defer: on-entry and on-semantics with anonymous functions May 21, 2019
@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented May 22, 2019

An equivalent but slightly-more-readable version: https://play.golang.org/p/h5m8NezzYro

@bcmills bcmills changed the title defer: on-entry and on-semantics with anonymous functions cmd/compile: return statement does not set result parameters before executing deferred function returned by a literal function May 22, 2019
@bcmills bcmills added this to the Go1.13 milestone May 22, 2019
@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented May 22, 2019

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented May 22, 2019

Works as expected with gccgo.

Works as expected with Go 1.4 and earlier, but not with Go 1.5 and later.

@mdempsky

This comment has been minimized.

Copy link
Member

@mdempsky mdempsky commented May 22, 2019

I suspect the issue is that return statements don't mark the result parameters as Assigned, so we perform the closure pass-by-value optimization. E.g., adding result = 0 after var f func() = ... in not_working makes it behave correctly.

@mdempsky

This comment has been minimized.

Copy link
Member

@mdempsky mdempsky commented May 22, 2019

Actually, I think it's even more mundane than that: the first occurrence of outer here should be outermost, like the rest of the line:

// out parameters will be assigned to implicitly upon return.
if outer.Class() != PPARAMOUT && !outermost.Addrtaken() && !outermost.Assigned() && v.Type.Width <= 128 {
v.Name.SetByval(true)
} else {
outermost.SetAddrtaken(true)
outer = nod(OADDR, outer, nil)
}

@StephanVerbeeck

This comment has been minimized.

Copy link

@StephanVerbeeck StephanVerbeeck commented May 22, 2019

Could be related to the introduction of named output arguments and the duality involved (#31630) ?
There are in fact two paths now to return the values:

  1. direct via the return statement (bypassing the output variables that were declared as function arguments)
  2. assignment to output variables where they are picked up by return statement without arguments.

The timeline (sequence of instructions) might not be correct in combination with one or more levels of defer.

@mdempsky

This comment has been minimized.

Copy link
Member

@mdempsky mdempsky commented May 22, 2019

@StephanVerbeeck I don't think so. I'm pretty sure it's like I stated in my last comment: outer.Class() != PPARAMOUT should be outermost.Class() != PPARAMOUT, because in multiply-nested function literals when referencing the outermost function's return parameters (like in the issue report), outer.Class() will be PAUTOHEAP instead.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented May 22, 2019

Change https://golang.org/cl/178541 mentions this issue: cmd/compile: fix capture-by-reference of return parameters

@gopherbot gopherbot closed this in 94a9dad May 22, 2019
@sandan

This comment has been minimized.

Copy link
Author

@sandan sandan commented May 28, 2019

Thanks! How do I find out what version of golang will have the fix?

@mdempsky

This comment has been minimized.

Copy link
Member

@mdempsky mdempsky commented May 28, 2019

@sandan The fix will be included in Go 1.13. Thanks for the report!

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