From cddb9307d634c7164b285aa7f2fa9363a812b0fa Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Sat, 9 Apr 2022 12:20:48 +0200 Subject: [PATCH 01/16] bevy_reflect: ReflectFromPtr to create &dyn Reflect --- .../bevy_reflect_derive/src/lib.rs | 1 + crates/bevy_reflect/src/impls/std.rs | 5 +- crates/bevy_reflect/src/type_registry.rs | 96 ++++++++++++++++++- 3 files changed, 99 insertions(+), 3 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs b/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs index 8128b292b8559..e70f2d2f8c0b2 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs @@ -719,6 +719,7 @@ fn impl_get_type_registration( impl #impl_generics #bevy_reflect_path::GetTypeRegistration for #type_name #ty_generics #where_clause { fn get_type_registration() -> #bevy_reflect_path::TypeRegistration { let mut registration = #bevy_reflect_path::TypeRegistration::of::<#type_name #ty_generics>(); + registration.insert::<#bevy_reflect_path::ReflectFromPtr>(#bevy_reflect_path::FromType::<#type_name #ty_generics>::from_type()); #(registration.insert::<#registration_data>(#bevy_reflect_path::FromType::<#type_name #ty_generics>::from_type());)* registration } diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index 0b1d4097e1e11..5d270f88d802a 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -1,4 +1,4 @@ -use crate as bevy_reflect; +use crate::{self as bevy_reflect, ReflectFromPtr}; use crate::{ map_partial_eq, serde::Serializable, DynamicMap, FromReflect, FromType, GetTypeRegistration, List, ListIter, Map, MapIter, Reflect, ReflectDeserialize, ReflectMut, ReflectRef, @@ -148,6 +148,7 @@ impl Deserialize<'de>> GetTypeRegistration for Vec fn get_type_registration() -> TypeRegistration { let mut registration = TypeRegistration::of::>(); registration.insert::(FromType::>::from_type()); + registration.insert::(FromType::>::from_type()); registration } } @@ -270,6 +271,7 @@ where fn get_type_registration() -> TypeRegistration { let mut registration = TypeRegistration::of::(); registration.insert::(FromType::::from_type()); + registration.insert::(FromType::::from_type()); registration } } @@ -355,6 +357,7 @@ impl GetTypeRegistration for Cow<'static, str> { fn get_type_registration() -> TypeRegistration { let mut registration = TypeRegistration::of::>(); registration.insert::(FromType::>::from_type()); + registration.insert::(FromType::>::from_type()); registration } } diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index a6aa64f487305..fbbcd949287a3 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -3,7 +3,7 @@ use bevy_utils::{HashMap, HashSet}; use downcast_rs::{impl_downcast, Downcast}; use parking_lot::{RwLock, RwLockReadGuard, RwLockWriteGuard}; use serde::Deserialize; -use std::{any::TypeId, fmt::Debug, sync::Arc}; +use std::{any::TypeId, fmt::Debug, marker::PhantomData, sync::Arc}; /// A registry of reflected types. #[derive(Default)] @@ -350,11 +350,103 @@ impl Deserialize<'a> + Reflect> FromType for ReflectDeserialize { } } +#[derive(Clone)] +pub struct ReflectFromPtr { + type_id: TypeId, + to_reflect: for<'a> unsafe fn(*const (), lt: PhantomData<&'a ()>) -> &'a dyn Reflect, + to_reflect_mut: for<'a> unsafe fn(*mut (), lt: PhantomData<&'a ()>) -> &'a mut dyn Reflect, +} + +impl ReflectFromPtr { + /// # Safety: + /// - `val` must be a pointer to a value of the type that the [`ReflectFromPtr`] was constructed for + /// - the lifetime `'a` of the return type can be arbitrarily chosen by the caller, who must ensure that during + /// that lifetime, `val` is valid + pub unsafe fn to_reflect_ptr<'a>(&self, val: *const ()) -> &'a dyn Reflect { + (self.to_reflect)(val, PhantomData) + } + + /// # Safety: + /// - `val` must be a pointer to a value of the type that the [`ReflectFromPtr`] was constructed for + /// - the lifetime `'a` of the return type can be arbitrarily chosen by the caller, who must ensure that during + /// that lifetime, `val` is valid and not aliased + pub unsafe fn to_reflect_ptr_mut<'a>(&self, val: *mut ()) -> &'a mut dyn Reflect { + (self.to_reflect_mut)(val, PhantomData) + } + + pub fn to_reflect<'a, T: 'static>(&self, val: &'a T) -> &'a dyn Reflect { + assert_eq!(self.type_id, std::any::TypeId::of::()); + // SAFE: the lifetime of `val` is the same as the lifetime of the return value + // and the type of `val` has been checked to be the same correct one + unsafe { self.to_reflect_ptr(val as *const _ as *const ()) } + } + + pub fn to_reflect_mut<'a, T: 'static>(&self, val: &'a mut T) -> &'a mut dyn Reflect { + assert_eq!(self.type_id, std::any::TypeId::of::()); + // SAFE: the lifetime of `val` is the same as the lifetime of the return value + // and the type of `val` has been checked to be the same correct one + unsafe { self.to_reflect_ptr_mut(val as *mut _ as *mut ()) } + } +} + +impl FromType for ReflectFromPtr { + fn from_type() -> Self { + ReflectFromPtr { + type_id: std::any::TypeId::of::(), + to_reflect: |ptr, _lt| { + // SAFE: can only be called by `to_reflect_ptr` where the caller promises the safety requirements + // or `to_reflect` which is typed and checks that the correct type is used. + let val: &T = unsafe { &*ptr.cast::() }; + val as &dyn Reflect + }, + to_reflect_mut: |ptr, _lt| { + // SAFE: can only be called by `to_reflect_ptr_mut` where the caller promises the safety requirements + // or `to_reflect_mut` which is typed and checks that the correct type is used. + let val: &mut T = unsafe { &mut *ptr.cast::() }; + val as &mut dyn Reflect + }, + } + } +} + #[cfg(test)] mod test { - use crate::TypeRegistration; + use crate::{GetTypeRegistration, ReflectFromPtr, TypeRegistration}; use bevy_utils::HashMap; + use crate as bevy_reflect; + use crate::Reflect; + + #[test] + fn test_reflect_from_ptr() { + #[derive(Reflect)] + struct Foo { + a: f32, + } + + let foo_registration = ::get_type_registration(); + let reflect_from_ptr = foo_registration.data::().unwrap(); + + let mut value = Foo { a: 1.0 }; + + let dyn_reflect = reflect_from_ptr.to_reflect_mut(&mut value); + match dyn_reflect.reflect_mut() { + bevy_reflect::ReflectMut::Struct(strukt) => { + strukt.field_mut("a").unwrap().apply(&2.0f32) + } + _ => panic!("invalid reflection"), + } + + let dyn_reflect = reflect_from_ptr.to_reflect(&value); + match dyn_reflect.reflect_ref() { + bevy_reflect::ReflectRef::Struct(strukt) => { + let a = strukt.field("a").unwrap().downcast_ref::().unwrap(); + assert_eq!(*a, 2.0); + } + _ => panic!("invalid reflection"), + } + } + #[test] fn test_get_short_name() { assert_eq!( From 3038e3ffa1e08adc9cc5e0dc84f7f5a7b5e4c985 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Mon, 18 Apr 2022 13:25:28 +0200 Subject: [PATCH 02/16] fix '# Safety' comment to be recognized by clippy --- crates/bevy_reflect/src/type_registry.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index fbbcd949287a3..893c24ef29e93 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -358,7 +358,8 @@ pub struct ReflectFromPtr { } impl ReflectFromPtr { - /// # Safety: + /// # Safety + /// /// - `val` must be a pointer to a value of the type that the [`ReflectFromPtr`] was constructed for /// - the lifetime `'a` of the return type can be arbitrarily chosen by the caller, who must ensure that during /// that lifetime, `val` is valid @@ -366,7 +367,8 @@ impl ReflectFromPtr { (self.to_reflect)(val, PhantomData) } - /// # Safety: + /// # Safety + /// /// - `val` must be a pointer to a value of the type that the [`ReflectFromPtr`] was constructed for /// - the lifetime `'a` of the return type can be arbitrarily chosen by the caller, who must ensure that during /// that lifetime, `val` is valid and not aliased From 6438f552276db1c0aa2dec10fa1f466a42764af5 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Mon, 18 Apr 2022 13:59:43 +0200 Subject: [PATCH 03/16] change names from to_ to as_ and get use *const dyn Reflect instead of &dyn Reflect --- crates/bevy_reflect/src/type_registry.rs | 48 +++++++++++------------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 893c24ef29e93..e01f14f021488 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -3,7 +3,7 @@ use bevy_utils::{HashMap, HashSet}; use downcast_rs::{impl_downcast, Downcast}; use parking_lot::{RwLock, RwLockReadGuard, RwLockWriteGuard}; use serde::Deserialize; -use std::{any::TypeId, fmt::Debug, marker::PhantomData, sync::Arc}; +use std::{any::TypeId, fmt::Debug, sync::Arc}; /// A registry of reflected types. #[derive(Default)] @@ -353,59 +353,55 @@ impl Deserialize<'a> + Reflect> FromType for ReflectDeserialize { #[derive(Clone)] pub struct ReflectFromPtr { type_id: TypeId, - to_reflect: for<'a> unsafe fn(*const (), lt: PhantomData<&'a ()>) -> &'a dyn Reflect, - to_reflect_mut: for<'a> unsafe fn(*mut (), lt: PhantomData<&'a ()>) -> &'a mut dyn Reflect, + to_reflect: unsafe fn(*const ()) -> *const dyn Reflect, + to_reflect_mut: unsafe fn(*mut ()) -> *mut dyn Reflect, } impl ReflectFromPtr { /// # Safety /// - /// - `val` must be a pointer to a value of the type that the [`ReflectFromPtr`] was constructed for - /// - the lifetime `'a` of the return type can be arbitrarily chosen by the caller, who must ensure that during - /// that lifetime, `val` is valid - pub unsafe fn to_reflect_ptr<'a>(&self, val: *const ()) -> &'a dyn Reflect { - (self.to_reflect)(val, PhantomData) + /// - `val` must be a pointer to a valid value of the type that the [`ReflectFromPtr`] was constructed for + pub unsafe fn as_reflect_ptr(&self, val: *const ()) -> *const dyn Reflect { + (self.to_reflect)(val) } /// # Safety /// - /// - `val` must be a pointer to a value of the type that the [`ReflectFromPtr`] was constructed for - /// - the lifetime `'a` of the return type can be arbitrarily chosen by the caller, who must ensure that during - /// that lifetime, `val` is valid and not aliased - pub unsafe fn to_reflect_ptr_mut<'a>(&self, val: *mut ()) -> &'a mut dyn Reflect { - (self.to_reflect_mut)(val, PhantomData) + /// - `val` must be a pointer to a valid and unaliased value of the type that the [`ReflectFromPtr`] was constructed for + pub unsafe fn as_reflect_ptr_mut(&self, val: *mut ()) -> *mut dyn Reflect { + (self.to_reflect_mut)(val) } - pub fn to_reflect<'a, T: 'static>(&self, val: &'a T) -> &'a dyn Reflect { + pub fn as_reflect<'a, T: Reflect>(&self, val: &'a T) -> &'a dyn Reflect { assert_eq!(self.type_id, std::any::TypeId::of::()); // SAFE: the lifetime of `val` is the same as the lifetime of the return value // and the type of `val` has been checked to be the same correct one - unsafe { self.to_reflect_ptr(val as *const _ as *const ()) } + unsafe { &*self.as_reflect_ptr(val as *const _ as *const ()) } } - pub fn to_reflect_mut<'a, T: 'static>(&self, val: &'a mut T) -> &'a mut dyn Reflect { + pub fn as_reflect_mut<'a, T: Reflect>(&self, val: &'a mut T) -> &'a mut dyn Reflect { assert_eq!(self.type_id, std::any::TypeId::of::()); // SAFE: the lifetime of `val` is the same as the lifetime of the return value // and the type of `val` has been checked to be the same correct one - unsafe { self.to_reflect_ptr_mut(val as *mut _ as *mut ()) } + unsafe { &mut *self.as_reflect_ptr_mut(val as *mut _ as *mut ()) } } } -impl FromType for ReflectFromPtr { +impl FromType for ReflectFromPtr { fn from_type() -> Self { ReflectFromPtr { type_id: std::any::TypeId::of::(), - to_reflect: |ptr, _lt| { - // SAFE: can only be called by `to_reflect_ptr` where the caller promises the safety requirements + to_reflect: |ptr| { + // SAFE: can only be called by `as_reflect_ptr` where the caller promises the safety requirements // or `to_reflect` which is typed and checks that the correct type is used. let val: &T = unsafe { &*ptr.cast::() }; - val as &dyn Reflect + val as &dyn Reflect as *const dyn Reflect }, - to_reflect_mut: |ptr, _lt| { - // SAFE: can only be called by `to_reflect_ptr_mut` where the caller promises the safety requirements + to_reflect_mut: |ptr| { + // SAFE: can only be called by `as_reflect_ptr_mut` where the caller promises the safety requirements // or `to_reflect_mut` which is typed and checks that the correct type is used. let val: &mut T = unsafe { &mut *ptr.cast::() }; - val as &mut dyn Reflect + val as &mut dyn Reflect as *mut dyn Reflect }, } } @@ -431,7 +427,7 @@ mod test { let mut value = Foo { a: 1.0 }; - let dyn_reflect = reflect_from_ptr.to_reflect_mut(&mut value); + let dyn_reflect = reflect_from_ptr.as_reflect_mut(&mut value); match dyn_reflect.reflect_mut() { bevy_reflect::ReflectMut::Struct(strukt) => { strukt.field_mut("a").unwrap().apply(&2.0f32) @@ -439,7 +435,7 @@ mod test { _ => panic!("invalid reflection"), } - let dyn_reflect = reflect_from_ptr.to_reflect(&value); + let dyn_reflect = reflect_from_ptr.as_reflect(&value); match dyn_reflect.reflect_ref() { bevy_reflect::ReflectRef::Struct(strukt) => { let a = strukt.field("a").unwrap().downcast_ref::().unwrap(); From 46651954388dc59ad752340413b6351f144b6c2b Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Mon, 18 Apr 2022 14:07:11 +0200 Subject: [PATCH 04/16] add more docs --- crates/bevy_reflect/src/type_registry.rs | 29 ++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index e01f14f021488..fcd5b710eb519 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -350,6 +350,35 @@ impl Deserialize<'a> + Reflect> FromType for ReflectDeserialize { } } +/// [`Reflect`] values are commonly used in situations where the actual types of values +/// are not known at runtime. In such situations you might have access to a `*const ()` +/// that you know implements [`Reflect`], but have no way of turning it into a `&dyn Reflect`. +/// +/// This is where [`ReflectFromPtr`] comes in, when creating a [`ReflectFromPtr`] for a given type `T: Reflect`, +/// it internally saves a concrete function `*const T -> const dyn Reflect` which lets you create a trait object of [`Reflect`] +/// from a pointer. +/// +/// # Example +/// ```rust +/// use bevy_reflect::{TypeRegistry, Reflect, ReflectFromPtr}; +/// +/// #[derive(Reflect)] +/// struct Reflected(String); +/// +/// let mut type_registry = TypeRegistry::default(); +/// type_registry.register::(); +/// +/// let value = Reflected("Hello world!".to_string()); +/// let value: *const () = &value as *const _ as *const (); +/// +/// let reflect_from_ptr = type_registry.get(std::any::TypeId::of::())?.data::()?; +/// // SAFE: `value` is a pointer to an valid value of `Reflected` +/// let value = unsafe { reflect_from_ptr.as_reflect_ptr(value) }; +/// +/// println!("{:?}", value); +/// +/// # Ok(()) +/// ``` #[derive(Clone)] pub struct ReflectFromPtr { type_id: TypeId, From 07b60d647ef5a70972438b909b06e32f25b58531 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Mon, 18 Apr 2022 14:16:57 +0200 Subject: [PATCH 05/16] fix doctest --- crates/bevy_reflect/src/type_registry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index fcd5b710eb519..401b71b87876c 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -371,7 +371,7 @@ impl Deserialize<'a> + Reflect> FromType for ReflectDeserialize { /// let value = Reflected("Hello world!".to_string()); /// let value: *const () = &value as *const _ as *const (); /// -/// let reflect_from_ptr = type_registry.get(std::any::TypeId::of::())?.data::()?; +/// let reflect_from_ptr = type_registry.get(std::any::TypeId::of::()).unwrap().data::().unwrap(); /// // SAFE: `value` is a pointer to an valid value of `Reflected` /// let value = unsafe { reflect_from_ptr.as_reflect_ptr(value) }; /// From e35f498f5b33d9773dadc5e99f711fb7f7e91285 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Mon, 18 Apr 2022 16:14:46 +0200 Subject: [PATCH 06/16] fix doctest again --- crates/bevy_reflect/src/type_registry.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 401b71b87876c..db653a46a6a66 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -376,8 +376,6 @@ impl Deserialize<'a> + Reflect> FromType for ReflectDeserialize { /// let value = unsafe { reflect_from_ptr.as_reflect_ptr(value) }; /// /// println!("{:?}", value); -/// -/// # Ok(()) /// ``` #[derive(Clone)] pub struct ReflectFromPtr { From 75bfd0a02c8584a02acb20bc8d5b16a243420b03 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Thu, 5 May 2022 12:28:53 +0200 Subject: [PATCH 07/16] use bevy_ptr --- crates/bevy_reflect/Cargo.toml | 1 + crates/bevy_reflect/src/type_registry.rs | 42 +++++++++++------------- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/crates/bevy_reflect/Cargo.toml b/crates/bevy_reflect/Cargo.toml index 8d74f36495520..14280c9320521 100644 --- a/crates/bevy_reflect/Cargo.toml +++ b/crates/bevy_reflect/Cargo.toml @@ -16,6 +16,7 @@ bevy = ["glam", "smallvec"] # bevy bevy_reflect_derive = { path = "bevy_reflect_derive", version = "0.8.0-dev" } bevy_utils = { path = "../bevy_utils", version = "0.8.0-dev" } +bevy_ptr = { path = "../bevy_ptr", version = "0.8.0-dev" } # other erased-serde = "0.3" diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index db653a46a6a66..6ecb6414851f9 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -1,9 +1,10 @@ use crate::Reflect; +use bevy_ptr::{Ptr, PtrMut}; use bevy_utils::{HashMap, HashSet}; use downcast_rs::{impl_downcast, Downcast}; use parking_lot::{RwLock, RwLockReadGuard, RwLockWriteGuard}; use serde::Deserialize; -use std::{any::TypeId, fmt::Debug, sync::Arc}; +use std::{any::TypeId, fmt::Debug, ptr::NonNull, sync::Arc}; /// A registry of reflected types. #[derive(Default)] @@ -361,6 +362,8 @@ impl Deserialize<'a> + Reflect> FromType for ReflectDeserialize { /// # Example /// ```rust /// use bevy_reflect::{TypeRegistry, Reflect, ReflectFromPtr}; +/// use bevy_ptr::Ptr; +/// use std::ptr::NonNull; /// /// #[derive(Reflect)] /// struct Reflected(String); @@ -369,48 +372,45 @@ impl Deserialize<'a> + Reflect> FromType for ReflectDeserialize { /// type_registry.register::(); /// /// let value = Reflected("Hello world!".to_string()); -/// let value: *const () = &value as *const _ as *const (); /// /// let reflect_from_ptr = type_registry.get(std::any::TypeId::of::()).unwrap().data::().unwrap(); -/// // SAFE: `value` is a pointer to an valid value of `Reflected` -/// let value = unsafe { reflect_from_ptr.as_reflect_ptr(value) }; +/// // SAFE: `value` is of type `Reflected`, which the `ReflectFromPtr` was created for +/// let value = unsafe { reflect_from_ptr.as_reflect(&value) }; /// /// println!("{:?}", value); /// ``` #[derive(Clone)] pub struct ReflectFromPtr { type_id: TypeId, - to_reflect: unsafe fn(*const ()) -> *const dyn Reflect, - to_reflect_mut: unsafe fn(*mut ()) -> *mut dyn Reflect, + to_reflect: for<'a> unsafe fn(Ptr<'a>) -> &'a dyn Reflect, + to_reflect_mut: for<'a> unsafe fn(PtrMut<'a>) -> &'a mut dyn Reflect, } impl ReflectFromPtr { /// # Safety /// /// - `val` must be a pointer to a valid value of the type that the [`ReflectFromPtr`] was constructed for - pub unsafe fn as_reflect_ptr(&self, val: *const ()) -> *const dyn Reflect { + pub unsafe fn as_reflect_ptr<'a>(&self, val: Ptr<'a>) -> &'a dyn Reflect { (self.to_reflect)(val) } /// # Safety /// /// - `val` must be a pointer to a valid and unaliased value of the type that the [`ReflectFromPtr`] was constructed for - pub unsafe fn as_reflect_ptr_mut(&self, val: *mut ()) -> *mut dyn Reflect { + pub unsafe fn as_reflect_ptr_mut<'a>(&self, val: PtrMut<'a>) -> &'a mut dyn Reflect { (self.to_reflect_mut)(val) } pub fn as_reflect<'a, T: Reflect>(&self, val: &'a T) -> &'a dyn Reflect { assert_eq!(self.type_id, std::any::TypeId::of::()); - // SAFE: the lifetime of `val` is the same as the lifetime of the return value - // and the type of `val` has been checked to be the same correct one - unsafe { &*self.as_reflect_ptr(val as *const _ as *const ()) } + // SAFE: `val` is of type `T` + unsafe { &*self.as_reflect_ptr(Ptr::new(NonNull::from(val).cast())) } } pub fn as_reflect_mut<'a, T: Reflect>(&self, val: &'a mut T) -> &'a mut dyn Reflect { assert_eq!(self.type_id, std::any::TypeId::of::()); - // SAFE: the lifetime of `val` is the same as the lifetime of the return value - // and the type of `val` has been checked to be the same correct one - unsafe { &mut *self.as_reflect_ptr_mut(val as *mut _ as *mut ()) } + // SAFE: `val` is of type `T` + unsafe { &mut *self.as_reflect_ptr_mut(PtrMut::new(NonNull::from(val).cast())) } } } @@ -419,16 +419,14 @@ impl FromType for ReflectFromPtr { ReflectFromPtr { type_id: std::any::TypeId::of::(), to_reflect: |ptr| { - // SAFE: can only be called by `as_reflect_ptr` where the caller promises the safety requirements - // or `to_reflect` which is typed and checks that the correct type is used. - let val: &T = unsafe { &*ptr.cast::() }; - val as &dyn Reflect as *const dyn Reflect + // SAFE: only called from `as_reflect`, where the `ptr` is guaranteed to be of type `T`, + // and `as_reflect_ptr`, where the caller promises to call it with type `T` + unsafe { ptr.deref::() as &dyn Reflect } }, to_reflect_mut: |ptr| { - // SAFE: can only be called by `as_reflect_ptr_mut` where the caller promises the safety requirements - // or `to_reflect_mut` which is typed and checks that the correct type is used. - let val: &mut T = unsafe { &mut *ptr.cast::() }; - val as &mut dyn Reflect as *mut dyn Reflect + // SAFE: only called from `as_reflect_mut`, where the `ptr` is guaranteed to be of type `T`, + // and `as_reflect_ptr_mut`, where the caller promises to call it with type `T` + unsafe { ptr.deref_mut::() as &mut dyn Reflect } }, } } From 156f9f933ec14906a03ce811b75834a6373bbe56 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Fri, 13 May 2022 15:26:29 +0200 Subject: [PATCH 08/16] expose TypeId of ReflectFromPtr --- crates/bevy_reflect/src/type_registry.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 6df29da5a3178..167f9cd14f199 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -416,16 +416,23 @@ pub struct ReflectFromPtr { } impl ReflectFromPtr { + /// Returns the [`TypeId`] that the [`ReflectFromPtr`] was constructed for + pub fn type_id(&self) -> TypeId { + self.type_id + } + /// # Safety /// - /// - `val` must be a pointer to a valid value of the type that the [`ReflectFromPtr`] was constructed for + /// `val` must be a pointer to a valid value of the type that the [`ReflectFromPtr`] was constructed for. + /// This can be verified by checking that the type id returned by [`ReflectFromPtr::type_id`] is the expected one. pub unsafe fn as_reflect_ptr<'a>(&self, val: Ptr<'a>) -> &'a dyn Reflect { (self.to_reflect)(val) } /// # Safety /// - /// - `val` must be a pointer to a valid and unaliased value of the type that the [`ReflectFromPtr`] was constructed for + /// `val` must be a pointer to a valid and unaliased value of the type that the [`ReflectFromPtr`] was constructed for + /// This can be verified by checking that the type id returned by [`ReflectFromPtr::type_id`] is the expected one. pub unsafe fn as_reflect_ptr_mut<'a>(&self, val: PtrMut<'a>) -> &'a mut dyn Reflect { (self.to_reflect_mut)(val) } From 55fb2eacd64790ab46dd37f547ea0eeecde27e99 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Fri, 13 May 2022 15:38:33 +0200 Subject: [PATCH 09/16] change print in doctest to assert --- crates/bevy_reflect/src/type_registry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 167f9cd14f199..7121f3eb114b8 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -406,7 +406,7 @@ impl Deserialize<'a> + Reflect> FromType for ReflectDeserialize { /// // SAFE: `value` is of type `Reflected`, which the `ReflectFromPtr` was created for /// let value = unsafe { reflect_from_ptr.as_reflect(&value) }; /// -/// println!("{:?}", value); +/// assert!(value.reflect_partial_eq(&Reflected("Hello world!".to_string())).unwrap()); /// ``` #[derive(Clone)] pub struct ReflectFromPtr { From 939d49fae41e1201ef850f0945ffa629da390718 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Fri, 13 May 2022 15:39:34 +0200 Subject: [PATCH 10/16] use assert_eq on downcasted value --- crates/bevy_reflect/src/type_registry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 7121f3eb114b8..99e49556f55e3 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -406,7 +406,7 @@ impl Deserialize<'a> + Reflect> FromType for ReflectDeserialize { /// // SAFE: `value` is of type `Reflected`, which the `ReflectFromPtr` was created for /// let value = unsafe { reflect_from_ptr.as_reflect(&value) }; /// -/// assert!(value.reflect_partial_eq(&Reflected("Hello world!".to_string())).unwrap()); +/// assert_eq!(value.downcast_ref::().unwrap().0, "Hello world!"); /// ``` #[derive(Clone)] pub struct ReflectFromPtr { From 31ec9384804915bbdee56a6412007eb1f82ad7de Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Tue, 17 May 2022 17:18:26 +0200 Subject: [PATCH 11/16] Apply suggestions from code review Co-authored-by: Alice Cecile --- crates/bevy_reflect/src/type_registry.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 99e49556f55e3..71c57953ac352 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -381,11 +381,11 @@ impl Deserialize<'a> + Reflect> FromType for ReflectDeserialize { } /// [`Reflect`] values are commonly used in situations where the actual types of values -/// are not known at runtime. In such situations you might have access to a `*const ()` +/// are not known at runtime. In such situations you might have access to a `*const ()` pointer /// that you know implements [`Reflect`], but have no way of turning it into a `&dyn Reflect`. /// -/// This is where [`ReflectFromPtr`] comes in, when creating a [`ReflectFromPtr`] for a given type `T: Reflect`, -/// it internally saves a concrete function `*const T -> const dyn Reflect` which lets you create a trait object of [`Reflect`] +/// This is where [`ReflectFromPtr`] comes in, when creating a [`ReflectFromPtr`] for a given type `T: Reflect`. +/// Internally, this saves a concrete function `*const T -> const dyn Reflect` which lets you create a trait object of [`Reflect`] /// from a pointer. /// /// # Example From be78fd7eaa9ee26a80fd5070ceddb8b011d03832 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Tue, 17 May 2022 17:18:17 +0200 Subject: [PATCH 12/16] split complicated line into two --- crates/bevy_reflect/src/type_registry.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 71c57953ac352..71db3f5e3f9ca 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -402,7 +402,8 @@ impl Deserialize<'a> + Reflect> FromType for ReflectDeserialize { /// /// let value = Reflected("Hello world!".to_string()); /// -/// let reflect_from_ptr = type_registry.get(std::any::TypeId::of::()).unwrap().data::().unwrap(); +/// let reflect_data = type_registry.get(std::any::TypeId::of::()).unwrap(); +/// let reflect_from_ptr = reflect_data.data::().unwrap(); /// // SAFE: `value` is of type `Reflected`, which the `ReflectFromPtr` was created for /// let value = unsafe { reflect_from_ptr.as_reflect(&value) }; /// From b9949287cff64110f285710fe245ff9c11cee62d Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Tue, 17 May 2022 17:21:44 +0200 Subject: [PATCH 13/16] remove unnecessary safety docs (they are required for the Ptr types) --- crates/bevy_reflect/src/type_registry.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 71db3f5e3f9ca..3eedda4945724 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -424,7 +424,7 @@ impl ReflectFromPtr { /// # Safety /// - /// `val` must be a pointer to a valid value of the type that the [`ReflectFromPtr`] was constructed for. + /// `val` must be a pointer to value of the type that the [`ReflectFromPtr`] was constructed for. /// This can be verified by checking that the type id returned by [`ReflectFromPtr::type_id`] is the expected one. pub unsafe fn as_reflect_ptr<'a>(&self, val: Ptr<'a>) -> &'a dyn Reflect { (self.to_reflect)(val) @@ -432,7 +432,7 @@ impl ReflectFromPtr { /// # Safety /// - /// `val` must be a pointer to a valid and unaliased value of the type that the [`ReflectFromPtr`] was constructed for + /// `val` must be a pointer to a value of the type that the [`ReflectFromPtr`] was constructed for /// This can be verified by checking that the type id returned by [`ReflectFromPtr::type_id`] is the expected one. pub unsafe fn as_reflect_ptr_mut<'a>(&self, val: PtrMut<'a>) -> &'a mut dyn Reflect { (self.to_reflect_mut)(val) From e51c5afae7c387ae0c7c58fed9794aa90d2aaac7 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Tue, 17 May 2022 21:00:27 +0200 Subject: [PATCH 14/16] remove unnecessary methods from ReflectFromPtr --- crates/bevy_reflect/src/type_registry.rs | 60 +++++++++++++----------- 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 3eedda4945724..00088bce614bd 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -4,7 +4,7 @@ use bevy_utils::{HashMap, HashSet}; use downcast_rs::{impl_downcast, Downcast}; use parking_lot::{RwLock, RwLockReadGuard, RwLockWriteGuard}; use serde::Deserialize; -use std::{any::TypeId, fmt::Debug, ptr::NonNull, sync::Arc}; +use std::{any::TypeId, fmt::Debug, sync::Arc}; /// A registry of reflected types. #[derive(Default)] @@ -400,12 +400,13 @@ impl Deserialize<'a> + Reflect> FromType for ReflectDeserialize { /// let mut type_registry = TypeRegistry::default(); /// type_registry.register::(); /// -/// let value = Reflected("Hello world!".to_string()); +/// let mut value = Reflected("Hello world!".to_string()); +/// let value = unsafe { Ptr::new(NonNull::from(&mut value).cast()) }; /// /// let reflect_data = type_registry.get(std::any::TypeId::of::()).unwrap(); /// let reflect_from_ptr = reflect_data.data::().unwrap(); /// // SAFE: `value` is of type `Reflected`, which the `ReflectFromPtr` was created for -/// let value = unsafe { reflect_from_ptr.as_reflect(&value) }; +/// let value = unsafe { reflect_from_ptr.as_reflect_ptr(value) }; /// /// assert_eq!(value.downcast_ref::().unwrap().0, "Hello world!"); /// ``` @@ -437,18 +438,6 @@ impl ReflectFromPtr { pub unsafe fn as_reflect_ptr_mut<'a>(&self, val: PtrMut<'a>) -> &'a mut dyn Reflect { (self.to_reflect_mut)(val) } - - pub fn as_reflect<'a, T: Reflect>(&self, val: &'a T) -> &'a dyn Reflect { - assert_eq!(self.type_id, std::any::TypeId::of::()); - // SAFE: `val` is of type `T` - unsafe { &*self.as_reflect_ptr(Ptr::new(NonNull::from(val).cast())) } - } - - pub fn as_reflect_mut<'a, T: Reflect>(&self, val: &'a mut T) -> &'a mut dyn Reflect { - assert_eq!(self.type_id, std::any::TypeId::of::()); - // SAFE: `val` is of type `T` - unsafe { &mut *self.as_reflect_ptr_mut(PtrMut::new(NonNull::from(val).cast())) } - } } impl FromType for ReflectFromPtr { @@ -471,7 +460,10 @@ 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; use crate as bevy_reflect; @@ -487,23 +479,37 @@ mod test { let foo_registration = ::get_type_registration(); let reflect_from_ptr = foo_registration.data::().unwrap(); - let mut value = Foo { a: 1.0 }; + // not required in this situation because we no nobody messed with the TypeRegistry, + // but in the general case somebody could have replaced the ReflectFromPtr with an + // instance for another type, so then we'd need to check that the type is the expected one + assert_eq!(reflect_from_ptr.type_id(), std::any::TypeId::of::()); - let dyn_reflect = reflect_from_ptr.as_reflect_mut(&mut value); - match dyn_reflect.reflect_mut() { - bevy_reflect::ReflectMut::Struct(strukt) => { - strukt.field_mut("a").unwrap().apply(&2.0f32) + 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()) }; + // 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() { + bevy_reflect::ReflectMut::Struct(strukt) => { + strukt.field_mut("a").unwrap().apply(&2.0f32) + } + _ => panic!("invalid reflection"), } - _ => panic!("invalid reflection"), } - let dyn_reflect = reflect_from_ptr.as_reflect(&value); - match dyn_reflect.reflect_ref() { - bevy_reflect::ReflectRef::Struct(strukt) => { - let a = strukt.field("a").unwrap().downcast_ref::().unwrap(); - assert_eq!(*a, 2.0); + { + // 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) }; + match dyn_reflect.reflect_ref() { + bevy_reflect::ReflectRef::Struct(strukt) => { + let a = strukt.field("a").unwrap().downcast_ref::().unwrap(); + assert_eq!(*a, 2.0); + } + _ => panic!("invalid reflection"), } - _ => panic!("invalid reflection"), } } From a9ce062089dbdf4d119ef6608c321e39d0a41ca2 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Tue, 17 May 2022 21:05:35 +0200 Subject: [PATCH 15/16] Update crates/bevy_reflect/src/impls/std.rs Co-authored-by: PROMETHIA-27 <42193387+PROMETHIA-27@users.noreply.github.com> --- crates/bevy_reflect/src/impls/std.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index d3d35391a0d60..1461dcead02d3 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -291,9 +291,9 @@ where V: Reflect + Clone + for<'de> Deserialize<'de>, { fn get_type_registration() -> TypeRegistration { - let mut registration = TypeRegistration::of::(); - registration.insert::(FromType::::from_type()); - registration.insert::(FromType::::from_type()); + let mut registration = TypeRegistration::of::>(); + registration.insert::(FromType::>::from_type()); + registration.insert::(FromType::>::from_type()); registration } } From 4b9e974d9f6b89d39ee595ef1afa43fef0ad163e Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Tue, 19 Jul 2022 16:00:01 -0700 Subject: [PATCH 16/16] clippy --- crates/bevy_reflect/src/type_registry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 3d2235a9200fa..3a44fe4da1499 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -523,7 +523,7 @@ mod test { let dyn_reflect = unsafe { reflect_from_ptr.as_reflect_ptr_mut(value) }; match dyn_reflect.reflect_mut() { bevy_reflect::ReflectMut::Struct(strukt) => { - strukt.field_mut("a").unwrap().apply(&2.0f32) + strukt.field_mut("a").unwrap().apply(&2.0f32); } _ => panic!("invalid reflection"), }