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

Mutex and RwLock need RefUnwindSafe too #34756

Merged
merged 2 commits into from
Jul 13, 2016

Conversation

habnabit
Copy link
Contributor

Incomplete, because I don't know what the appropriate stability annotation is here, but this is an attempt to bring the documentation for std::panic in line with reality. Right now, it says:

Types like &Mutex<T>, however, are unwind safe because they implement poisoning by default.

But only Mutex<T>, not &Mutex<T>, is unwind-safe.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@brson
Copy link
Contributor

brson commented Jul 10, 2016

r? @sfackler

@rust-highfive rust-highfive assigned sfackler and unassigned brson Jul 10, 2016
@@ -227,6 +227,12 @@ impl<T: ?Sized> !RefUnwindSafe for UnsafeCell<T> {}
#[stable(feature = "catch_unwind", since = "1.9.0")]
impl<T> RefUnwindSafe for AssertUnwindSafe<T> {}

// XXX: Obviously wrong, but what should it be?
#[stable(feature = "catch_unwind", since = "1.9.0")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just make up a new feature name and change the since version to 1.12.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

This is the first (and presumably only) use of this feature.
@sfackler
Copy link
Member

LGTM but ping @aturon

@alexcrichton
Copy link
Member

@bors: r+ fbec232

Hm yes this was indeed the original intention! Not sure how it slipped through the cracks...

@bors
Copy link
Contributor

bors commented Jul 13, 2016

⌛ Testing commit fbec232 with merge 617039b...

bors added a commit that referenced this pull request Jul 13, 2016
Mutex and RwLock need RefUnwindSafe too

Incomplete, because I don't know what the appropriate stability annotation is here, but this is an attempt to bring the documentation for `std::panic` in line with reality. Right now, it says:

>Types like `&Mutex<T>`, however, are unwind safe because they implement poisoning by default.

But only `Mutex<T>`, not `&Mutex<T>`, is unwind-safe.
@bors bors merged commit fbec232 into rust-lang:master Jul 13, 2016
@habnabit habnabit deleted the mutex-refunwindsafe branch August 21, 2016 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants