Rollup merge of #41064 - Gankro:ptr-redux, r=alexcrichton

refactor NonZero, Shared, and Unique APIs

Major difference is that I removed Deref impls, as apparently LLVM has
trouble maintaining metadata with a `&ptr -> &ptr` API. This was cited
as a blocker for ever stabilizing this API. It wasn't that ergonomic
anyway.

* Added `get` to NonZero to replace Deref impl
* Added `ptr` getter to Shared/Unique to replace Deref impl
* Added Unique's `get` and `get_mut` conveniences to Shared
* Deprecated `as_mut_ptr` on Shared in favour of `ptr`

Note that Shared used to primarily expose only `*const` but there isn't
a good justification for that, so I made it `*mut`.

5 files changed, 138 insertions, 62 deletions

diff --git a/src/libcore/cell.rs b/src/libcore/cell.rs
index f62057b3a52..7886f90b66e 100644
--- a/src/libcore/cell.rs
+++ b/src/libcore/cell.rs
@@ -132,7 +132,6 @@
 //! use std::cell::Cell;
 //! use std::ptr::Shared;
 //! use std::intrinsics::abort;
-//! use std::intrinsics::assume;
 //!
 //! struct Rc<T: ?Sized> {
 //!     ptr: Shared<RcBox<T>>
@@ -171,8 +170,7 @@
 //! impl<T: ?Sized> RcBoxPtr<T> for Rc<T> {
 //!    fn inner(&self) -> &RcBox<T> {
 //!        unsafe {
-//!            assume(!(*(&self.ptr as *const _ as *const *const ())).is_null());
-//!            &(**self.ptr)
+//!            self.ptr.as_ref()
 //!        }
 //!    }
 //! }
diff --git a/src/libcore/nonzero.rs b/src/libcore/nonzero.rs
index d7382501bc3..d93085e96db 100644
--- a/src/libcore/nonzero.rs
+++ b/src/libcore/nonzero.rs
@@ -13,7 +13,7 @@
             reason = "needs an RFC to flesh out the design",
             issue = "27730")]
 
-use ops::{CoerceUnsized, Deref};
+use ops::CoerceUnsized;
 
 /// Unsafe trait to indicate what types are usable with the NonZero struct
 pub unsafe trait Zeroable {}
@@ -46,15 +46,10 @@ impl<T: Zeroable> NonZero<T> {
     pub const unsafe fn new(inner: T) -> NonZero<T> {
         NonZero(inner)
     }
-}
-
-impl<T: Zeroable> Deref for NonZero<T> {
-    type Target = T;
 
-    #[inline]
-    fn deref(&self) -> &T {
-        let NonZero(ref inner) = *self;
-        inner
+    /// Gets the inner value.
+    pub fn get(self) -> T {
+        self.0
     }
 }
 
diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs
index 115326bb916..a60abefc076 100644
--- a/src/libcore/ptr.rs
+++ b/src/libcore/ptr.rs
@@ -17,7 +17,7 @@
 #![stable(feature = "rust1", since = "1.0.0")]
 
 use intrinsics;
-use ops::{CoerceUnsized, Deref};
+use ops::CoerceUnsized;
 use fmt;
 use hash;
 use marker::{PhantomData, Unsize};
@@ -957,13 +957,25 @@ impl<T: ?Sized> PartialOrd for *mut T {
 }
 
 /// A wrapper around a raw non-null `*mut T` that indicates that the possessor
-/// of this wrapper owns the referent. This in turn implies that the
-/// `Unique<T>` is `Send`/`Sync` if `T` is `Send`/`Sync`, unlike a raw
-/// `*mut T` (which conveys no particular ownership semantics).  It
-/// also implies that the referent of the pointer should not be
-/// modified without a unique path to the `Unique` reference. Useful
-/// for building abstractions like `Vec<T>` or `Box<T>`, which
-/// internally use raw pointers to manage the memory that they own.
+/// of this wrapper owns the referent. Useful for building abstractions like
+/// `Box<T>`, `Vec<T>`, `String`, and `HashMap<K, V>`.
+///
+/// Unlike `*mut T`, `Unique<T>` behaves "as if" it were an instance of `T`.
+/// It implements `Send`/`Sync` if `T` is `Send`/`Sync`. It also implies
+/// the kind of strong aliasing guarantees an instance of `T` can expect:
+/// the referent of the pointer should not be modified without a unique path to
+/// its owning Unique.
+///
+/// If you're uncertain of whether it's correct to use `Unique` for your purposes,
+/// consider using `Shared`, which has weaker semantics.
+///
+/// Unlike `*mut T`, the pointer must always be non-null, even if the pointer
+/// is never dereferenced. This is so that enums may use this forbidden value
+/// as a discriminant -- `Option<Unique<T>>` has the same size as `Unique<T>`.
+/// However the pointer may still dangle if it isn't dereferenced.
+///
+/// Unlike `*mut T`, `Unique<T>` is covariant over `T`. This should always be correct
+/// for any type which upholds Unique's aliasing requirements.
 #[allow(missing_debug_implementations)]
 #[unstable(feature = "unique", reason = "needs an RFC to flesh out design",
            issue = "27730")]
