Skip to content

Commit

Permalink
Remove ptr::Unique
Browse files Browse the repository at this point in the history
rust-lang#71507 (comment) discusses whether `Unique` not actually being unique is UB.

`Unique` has very limited use over `NonNull`, so it doesn’t seem worth
spending a lot of effort defining an abstraction and coming up with
rules about what uses of it are or aren’t sound.
  • Loading branch information
SimonSapin committed Apr 24, 2020
1 parent 0612568 commit fa98ff5
Show file tree
Hide file tree
Showing 31 changed files with 112 additions and 344 deletions.
10 changes: 5 additions & 5 deletions src/liballoc/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#![stable(feature = "alloc_module", since = "1.28.0")]

use core::intrinsics::{self, min_align_of_val, size_of_val};
use core::ptr::{NonNull, Unique};
use core::ptr::NonNull;

#[stable(feature = "alloc_module", since = "1.28.0")]
#[doc(inline)]
Expand Down Expand Up @@ -258,7 +258,7 @@ unsafe impl AllocRef for Global {
}
}

/// The allocator for unique pointers.
/// The allocator for Box.
// This function must not unwind. If it does, MIR codegen will fail.
#[cfg(not(test))]
#[lang = "exchange_malloc"]
Expand All @@ -276,9 +276,9 @@ unsafe fn exchange_malloc(size: usize, align: usize) -> *mut u8 {
// This signature has to be the same as `Box`, otherwise an ICE will happen.
// When an additional parameter to `Box` is added (like `A: AllocRef`), this has to be added here as
// well.
// For example if `Box` is changed to `struct Box<T: ?Sized, A: AllocRef>(Unique<T>, A)`,
// this function has to be changed to `fn box_free<T: ?Sized, A: AllocRef>(Unique<T>, A)` as well.
pub(crate) unsafe fn box_free<T: ?Sized>(ptr: Unique<T>) {
// For example if `Box` is changed to `struct Box<T: ?Sized, A: AllocRef>(NonNull<T>, A)`,
// this function has to be changed to `fn box_free<T: ?Sized, A: AllocRef>(NonNull<T>, A)` as well.
pub(crate) unsafe fn box_free<T: ?Sized>(ptr: NonNull<T>) {
let size = size_of_val(ptr.as_ref());
let align = min_align_of_val(ptr.as_ref());
let layout = Layout::from_size_align_unchecked(size, align);
Expand Down
43 changes: 28 additions & 15 deletions src/liballoc/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,13 @@ use core::fmt;
use core::future::Future;
use core::hash::{Hash, Hasher};
use core::iter::{FromIterator, FusedIterator, Iterator};
use core::marker::{Unpin, Unsize};
use core::marker::{PhantomData, Unpin, Unsize};
use core::mem;
use core::ops::{
CoerceUnsized, Deref, DerefMut, DispatchFromDyn, Generator, GeneratorState, Receiver,
};
use core::pin::Pin;
use core::ptr::{self, NonNull, Unique};
use core::ptr::{self, NonNull};
use core::task::{Context, Poll};

use crate::alloc::{self, AllocInit, AllocRef, Global};
Expand All @@ -156,7 +156,26 @@ use crate::vec::Vec;
#[lang = "owned_box"]
#[fundamental]
#[stable(feature = "rust1", since = "1.0.0")]
pub struct Box<T: ?Sized>(Unique<T>);
pub struct Box<T: ?Sized>(BoxPtr<T>);

// FIXME: remove this struct and give `Box` two fields.
// Currently this causes an ICE with 'assertion failed: sig.c_variadic || extra_args.is_empty()'
struct BoxPtr<T: ?Sized> {
ptr: NonNull<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
_phantom: PhantomData<T>,
}

#[stable(feature = "rust1", since = "1.0.0")]
unsafe impl<T: Send + ?Sized> Send for Box<T> {}

#[stable(feature = "rust1", since = "1.0.0")]
unsafe impl<T: Sync + ?Sized> Sync for Box<T> {}

impl<T> Box<T> {
/// Allocates memory on the heap and then places `x` into it.
Expand Down Expand Up @@ -382,7 +401,7 @@ impl<T: ?Sized> Box<T> {
#[stable(feature = "box_raw", since = "1.4.0")]
#[inline]
pub unsafe fn from_raw(raw: *mut T) -> Self {
Box(Unique::new_unchecked(raw))
Box(BoxPtr { ptr: NonNull::new_unchecked(raw), _phantom: PhantomData })
}

/// Consumes the `Box`, returning a wrapped raw pointer.
Expand Down Expand Up @@ -462,22 +481,14 @@ impl<T: ?Sized> Box<T> {
#[unstable(feature = "box_into_raw_non_null", issue = "47336")]
#[inline]
pub fn into_raw_non_null(b: Box<T>) -> NonNull<T> {
Box::into_unique(b).into()
}

#[unstable(feature = "ptr_internals", issue = "none", reason = "use into_raw_non_null instead")]
#[inline]
#[doc(hidden)]
pub fn into_unique(b: Box<T>) -> Unique<T> {
let b = mem::ManuallyDrop::new(b);
let mut unique = b.0;
let mut b = mem::ManuallyDrop::new(b);
// Box is kind-of a library type, but recognized as a "unique pointer" by
// Stacked Borrows. This function here corresponds to "reborrowing to
// a raw pointer", but there is no actual reborrow here -- so
// without some care, the pointer we are returning here still carries
// the tag of `b`, with `Unique` permission.
// We round-trip through a mutable reference to avoid that.
unsafe { Unique::new_unchecked(unique.as_mut() as *mut T) }
unsafe { NonNull::new_unchecked(b.0.ptr.as_mut() as *mut T) }
}

/// Consumes and leaks the `Box`, returning a mutable reference,
Expand Down Expand Up @@ -910,7 +921,7 @@ impl<T: fmt::Debug + ?Sized> fmt::Debug for Box<T> {
impl<T: ?Sized> fmt::Pointer for Box<T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
// It's not possible to extract the inner Uniq directly from the Box,
// instead we cast it to a *const which aliases the Unique
// instead we cast it to a *const which aliases the NonNull
let ptr: *const T = &**self;
fmt::Pointer::fmt(&ptr, f)
}
Expand Down Expand Up @@ -1025,9 +1036,11 @@ impl<A, F: Fn<A> + ?Sized> Fn<A> for Box<F> {

#[unstable(feature = "coerce_unsized", issue = "27732")]
impl<T: ?Sized + Unsize<U>, U: ?Sized> CoerceUnsized<Box<U>> for Box<T> {}
impl<T: ?Sized + Unsize<U>, U: ?Sized> CoerceUnsized<BoxPtr<U>> for BoxPtr<T> {}

#[unstable(feature = "dispatch_from_dyn", issue = "none")]
impl<T: ?Sized + Unsize<U>, U: ?Sized> DispatchFromDyn<Box<U>> for Box<T> {}
impl<T: ?Sized + Unsize<U>, U: ?Sized> DispatchFromDyn<BoxPtr<U>> for BoxPtr<T> {}

#[stable(feature = "boxed_slice_from_iter", since = "1.32.0")]
impl<A> FromIterator<A> for Box<[A]> {
Expand Down
10 changes: 5 additions & 5 deletions src/liballoc/collections/btree/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
use core::cmp::Ordering;
use core::marker::PhantomData;
use core::mem::{self, MaybeUninit};
use core::ptr::{self, NonNull, Unique};
use core::ptr::{self, NonNull};
use core::slice;

use crate::alloc::{AllocRef, Global, Layout};
Expand Down Expand Up @@ -118,20 +118,20 @@ impl<K, V> InternalNode<K, V> {
/// of nodes it actually contains, and, partially due to this lack of information,
/// has no destructor.
struct BoxedNode<K, V> {
ptr: Unique<LeafNode<K, V>>,
ptr: NonNull<LeafNode<K, V>>,
}

impl<K, V> BoxedNode<K, V> {
fn from_leaf(node: Box<LeafNode<K, V>>) -> Self {
BoxedNode { ptr: Box::into_unique(node) }
BoxedNode { ptr: Box::into_raw_non_null(node) }
}

fn from_internal(node: Box<InternalNode<K, V>>) -> Self {
BoxedNode { ptr: Box::into_unique(node).cast() }
BoxedNode { ptr: Box::into_raw_non_null(node).cast() }
}

unsafe fn from_ptr(ptr: NonNull<LeafNode<K, V>>) -> Self {
BoxedNode { ptr: Unique::from(ptr) }
BoxedNode { ptr: NonNull::from(ptr) }
}

fn as_ptr(&self) -> NonNull<LeafNode<K, V>> {
Expand Down
1 change: 0 additions & 1 deletion src/liballoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@
#![feature(optin_builtin_traits)]
#![feature(or_patterns)]
#![feature(pattern)]
#![feature(ptr_internals)]
#![feature(ptr_offset_from)]
#![feature(rustc_attrs)]
#![feature(receiver_trait)]
Expand Down
34 changes: 25 additions & 9 deletions src/liballoc/raw_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@

use core::alloc::MemoryBlock;
use core::cmp;
use core::marker::PhantomData;
use core::mem::{self, ManuallyDrop, MaybeUninit};
use core::ops::Drop;
use core::ptr::{NonNull, Unique};
use core::ptr::NonNull;
use core::slice;

use crate::alloc::{
Expand All @@ -25,14 +26,15 @@ mod tests;
/// involved. This type is excellent for building your own data structures like Vec and VecDeque.
/// In particular:
///
/// * Produces `Unique::empty()` on zero-sized types.
/// * Produces `Unique::empty()` on zero-length allocations.
/// * Avoids freeing `Unique::empty()`.
/// * Produces `NonNull::dangling()` on zero-sized types.
/// * Produces `NonNull::dangling()` on zero-length allocations.
/// * Avoids freeing `NonNull::dangling()`.
/// * Catches all overflows in capacity computations (promotes them to "capacity overflow" panics).
/// * Guards against 32-bit systems allocating more than isize::MAX bytes.
/// * Guards against overflowing your length.
/// * Calls `handle_alloc_error` for fallible allocations.
/// * Contains a `ptr::Unique` and thus endows the user with all related benefits.
/// * Contains a `ptr::NonNull` and thus endows the user with enum layout optimizations.
/// * Is `Send` and `Sync` when items are.
/// * Uses the excess returned from the allocator to use the largest available capacity.
///
/// This type does not in anyway inspect the memory that it manages. When dropped it *will*
Expand All @@ -44,11 +46,24 @@ mod tests;
/// `Box<[T]>`, since `capacity()` won't yield the length.
#[allow(missing_debug_implementations)]
pub struct RawVec<T, A: AllocRef = Global> {
ptr: Unique<T>,
ptr: NonNull<T>,
cap: usize,
alloc: A,

// 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
_phantom: PhantomData<T>,
}

#[stable(feature = "rust1", since = "1.0.0")]
unsafe impl<T: Send, A: AllocRef + Send> Send for RawVec<T, A> {}

#[stable(feature = "rust1", since = "1.0.0")]
unsafe impl<T: Sync, A: AllocRef + Sync> Sync for RawVec<T, A> {}

impl<T> RawVec<T, Global> {
/// HACK(Centril): This exists because `#[unstable]` `const fn`s needn't conform
/// to `min_const_fn` and so they cannot be called in `min_const_fn`s either.
Expand Down Expand Up @@ -125,7 +140,7 @@ impl<T, A: AllocRef> RawVec<T, A> {
/// the returned `RawVec`.
pub const fn new_in(alloc: A) -> Self {
// `cap: 0` means "unallocated". zero-sized types are ignored.
Self { ptr: Unique::empty(), cap: 0, alloc }
Self { ptr: NonNull::dangling(), cap: 0, alloc, _phantom: PhantomData }
}

/// Like `with_capacity`, but parameterized over the choice of
Expand Down Expand Up @@ -154,6 +169,7 @@ impl<T, A: AllocRef> RawVec<T, A> {
ptr: memory.ptr.cast().into(),
cap: Self::capacity_from_bytes(memory.size),
alloc,
_phantom: PhantomData,
}
}
}
Expand All @@ -168,11 +184,11 @@ impl<T, A: AllocRef> RawVec<T, A> {
/// If the `ptr` and `capacity` come from a `RawVec` created via `a`, then this is guaranteed.
#[inline]
pub unsafe fn from_raw_parts_in(ptr: *mut T, capacity: usize, a: A) -> Self {
Self { ptr: Unique::new_unchecked(ptr), cap: capacity, alloc: a }
Self { ptr: NonNull::new_unchecked(ptr), cap: capacity, alloc: a, _phantom: PhantomData }
}

/// Gets a raw pointer to the start of the allocation. Note that this is
/// `Unique::empty()` if `capacity == 0` or `T` is zero-sized. In the former case, you must
/// `NonNull::dangling()` if `capacity == 0` or `T` is zero-sized. In the former case, you must
/// be careful.
pub fn ptr(&self) -> *mut T {
self.ptr.as_ptr()
Expand Down
6 changes: 3 additions & 3 deletions src/liballoc/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -984,8 +984,8 @@ impl<T: ?Sized> Rc<T> {

fn from_box(v: Box<T>) -> Rc<T> {
unsafe {
let box_unique = Box::into_unique(v);
let bptr = box_unique.as_ptr();
let box_non_null = Box::into_raw_non_null(v);
let bptr = box_non_null.as_ptr();

let value_size = size_of_val(&*bptr);
let ptr = Self::allocate_for_ptr(bptr);
Expand All @@ -998,7 +998,7 @@ impl<T: ?Sized> Rc<T> {
);

// Free the allocation without dropping its contents
box_free(box_unique);
box_free(box_non_null);

Self::from_ptr(ptr)
}
Expand Down
6 changes: 3 additions & 3 deletions src/liballoc/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -862,8 +862,8 @@ impl<T: ?Sized> Arc<T> {

fn from_box(v: Box<T>) -> Arc<T> {
unsafe {
let box_unique = Box::into_unique(v);
let bptr = box_unique.as_ptr();
let box_non_null = Box::into_raw_non_null(v);
let bptr = box_non_null.as_ptr();

let value_size = size_of_val(&*bptr);
let ptr = Self::allocate_for_ptr(bptr);
Expand All @@ -876,7 +876,7 @@ impl<T: ?Sized> Arc<T> {
);

// Free the allocation without dropping its contents
box_free(box_unique);
box_free(box_non_null);

Self::from_ptr(ptr)
}
Expand Down
4 changes: 0 additions & 4 deletions src/libcore/ptr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,6 @@ mod non_null;
#[stable(feature = "nonnull", since = "1.25.0")]
pub use non_null::NonNull;

mod unique;
#[unstable(feature = "ptr_internals", issue = "none")]
pub use unique::Unique;

mod const_ptr;
mod mut_ptr;

Expand Down
9 changes: 0 additions & 9 deletions src/libcore/ptr/non_null.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use crate::hash;
use crate::marker::Unsize;
use crate::mem;
use crate::ops::{CoerceUnsized, DispatchFromDyn};
use crate::ptr::Unique;

// ignore-tidy-undocumented-unsafe

Expand Down Expand Up @@ -201,14 +200,6 @@ impl<T: ?Sized> hash::Hash for NonNull<T> {
}
}

#[unstable(feature = "ptr_internals", issue = "none")]
impl<T: ?Sized> From<Unique<T>> for NonNull<T> {
#[inline]
fn from(unique: Unique<T>) -> Self {
unsafe { NonNull::new_unchecked(unique.as_ptr()) }
}
}

#[stable(feature = "nonnull", since = "1.25.0")]
impl<T: ?Sized> From<&mut T> for NonNull<T> {
#[inline]
Expand Down
Loading

0 comments on commit fa98ff5

Please sign in to comment.