From cb3826d9adcb185fbf438602e82ae99ff1878779 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Mon, 21 Dec 2015 09:39:45 -0800
Subject: [PATCH] std: Ensure AssertRecoverSafe indeed is more often

Types like `&AssertRecoverSafe<T>` and `Rc<AssertRecoverSafe<T>>` were
mistakenly not considered recover safe, but the point of the assertion wrapper
is that it indeed is! This was caused by an interaction between the
`RecoverSafe` and `NoUnsafeCell` marker traits, and this is updated by adding an
impl of the `NoUnsafeCell` marker trait for `AssertRecoverSafe` to ensure that
it never interacts with the other negative impls of `RecoverSafe`.

cc #30510
---
 src/libstd/panic.rs             | 34 ++++++++++++++++++---------------
 src/test/run-pass/panic-safe.rs |  6 +++++-
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/src/libstd/panic.rs b/src/libstd/panic.rs
index 6e4ba337b08ee..0f5a08ba3ffe7 100644
--- a/src/libstd/panic.rs
+++ b/src/libstd/panic.rs
@@ -99,8 +99,11 @@ use thread::Result;
                             across a recover boundary"]
 pub trait RecoverSafe {}
 
-/// A marker trait representing types which do not contain an `UnsafeCell` by
-/// value internally.
+/// A marker trait representing types where a shared reference is considered
+/// recover safe.
+///
+/// This trait is namely not implemented by `UnsafeCell`, the root of all
+/// interior mutability.
 ///
 /// This is a "helper marker trait" used to provide impl blocks for the
 /// `RecoverSafe` trait, for more information see that documentation.
@@ -108,7 +111,7 @@ pub trait RecoverSafe {}
 #[rustc_on_unimplemented = "the type {Self} contains interior mutability \
                             and a reference may not be safely transferrable \
                             across a recover boundary"]
-pub trait NoUnsafeCell {}
+pub trait RefRecoverSafe {}
 
 /// A simple wrapper around a type to assert that it is panic safe.
 ///
@@ -157,11 +160,11 @@ pub struct AssertRecoverSafe<T>(T);
 // * Our custom AssertRecoverSafe wrapper is indeed recover safe
 impl RecoverSafe for .. {}
 impl<'a, T: ?Sized> !RecoverSafe for &'a mut T {}
-impl<'a, T: NoUnsafeCell + ?Sized> RecoverSafe for &'a T {}
-impl<T: NoUnsafeCell + ?Sized> RecoverSafe for *const T {}
-impl<T: NoUnsafeCell + ?Sized> RecoverSafe for *mut T {}
+impl<'a, T: RefRecoverSafe + ?Sized> RecoverSafe for &'a T {}
+impl<T: RefRecoverSafe + ?Sized> RecoverSafe for *const T {}
+impl<T: RefRecoverSafe + ?Sized> RecoverSafe for *mut T {}
 impl<T: RecoverSafe> RecoverSafe for Unique<T> {}
-impl<T: NoUnsafeCell + ?Sized> RecoverSafe for Shared<T> {}
+impl<T: RefRecoverSafe + ?Sized> RecoverSafe for Shared<T> {}
 impl<T: ?Sized> RecoverSafe for Mutex<T> {}
 impl<T: ?Sized> RecoverSafe for RwLock<T> {}
 impl<T> RecoverSafe for AssertRecoverSafe<T> {}
@@ -169,15 +172,16 @@ impl<T> RecoverSafe for AssertRecoverSafe<T> {}
 // not covered via the Shared impl above b/c the inner contents use
 // Cell/AtomicUsize, but the usage here is recover safe so we can lift the
 // impl up one level to Arc/Rc itself
-impl<T: NoUnsafeCell + ?Sized> RecoverSafe for Rc<T> {}
-impl<T: NoUnsafeCell + ?Sized> RecoverSafe for Arc<T> {}
+impl<T: RefRecoverSafe + ?Sized> RecoverSafe for Rc<T> {}
+impl<T: RefRecoverSafe + ?Sized> RecoverSafe for Arc<T> {}
 
-// Pretty simple implementations for the `NoUnsafeCell` marker trait, basically
-// just saying that this is a marker trait and `UnsafeCell` is the only thing
-// which doesn't implement it (which then transitively applies to everything
-// else.
-impl NoUnsafeCell for .. {}
-impl<T: ?Sized> !NoUnsafeCell for UnsafeCell<T> {}
+// Pretty simple implementations for the `RefRecoverSafe` marker trait,
+// basically just saying that this is a marker trait and `UnsafeCell` is the
+// only thing which doesn't implement it (which then transitively applies to
+// everything else.
+impl RefRecoverSafe for .. {}
+impl<T: ?Sized> !RefRecoverSafe for UnsafeCell<T> {}
+impl<T> RefRecoverSafe for AssertRecoverSafe<T> {}
 
 impl<T> AssertRecoverSafe<T> {
     /// Creates a new `AssertRecoverSafe` wrapper around the provided type.
diff --git a/src/test/run-pass/panic-safe.rs b/src/test/run-pass/panic-safe.rs
index cd2457e8a52f7..9949b79278c11 100644
--- a/src/test/run-pass/panic-safe.rs
+++ b/src/test/run-pass/panic-safe.rs
@@ -11,7 +11,7 @@
 #![allow(dead_code)]
 #![feature(recover)]
 
-use std::panic::RecoverSafe;
+use std::panic::{RecoverSafe, AssertRecoverSafe};
 use std::cell::RefCell;
 use std::sync::{Mutex, RwLock, Arc};
 use std::rc::Rc;
@@ -47,5 +47,9 @@ fn main() {
         assert::<Box<T>>();
         assert::<Vec<T>>();
         assert::<RefCell<T>>();
+        assert::<AssertRecoverSafe<T>>();
+        assert::<&AssertRecoverSafe<T>>();
+        assert::<Rc<AssertRecoverSafe<T>>>();
+        assert::<Arc<AssertRecoverSafe<T>>>();
     }
 }