Add Shared pointer and have {Arc, Rc} use it

apasel422 · apasel422 · commit d6bd8d8491e8 · 2015-10-16T18:35:31.000-04:00
This change has two consequences: 1. It makes `Arc<T>` and `Rc<T>` covariant in `T`. 2. It causes the compiler to reject code that was unsound with respect to dropck. See compile-fail/issue-29106.rs for an example of code that no longer compiles. Because of this, this is a [breaking-change]. Fixes #29037. Fixes #29106.
diff --git a/src/liballoc/arc.rs b/src/liballoc/arc.rs
@@ -79,9 +79,8 @@ use core::cmp::Ordering;
 use core::mem::{align_of_val, size_of_val};
 use core::intrinsics::{drop_in_place, abort};
 use core::mem;
-use core::nonzero::NonZero;
 use core::ops::{Deref, CoerceUnsized};
-use core::ptr;
+use core::ptr::{self, Shared};
 use core::marker::Unsize;
 use core::hash::{Hash, Hasher};
 use core::{usize, isize};
@@ -124,12 +123,13 @@ const MAX_REFCOUNT: usize = (isize::MAX) as usize;
 pub struct Arc<T: ?Sized> {
     // FIXME #12808: strange name to try to avoid interfering with
     // field accesses of the contained type via Deref
-    _ptr: NonZero<*mut ArcInner<T>>,
+    _ptr: Shared<ArcInner<T>>,
 }
 
 unsafe impl<T: ?Sized + Sync + Send> Send for Arc<T> { }
 unsafe impl<T: ?Sized + Sync + Send> Sync for Arc<T> { }
 
+#[cfg(not(stage0))] // remove cfg after new snapshot
 impl<T: ?Sized + Unsize<U>, U: ?Sized> CoerceUnsized<Arc<U>> for Arc<T> {}
 
 /// A weak pointer to an `Arc`.
@@ -141,12 +141,13 @@ impl<T: ?Sized + Unsize<U>, U: ?Sized> CoerceUnsized<Arc<U>> for Arc<T> {}
 pub struct Weak<T: ?Sized> {
     // FIXME #12808: strange name to try to avoid interfering with
     // field accesses of the contained type via Deref
-    _ptr: NonZero<*mut ArcInner<T>>,
+    _ptr: Shared<ArcInner<T>>,
 }
 
 unsafe impl<T: ?Sized + Sync + Send> Send for Weak<T> { }
 unsafe impl<T: ?Sized + Sync + Send> Sync for Weak<T> { }
 
+#[cfg(not(stage0))] // remove cfg after new snapshot
 impl<T: ?Sized + Unsize<U>, U: ?Sized> CoerceUnsized<Weak<U>> for Weak<T> {}
 
 #[stable(feature = "rust1", since = "1.0.0")]
@@ -190,7 +191,7 @@ impl<T> Arc<T> {
             weak: atomic::AtomicUsize::new(1),
             data: data,
         };
-        Arc { _ptr: unsafe { NonZero::new(Box::into_raw(x)) } }
+        Arc { _ptr: unsafe { Shared::new(Box::into_raw(x)) } }
     }
 
     /// Unwraps the contained value if the `Arc<T>` has only one strong reference.
diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs
@@ -90,6 +90,7 @@
 #![feature(placement_in_syntax)]
 #![feature(placement_new_protocol)]
 #![feature(raw)]
+#![feature(shared)]
 #![feature(staged_api)]
 #![feature(unboxed_closures)]
 #![feature(unique)]
diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs
@@ -163,9 +163,8 @@ use core::hash::{Hasher, Hash};
 use core::intrinsics::{assume, drop_in_place, abort};
 use core::marker::{self, Unsize};
 use core::mem::{self, align_of_val, size_of_val, forget};
-use core::nonzero::NonZero;
 use core::ops::{CoerceUnsized, Deref};
-use core::ptr;
+use core::ptr::{self, Shared};
 
 use heap::deallocate;
 
@@ -184,12 +183,13 @@ struct RcBox<T: ?Sized> {
 pub struct Rc<T: ?Sized> {
     // FIXME #12808: strange names to try to avoid interfering with field
     // accesses of the contained type via Deref
-    _ptr: NonZero<*mut RcBox<T>>,
+    _ptr: Shared<RcBox<T>>,
 }
 
 impl<T: ?Sized> !marker::Send for Rc<T> {}
 impl<T: ?Sized> !marker::Sync for Rc<T> {}
 
+#[cfg(not(stage0))] // remove cfg after new snapshot
 impl<T: ?Sized+Unsize<U>, U: ?Sized> CoerceUnsized<Rc<U>> for Rc<T> {}
 
 impl<T> Rc<T> {
@@ -210,7 +210,7 @@ impl<T> Rc<T> {
                 // pointers, which ensures that the weak destructor never frees
                 // the allocation while the strong destructor is running, even
                 // if the weak pointer is stored inside the strong one.
-                _ptr: NonZero::new(Box::into_raw(box RcBox {
+                _ptr: Shared::new(Box::into_raw(box RcBox {
                     strong: Cell::new(1),
                     weak: Cell::new(1),
                     value: value,
@@ -712,12 +712,13 @@ impl<T> fmt::Pointer for Rc<T> {
 pub struct Weak<T: ?Sized> {
     // FIXME #12808: strange names to try to avoid interfering with
     // field accesses of the contained type via Deref
-    _ptr: NonZero<*mut RcBox<T>>,
+    _ptr: Shared<RcBox<T>>,
 }
 
 impl<T: ?Sized> !marker::Send for Weak<T> {}
 impl<T: ?Sized> !marker::Sync for Weak<T> {}
 
+#[cfg(not(stage0))] // remove cfg after new snapshot
 impl<T: ?Sized+Unsize<U>, U: ?Sized> CoerceUnsized<Weak<U>> for Weak<T> {}
 
 impl<T: ?Sized> Weak<T> {
diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs
@@ -18,11 +18,11 @@
 
 use clone::Clone;
 use intrinsics;
-use ops::Deref;
+use ops::{CoerceUnsized, Deref};
 use fmt;
 use hash;
 use option::Option::{self, Some, None};
-use marker::{PhantomData, Send, Sized, Sync};
+use marker::{Copy, PhantomData, Send, Sized, Sync, Unsize};
 use mem;
 use nonzero::NonZero;
 
@@ -532,3 +532,68 @@ impl<T> fmt::Pointer for Unique<T> {
         fmt::Pointer::fmt(&*self.pointer, f)
     }
 }
+
+/// A wrapper around a raw `*mut T` that indicates that the possessor
+/// of this wrapper has shared ownership of the referent. Useful for
+/// building abstractions like `Rc<T>` or `Arc<T>`, which internally
+/// use raw pointers to manage the memory that they own.
+#[unstable(feature = "shared", reason = "needs an RFC to flesh out design",
+           issue = "0")]
+pub struct Shared<T: ?Sized> {
+    pointer: NonZero<*const T>,
+    // NOTE: this marker has no consequences for variance, but is necessary
+    // for dropck to understand that we logically own a `T`.
+    //
+    // For details, see:
+    // https://github.com/rust-lang/rfcs/blob/master/text/0769-sound-generic-drop.md#phantom-data
+    _marker: PhantomData<T>,
+}
+
+/// `Shared` pointers are not `Send` because the data they reference may be aliased.
+// NB: This impl is unnecessary, but should provide better error messages.
+#[unstable(feature = "shared", issue = "0")]
+impl<T: ?Sized> !Send for Shared<T> { }
+
+/// `Shared` pointers are not `Sync` because the data they reference may be aliased.
+// NB: This impl is unnecessary, but should provide better error messages.
+#[unstable(feature = "shared", issue = "0")]
+impl<T: ?Sized> !Sync for Shared<T> { }
+
+#[unstable(feature = "shared", issue = "0")]
+impl<T: ?Sized> Shared<T> {
+    /// Creates a new `Shared`.
+    pub unsafe fn new(ptr: *mut T) -> Self {
+        Shared { pointer: NonZero::new(ptr), _marker: PhantomData }
+    }
+}
+
+#[unstable(feature = "shared", issue = "0")]
+impl<T: ?Sized> Clone for Shared<T> {
+    fn clone(&self) -> Self {
+        *self
+    }
+}
+
+#[unstable(feature = "shared", issue = "0")]
+impl<T: ?Sized> Copy for Shared<T> { }
+
+#[cfg(not(stage0))] // remove cfg after new snapshot
+#[unstable(feature = "shared", issue = "0")]
+impl<T: ?Sized, U: ?Sized> CoerceUnsized<Shared<U>> for Shared<T> where T: Unsize<U> { }
+
+#[unstable(feature = "shared", issue = "0")]
+impl<T: ?Sized> Deref for Shared<T> {
+    type Target = *mut T;
+
+    #[inline]
+    fn deref(&self) -> &*mut T {
+        unsafe { mem::transmute(&*self.pointer) }
+    }
+}
+
+#[unstable(feature = "shared", issue = "0")]
+impl<T> fmt::Pointer for Shared<T> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Pointer::fmt(&*self.pointer, f)
+    }
+}
diff --git a/src/libstd/sync/mutex.rs b/src/libstd/sync/mutex.rs
@@ -293,6 +293,7 @@ impl<T: ?Sized> Mutex<T> {
 
 #[stable(feature = "rust1", since = "1.0.0")]
 impl<T: ?Sized> Drop for Mutex<T> {
+    #[unsafe_destructor_blind_to_params]
     fn drop(&mut self) {
         // This is actually safe b/c we know that there is no further usage of
         // this mutex (it's up to the user to arrange for a mutex to get
diff --git a/src/libstd/sync/rwlock.rs b/src/libstd/sync/rwlock.rs
@@ -314,6 +314,7 @@ impl<T: ?Sized> RwLock<T> {
 
 #[stable(feature = "rust1", since = "1.0.0")]
 impl<T: ?Sized> Drop for RwLock<T> {
+    #[unsafe_destructor_blind_to_params]
     fn drop(&mut self) {
         // IMPORTANT: This code needs to be kept in sync with `RwLock::into_inner`.
         unsafe { self.inner.lock.destroy() }
diff --git a/src/test/compile-fail/issue-29106.rs b/src/test/compile-fail/issue-29106.rs
@@ -0,0 +1,34 @@
+// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use std::rc::Rc;
+use std::sync::Arc;
+
+struct Foo<'a>(&'a String);
+
+impl<'a> Drop for Foo<'a> {
+    fn drop(&mut self) {
+        println!("{:?}", self.0);
+    }
+}
+
+fn main() {
+    {
+        let (y, x);
+        x = "alive".to_string();
+        y = Arc::new(Foo(&x)); //~ ERROR `x` does not live long enough
+    }
+
+    {
+        let (y, x);
+        x = "alive".to_string();
+        y = Rc::new(Foo(&x)); //~ ERROR `x` does not live long enough
+    }
+}
diff --git a/src/test/run-pass/issue-29037.rs b/src/test/run-pass/issue-29037.rs
@@ -0,0 +1,32 @@
+// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// This test ensures that each pointer type `P<X>` is covariant in `X`.
+
+use std::rc::Rc;
+use std::sync::Arc;
+
+fn a<'r>(x: Box<&'static str>) -> Box<&'r str> {
+    x
+}
+
+fn b<'r, 'w>(x: &'w &'static str) -> &'w &'r str {
+    x
+}
+
+fn c<'r>(x: Arc<&'static str>) -> Arc<&'r str> {
+    x
+}
+
+fn d<'r>(x: Rc<&'static str>) -> Rc<&'r str> {
+    x
+}
+
+fn main() {}
