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

[Merged by Bors] - Document alignment requirements of Ptr, PtrMut and OwningPtr #7151

Closed
wants to merge 1 commit into from
Closed
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
130 changes: 101 additions & 29 deletions crates/bevy_ptr/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,37 @@ use core::{
cell::UnsafeCell, marker::PhantomData, mem::ManuallyDrop, num::NonZeroUsize, ptr::NonNull,
};

#[derive(Copy, Clone)]
/// Used as a type argument to [`Ptr`], [`PtrMut`] and [`OwningPtr`] to specify that the pointer is aligned.
pub struct Aligned;
#[derive(Copy, Clone)]
/// Used as a type argument to [`Ptr`], [`PtrMut`] and [`OwningPtr`] to specify that the pointer is not aligned.
pub struct Unaligned;

/// Trait that is only implemented for [`Aligned`] and [`Unaligned`] to work around the lack of ability
/// to have const generics of an enum.
pub trait IsAligned: sealed::Sealed {}
impl IsAligned for Aligned {}
Copy link
Member

Choose a reason for hiding this comment

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

Actually wonder if we could use const generics to enforce debug checks on byte_add and byte_offset, but I'll leave that for a separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

you wouldnt need const generics, it ought to be possible to look at the TypeId of A to see if its Aligned or not

impl IsAligned for Unaligned {}

mod sealed {
pub trait Sealed {}
impl Sealed for super::Aligned {}
impl Sealed for super::Unaligned {}
}

/// Type-erased borrow of some unknown type chosen when constructing this type.
///
/// This type tries to act "borrow-like" which means that:
/// - It should be considered immutable: its target must not be changed while this pointer is alive.
/// - It must always points to a valid value of whatever the pointee type is.
/// - The lifetime `'a` accurately represents how long the pointer is valid for.
/// - Must be sufficiently aligned for the unknown pointee type.
///
/// It may be helpful to think of this type as similar to `&'a dyn Any` but without
/// the metadata and able to point to data that does not correspond to a Rust type.
#[derive(Copy, Clone)]
pub struct Ptr<'a>(NonNull<u8>, PhantomData<&'a u8>);
pub struct Ptr<'a, A: IsAligned = Aligned>(NonNull<u8>, PhantomData<(&'a u8, A)>);

/// Type-erased mutable borrow of some unknown type chosen when constructing this type.
///
Expand All @@ -26,10 +46,11 @@ pub struct Ptr<'a>(NonNull<u8>, PhantomData<&'a u8>);
/// aliased mutability.
/// - It must always points to a valid value of whatever the pointee type is.
/// - The lifetime `'a` accurately represents how long the pointer is valid for.
/// - Must be sufficiently aligned for the unknown pointee type.
///
/// It may be helpful to think of this type as similar to `&'a mut dyn Any` but without
/// the metadata and able to point to data that does not correspond to a Rust type.
pub struct PtrMut<'a>(NonNull<u8>, PhantomData<&'a mut u8>);
pub struct PtrMut<'a, A: IsAligned = Aligned>(NonNull<u8>, PhantomData<(&'a mut u8, A)>);

/// Type-erased Box-like pointer to some unknown type chosen when constructing this type.
/// Conceptually represents ownership of whatever data is being pointed to and so is
Expand All @@ -42,22 +63,32 @@ pub struct PtrMut<'a>(NonNull<u8>, PhantomData<&'a mut u8>);
/// to aliased mutability and potentially use after free bugs.
/// - It must always points to a valid value of whatever the pointee type is.
/// - The lifetime `'a` accurately represents how long the pointer is valid for.
/// - Must be sufficiently aligned for the unknown pointee type.
///
/// It may be helpful to think of this type as similar to `&'a mut ManuallyDrop<dyn Any>` but
/// without the metadata and able to point to data that does not correspond to a Rust type.
pub struct OwningPtr<'a>(NonNull<u8>, PhantomData<&'a mut u8>);
pub struct OwningPtr<'a, A: IsAligned = Aligned>(NonNull<u8>, PhantomData<(&'a mut u8, A)>);

