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

Jump to bottom

builtin: can't "defer recover()" without an anonymous func #20768

Closed
mvdan opened this issue Jun 23, 2017 · 6 comments
Closed

builtin: can't "defer recover()" without an anonymous func #20768

mvdan opened this issue Jun 23, 2017 · 6 comments
Labels
Documentation FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Go1.10

Comments

@mvdan
Copy link
Member

mvdan commented Jun 23, 2017

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

go version devel +ded29e7b39 Wed Jun 21 04:56:15 2017 +0000 linux/amd64

Also on 1.8.3.

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/mvdan/go/land:/home/mvdan/go"
GORACE=""
GOROOT="/home/mvdan/tip"
GOTOOLDIR="/home/mvdan/tip/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build990881875=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

This works: https://play.golang.org/p/YYvq0OrB8H

This doesn't: https://play.golang.org/p/7u3lbkXkVp

What did you expect to see?

I expected the absence of an anonymous function to not change the behaviour of defer and/or panic.

What did you see instead?

The opposite - the deferred recover doesn't do its job.

Perhaps there's something I'm missing. I also don't know where this bug - if there is one - resides. Could well be the compiler, or perhaps even a spec clarification.

Note that I realise that the defer statement spec says:

Calls of built-in functions are restricted as for expression statements.
[which says]
The following built-in functions are not permitted in statement context:
append cap complex imag len make new real
unsafe.Alignof unsafe.Offsetof unsafe.Sizeof

Note how recover isn't one of them. And if it was restricted, it should be a compile-time error, not a change in run-time behaviour.
@mvdan
Copy link
Member Author

mvdan commented Jun 23, 2017

Perhaps worth noting that defer panic(...) does work: https://play.golang.org/p/-KtW4CI0o7

@bradfitz
Copy link
Contributor

I feel like I've seen a duplicate issue (or mailing list post?) about this before, where I recall hearing this is all working as intended.

But that probably means there's missing documentation somewhere.

@griesemer ?

@bradfitz bradfitz added Documentation NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Jun 23, 2017
@bradfitz bradfitz added this to the Go1.10 milestone Jun 23, 2017
@mvdan
Copy link
Member Author

mvdan commented Jun 23, 2017

I tried searching for a dup, but I couldn't find one.

I agree that there's something to do, even if it's just a doc/spec change. I could not find an explanation for this.

@mvdan
Copy link
Member Author

mvdan commented Jun 23, 2017

After a second look, I've found in https://golang.org/pkg/builtin/#recover:

Executing a call to recover inside a deferred function (but not any function called by it) stops the panicking sequence by restoring normal execution and retrieves the error value passed to the call of panic.

Technically, doing defer recover() means that the recover is not in a deferred function body, as it's directly part of the deferred statement.

But I don't quite understand this restriction or the reason behind it.

@ianlancetaylor
Copy link
Contributor

Searching for "defer recover()" in the golang-nuts mailing list will bring up a few discussion about this. Here's one of the times I've answered it: https://groups.google.com/d/msg/golang-nuts/SwmjC_j5q90/99rdN1LEN1kJ .

Closing because this is working as documented and expected.

@mvdan
Copy link
Member Author

mvdan commented Jun 23, 2017

Thanks @ianlancetaylor - I tried Google, but I didn't try the search within the mailing list. Apologies.

At least the next person will have it easier to find, as it's now in the issue tracker too :)

@golang golang locked and limited conversation to collaborators Jun 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Documentation FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Go1.10
Development

No branches or pull requests
4 participants
@bradfitz @ianlancetaylor @mvdan @gopherbot