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] - Add safe constructors for untyped pointers Ptr and PtrMut #6539

Closed
wants to merge 8 commits 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
35 changes: 28 additions & 7 deletions crates/bevy_ptr/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#![warn(missing_docs)]

use core::{
cell::UnsafeCell, marker::PhantomData, mem::MaybeUninit, num::NonZeroUsize, ptr::NonNull,
cell::UnsafeCell, marker::PhantomData, mem::ManuallyDrop, num::NonZeroUsize, ptr::NonNull,
};

/// Type-erased borrow of some unknown type chosen when constructing this type.
Expand Down Expand Up @@ -98,6 +98,9 @@ macro_rules! impl_ptr {
}

impl_ptr!(Ptr);
impl_ptr!(PtrMut);
impl_ptr!(OwningPtr);

impl<'a> Ptr<'a> {
/// Transforms this [`Ptr`] into an [`PtrMut`]
///
Expand Down Expand Up @@ -127,7 +130,16 @@ impl<'a> Ptr<'a> {
self.0.as_ptr()
}
}
impl_ptr!(PtrMut);

impl<'a, T> From<&'a T> for Ptr<'a> {
JoJoJet marked this conversation as resolved.
Show resolved Hide resolved
#[inline]
fn from(val: &'a T) -> Self {
// SAFETY: The returned pointer has the same lifetime as the passed reference.
JoJoJet marked this conversation as resolved.
Show resolved Hide resolved
// Access is immutable.
unsafe { Self::new(NonNull::from(val).cast()) }
}
}

impl<'a> PtrMut<'a> {
/// Transforms this [`PtrMut`] into an [`OwningPtr`]
///
Expand Down Expand Up @@ -157,15 +169,24 @@ impl<'a> PtrMut<'a> {
self.0.as_ptr()
}
}
impl_ptr!(OwningPtr);

impl<'a, T> From<&'a mut T> for PtrMut<'a> {
JoJoJet marked this conversation as resolved.
Show resolved Hide resolved
#[inline]
fn from(val: &'a mut T) -> Self {
// SAFETY: The returned pointer has the same lifetime as the passed reference.
JoJoJet marked this conversation as resolved.
Show resolved Hide resolved
// The reference is mutable, and thus will not alias.
unsafe { Self::new(NonNull::from(val).cast()) }
}
}

impl<'a> OwningPtr<'a> {
/// Consumes a value and creates an [`OwningPtr`] to it while ensuring a double drop does not happen.
#[inline]
pub fn make<T, F: FnOnce(OwningPtr<'_>) -> R, R>(val: T, f: F) -> R {
let mut temp = MaybeUninit::new(val);
// SAFETY: `temp.as_mut_ptr()` is a reference to a local value on the stack, so it cannot be null
let ptr = unsafe { NonNull::new_unchecked(temp.as_mut_ptr().cast::<u8>()) };
f(Self(ptr, PhantomData))
let mut temp = ManuallyDrop::new(val);
// SAFETY: The value behind the pointer will not get dropped or observed later,
// so it's safe to promote it to an owning pointer.
f(unsafe { PtrMut::from(&mut *temp).promote() })
}

/// Consumes the [`OwningPtr`] to obtain ownership of the underlying data of type `T`.
Expand Down
11 changes: 3 additions & 8 deletions crates/bevy_reflect/src/type_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ impl<T: for<'a> Deserialize<'a> + Reflect> FromType<T> for ReflectDeserialize {
/// type_registry.register::<Reflected>();
///
/// let mut value = Reflected("Hello world!".to_string());
/// let value = unsafe { Ptr::new(NonNull::from(&mut value).cast()) };
/// let value = Ptr::from(&value);
///
/// let reflect_data = type_registry.get(std::any::TypeId::of::<Reflected>()).unwrap();
/// let reflect_from_ptr = reflect_data.data::<ReflectFromPtr>().unwrap();
Expand Down Expand Up @@ -534,8 +534,6 @@ impl<T: Reflect> FromType<T> for ReflectFromPtr {

#[cfg(test)]
mod test {
use std::ptr::NonNull;

use crate::{GetTypeRegistration, ReflectFromPtr, TypeRegistration};
use bevy_ptr::{Ptr, PtrMut};
use bevy_utils::HashMap;
Expand All @@ -560,8 +558,7 @@ mod test {

let mut value = Foo { a: 1.0 };
{
// SAFETY: lifetime doesn't outlive original value, access is unique
let value = unsafe { PtrMut::new(NonNull::from(&mut value).cast()) };
let value = PtrMut::from(&mut value);
// SAFETY: reflect_from_ptr was constructed for the correct type
let dyn_reflect = unsafe { reflect_from_ptr.as_reflect_ptr_mut(value) };
match dyn_reflect.reflect_mut() {
Expand All @@ -573,10 +570,8 @@ mod test {
}

{
// SAFETY: lifetime doesn't outlive original value
let value = unsafe { Ptr::new(NonNull::from(&mut value).cast()) };
// SAFETY: reflect_from_ptr was constructed for the correct type
let dyn_reflect = unsafe { reflect_from_ptr.as_reflect_ptr(value) };
let dyn_reflect = unsafe { reflect_from_ptr.as_reflect_ptr(Ptr::from(&value)) };
match dyn_reflect.reflect_ref() {
bevy_reflect::ReflectRef::Struct(strukt) => {
let a = strukt.field("a").unwrap().downcast_ref::<f32>().unwrap();
Expand Down