From 776cbe3e0dbd97b7c5a2cb585290d215cd6f23c6 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 <mcanders1@gmail.com>
---
 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<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`.
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<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();
@@ -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;
@@ -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::<f32>().unwrap();