Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove ptr::Unique #71530

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this PR conflicts badly with #71168.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ll deal with it after one of them lands.

// 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.
Comment on lines 485 to 490
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RalfJung Could you say some more about how and why Box is special in Stacked Borrows? Could it be "just" another library type? Are Vec or Rc for example also special?

Copy link
Member

@RalfJung RalfJung Apr 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The how: Box is basically like &mut (except that unlike &mut we never add "protectors" for Box, but that's a detail).

The why: As far as I know, rustc emits noalias annotations for Box. This demonstrates that it is not "just another library type". Those annotations are incorrect unless our aliasing model (i.e., Stacked Borrows) actually treats Box special. Additionally, back when I started with Stacked Borrows, we also added dereferencable annotations. Right now we do not do that any more, but the plan is to start doing that again once LLVM weakened dereferencable semantics to mean "dereferencable on entry to the function" (as opposed to "dereferencable throughout the entire execution of the function", which is what it means now). That would be another way in which Box is not just a library type.

Long-term, the plan is to not make Box special but to make Unique special instead. At least, @Gankra told me that that was the plan. ;) But that is blocked on figuring out better how exactly we want it to be special.

Copy link
Member

@RalfJung RalfJung Apr 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are Vec or Rc for example also special?

No, right now only Box is special.

Eventually, when Unique is special, that would be inherited by Vec. Vec is, in fact, the main motivation for making Unique special. See e.g. #71354.

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