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

implement recover #891

Closed
carnott-snap opened this issue Feb 4, 2020 · 9 comments
Closed

implement recover #891

carnott-snap opened this issue Feb 4, 2020 · 9 comments
Labels
enhancement New feature or request

Comments

@carnott-snap
Copy link

While recover is called out as a little used feature in the docs, it is unfortunately widespread as a custom control flow primitive, see encoding/gob01. It has also been clarified and approved by the Go authors as canonical, see #35093.

@aykevl
Copy link
Member

aykevl commented Feb 8, 2020

"Little used" is relative, it is little used compared to many other language features.

I personally don't like recover as it seems to me even worse than exceptions, but I guess it will eventually need to be implemented. Because encoding/gob isn't currently supported anyway (it heavily depends on reflection) I think proper support for recover will only become a priority once that package compiles at all.

At the moment, you can in fact use recover but it will always return nil without any stack unwinding at panic. Full support for recover will likely cause a significant increase in code size and is definitely platform dependent.

@urandom2
Copy link

urandom2 commented Feb 8, 2020

That makes sense, just highlighting its use for custom control flow. It also looks like you have reimplemented or do not support most of the stdlib functionality that would be affected, but I thought I would document the usages that I found:

  • bytes
  • debug/gosym
  • encoding/gob
  • encoding/json
  • fmt
  • go/ast
  • go/parser
  • go/types
  • io/ioutil
  • net/http
  • os/exec
  • runtime
  • syscall
  • testing
  • text/tabwriter
  • text/template/parse
  • text/template

@deadprogram deadprogram added the enhancement New feature or request label Feb 15, 2020
@carnott-snap
Copy link
Author

There are some other cases where recover is required that I came across recently: golang/go#39545 (comment). I think it makes sense to move this out of the "Little used features" section, if not just implement it.

@aykevl
Copy link
Member

aykevl commented Jul 30, 2020

So far, we have managed to compile quite a lot of code correctly without needing it. I still think it is not used a lot. Compare it for example with the select keyword, which is not used a lot but still crucial in many cases.

Unfortunately it's not as simple as just implementing this feature. Implementing recover would mean that every defer would become significantly more expensive (in code size, RAM consumption, and runtime execution speed), even for people who don't actually need recover. There might be ways around that but it's going to be difficult. Also, recover will likely be highly architecture specific so would need some work for every new architecture.

Honestly I think I would need to see a good practical use case before adding this feature. So far I haven't really seen one where recover was worth the cost. Practical uses are for example popular packages that won't work correctly without support for recover.

@carnott-snap
Copy link
Author

The argument I am trying to make is more about interop. Today tinygo cannot use most of the code that exists as part of the go ecosystem. If you are writing a few lines of code to run on a microcontroller, this is fine, but it means tinygo is not go.

I sympathise with the desire to target microcontrollers and am not suggesting that recover needs to be implemented, just trying to get the wording of the public facing site fixed for now.

@aykevl
Copy link
Member

aykevl commented May 30, 2022

Here is a PR that implements this feature on most architectures: #2331

@deadprogram deadprogram added the next-release Will be part of next release label Jun 16, 2022
@deadprogram
Copy link
Member

#2331 has been merged, so we will close this issue after the next release. Thanks!

@Vilsol
Copy link

Vilsol commented Jun 16, 2022

@deadprogram should a new issue be opened for recover support for wasm? It seems like it will still be unsupported after this: https://github.com/tinygo-org/tinygo/pull/2331/files#diff-f29f21a0c5d9dd43724e58b58c27f718331cf3cb1096e0a7c71b98ef7872e870R29-R33

@deadprogram
Copy link
Member

@Vilsol that is a very good idea. Can you please create the issue to track it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants