From f2f8f9097f13081389e1377a151d5ab10606e78d Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Mon, 14 Nov 2022 22:53:50 +0000 Subject: [PATCH] Add safe constructors for untyped pointers `Ptr` and `PtrMut` (#6539) # Objective Currently, `Ptr` and `PtrMut` can only be constructed via unsafe code. This means that downgrading a reference to an untyped pointer is very cumbersome, despite being a very simple operation. ## Solution Define conversions for easily and safely constructing untyped pointers. This is the non-owned counterpart to `OwningPtr::make`. Before: ```rust let ptr = unsafe { PtrMut::new(NonNull::from(&mut value).cast()) }; ``` After: ```rust let ptr = PtrMut::from(&mut value); ``` Co-authored-by: Carter Anderson --- crates/bevy_ptr/src/lib.rs | 35 +++++++++++++++++++----- crates/bevy_reflect/src/type_registry.rs | 11 ++------ 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/crates/bevy_ptr/src/lib.rs b/crates/bevy_ptr/src/lib.rs index 3107ab2d5ecc8..4aec221350cb6 100644 --- a/crates/bevy_ptr/src/lib.rs +++ b/crates/bevy_ptr/src/lib.rs @@ -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. @@ -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`] /// @@ -127,7 +130,16 @@ impl<'a> Ptr<'a> { self.0.as_ptr() } } -impl_ptr!(PtrMut); + +impl<'a, T> From<&'a T> for Ptr<'a> { + #[inline] + fn from(val: &'a T) -> Self { + // SAFETY: The returned pointer has the same lifetime as the passed reference. + // Access is immutable. + unsafe { Self::new(NonNull::from(val).cast()) } + } +} + impl<'a> PtrMut<'a> { /// Transforms this [`PtrMut`] into an [`OwningPtr`] /// @@ -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> { + #[inline] + fn from(val: &'a mut T) -> Self { + // SAFETY: The returned pointer has the same lifetime as the passed reference. + // 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) -> 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::()) }; - 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`. diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 00f3bdb2f7177..2a6fc6f8168be 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -475,7 +475,7 @@ impl Deserialize<'a> + Reflect> FromType for ReflectDeserialize { /// type_registry.register::(); /// /// 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::()).unwrap(); /// let reflect_from_ptr = reflect_data.data::().unwrap(); @@ -534,8 +534,6 @@ impl FromType for ReflectFromPtr { #[cfg(test)] mod test { - use std::ptr::NonNull; - use crate::{GetTypeRegistration, ReflectFromPtr, TypeRegistration}; use bevy_ptr::{Ptr, PtrMut}; use bevy_utils::HashMap; @@ -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() { @@ -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::().unwrap();