macro_rules! impl_ptr {
($ptr:ident) => {
impl $ptr<'_> {
impl<'a> $ptr<'a, Aligned> {
/// Removes the alignment requirement of this pointer
pub fn to_unaligned(self) -> $ptr<'a, Unaligned> {
$ptr(self.0, PhantomData)
}
}

impl<A: IsAligned> $ptr<'_, A> {
/// Calculates the offset from a pointer.
/// As the pointer is type-erased, there is no size information available. The provided
/// `count` parameter is in raw bytes.
///
/// *See also: [`ptr::offset`][ptr_offset]*
///
/// # Safety
/// the offset cannot make the existing ptr null, or take it out of bounds for its allocation.
/// - The offset cannot make the existing ptr null, or take it out of bounds for its allocation.
/// - If the `A` type parameter is [`Aligned`] then the offset must not make the resulting pointer
/// be unaligned for the pointee type.
///
/// [ptr_offset]: https://doc.rust-lang.org/std/primitive.pointer.html#method.offset
#[inline]
Expand All @@ -75,7 +106,9 @@ macro_rules! impl_ptr {
/// *See also: [`ptr::add`][ptr_add]*
///
/// # Safety
/// the offset cannot make the existing ptr null, or take it out of bounds for its allocation.
/// - The offset cannot make the existing ptr null, or take it out of bounds for its allocation.
/// - If the `A` type parameter is [`Aligned`] then the offset must not make the resulting pointer
/// be unaligned for the pointee type.
///
/// [ptr_add]: https://doc.rust-lang.org/std/primitive.pointer.html#method.add
#[inline]
Expand All @@ -85,18 +118,9 @@ macro_rules! impl_ptr {
PhantomData,
)
}

/// Creates a new instance from a raw pointer.
///
/// # Safety
/// The lifetime for the returned item must not exceed the lifetime `inner` is valid for
#[inline]
pub unsafe fn new(inner: NonNull<u8>) -> Self {
Self(inner, PhantomData)
}
}

impl Pointer for $ptr<'_> {
impl<A: IsAligned> Pointer for $ptr<'_, A> {
#[inline]
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
Pointer::fmt(&self.0, f)
Expand All @@ -109,20 +133,35 @@ impl_ptr!(Ptr);
impl_ptr!(PtrMut);
impl_ptr!(OwningPtr);

impl<'a> Ptr<'a> {
impl<'a, A: IsAligned> Ptr<'a, A> {
/// Creates a new instance from a raw pointer.
///
/// # Safety
/// - `inner` must point to valid value of whatever the pointee type is.
/// - If the `A` type parameter is [`Aligned`] then `inner` must be sufficiently aligned for the pointee type.
/// - `inner` must have correct provenance to allow reads of the pointee type.
/// - The lifetime `'a` must be constrained such that this [`Ptr`] will stay valid and nothing
/// can mutate the pointee while this [`Ptr`] is live except through an `UnsafeCell`.
#[inline]
pub unsafe fn new(inner: NonNull<u8>) -> Self {
Self(inner, PhantomData)
}

/// Transforms this [`Ptr`] into an [`PtrMut`]
///
/// # Safety
/// Another [`PtrMut`] for the same [`Ptr`] must not be created until the first is dropped.
#[inline]
pub unsafe fn assert_unique(self) -> PtrMut<'a> {
pub unsafe fn assert_unique(self) -> PtrMut<'a, A> {
PtrMut(self.0, PhantomData)
}

/// Transforms this [`Ptr<T>`] into a `&T` with the same lifetime
///
/// # Safety
/// Must point to a valid `T`
/// - `T` must be the erased pointee type for this [`Ptr`].
/// - If the type parameter `A` is `Unaligned` then this pointer must be sufficiently aligned
/// for the pointee type `T`.
#[inline]
pub unsafe fn deref<T>(self) -> &'a T {
&*self.as_ptr().cast()
Expand All @@ -148,20 +187,35 @@ impl<'a, T> From<&'a T> for Ptr<'a> {
}
}

impl<'a> PtrMut<'a> {
impl<'a, A: IsAligned> PtrMut<'a, A> {
/// Creates a new instance from a raw pointer.
///
/// # Safety
/// - `inner` must point to valid value of whatever the pointee type is.
/// - If the `A` type parameter is [`Aligned`] then `inner` must be sufficiently aligned for the pointee type.
/// - `inner` must have correct provenance to allow read and writes of the pointee type.
/// - The lifetime `'a` must be constrained such that this [`PtrMut`] will stay valid and nothing
/// else can read or mutate the pointee while this [`PtrMut`] is live.
#[inline]
pub unsafe fn new(inner: NonNull<u8>) -> Self {
Self(inner, PhantomData)
}

/// Transforms this [`PtrMut`] into an [`OwningPtr`]
///
/// # Safety
/// Must have right to drop or move out of [`PtrMut`].
#[inline]
pub unsafe fn promote(self) -> OwningPtr<'a> {
pub unsafe fn promote(self) -> OwningPtr<'a, A> {
OwningPtr(self.0, PhantomData)
}

/// Transforms this [`PtrMut<T>`] into a `&mut T` with the same lifetime
///
/// # Safety
/// Must point to a valid `T`
/// - `T` must be the erased pointee type for this [`PtrMut`].
/// - If the type parameter `A` is [`Unaligned`] then this pointer must be sufficiently aligned
/// for the pointee type `T`.
#[inline]
pub unsafe fn deref_mut<T>(self) -> &'a mut T {
&mut *self.as_ptr().cast()
Expand All @@ -177,16 +231,16 @@ impl<'a> PtrMut<'a> {
self.0.as_ptr()
}

/// Gets a `PtrMut` from this with a smaller lifetime.
/// Gets a [`PtrMut`] from this with a smaller lifetime.
#[inline]
pub fn reborrow(&mut self) -> PtrMut<'_> {
pub fn reborrow(&mut self) -> PtrMut<'_, A> {
// SAFE: the ptrmut we're borrowing from is assumed to be valid
unsafe { PtrMut::new(self.0) }
}

/// Gets an immutable reference from this mutable reference
#[inline]
pub fn as_ref(&self) -> Ptr<'_> {
pub fn as_ref(&self) -> Ptr<'_, A> {
// SAFE: The `PtrMut` type's guarantees about the validity of this pointer are a superset of `Ptr` s guarantees
unsafe { Ptr::new(self.0) }
}
Expand All @@ -210,11 +264,27 @@ impl<'a> OwningPtr<'a> {
// so it's safe to promote it to an owning pointer.
f(unsafe { PtrMut::from(&mut *temp).promote() })
}
}
impl<'a, A: IsAligned> OwningPtr<'a, A> {
/// Creates a new instance from a raw pointer.
///
/// # Safety
/// - `inner` must point to valid value of whatever the pointee type is.
/// - If the `A` type parameter is [`Aligned`] then `inner` must be sufficiently aligned for the pointee type.
/// - `inner` must have correct provenance to allow read and writes of the pointee type.
/// - The lifetime `'a` must be constrained such that this [`OwningPtr`] will stay valid and nothing
/// else can read or mutate the pointee while this [`OwningPtr`] is live.
#[inline]
pub unsafe fn new(inner: NonNull<u8>) -> Self {
Self(inner, PhantomData)
}

/// Consumes the [`OwningPtr`] to obtain ownership of the underlying data of type `T`.
///
/// # Safety
/// Must point to a valid `T`.
/// - `T` must be the erased pointee type for this [`OwningPtr`].
/// - If the type parameter `A` is `Unaligned` then this pointer must be sufficiently aligned
/// for the pointee type `T`.
#[inline]
pub unsafe fn read<T>(self) -> T {
self.as_ptr().cast::<T>().read()
Expand All @@ -223,7 +293,9 @@ impl<'a> OwningPtr<'a> {
/// Consumes the [`OwningPtr`] to drop the underlying data of type `T`.
///
/// # Safety
/// Must point to a valid `T`.
/// - `T` must be the erased pointee type for this [`OwningPtr`].
/// - If the type parameter `A` is `Unaligned` then this pointer must be sufficiently aligned
/// for the pointee type `T`.
#[inline]
pub unsafe fn drop_as<T>(self) {
self.as_ptr().cast::<T>().drop_in_place();
Expand All @@ -241,14 +313,14 @@ impl<'a> OwningPtr<'a> {

/// Gets an immutable pointer from this owned pointer.
#[inline]
pub fn as_ref(&self) -> Ptr<'_> {
pub fn as_ref(&self) -> Ptr<'_, A> {
// SAFE: The `Owning` type's guarantees about the validity of this pointer are a superset of `Ptr` s guarantees
unsafe { Ptr::new(self.0) }
}

/// Gets a mutable pointer from this owned pointer.
#[inline]
pub fn as_mut(&mut self) -> PtrMut<'_> {
pub fn as_mut(&mut self) -> PtrMut<'_, A> {
// SAFE: The `Owning` type's guarantees about the validity of this pointer are a superset of `Ptr` s guarantees
unsafe { PtrMut::new(self.0) }
}
Expand Down