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

Update std::thread::Result and std::panic::catch_unwind documentation to recommend std::panic::resume_unwind instead of unwrap #79950

Closed
yaahc opened this issue Dec 11, 2020 · 1 comment · Fixed by #80169
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@yaahc
Copy link
Member

yaahc commented Dec 11, 2020

The current version of the std::thread::Result documentation recommends unwrapping a Result<T, Box<dyn Any + ...>> in order to re-start propagation of a caught panic. This recommendation will not simply resume the panic as it was originally. Once the error type has been erased and Boxed the panic can no longer be downcast directly back to the actual payload type. Instead you have to also try downcasting twice.

std::panic::catch_unwind on the other hand has no references to resume_unwind or how to repropagate caught panics at all, which ends up pushing users into the std::thread::Result if they're trying to figure out how to handle the Err variant of catch_unwind.

These docs should reference resume_unwind as the recommended way for re-propagating type erased panic payloads.

@frewsxcv frewsxcv added the A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools label Dec 12, 2020
@camelid camelid added A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Dec 12, 2020
@frewsxcv
Copy link
Member

frewsxcv commented Dec 18, 2020

PR opened #80169

JohnTitor added a commit to JohnTitor/rust that referenced this issue Jan 14, 2021
Recommend panic::resume_unwind instead of panicking.

Fixes rust-lang#79950.
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jan 14, 2021
Recommend panic::resume_unwind instead of panicking.

Fixes rust-lang#79950.
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jan 14, 2021
Recommend panic::resume_unwind instead of panicking.

Fixes rust-lang#79950.
m-ou-se added a commit to m-ou-se/rust that referenced this issue Jan 14, 2021
Recommend panic::resume_unwind instead of panicking.

Fixes rust-lang#79950.
m-ou-se added a commit to m-ou-se/rust that referenced this issue Jan 14, 2021
Recommend panic::resume_unwind instead of panicking.

Fixes rust-lang#79950.
@bors bors closed this as completed in 3ea744e Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants