Skip to content

Commit

Permalink
Auto merge of #46749 - SimonSapin:exorcism, r=nikomatsakis
Browse files Browse the repository at this point in the history
Move PhantomData<T> from Shared<T> to users of both Shared and #[may_dangle]

After discussing #27730 (comment) today with @pnkfelix and @gankro, we concluded that it’s ok for drop checking not to be much smarter than the current `#[may_dangle]` design which requires an explicit unsafe opt-in.
  • Loading branch information
bors committed Dec 19, 2017
2 parents a9f047c + 60dc104 commit b39c4bc
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 36 deletions.
16 changes: 9 additions & 7 deletions src/liballoc/arc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use core::mem::{self, align_of_val, size_of_val, uninitialized};
use core::ops::Deref;
use core::ops::CoerceUnsized;
use core::ptr::{self, Shared};
use core::marker::Unsize;
use core::marker::{Unsize, PhantomData};
use core::hash::{Hash, Hasher};
use core::{isize, usize};
use core::convert::From;
Expand Down Expand Up @@ -198,6 +198,7 @@ const MAX_REFCOUNT: usize = (isize::MAX) as usize;
#[stable(feature = "rust1", since = "1.0.0")]
pub struct Arc<T: ?Sized> {
ptr: Shared<ArcInner<T>>,
phantom: PhantomData<T>,
}

#[stable(feature = "rust1", since = "1.0.0")]
Expand Down Expand Up @@ -285,7 +286,7 @@ impl<T> Arc<T> {
weak: atomic::AtomicUsize::new(1),
data,
};
Arc { ptr: Shared::from(Box::into_unique(x)) }
Arc { ptr: Shared::from(Box::into_unique(x)), phantom: PhantomData }
}

/// Returns the contained value, if the `Arc` has exactly one strong reference.
Expand Down Expand Up @@ -397,6 +398,7 @@ impl<T: ?Sized> Arc<T> {

Arc {
ptr: Shared::new_unchecked(arc_ptr),
phantom: PhantomData,
}
}

Expand Down Expand Up @@ -580,7 +582,7 @@ impl<T: ?Sized> Arc<T> {
// Free the allocation without dropping its contents
box_free(bptr);

Arc { ptr: Shared::new_unchecked(ptr) }
Arc { ptr: Shared::new_unchecked(ptr), phantom: PhantomData }
}
}
}
Expand All @@ -607,7 +609,7 @@ impl<T> Arc<[T]> {
&mut (*ptr).data as *mut [T] as *mut T,
v.len());

Arc { ptr: Shared::new_unchecked(ptr) }
Arc { ptr: Shared::new_unchecked(ptr), phantom: PhantomData }
}
}

Expand Down Expand Up @@ -667,7 +669,7 @@ impl<T: Clone> ArcFromSlice<T> for Arc<[T]> {
// All clear. Forget the guard so it doesn't free the new ArcInner.
mem::forget(guard);

Arc { ptr: Shared::new_unchecked(ptr) }
Arc { ptr: Shared::new_unchecked(ptr), phantom: PhantomData }
}
}
}
Expand Down Expand Up @@ -725,7 +727,7 @@ impl<T: ?Sized> Clone for Arc<T> {
}
}

Arc { ptr: self.ptr }
Arc { ptr: self.ptr, phantom: PhantomData }
}
}

Expand Down Expand Up @@ -1052,7 +1054,7 @@ impl<T: ?Sized> Weak<T> {

// Relaxed is valid for the same reason it is on Arc's Clone impl
match inner.strong.compare_exchange_weak(n, n + 1, Relaxed, Relaxed) {
Ok(_) => return Some(Arc { ptr: self.ptr }),
Ok(_) => return Some(Arc { ptr: self.ptr, phantom: PhantomData }),
Err(old) => n = old,
}
}
Expand Down
16 changes: 10 additions & 6 deletions src/liballoc/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ use core::fmt;
use core::hash::{Hash, Hasher};
use core::intrinsics::abort;
use core::marker;
use core::marker::Unsize;
use core::marker::{Unsize, PhantomData};
use core::mem::{self, align_of_val, forget, size_of_val, uninitialized};
use core::ops::Deref;
use core::ops::CoerceUnsized;
Expand Down Expand Up @@ -283,6 +283,7 @@ struct RcBox<T: ?Sized> {
#[stable(feature = "rust1", since = "1.0.0")]
pub struct Rc<T: ?Sized> {
ptr: Shared<RcBox<T>>,
phantom: PhantomData<T>,
}

#[stable(feature = "rust1", since = "1.0.0")]
Expand Down Expand Up @@ -315,6 +316,7 @@ impl<T> Rc<T> {
weak: Cell::new(1),
value,
})),
phantom: PhantomData,
}
}

Expand Down Expand Up @@ -427,6 +429,7 @@ impl<T: ?Sized> Rc<T> {

Rc {
ptr: Shared::new_unchecked(rc_ptr),
phantom: PhantomData,
}
}

Expand Down Expand Up @@ -647,6 +650,7 @@ impl Rc<Any> {
forget(self);
Ok(Rc {
ptr: Shared::new_unchecked(raw as *const RcBox<T> as *mut _),
phantom: PhantomData,
})
}
} else {
Expand Down Expand Up @@ -691,7 +695,7 @@ impl<T: ?Sized> Rc<T> {
// Free the allocation without dropping its contents
box_free(bptr);

Rc { ptr: Shared::new_unchecked(ptr) }
Rc { ptr: Shared::new_unchecked(ptr), phantom: PhantomData }
}
}
}
Expand All @@ -718,7 +722,7 @@ impl<T> Rc<[T]> {
&mut (*ptr).value as *mut [T] as *mut T,
v.len());

Rc { ptr: Shared::new_unchecked(ptr) }
Rc { ptr: Shared::new_unchecked(ptr), phantom: PhantomData }
}
}

Expand Down Expand Up @@ -777,7 +781,7 @@ impl<T: Clone> RcFromSlice<T> for Rc<[T]> {
// All clear. Forget the guard so it doesn't free the new RcBox.
forget(guard);

Rc { ptr: Shared::new_unchecked(ptr) }
Rc { ptr: Shared::new_unchecked(ptr), phantom: PhantomData }
}
}
}
Expand Down Expand Up @@ -868,7 +872,7 @@ impl<T: ?Sized> Clone for Rc<T> {
#[inline]
fn clone(&self) -> Rc<T> {
self.inc_strong();
Rc { ptr: self.ptr }
Rc { ptr: self.ptr, phantom: PhantomData }
}
}

Expand Down Expand Up @@ -1228,7 +1232,7 @@ impl<T: ?Sized> Weak<T> {
None
} else {
self.inc_strong();
Some(Rc { ptr: self.ptr })
Some(Rc { ptr: self.ptr, phantom: PhantomData })
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/liballoc/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ use core::fmt;
use core::hash::{self, Hash};
use core::intrinsics::{arith_offset, assume};
use core::iter::{FromIterator, FusedIterator, TrustedLen};
use core::marker::PhantomData;
use core::mem;
#[cfg(not(test))]
use core::num::Float;
Expand Down Expand Up @@ -1743,6 +1744,7 @@ impl<T> IntoIterator for Vec<T> {
mem::forget(self);
IntoIter {
buf: Shared::new_unchecked(begin),
phantom: PhantomData,
cap,
ptr: begin,
end,
Expand Down Expand Up @@ -2264,6 +2266,7 @@ impl<'a, T> FromIterator<T> for Cow<'a, [T]> where T: Clone {
#[stable(feature = "rust1", since = "1.0.0")]
pub struct IntoIter<T> {
buf: Shared<T>,
phantom: PhantomData<T>,
cap: usize,
ptr: *const T,
end: *const T,
Expand Down
35 changes: 12 additions & 23 deletions src/libcore/ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2450,16 +2450,11 @@ impl<'a, T: ?Sized> From<&'a T> for Unique<T> {
}
}

/// 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>`, `Arc<T>`, or doubly-linked lists, which
/// internally use aliased raw pointers to manage the memory that they own.
/// `*mut T` but non-zero and covariant.
///
/// 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.
/// This is often the correct thing to use when building data structures using
/// raw pointers, but is ultimately more dangerous to use because of its additional
/// properties. If you're not sure if you should use `Shared<T>`, just use `*mut T`!
///
/// 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
Expand All @@ -2469,20 +2464,14 @@ impl<'a, T: ?Sized> From<&'a T> for Unique<T> {
/// 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.
/// Usually this won't be necessary; covariance is correct for most safe abstractions,
/// such as Box, Rc, Arc, Vec, and LinkedList. This is the case 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")]
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.
Expand Down Expand Up @@ -2518,12 +2507,12 @@ impl<T: ?Sized> Shared<T> {
/// `ptr` must be non-null.
#[unstable(feature = "shared", issue = "27730")]
pub const unsafe fn new_unchecked(ptr: *mut T) -> Self {
Shared { pointer: NonZero::new_unchecked(ptr), _marker: PhantomData }
Shared { pointer: NonZero::new_unchecked(ptr) }
}

/// Creates a new `Shared` if `ptr` is non-null.
pub fn new(ptr: *mut T) -> Option<Self> {
NonZero::new(ptr as *const T).map(|nz| Shared { pointer: nz, _marker: PhantomData })
NonZero::new(ptr as *const T).map(|nz| Shared { pointer: nz })
}

/// Acquires the underlying `*mut` pointer.
Expand Down Expand Up @@ -2580,20 +2569,20 @@ impl<T: ?Sized> fmt::Pointer for Shared<T> {
#[unstable(feature = "shared", issue = "27730")]
impl<T: ?Sized> From<Unique<T>> for Shared<T> {
fn from(unique: Unique<T>) -> Self {
Shared { pointer: unique.pointer, _marker: PhantomData }
Shared { pointer: unique.pointer }
}
}

#[unstable(feature = "shared", issue = "27730")]
impl<'a, T: ?Sized> From<&'a mut T> for Shared<T> {
fn from(reference: &'a mut T) -> Self {
Shared { pointer: NonZero::from(reference), _marker: PhantomData }
Shared { pointer: NonZero::from(reference) }
}
}

#[unstable(feature = "shared", issue = "27730")]
impl<'a, T: ?Sized> From<&'a T> for Shared<T> {
fn from(reference: &'a T) -> Self {
Shared { pointer: NonZero::from(reference), _marker: PhantomData }
Shared { pointer: NonZero::from(reference) }
}
}

0 comments on commit b39c4bc

Please sign in to comment.