@@ -992,6 +1004,20 @@ unsafe impl<T: Send + ?Sized> Send for Unique<T> { }
 unsafe impl<T: Sync + ?Sized> Sync for Unique<T> { }
 
 #[unstable(feature = "unique", issue = "27730")]
+impl<T: Sized> Unique<T> {
+    /// Creates a new `Shared` that is dangling, but well-aligned.
+    ///
+    /// This is useful for initializing types which lazily allocate, like
+    /// `Vec::new` does.
+    pub fn empty() -> Self {
+        unsafe {
+            let ptr = mem::align_of::<T>() as *mut T;
+            Unique::new(ptr)
+        }
+    }
+}
+
+#[unstable(feature = "unique", issue = "27730")]
 impl<T: ?Sized> Unique<T> {
     /// Creates a new `Unique`.
     ///
@@ -1002,41 +1028,72 @@ impl<T: ?Sized> Unique<T> {
         Unique { pointer: NonZero::new(ptr), _marker: PhantomData }
     }
 
+    /// Acquires the underlying `*mut` pointer.
+    pub fn as_ptr(self) -> *mut T {
+        self.pointer.get() as *mut T
+    }
+
     /// Dereferences the content.
-    pub unsafe fn get(&self) -> &T {
-        &**self.pointer
+    ///
+    /// The resulting lifetime is bound to self so this behaves "as if"
+    /// it were actually an instance of T that is getting borrowed. If a longer
+    /// (unbound) lifetime is needed, use `&*my_ptr.ptr()`.
+    pub unsafe fn as_ref(&self) -> &T {
+        &*self.as_ptr()
     }
 
     /// Mutably dereferences the content.
-    pub unsafe fn get_mut(&mut self) -> &mut T {
-        &mut ***self
+    ///
+    /// The resulting lifetime is bound to self so this behaves "as if"
+    /// it were actually an instance of T that is getting borrowed. If a longer
+    /// (unbound) lifetime is needed, use `&mut *my_ptr.ptr()`.
+    pub unsafe fn as_mut(&mut self) -> &mut T {
+        &mut *self.as_ptr()
     }
 }
 
-#[unstable(feature = "unique", issue = "27730")]
-impl<T: ?Sized, U: ?Sized> CoerceUnsized<Unique<U>> for Unique<T> where T: Unsize<U> { }
-
-#[unstable(feature = "unique", issue= "27730")]
-impl<T:?Sized> Deref for Unique<T> {
-    type Target = *mut T;
-
-    #[inline]
-    fn deref(&self) -> &*mut T {
-        unsafe { mem::transmute(&*self.pointer) }
+#[unstable(feature = "shared", issue = "27730")]
+impl<T: ?Sized> Clone for Unique<T> {
+    fn clone(&self) -> Self {
+        *self
     }
 }
 
+#[unstable(feature = "shared", issue = "27730")]
+impl<T: ?Sized> Copy for Unique<T> { }
+
+#[unstable(feature = "unique", issue = "27730")]
+impl<T: ?Sized, U: ?Sized> CoerceUnsized<Unique<U>> for Unique<T> where T: Unsize<U> { }
+
 #[unstable(feature = "unique", issue = "27730")]
-impl<T> fmt::Pointer for Unique<T> {
+impl<T: ?Sized> fmt::Pointer for Unique<T> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        fmt::Pointer::fmt(&*self.pointer, f)
+        fmt::Pointer::fmt(&self.as_ptr(), f)
     }
 }
 
-/// A wrapper around a raw non-null `*mut T` that indicates that the possessor
+/// 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.
+/// building abstractions like `Rc<T>`, `Arc<T>`, or doubly-linked lists, which
+/// internally use aliased raw pointers to manage the memory that they own.
+///
+/// This is similar to `Unique`, except that it doesn't make any aliasing
+/// guarantees, and doesn't derive Send and Sync. Note that unlike `&T`,
+/// Shared has no special mutability requirements. Shared may mutate data
+/// aliased by other Shared pointers. More precise rules require Rust to
+/// develop an actual aliasing model.
+///
+/// Unlike `*mut T`, the pointer must always be non-null, even if the pointer
+/// is never dereferenced. This is so that enums may use this forbidden value
+/// as a discriminant -- `Option<Shared<T>>` has the same size as `Shared<T>`.
+/// However the pointer may still dangle if it isn't dereferenced.
+///
+/// Unlike `*mut T`, `Shared<T>` is covariant over `T`. If this is incorrect
+/// for your use case, you should include some PhantomData in your type to
+/// provide invariance, such as `PhantomData<Cell<T>>` or `PhantomData<&'a mut T>`.
+/// Usually this won't be necessary; covariance is correct for Rc, Arc, and LinkedList
+/// because they provide a public API that follows the normal shared XOR mutable
+/// rules of Rust.
 #[allow(missing_debug_implementations)]
 #[unstable(feature = "shared", reason = "needs an RFC to flesh out design",
            issue = "27730")]
@@ -1061,22 +1118,58 @@ impl<T: ?Sized> !Send for Shared<T> { }
 impl<T: ?Sized> !Sync for Shared<T> { }
 
 #[unstable(feature = "shared", issue = "27730")]
+impl<T: Sized> Shared<T> {
+    /// Creates a new `Shared` that is dangling, but well-aligned.
+    ///
+    /// This is useful for initializing types which lazily allocate, like
+    /// `Vec::new` does.
+    pub fn empty() -> Self {
+        unsafe {
+            let ptr = mem::align_of::<T>() as *mut T;
+            Shared::new(ptr)
+        }
+    }
+}
+
+#[unstable(feature = "shared", issue = "27730")]
 impl<T: ?Sized> Shared<T> {
     /// Creates a new `Shared`.
     ///
     /// # Safety
     ///
     /// `ptr` must be non-null.
-    pub unsafe fn new(ptr: *const T) -> Self {
+    pub unsafe fn new(ptr: *mut T) -> Self {
         Shared { pointer: NonZero::new(ptr), _marker: PhantomData }
     }
-}
 
-#[unstable(feature = "shared", issue = "27730")]
-impl<T: ?Sized> Shared<T> {
+    /// Acquires the underlying `*mut` pointer.
+    pub fn as_ptr(self) -> *mut T {
+        self.pointer.get() as *mut T
+    }
+
+    /// Dereferences the content.
+    ///
+    /// The resulting lifetime is bound to self so this behaves "as if"
+    /// it were actually an instance of T that is getting borrowed. If a longer
+    /// (unbound) lifetime is needed, use `&*my_ptr.ptr()`.
+    pub unsafe fn as_ref(&self) -> &T {
+        &*self.as_ptr()
+    }
+
+    /// Mutably dereferences the content.
+    ///
+    /// The resulting lifetime is bound to self so this behaves "as if"
+    /// it were actually an instance of T that is getting borrowed. If a longer
+    /// (unbound) lifetime is needed, use `&mut *my_ptr.ptr_mut()`.
+    pub unsafe fn as_mut(&mut self) -> &mut T {
+        &mut *self.as_ptr()
+    }
+
     /// Acquires the underlying pointer as a `*mut` pointer.
+    #[rustc_deprecated(since = "1.19", reason = "renamed to `as_ptr` for ergonomics/consistency")]
+    #[unstable(feature = "shared", issue = "27730")]
     pub unsafe fn as_mut_ptr(&self) -> *mut T {
-        **self as _
+        self.as_ptr()
     }
 }
 
@@ -1094,18 +1187,8 @@ impl<T: ?Sized> Copy for Shared<T> { }
 impl<T: ?Sized, U: ?Sized> CoerceUnsized<Shared<U>> for Shared<T> where T: Unsize<U> { }
 
 #[unstable(feature = "shared", issue = "27730")]
-impl<T: ?Sized> Deref for Shared<T> {
-    type Target = *const T;
-
-    #[inline]
-    fn deref(&self) -> &*const T {
-        unsafe { mem::transmute(&*self.pointer) }
-    }
-}
-
-#[unstable(feature = "shared", issue = "27730")]
-impl<T> fmt::Pointer for Shared<T> {
+impl<T: ?Sized> fmt::Pointer for Shared<T> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        fmt::Pointer::fmt(&*self.pointer, f)
+        fmt::Pointer::fmt(&self.as_ptr(), f)
     }
 }
diff --git a/src/libcore/tests/nonzero.rs b/src/libcore/tests/nonzero.rs
index 7a367ddeec8..588fffda35f 100644
--- a/src/libcore/tests/nonzero.rs
+++ b/src/libcore/tests/nonzero.rs
@@ -31,12 +31,12 @@ fn test_match_on_nonzero_option() {
         NonZero::new(42)
     });
     match a {
-        Some(val) => assert_eq!(*val, 42),
+        Some(val) => assert_eq!(val.get(), 42),
         None => panic!("unexpected None while matching on Some(NonZero(_))")
     }
 
     match unsafe { Some(NonZero::new(43)) } {
-        Some(val) => assert_eq!(*val, 43),
+        Some(val) => assert_eq!(val.get(), 43),
         None => panic!("unexpected None while matching on Some(NonZero(_))")
     }
 }
diff --git a/src/libcore/tests/ptr.rs b/src/libcore/tests/ptr.rs
index 7f6f472bfbb..e28dc6a6881 100644
--- a/src/libcore/tests/ptr.rs
+++ b/src/libcore/tests/ptr.rs
@@ -166,10 +166,10 @@ fn test_set_memory() {
 
 #[test]
 fn test_unsized_unique() {
-    let xs: &mut [i32] = &mut [1, 2, 3];
-    let ptr = unsafe { Unique::new(xs as *mut [i32]) };
-    let ys = unsafe { &mut **ptr };
-    let zs: &mut [i32] = &mut [1, 2, 3];
+    let xs: &[i32] = &[1, 2, 3];
+    let ptr = unsafe { Unique::new(xs as *const [i32] as *mut [i32]) };
+    let ys = unsafe { ptr.as_ref() };
+    let zs: &[i32] = &[1, 2, 3];
     assert!(ys == zs);
 }