From 23b66dba488cf3f30cdc9381692e24d5bda59eb8 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sun, 18 Sep 2022 21:21:48 -0700 Subject: [PATCH 01/31] Added reflect_remote macro --- crates/bevy_reflect/derive/src/derive_data.rs | 66 ++++++++++++- crates/bevy_reflect/derive/src/impls/enums.rs | 42 +++++--- .../bevy_reflect/derive/src/impls/structs.rs | 25 +++-- .../derive/src/impls/tuple_structs.rs | 18 +++- crates/bevy_reflect/derive/src/lib.rs | 65 ++++++++++++ crates/bevy_reflect/derive/src/remote.rs | 99 +++++++++++++++++++ crates/bevy_reflect/src/lib.rs | 31 ++++++ 7 files changed, 318 insertions(+), 28 deletions(-) create mode 100644 crates/bevy_reflect/derive/src/remote.rs diff --git a/crates/bevy_reflect/derive/src/derive_data.rs b/crates/bevy_reflect/derive/src/derive_data.rs index 8825febfffb0a..a75a9fda7338a 100644 --- a/crates/bevy_reflect/derive/src/derive_data.rs +++ b/crates/bevy_reflect/derive/src/derive_data.rs @@ -17,7 +17,7 @@ use syn::punctuated::Punctuated; use syn::spanned::Spanned; use syn::{ parse_str, Data, DeriveInput, Field, Fields, GenericParam, Generics, Ident, LitStr, Meta, Path, - PathSegment, Type, TypeParam, Variant, + PathSegment, Type, TypeParam, TypePath, Variant, }; pub(crate) enum ReflectDerive<'a> { @@ -70,6 +70,7 @@ pub(crate) struct ReflectStruct<'a> { meta: ReflectMeta<'a>, serialization_data: Option, fields: Vec>, + remote_ty: Option<&'a TypePath>, } /// Enum data used by derive macros for `Reflect` and `FromReflect`. @@ -88,6 +89,7 @@ pub(crate) struct ReflectStruct<'a> { pub(crate) struct ReflectEnum<'a> { meta: ReflectMeta<'a>, variants: Vec>, + remote_ty: Option<&'a TypePath>, } /// Represents a field on a struct or tuple struct. @@ -146,8 +148,12 @@ enum ReflectMode { /// How the macro was invoked. #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub(crate) enum ReflectImplSource { + /// Using `impl_reflect!`. ImplRemoteType, + /// Using `#[derive(...)]`. DeriveLocalType, + /// Using `#[reflect_remote]`. + RemoteReflect, } /// Which trait the macro explicitly implements. @@ -173,7 +179,9 @@ impl fmt::Display for ReflectProvenance { (S::DeriveLocalType, T::Reflect) => "`#[derive(Reflect)]`", (S::DeriveLocalType, T::FromReflect) => "`#[derive(FromReflect)]`", (S::DeriveLocalType, T::TypePath) => "`#[derive(TypePath)]`", - (S::ImplRemoteType, T::FromReflect | T::TypePath) => unreachable!(), + (S::RemoteReflect, T::Reflect) => "`#[reflect_remote]`", + (S::RemoteReflect, T::FromReflect | T::TypePath) + | (S::ImplRemoteType, T::FromReflect | T::TypePath) => unreachable!(), }; f.write_str(str) } @@ -322,6 +330,7 @@ impl<'a> ReflectDerive<'a> { meta, serialization_data: SerializationDataDef::new(&fields)?, fields, + remote_ty: None, }; match data.fields { @@ -333,7 +342,11 @@ impl<'a> ReflectDerive<'a> { Data::Enum(data) => { let variants = Self::collect_enum_variants(&data.variants)?; - let reflect_enum = ReflectEnum { meta, variants }; + let reflect_enum = ReflectEnum { + meta, + variants, + remote_ty: None, + }; Ok(Self::Enum(reflect_enum)) } Data::Union(..) => Err(syn::Error::new( @@ -343,6 +356,23 @@ impl<'a> ReflectDerive<'a> { }; } + /// Set the remote type for this derived type. + /// + /// # Panics + /// + /// Panics when called on [`ReflectDerive::Value`]. + pub fn set_remote(&mut self, remote_ty: Option<&'a TypePath>) { + match self { + Self::Struct(data) | Self::TupleStruct(data) | Self::UnitStruct(data) => { + data.remote_ty = remote_ty; + } + Self::Enum(data) => { + data.remote_ty = remote_ty; + } + _ => panic!("cannot create a reflected value type for a remote type"), + } + } + pub fn meta(&self) -> &ReflectMeta<'a> { match self { ReflectDerive::Struct(data) @@ -531,6 +561,17 @@ impl<'a> ReflectStruct<'a> { self.serialization_data.as_ref() } + /// Whether this reflected struct represents a remote type or not. + pub fn is_remote(&self) -> bool { + self.remote_ty.is_some() + } + + #[allow(dead_code)] + /// Get the remote type path, if any. + pub fn remote_ty(&self) -> Option<&'a TypePath> { + self.remote_ty + } + /// Returns the `GetTypeRegistration` impl as a `TokenStream`. /// /// Returns a specific implementation for structs and this method should be preferred over the generic [`get_type_registration`](ReflectMeta) method @@ -630,9 +671,26 @@ impl<'a> ReflectEnum<'a> { &self.meta } + /// Whether this reflected enum represents a remote type or not. + pub fn is_remote(&self) -> bool { + self.remote_ty.is_some() + } + + #[allow(dead_code)] + /// Get the remote type path, if any. + pub fn remote_ty(&self) -> Option<&'a TypePath> { + self.remote_ty + } + /// Returns the given ident as a qualified unit variant of this enum. + /// + /// This takes into account the remote type, if any. pub fn get_unit(&self, variant: &Ident) -> proc_macro2::TokenStream { - let name = self.meta.type_path(); + let name = self + .remote_ty + .map(|path| path.to_token_stream()) + .unwrap_or_else(|| self.meta.type_path().to_token_stream()); + quote! { #name::#variant } diff --git a/crates/bevy_reflect/derive/src/impls/enums.rs b/crates/bevy_reflect/derive/src/impls/enums.rs index 74c7848afe768..f4c974253dc68 100644 --- a/crates/bevy_reflect/derive/src/impls/enums.rs +++ b/crates/bevy_reflect/derive/src/impls/enums.rs @@ -9,6 +9,26 @@ use syn::Fields; pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream { let bevy_reflect_path = reflect_enum.meta().bevy_reflect_path(); let enum_path = reflect_enum.meta().type_path(); + let is_remote = reflect_enum.is_remote(); + + // For `match self` expressions where self is a reference + let match_this = if is_remote { + quote!(&self.0) + } else { + quote!(self) + }; + // For `match self` expressions where self is a mutable reference + let match_this_mut = if is_remote { + quote!(&mut self.0) + } else { + quote!(self) + }; + // For `*self` assignments + let deref_this = if is_remote { + quote!(self.0) + } else { + quote!(*self) + }; let ref_name = Ident::new("__name_param", Span::call_site()); let ref_index = Ident::new("__index_param", Span::call_site()); @@ -73,42 +93,42 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream impl #impl_generics #bevy_reflect_path::Enum for #enum_path #ty_generics #where_reflect_clause { fn field(&self, #ref_name: &str) -> #FQOption<&dyn #bevy_reflect_path::PartialReflect> { - match self { + match #match_this { #(#enum_field,)* _ => #FQOption::None, } } fn field_at(&self, #ref_index: usize) -> #FQOption<&dyn #bevy_reflect_path::PartialReflect> { - match self { + match #match_this { #(#enum_field_at,)* _ => #FQOption::None, } } fn field_mut(&mut self, #ref_name: &str) -> #FQOption<&mut dyn #bevy_reflect_path::PartialReflect> { - match self { + match #match_this_mut { #(#enum_field,)* _ => #FQOption::None, } } fn field_at_mut(&mut self, #ref_index: usize) -> #FQOption<&mut dyn #bevy_reflect_path::PartialReflect> { - match self { + match #match_this_mut { #(#enum_field_at,)* _ => #FQOption::None, } } fn index_of(&self, #ref_name: &str) -> #FQOption { - match self { + match #match_this { #(#enum_index_of,)* _ => #FQOption::None, } } fn name_at(&self, #ref_index: usize) -> #FQOption<&str> { - match self { + match #match_this { #(#enum_name_at,)* _ => #FQOption::None, } @@ -120,7 +140,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream #[inline] fn field_len(&self) -> usize { - match self { + match #match_this { #(#enum_field_len,)* _ => 0, } @@ -128,7 +148,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream #[inline] fn variant_name(&self) -> &str { - match self { + match #match_this { #(#enum_variant_name,)* _ => unreachable!(), } @@ -136,7 +156,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream #[inline] fn variant_index(&self) -> usize { - match self { + match #match_this { #(#enum_variant_index,)* _ => unreachable!(), } @@ -144,7 +164,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream #[inline] fn variant_type(&self) -> #bevy_reflect_path::VariantType { - match self { + match #match_this { #(#enum_variant_type,)* _ => unreachable!(), } @@ -197,7 +217,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream // New variant -> perform a switch match #bevy_reflect_path::Enum::variant_name(#ref_value) { #(#variant_names => { - *self = #variant_constructors + #deref_this = #variant_constructors })* name => { return #FQResult::Err( diff --git a/crates/bevy_reflect/derive/src/impls/structs.rs b/crates/bevy_reflect/derive/src/impls/structs.rs index 964b91e788396..76e1da8d1a62f 100644 --- a/crates/bevy_reflect/derive/src/impls/structs.rs +++ b/crates/bevy_reflect/derive/src/impls/structs.rs @@ -10,6 +10,7 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> proc_macro2::TokenS let bevy_reflect_path = reflect_struct.meta().bevy_reflect_path(); let struct_path = reflect_struct.meta().type_path(); + let is_remote = reflect_struct.is_remote(); let field_names = reflect_struct .active_fields() @@ -22,11 +23,19 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> proc_macro2::TokenS .unwrap_or_else(|| field.declaration_index.to_string()) }) .collect::>(); - let field_idents = reflect_struct + let field_accessors = reflect_struct .active_fields() - .map(|field| ident_or_index(field.data.ident.as_ref(), field.declaration_index)) + .map(|field| { + let member = ident_or_index(field.data.ident.as_ref(), field.declaration_index); + + if is_remote { + quote!(0.#member) + } else { + quote!(#member) + } + }) .collect::>(); - let field_count = field_idents.len(); + let field_count = field_accessors.len(); let field_indices = (0..field_count).collect::>(); let where_clause_options = reflect_struct.where_clause_options(); @@ -74,28 +83,28 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> proc_macro2::TokenS impl #impl_generics #bevy_reflect_path::Struct for #struct_path #ty_generics #where_reflect_clause { fn field(&self, name: &str) -> #FQOption<&dyn #bevy_reflect_path::PartialReflect> { match name { - #(#field_names => #fqoption::Some(&self.#field_idents),)* + #(#field_names => #fqoption::Some(&self.#field_accessors),)* _ => #FQOption::None, } } fn field_mut(&mut self, name: &str) -> #FQOption<&mut dyn #bevy_reflect_path::PartialReflect> { match name { - #(#field_names => #fqoption::Some(&mut self.#field_idents),)* + #(#field_names => #fqoption::Some(&mut self.#field_accessors),)* _ => #FQOption::None, } } fn field_at(&self, index: usize) -> #FQOption<&dyn #bevy_reflect_path::PartialReflect> { match index { - #(#field_indices => #fqoption::Some(&self.#field_idents),)* + #(#field_indices => #fqoption::Some(&self.#field_accessors),)* _ => #FQOption::None, } } fn field_at_mut(&mut self, index: usize) -> #FQOption<&mut dyn #bevy_reflect_path::PartialReflect> { match index { - #(#field_indices => #fqoption::Some(&mut self.#field_idents),)* + #(#field_indices => #fqoption::Some(&mut self.#field_accessors),)* _ => #FQOption::None, } } @@ -118,7 +127,7 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> proc_macro2::TokenS fn clone_dynamic(&self) -> #bevy_reflect_path::DynamicStruct { let mut dynamic: #bevy_reflect_path::DynamicStruct = #FQDefault::default(); dynamic.set_represented_type(#bevy_reflect_path::PartialReflect::get_represented_type_info(self)); - #(dynamic.insert_boxed(#field_names, #bevy_reflect_path::PartialReflect::clone_value(&self.#field_idents));)* + #(dynamic.insert_boxed(#field_names, #bevy_reflect_path::PartialReflect::clone_value(&self.#field_accessors));)* dynamic } } diff --git a/crates/bevy_reflect/derive/src/impls/tuple_structs.rs b/crates/bevy_reflect/derive/src/impls/tuple_structs.rs index 50424c5dbf2fe..ef27a8441c395 100644 --- a/crates/bevy_reflect/derive/src/impls/tuple_structs.rs +++ b/crates/bevy_reflect/derive/src/impls/tuple_structs.rs @@ -10,12 +10,20 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> proc_macro2:: let bevy_reflect_path = reflect_struct.meta().bevy_reflect_path(); let struct_path = reflect_struct.meta().type_path(); + let is_remote = reflect_struct.is_remote(); - let field_idents = reflect_struct + let field_accessors = reflect_struct .active_fields() .map(|field| Member::Unnamed(Index::from(field.declaration_index))) + .map(|member| { + if is_remote { + quote!(0.#member) + } else { + quote!(#member) + } + }) .collect::>(); - let field_count = field_idents.len(); + let field_count = field_accessors.len(); let field_indices = (0..field_count).collect::>(); let where_clause_options = reflect_struct.where_clause_options(); @@ -63,14 +71,14 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> proc_macro2:: impl #impl_generics #bevy_reflect_path::TupleStruct for #struct_path #ty_generics #where_reflect_clause { fn field(&self, index: usize) -> #FQOption<&dyn #bevy_reflect_path::PartialReflect> { match index { - #(#field_indices => #fqoption::Some(&self.#field_idents),)* + #(#field_indices => #fqoption::Some(&self.#field_accessors),)* _ => #FQOption::None, } } fn field_mut(&mut self, index: usize) -> #FQOption<&mut dyn #bevy_reflect_path::PartialReflect> { match index { - #(#field_indices => #fqoption::Some(&mut self.#field_idents),)* + #(#field_indices => #fqoption::Some(&mut self.#field_accessors),)* _ => #FQOption::None, } } @@ -86,7 +94,7 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> proc_macro2:: fn clone_dynamic(&self) -> #bevy_reflect_path::DynamicTupleStruct { let mut dynamic: #bevy_reflect_path::DynamicTupleStruct = #FQDefault::default(); dynamic.set_represented_type(#bevy_reflect_path::PartialReflect::get_represented_type_info(self)); - #(dynamic.insert_boxed(#bevy_reflect_path::PartialReflect::clone_value(&self.#field_idents));)* + #(dynamic.insert_boxed(#bevy_reflect_path::PartialReflect::clone_value(&self.#field_accessors));)* dynamic } } diff --git a/crates/bevy_reflect/derive/src/lib.rs b/crates/bevy_reflect/derive/src/lib.rs index 86e0069a887fb..4e0b1b10a2099 100644 --- a/crates/bevy_reflect/derive/src/lib.rs +++ b/crates/bevy_reflect/derive/src/lib.rs @@ -27,6 +27,7 @@ mod from_reflect; mod impls; mod reflect_value; mod registration; +mod remote; mod serialization; mod trait_reflection; mod type_path; @@ -511,6 +512,70 @@ pub fn reflect_trait(args: TokenStream, input: TokenStream) -> TokenStream { trait_reflection::reflect_trait(&args, input) } +/// Generates a wrapper type that can be used to "derive `Reflect`" for remote types. +/// +/// This works by wrapping the remote type in a generated wrapper that has the `#[repr(transparent)]` attribute. +/// This allows the two types to be safely transmuted back-and-forth. +/// +/// # Defining the Wrapper +/// +/// Before defining the wrapper type, please note that it is _required_ that all fields of the remote type are public. +/// The generated code will, at times, need to access or mutate them, +/// and we do not currently have a way to assign getters/setters to each field +/// (but this may change in the future). +/// +/// The wrapper definition should match the remote type 1-to-1. +/// This includes the naming and ordering of the fields and variants. +/// +/// Generics and lifetimes do _not_ need to have the same names, however, they _do_ need to follow the same order. +/// Additionally, whether generics are inlined or placed in a where clause should not matter. +/// +/// Lastly, all macros and doc-comments should be placed __below__ this attribute. +/// If they are placed above, they will not be properly passed to the generated wrapper type. +/// +/// # Example +/// +/// Given a remote type, `RemoteType`: +/// +/// ``` +/// #[derive(Default)] +/// struct RemoteType +/// where +/// T: Default + Clone, +/// { +/// pub foo: T, +/// pub bar: usize +/// } +/// ``` +/// +/// We would define our wrapper type as such: +/// +/// ```ignore +/// use external_crate::RemoteType; +/// +/// #[reflect_remote(RemoteType)] +/// #[derive(Default)] +/// pub struct WrapperType { +/// pub foo: T, +/// pub bar: usize +/// } +/// ``` +/// +/// Apart from all the reflection trait implementations, this generates something like the following: +/// +/// ```ignore +/// use external_crate::RemoteType; +/// +/// #[derive(Default)] +/// #[repr(transparent)] +/// pub struct Wrapper(RemoteType); +/// ``` +/// +#[proc_macro_attribute] +pub fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStream { + remote::reflect_remote(args, input) +} + /// A macro used to generate reflection trait implementations for the given type. /// /// This is functionally the same as [deriving `Reflect`] using the `#[reflect_value]` container attribute. diff --git a/crates/bevy_reflect/derive/src/remote.rs b/crates/bevy_reflect/derive/src/remote.rs new file mode 100644 index 0000000000000..af077dbbc667f --- /dev/null +++ b/crates/bevy_reflect/derive/src/remote.rs @@ -0,0 +1,99 @@ +use crate::derive_data::{ReflectImplSource, ReflectProvenance, ReflectTraitToImpl}; +use crate::{from_reflect, impls, ReflectDerive, REFLECT_ATTRIBUTE_NAME}; +use proc_macro::TokenStream; +use quote::quote; +use syn::spanned::Spanned; +use syn::{parse_macro_input, DeriveInput, TypePath}; + +/// Generates the remote wrapper type and implements all the necessary traits. +pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStream { + let remote_ty = match syn::parse::(args) { + Ok(path) => path, + Err(err) => return err.to_compile_error().into(), + }; + + let ast = parse_macro_input!(input as DeriveInput); + let wrapper_definition = generate_remote_wrapper(&ast, &remote_ty); + + let mut derive_data = match ReflectDerive::from_input( + &ast, + ReflectProvenance { + source: ReflectImplSource::RemoteReflect, + trait_: ReflectTraitToImpl::Reflect, + }, + ) { + Ok(data) => data, + Err(err) => return err.into_compile_error().into(), + }; + + derive_data.set_remote(Some(&remote_ty)); + + let (reflect_impls, from_reflect_impl) = match derive_data { + ReflectDerive::Struct(struct_data) | ReflectDerive::UnitStruct(struct_data) => ( + impls::impl_struct(&struct_data), + if struct_data.meta().from_reflect().should_auto_derive() { + Some(from_reflect::impl_struct(&struct_data)) + } else { + None + }, + ), + ReflectDerive::TupleStruct(struct_data) => ( + impls::impl_tuple_struct(&struct_data), + if struct_data.meta().from_reflect().should_auto_derive() { + Some(from_reflect::impl_tuple_struct(&struct_data)) + } else { + None + }, + ), + ReflectDerive::Enum(enum_data) => ( + impls::impl_enum(&enum_data), + if enum_data.meta().from_reflect().should_auto_derive() { + Some(from_reflect::impl_enum(&enum_data)) + } else { + None + }, + ), + _ => { + return syn::Error::new(ast.span(), "cannot reflect a remote value type") + .into_compile_error() + .into() + } + }; + + TokenStream::from(quote! { + #wrapper_definition + + #reflect_impls + + #from_reflect_impl + }) +} + +/// Generates the remote wrapper type. +/// +/// # Example +/// +/// If the supplied remote type is `Bar`, then the wrapper type— named `Foo`— would look like: +/// +/// ``` +/// # struct Bar(T); +/// +/// #[repr(transparent)] +/// struct Foo(Bar); +/// ``` +fn generate_remote_wrapper(input: &DeriveInput, remote_ty: &TypePath) -> proc_macro2::TokenStream { + let ident = &input.ident; + let vis = &input.vis; + let ty_generics = &input.generics; + let where_clause = &input.generics.where_clause; + let attrs = input + .attrs + .iter() + .filter(|attr| !attr.path().is_ident(REFLECT_ATTRIBUTE_NAME)); + + quote! { + #(#attrs)* + #[repr(transparent)] + #vis struct #ident #ty_generics (pub #remote_ty) #where_clause; + } +} diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 1c5d9d12c6322..d2df27d6f544e 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -2476,6 +2476,37 @@ bevy_reflect::tests::Test { assert_impl_all!(TupleStruct: Reflect); assert_impl_all!(Enum: Reflect); } + + #[test] + fn should_reflect_remote_type() { + mod external_crate { + #[derive(Debug, Default)] + pub struct TheirType { + pub value: String, + } + } + + #[reflect_remote(external_crate::TheirType)] + // TODO: Remove + #[reflect(from_reflect = false)] + #[derive(Debug, Default)] + #[reflect(Debug, Default)] + struct MyType { + pub value: String, + } + + let mut patch = DynamicStruct::default(); + patch.set_represented_type(Some(MyType::type_info())); + patch.insert("value", "Goodbye".to_string()); + + let mut data = MyType(external_crate::TheirType { + value: "Hello".to_string(), + }); + + assert_eq!("Hello", data.0.value); + data.apply(&patch); + assert_eq!("Goodbye", data.0.value); + } #[cfg(feature = "glam")] mod glam { From c8b3167570388a524258eac6671401e95767aa74 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Mon, 19 Sep 2022 22:41:54 -0700 Subject: [PATCH 02/31] Added remote field attribute --- crates/bevy_reflect/derive/src/derive_data.rs | 14 ++- .../derive/src/field_attributes.rs | 29 +++++- crates/bevy_reflect/derive/src/impls/enums.rs | 57 +++++++++++- .../bevy_reflect/derive/src/impls/structs.rs | 33 +++---- .../derive/src/impls/tuple_structs.rs | 31 +++---- crates/bevy_reflect/derive/src/lib.rs | 1 + .../bevy_reflect/derive/src/struct_utility.rs | 92 +++++++++++++++++++ crates/bevy_reflect/src/lib.rs | 49 ++++++++++ 8 files changed, 258 insertions(+), 48 deletions(-) create mode 100644 crates/bevy_reflect/derive/src/struct_utility.rs diff --git a/crates/bevy_reflect/derive/src/derive_data.rs b/crates/bevy_reflect/derive/src/derive_data.rs index a75a9fda7338a..30e1c1b22d77e 100644 --- a/crates/bevy_reflect/derive/src/derive_data.rs +++ b/crates/bevy_reflect/derive/src/derive_data.rs @@ -530,7 +530,7 @@ impl<'a> StructField<'a> { } }; - let ty = &self.data.ty; + let ty = self.reflected_type(); let custom_attributes = self.attrs.custom_attributes.to_tokens(bevy_reflect_path); #[allow(unused_mut)] // Needs mutability for the feature gate @@ -548,6 +548,14 @@ impl<'a> StructField<'a> { info } + + /// Returns the reflected type of this field. + /// + /// Normally this is just the field's defined type. + /// However, this can be adjusted to use a different type, like for representing remote types. + pub fn reflected_type(&self) -> &Type { + self.attrs.remote.as_ref().unwrap_or(&self.data.ty) + } } impl<'a> ReflectStruct<'a> { @@ -590,7 +598,7 @@ impl<'a> ReflectStruct<'a> { /// Get a collection of types which are exposed to the reflection API pub fn active_types(&self) -> Vec { self.active_fields() - .map(|field| field.data.ty.clone()) + .map(|field| field.reflected_type().clone()) .collect() } @@ -704,7 +712,7 @@ impl<'a> ReflectEnum<'a> { /// Get a collection of types which are exposed to the reflection API pub fn active_types(&self) -> Vec { self.active_fields() - .map(|field| field.data.ty.clone()) + .map(|field| field.reflected_type().clone()) .collect() } diff --git a/crates/bevy_reflect/derive/src/field_attributes.rs b/crates/bevy_reflect/derive/src/field_attributes.rs index d66daf8382b4d..e04788cb522a6 100644 --- a/crates/bevy_reflect/derive/src/field_attributes.rs +++ b/crates/bevy_reflect/derive/src/field_attributes.rs @@ -7,13 +7,15 @@ use crate::custom_attributes::CustomAttributes; use crate::utility::terminated_parser; use crate::REFLECT_ATTRIBUTE_NAME; +use quote::ToTokens; use syn::parse::ParseStream; -use syn::{Attribute, LitStr, Meta, Token}; +use syn::{Attribute, LitStr, Meta, Token, Type}; mod kw { syn::custom_keyword!(ignore); syn::custom_keyword!(skip_serializing); syn::custom_keyword!(default); + syn::custom_keyword!(remote); } pub(crate) const IGNORE_SERIALIZATION_ATTR: &str = "skip_serializing"; @@ -76,6 +78,8 @@ pub(crate) struct FieldAttributes { pub default: DefaultBehavior, /// Custom attributes created via `#[reflect(@...)]`. pub custom_attributes: CustomAttributes, + /// For defining the remote wrapper type that should be used in place of the field for reflection logic. + pub remote: Option, } impl FieldAttributes { @@ -119,6 +123,8 @@ impl FieldAttributes { self.parse_skip_serializing(input) } else if lookahead.peek(kw::default) { self.parse_default(input) + } else if lookahead.peek(kw::remote) { + self.parse_remote(input) } else { Err(lookahead.error()) } @@ -190,4 +196,25 @@ impl FieldAttributes { fn parse_custom_attribute(&mut self, input: ParseStream) -> syn::Result<()> { self.custom_attributes.parse_custom_attribute(input) } + + /// Parse `remote` attribute. + /// + /// Examples: + /// - `#[reflect(remote = "path::to::RemoteType")]` + fn parse_remote(&mut self, input: ParseStream) -> syn::Result<()> { + if let Some(remote) = self.remote.as_ref() { + return Err(input.error(format!( + "remote type already specified as {}", + remote.to_token_stream() + ))); + } + + input.parse::()?; + input.parse::()?; + + let lit = input.parse::()?; + self.remote = Some(lit.parse()?); + + Ok(()) + } } diff --git a/crates/bevy_reflect/derive/src/impls/enums.rs b/crates/bevy_reflect/derive/src/impls/enums.rs index f4c974253dc68..87f60bd7e7635 100644 --- a/crates/bevy_reflect/derive/src/impls/enums.rs +++ b/crates/bevy_reflect/derive/src/impls/enums.rs @@ -38,7 +38,9 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream let EnumImpls { enum_field, + enum_field_mut, enum_field_at, + enum_field_at_mut, enum_index_of, enum_name_at, enum_field_len, @@ -108,14 +110,14 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream fn field_mut(&mut self, #ref_name: &str) -> #FQOption<&mut dyn #bevy_reflect_path::PartialReflect> { match #match_this_mut { - #(#enum_field,)* + #(#enum_field_mut,)* _ => #FQOption::None, } } fn field_at_mut(&mut self, #ref_index: usize) -> #FQOption<&mut dyn #bevy_reflect_path::PartialReflect> { match #match_this_mut { - #(#enum_field_at,)* + #(#enum_field_at_mut,)* _ => #FQOption::None, } } @@ -263,7 +265,9 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream struct EnumImpls { enum_field: Vec, + enum_field_mut: Vec, enum_field_at: Vec, + enum_field_at_mut: Vec, enum_index_of: Vec, enum_name_at: Vec, enum_field_len: Vec, @@ -276,7 +280,9 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden let bevy_reflect_path = reflect_enum.meta().bevy_reflect_path(); let mut enum_field = Vec::new(); + let mut enum_field_mut = Vec::new(); let mut enum_field_at = Vec::new(); + let mut enum_field_at_mut = Vec::new(); let mut enum_index_of = Vec::new(); let mut enum_name_at = Vec::new(); let mut enum_field_len = Vec::new(); @@ -324,6 +330,27 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden field_len } + /// Process the field value to account for remote types. + /// + /// If the field is a remote type, then the value will be transmuted accordingly. + fn process_field_value( + ident: &Ident, + field: &StructField, + is_mutable: bool, + ) -> proc_macro2::TokenStream { + let ref_token = if is_mutable { quote!(&mut) } else { quote!(&) }; + field + .attrs + .remote + .as_ref() + .map(|ty| { + quote! { + unsafe { ::core::mem::transmute::<#ref_token _, #ref_token #ty>(#ident) } + } + }) + .unwrap_or_else(|| quote!(#ident)) + } + match &variant.fields { EnumVariantFields::Unit => { let field_len = process_fields(&[], |_| {}); @@ -339,8 +366,16 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden .expect("reflection index should exist for active field"); let declare_field = syn::Index::from(field.declaration_index); + + let __value = Ident::new("__value", Span::call_site()); + let value_ref = process_field_value(&__value, field, false); + let value_mut = process_field_value(&__value, field, true); + enum_field_at.push(quote! { - #unit { #declare_field : value, .. } if #ref_index == #reflection_index => #FQOption::Some(value) + #unit { #declare_field : #__value, .. } if #ref_index == #reflection_index => #FQOption::Some(#value_ref) + }); + enum_field_at_mut.push(quote! { + #unit { #declare_field : #__value, .. } if #ref_index == #reflection_index => #FQOption::Some(#value_mut) }); }); @@ -356,11 +391,21 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden .reflection_index .expect("reflection index should exist for active field"); + let __value = Ident::new("__value", Span::call_site()); + let value_ref = process_field_value(&__value, field, false); + let value_mut = process_field_value(&__value, field, true); + enum_field.push(quote! { - #unit{ #field_ident, .. } if #ref_name == #field_name => #FQOption::Some(#field_ident) + #unit{ #field_ident: #__value, .. } if #ref_name == #field_name => #FQOption::Some(#value_ref) + }); + enum_field_mut.push(quote! { + #unit{ #field_ident: #__value, .. } if #ref_name == #field_name => #FQOption::Some(#value_mut) }); enum_field_at.push(quote! { - #unit{ #field_ident, .. } if #ref_index == #reflection_index => #FQOption::Some(#field_ident) + #unit{ #field_ident: #__value, .. } if #ref_index == #reflection_index => #FQOption::Some(#value_ref) + }); + enum_field_at_mut.push(quote! { + #unit{ #field_ident: #__value, .. } if #ref_index == #reflection_index => #FQOption::Some(#value_mut) }); enum_index_of.push(quote! { #unit{ .. } if #ref_name == #field_name => #FQOption::Some(#reflection_index) @@ -379,7 +424,9 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden EnumImpls { enum_field, + enum_field_mut, enum_field_at, + enum_field_at_mut, enum_index_of, enum_name_at, enum_field_len, diff --git a/crates/bevy_reflect/derive/src/impls/structs.rs b/crates/bevy_reflect/derive/src/impls/structs.rs index 76e1da8d1a62f..397dabec9b77f 100644 --- a/crates/bevy_reflect/derive/src/impls/structs.rs +++ b/crates/bevy_reflect/derive/src/impls/structs.rs @@ -1,5 +1,5 @@ use crate::impls::{common_partial_reflect_methods, impl_full_reflect, impl_type_path, impl_typed}; -use crate::utility::ident_or_index; +use crate::struct_utility::FieldAccessors; use crate::ReflectStruct; use bevy_macro_utils::fq_std::{FQBox, FQDefault, FQOption, FQResult}; use quote::{quote, ToTokens}; @@ -10,7 +10,6 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> proc_macro2::TokenS let bevy_reflect_path = reflect_struct.meta().bevy_reflect_path(); let struct_path = reflect_struct.meta().type_path(); - let is_remote = reflect_struct.is_remote(); let field_names = reflect_struct .active_fields() @@ -23,20 +22,14 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> proc_macro2::TokenS .unwrap_or_else(|| field.declaration_index.to_string()) }) .collect::>(); - let field_accessors = reflect_struct - .active_fields() - .map(|field| { - let member = ident_or_index(field.data.ident.as_ref(), field.declaration_index); - if is_remote { - quote!(0.#member) - } else { - quote!(#member) - } - }) - .collect::>(); - let field_count = field_accessors.len(); - let field_indices = (0..field_count).collect::>(); + let FieldAccessors { + fields, + fields_ref, + fields_mut, + field_indices, + field_count, + } = FieldAccessors::new(reflect_struct); let where_clause_options = reflect_struct.where_clause_options(); let typed_impl = impl_typed( @@ -83,28 +76,28 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> proc_macro2::TokenS impl #impl_generics #bevy_reflect_path::Struct for #struct_path #ty_generics #where_reflect_clause { fn field(&self, name: &str) -> #FQOption<&dyn #bevy_reflect_path::PartialReflect> { match name { - #(#field_names => #fqoption::Some(&self.#field_accessors),)* + #(#field_names => #fqoption::Some(#fields_ref),)* _ => #FQOption::None, } } fn field_mut(&mut self, name: &str) -> #FQOption<&mut dyn #bevy_reflect_path::PartialReflect> { match name { - #(#field_names => #fqoption::Some(&mut self.#field_accessors),)* + #(#field_names => #fqoption::Some(#fields_mut),)* _ => #FQOption::None, } } fn field_at(&self, index: usize) -> #FQOption<&dyn #bevy_reflect_path::PartialReflect> { match index { - #(#field_indices => #fqoption::Some(&self.#field_accessors),)* + #(#field_indices => #fqoption::Some(#fields_ref),)* _ => #FQOption::None, } } fn field_at_mut(&mut self, index: usize) -> #FQOption<&mut dyn #bevy_reflect_path::PartialReflect> { match index { - #(#field_indices => #fqoption::Some(&mut self.#field_accessors),)* + #(#field_indices => #fqoption::Some(#fields_mut),)* _ => #FQOption::None, } } @@ -127,7 +120,7 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> proc_macro2::TokenS fn clone_dynamic(&self) -> #bevy_reflect_path::DynamicStruct { let mut dynamic: #bevy_reflect_path::DynamicStruct = #FQDefault::default(); dynamic.set_represented_type(#bevy_reflect_path::PartialReflect::get_represented_type_info(self)); - #(dynamic.insert_boxed(#field_names, #bevy_reflect_path::PartialReflect::clone_value(&self.#field_accessors));)* + #(dynamic.insert_boxed(#field_names, #bevy_reflect_path::PartialReflect::clone_value(&#fields));)* dynamic } } diff --git a/crates/bevy_reflect/derive/src/impls/tuple_structs.rs b/crates/bevy_reflect/derive/src/impls/tuple_structs.rs index ef27a8441c395..115ad872eae0d 100644 --- a/crates/bevy_reflect/derive/src/impls/tuple_structs.rs +++ b/crates/bevy_reflect/derive/src/impls/tuple_structs.rs @@ -1,8 +1,8 @@ use crate::impls::{common_partial_reflect_methods, impl_full_reflect, impl_type_path, impl_typed}; +use crate::struct_utility::FieldAccessors; use crate::ReflectStruct; use bevy_macro_utils::fq_std::{FQBox, FQDefault, FQOption, FQResult}; use quote::{quote, ToTokens}; -use syn::{Index, Member}; /// Implements `TupleStruct`, `GetTypeRegistration`, and `Reflect` for the given derive data. pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> proc_macro2::TokenStream { @@ -10,21 +10,14 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> proc_macro2:: let bevy_reflect_path = reflect_struct.meta().bevy_reflect_path(); let struct_path = reflect_struct.meta().type_path(); - let is_remote = reflect_struct.is_remote(); - - let field_accessors = reflect_struct - .active_fields() - .map(|field| Member::Unnamed(Index::from(field.declaration_index))) - .map(|member| { - if is_remote { - quote!(0.#member) - } else { - quote!(#member) - } - }) - .collect::>(); - let field_count = field_accessors.len(); - let field_indices = (0..field_count).collect::>(); + + let FieldAccessors { + fields, + fields_ref, + fields_mut, + field_indices, + field_count, + } = FieldAccessors::new(reflect_struct); let where_clause_options = reflect_struct.where_clause_options(); let get_type_registration_impl = reflect_struct.get_type_registration(&where_clause_options); @@ -71,14 +64,14 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> proc_macro2:: impl #impl_generics #bevy_reflect_path::TupleStruct for #struct_path #ty_generics #where_reflect_clause { fn field(&self, index: usize) -> #FQOption<&dyn #bevy_reflect_path::PartialReflect> { match index { - #(#field_indices => #fqoption::Some(&self.#field_accessors),)* + #(#field_indices => #fqoption::Some(#fields_ref),)* _ => #FQOption::None, } } fn field_mut(&mut self, index: usize) -> #FQOption<&mut dyn #bevy_reflect_path::PartialReflect> { match index { - #(#field_indices => #fqoption::Some(&mut self.#field_accessors),)* + #(#field_indices => #fqoption::Some(#fields_mut),)* _ => #FQOption::None, } } @@ -94,7 +87,7 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> proc_macro2:: fn clone_dynamic(&self) -> #bevy_reflect_path::DynamicTupleStruct { let mut dynamic: #bevy_reflect_path::DynamicTupleStruct = #FQDefault::default(); dynamic.set_represented_type(#bevy_reflect_path::PartialReflect::get_represented_type_info(self)); - #(dynamic.insert_boxed(#bevy_reflect_path::PartialReflect::clone_value(&self.#field_accessors));)* + #(dynamic.insert_boxed(#bevy_reflect_path::PartialReflect::clone_value(&#fields));)* dynamic } } diff --git a/crates/bevy_reflect/derive/src/lib.rs b/crates/bevy_reflect/derive/src/lib.rs index 4e0b1b10a2099..e09b437cfb7f0 100644 --- a/crates/bevy_reflect/derive/src/lib.rs +++ b/crates/bevy_reflect/derive/src/lib.rs @@ -29,6 +29,7 @@ mod reflect_value; mod registration; mod remote; mod serialization; +mod struct_utility; mod trait_reflection; mod type_path; mod utility; diff --git a/crates/bevy_reflect/derive/src/struct_utility.rs b/crates/bevy_reflect/derive/src/struct_utility.rs new file mode 100644 index 0000000000000..bc7f27adc371b --- /dev/null +++ b/crates/bevy_reflect/derive/src/struct_utility.rs @@ -0,0 +1,92 @@ +use crate::derive_data::StructField; +use crate::{utility, ReflectStruct}; +use quote::quote; + +/// A helper struct for creating remote-aware field accessors. +/// +/// These are "remote-aware" because when a field is a remote field, it uses a [`transmute`] internally +/// to access the field. +/// +/// [`transmute`]: core::mem::transmute +pub(crate) struct FieldAccessors { + /// The owned field accessors, such as `self.foo`. + pub fields: Vec, + /// The referenced field accessors, such as `&self.foo`. + pub fields_ref: Vec, + /// The mutably referenced field accessors, such as `&mut self.foo`. + pub fields_mut: Vec, + /// The ordered set of field indices (basically just the range of [0, `field_count`). + pub field_indices: Vec, + /// The number of fields in the reflected struct. + pub field_count: usize, +} + +impl FieldAccessors { + pub fn new(reflect_struct: &ReflectStruct) -> Self { + let fields = Self::get_fields(reflect_struct, |field, accessor| { + match &field.attrs.remote { + Some(wrapper_ty) => { + quote! { + unsafe { ::core::mem::transmute_copy::<_, #wrapper_ty>(&#accessor) } + } + } + None => accessor, + } + }); + let fields_ref = Self::get_fields(reflect_struct, |field, accessor| { + match &field.attrs.remote { + Some(wrapper_ty) => { + quote! { + unsafe { ::core::mem::transmute::<&_, &#wrapper_ty>(&#accessor) } + } + } + None => quote!(& #accessor), + } + }); + let fields_mut = Self::get_fields(reflect_struct, |field, accessor| { + match &field.attrs.remote { + Some(wrapper_ty) => { + quote! { + unsafe { ::core::mem::transmute::<&mut _, &mut #wrapper_ty>(&mut #accessor) } + } + } + None => quote!(&mut #accessor), + } + }); + + let field_count = fields.len(); + let field_indices = (0..field_count).collect(); + + Self { + fields, + fields_ref, + fields_mut, + field_indices, + field_count, + } + } + + fn get_fields( + reflect_struct: &ReflectStruct, + mut wrapper_fn: F, + ) -> Vec + where + F: FnMut(&StructField, proc_macro2::TokenStream) -> proc_macro2::TokenStream, + { + let is_remote = reflect_struct.is_remote(); + reflect_struct + .active_fields() + .map(|field| { + let member = + utility::ident_or_index(field.data.ident.as_ref(), field.declaration_index); + let accessor = if is_remote { + quote!(self.0.#member) + } else { + quote!(self.#member) + }; + + wrapper_fn(field, accessor) + }) + .collect::>() + } +} diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index d2df27d6f544e..65762c5382823 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -2486,6 +2486,7 @@ bevy_reflect::tests::Test { } } + // === Remote Wrapper === // #[reflect_remote(external_crate::TheirType)] // TODO: Remove #[reflect(from_reflect = false)] @@ -2506,6 +2507,54 @@ bevy_reflect::tests::Test { assert_eq!("Hello", data.0.value); data.apply(&patch); assert_eq!("Goodbye", data.0.value); + + // === Struct Container === // + #[derive(Reflect, Debug)] + // TODO: Remove + #[reflect(from_reflect = false)] + struct ContainerStruct { + #[reflect(remote = "MyType")] + their_type: external_crate::TheirType, + } + + let mut patch = DynamicStruct::default(); + patch.set_represented_type(Some(ContainerStruct::type_info())); + patch.insert( + "their_type", + MyType(external_crate::TheirType { + value: "Goodbye".to_string(), + }), + ); + + let mut data = ContainerStruct { + their_type: external_crate::TheirType { + value: "Hello".to_string(), + }, + }; + + assert_eq!("Hello", data.their_type.value); + data.apply(&patch); + assert_eq!("Goodbye", data.their_type.value); + + // === Tuple Struct Container === // + #[derive(Reflect, Debug)] + // TODO: Remove + #[reflect(from_reflect = false)] + struct ContainerTupleStruct(#[reflect(remote = "MyType")] external_crate::TheirType); + + let mut patch = DynamicTupleStruct::default(); + patch.set_represented_type(Some(ContainerTupleStruct::type_info())); + patch.insert(MyType(external_crate::TheirType { + value: "Goodbye".to_string(), + })); + + let mut data = ContainerTupleStruct(external_crate::TheirType { + value: "Hello".to_string(), + }); + + assert_eq!("Hello", data.0.value); + data.apply(&patch); + assert_eq!("Goodbye", data.0.value); } #[cfg(feature = "glam")] From 0865c493910dc5e5124f2a74a055c0c9a9dd422a Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Mon, 19 Sep 2022 22:48:31 -0700 Subject: [PATCH 03/31] Add test for nested remote types --- crates/bevy_reflect/src/lib.rs | 35 ++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 65762c5382823..7150cea0567e4 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -2557,6 +2557,41 @@ bevy_reflect::tests::Test { assert_eq!("Goodbye", data.0.value); } + #[test] + fn should_reflect_nested_remote_type() { + mod external_crate { + pub struct TheirOuter { + pub inner: TheirInner, + } + pub struct TheirInner(pub usize); + } + + #[reflect_remote(external_crate::TheirOuter)] + // TODO: Remove + #[reflect(from_reflect = false)] + struct MyOuter { + #[reflect(remote = "MyInner")] + pub inner: external_crate::TheirInner, + } + + #[reflect_remote(external_crate::TheirInner)] + // TODO: Remove + #[reflect(from_reflect = false)] + struct MyInner(usize); + + let mut patch = DynamicStruct::default(); + patch.set_represented_type(Some(MyOuter::type_info())); + patch.insert("inner", MyInner(external_crate::TheirInner(321))); + + let mut data = MyOuter(external_crate::TheirOuter { + inner: external_crate::TheirInner(123), + }); + + assert_eq!(123, data.0.inner.0); + data.apply(&patch); + assert_eq!(321, data.0.inner.0); + } + #[cfg(feature = "glam")] mod glam { use super::*; From ae72190df52ca6549acd0b326b03eb0688b32a2f Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Tue, 20 Sep 2022 21:04:24 -0700 Subject: [PATCH 04/31] Add remote FromReflect support --- crates/bevy_reflect/derive/src/derive_data.rs | 7 +- .../bevy_reflect/derive/src/enum_utility.rs | 17 ++- .../bevy_reflect/derive/src/from_reflect.rs | 125 ++++++++++++------ crates/bevy_reflect/derive/src/lib.rs | 16 +++ crates/bevy_reflect/derive/src/remote.rs | 20 ++- crates/bevy_reflect/src/lib.rs | 76 +++++++++-- 6 files changed, 210 insertions(+), 51 deletions(-) diff --git a/crates/bevy_reflect/derive/src/derive_data.rs b/crates/bevy_reflect/derive/src/derive_data.rs index 30e1c1b22d77e..71dd636580fbd 100644 --- a/crates/bevy_reflect/derive/src/derive_data.rs +++ b/crates/bevy_reflect/derive/src/derive_data.rs @@ -553,9 +553,14 @@ impl<'a> StructField<'a> { /// /// Normally this is just the field's defined type. /// However, this can be adjusted to use a different type, like for representing remote types. + /// In those cases, the returned value is the remote wrapper type. pub fn reflected_type(&self) -> &Type { self.attrs.remote.as_ref().unwrap_or(&self.data.ty) } + + pub fn attrs(&self) -> &FieldAttributes { + &self.attrs + } } impl<'a> ReflectStruct<'a> { @@ -736,7 +741,7 @@ impl<'a> ReflectEnum<'a> { self.meta(), where_clause_options, None, - Some(self.active_fields().map(|field| &field.data.ty)), + Some(self.active_fields().map(StructField::reflected_type)), ) } diff --git a/crates/bevy_reflect/derive/src/enum_utility.rs b/crates/bevy_reflect/derive/src/enum_utility.rs index 50ea343b7a10a..a67c4f35d18ae 100644 --- a/crates/bevy_reflect/derive/src/enum_utility.rs +++ b/crates/bevy_reflect/derive/src/enum_utility.rs @@ -84,7 +84,7 @@ pub(crate) trait VariantBuilder: Sized { let alias = field.alias; let field_constructor = self.construct_field(field); - match &field.field.attrs.default { + let construction = match &field.field.attrs.default { DefaultBehavior::Func(path) => quote! { if let #FQOption::Some(#alias) = #field_accessor { #field_constructor @@ -109,6 +109,17 @@ pub(crate) trait VariantBuilder: Sized { #field_constructor }} } + }; + + if field.field.attrs().remote.is_some() { + quote! { + // SAFETY: The remote type should always be a `#[repr(transparent)]` for the actual field type + unsafe { + ::core::mem::transmute(#construction) + } + } + } else { + construction } } @@ -200,7 +211,7 @@ impl<'a> VariantBuilder for FromReflectVariantBuilder<'a> { fn construct_field(&self, field: VariantField) -> TokenStream { let bevy_reflect_path = self.reflect_enum.meta().bevy_reflect_path(); - let field_ty = &field.field.data.ty; + let field_ty = field.field.reflected_type(); let alias = field.alias; quote! { @@ -251,7 +262,7 @@ impl<'a> VariantBuilder for TryApplyVariantBuilder<'a> { fn construct_field(&self, field: VariantField) -> TokenStream { let bevy_reflect_path = self.reflect_enum.meta().bevy_reflect_path(); let alias = field.alias; - let field_ty = &field.field.data.ty; + let field_ty = field.field.reflected_type(); quote! { <#field_ty as #bevy_reflect_path::FromReflect>::from_reflect(#alias) diff --git a/crates/bevy_reflect/derive/src/from_reflect.rs b/crates/bevy_reflect/derive/src/from_reflect.rs index 5aa637794ad5b..64ae7839505e4 100644 --- a/crates/bevy_reflect/derive/src/from_reflect.rs +++ b/crates/bevy_reflect/derive/src/from_reflect.rs @@ -52,6 +52,16 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream .. } = FromReflectVariantBuilder::new(reflect_enum).build(&ref_value); + let match_branches = if reflect_enum.is_remote() { + quote! { + #(#variant_names => #fqoption::Some(Self(#variant_constructors)),)* + } + } else { + quote! { + #(#variant_names => #fqoption::Some(#variant_constructors),)* + } + }; + let (impl_generics, ty_generics, where_clause) = enum_path.generics().split_for_impl(); // Add FromReflect bound for each active field @@ -66,7 +76,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream #bevy_reflect_path::PartialReflect::reflect_ref(#ref_value) { match #bevy_reflect_path::Enum::variant_name(#ref_value) { - #(#variant_names => #fqoption::Some(#variant_constructors),)* + #match_branches name => panic!("variant with name `{}` does not exist on enum `{}`", name, ::type_path()), } } else { @@ -105,30 +115,40 @@ fn impl_struct_internal( let MemberValuePair(active_members, active_values) = get_active_fields(reflect_struct, &ref_struct, &ref_struct_type, is_tuple); - + let is_defaultable = reflect_struct.meta().attrs().contains(REFLECT_DEFAULT); + + // The constructed "Self" ident + let __this = Ident::new("__this", Span::call_site()); + + // The reflected type: either `Self` or a remote type + let (reflect_ty, retval) = if let Some(remote_ty) = reflect_struct.remote_ty() { + (quote!(#remote_ty), quote!(Self(#__this))) + } else { + (quote!(Self), quote!(#__this)) + }; + let constructor = if is_defaultable { - quote!( - let mut __this: Self = #FQDefault::default(); + quote! { + let mut #__this = <#reflect_ty as #FQDefault>::default(); #( if let #fqoption::Some(__field) = #active_values() { // Iff field exists -> use its value - __this.#active_members = __field; + #__this.#active_members = __field; } )* - #FQOption::Some(__this) - ) + #FQOption::Some(#retval) + } } else { let MemberValuePair(ignored_members, ignored_values) = get_ignored_fields(reflect_struct); - quote!( - #FQOption::Some( - Self { - #(#active_members: #active_values()?,)* - #(#ignored_members: #ignored_values,)* - } - ) - ) + quote! { + let #__this = #reflect_ty { + #(#active_members: #active_values()?,)* + #(#ignored_members: #ignored_values,)* + }; + #FQOption::Some(#retval) + } }; let (impl_generics, ty_generics, where_clause) = reflect_struct @@ -201,34 +221,65 @@ fn get_active_fields( field.reflection_index.expect("field should be active"), is_tuple, ); - let ty = field.data.ty.clone(); + let ty = field.reflected_type().clone(); + let remote_ty = &field.data.ty; let get_field = quote! { #bevy_reflect_path::#struct_type::field(#dyn_struct_name, #accessor) }; + let into_remote = |value: proc_macro2::TokenStream| { + if field.attrs().remote.is_some() { + quote! { + #FQOption::Some( + // SAFETY: The remote type should always be a `#[repr(transparent)]` for the actual field type + unsafe { + ::core::mem::transmute::<#ty, #remote_ty>(#value?) + } + ) + } + } else { + value + } + }; + let value = match &field.attrs.default { - DefaultBehavior::Func(path) => quote! { - (|| - if let #FQOption::Some(field) = #get_field { - <#ty as #bevy_reflect_path::FromReflect>::from_reflect(field) - } else { - #FQOption::Some(#path()) - } - ) - }, - DefaultBehavior::Default => quote! { - (|| - if let #FQOption::Some(field) = #get_field { - <#ty as #bevy_reflect_path::FromReflect>::from_reflect(field) - } else { - #FQOption::Some(#FQDefault::default()) - } - ) - }, - DefaultBehavior::Required => quote! { - (|| <#ty as #bevy_reflect_path::FromReflect>::from_reflect(#get_field?)) - }, + DefaultBehavior::Func(path) => { + let value = into_remote(quote! { + <#ty as #bevy_reflect_path::FromReflect>::from_reflect(field) + }); + quote! { + (|| + if let #FQOption::Some(field) = #get_field { + #value + } else { + #FQOption::Some(#path()) + } + ) + } + } + DefaultBehavior::Default => { + let value = into_remote(quote! { + <#ty as #bevy_reflect_path::FromReflect>::from_reflect(field) + }); + quote! { + (|| + if let #FQOption::Some(field) = #get_field { + #value + } else { + #FQOption::Some(#FQDefault::default()) + } + ) + } + } + DefaultBehavior::Required => { + let value = into_remote(quote! { + <#ty as #bevy_reflect_path::FromReflect>::from_reflect(#get_field?) + }); + quote! { + (|| #value) + } + } }; (member, value) diff --git a/crates/bevy_reflect/derive/src/lib.rs b/crates/bevy_reflect/derive/src/lib.rs index e09b437cfb7f0..37f83e72f9264 100644 --- a/crates/bevy_reflect/derive/src/lib.rs +++ b/crates/bevy_reflect/derive/src/lib.rs @@ -572,6 +572,22 @@ pub fn reflect_trait(args: TokenStream, input: TokenStream) -> TokenStream { /// pub struct Wrapper(RemoteType); /// ``` /// +/// # `FromReflect` +/// +/// Because of the way this code modifies the item it's defined on, it is not possible to implement `FromReflect` +/// using a simple derive macro. +/// Instead, you will need to opt-in to to it by adding `FromReflect` +/// (just the identifier, no need to import or qualify the actual type) +/// as the last item in the attribute's argument list: +/// +/// ```ignore +/// #[reflect_remote(foo::Foo, FromReflect)] +/// struct FooWrapper; +/// ``` +/// +/// This is the _only_ trait this works with. You cannot derive any other traits using this method. +/// For those, use regular derive macros below this one. +/// #[proc_macro_attribute] pub fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStream { remote::reflect_remote(args, input) diff --git a/crates/bevy_reflect/derive/src/remote.rs b/crates/bevy_reflect/derive/src/remote.rs index af077dbbc667f..ec14e4317b395 100644 --- a/crates/bevy_reflect/derive/src/remote.rs +++ b/crates/bevy_reflect/derive/src/remote.rs @@ -2,16 +2,19 @@ use crate::derive_data::{ReflectImplSource, ReflectProvenance, ReflectTraitToImp use crate::{from_reflect, impls, ReflectDerive, REFLECT_ATTRIBUTE_NAME}; use proc_macro::TokenStream; use quote::quote; +use syn::parse::{Parse, ParseStream}; use syn::spanned::Spanned; use syn::{parse_macro_input, DeriveInput, TypePath}; /// Generates the remote wrapper type and implements all the necessary traits. pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStream { - let remote_ty = match syn::parse::(args) { + let remote_args = match syn::parse::(args) { Ok(path) => path, Err(err) => return err.to_compile_error().into(), }; + let remote_ty = remote_args.remote_ty; + let ast = parse_macro_input!(input as DeriveInput); let wrapper_definition = generate_remote_wrapper(&ast, &remote_ty); @@ -97,3 +100,18 @@ fn generate_remote_wrapper(input: &DeriveInput, remote_ty: &TypePath) -> proc_ma #vis struct #ident #ty_generics (pub #remote_ty) #where_clause; } } + +/// Metadata from the arguments defined in the `reflect_remote` attribute. +/// +/// The syntax for the arguments is: `#[reflect_remote(REMOTE_TYPE_PATH)]`. +struct RemoteArgs { + remote_ty: TypePath, +} + +impl Parse for RemoteArgs { + fn parse(input: ParseStream) -> syn::Result { + Ok(Self { + remote_ty: input.parse()?, + }) + } +} diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 7150cea0567e4..0eb8e37c34f52 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -2488,8 +2488,6 @@ bevy_reflect::tests::Test { // === Remote Wrapper === // #[reflect_remote(external_crate::TheirType)] - // TODO: Remove - #[reflect(from_reflect = false)] #[derive(Debug, Default)] #[reflect(Debug, Default)] struct MyType { @@ -2510,7 +2508,6 @@ bevy_reflect::tests::Test { // === Struct Container === // #[derive(Reflect, Debug)] - // TODO: Remove #[reflect(from_reflect = false)] struct ContainerStruct { #[reflect(remote = "MyType")] @@ -2538,8 +2535,6 @@ bevy_reflect::tests::Test { // === Tuple Struct Container === // #[derive(Reflect, Debug)] - // TODO: Remove - #[reflect(from_reflect = false)] struct ContainerTupleStruct(#[reflect(remote = "MyType")] external_crate::TheirType); let mut patch = DynamicTupleStruct::default(); @@ -2557,6 +2552,73 @@ bevy_reflect::tests::Test { assert_eq!("Goodbye", data.0.value); } + #[test] + fn should_reflect_remote_enum() { + mod external_crate { + #[derive(Debug, PartialEq, Eq)] + pub enum TheirType { + Unit, + Tuple(usize), + Struct { value: String }, + } + } + + // === Remote Wrapper === // + #[reflect_remote(external_crate::TheirType)] + #[derive(Debug)] + #[reflect(Debug)] + enum MyType { + Unit, + Tuple(usize), + Struct { value: String }, + } + + let mut patch = DynamicEnum::from(MyType(external_crate::TheirType::Tuple(123))); + + let mut data = MyType(external_crate::TheirType::Unit); + + assert_eq!(external_crate::TheirType::Unit, data.0); + data.apply(&patch); + assert_eq!(external_crate::TheirType::Tuple(123), data.0); + + patch = DynamicEnum::from(MyType(external_crate::TheirType::Struct { + value: "Hello world!".to_string(), + })); + + data.apply(&patch); + assert_eq!( + external_crate::TheirType::Struct { + value: "Hello world!".to_string() + }, + data.0 + ); + + // === Enum Container === // + #[derive(Reflect, Debug, PartialEq)] + enum ContainerEnum { + Foo, + Bar { + #[reflect(remote = "MyType")] + their_type: external_crate::TheirType, + }, + } + + let patch = DynamicEnum::from(ContainerEnum::Bar { + their_type: external_crate::TheirType::Tuple(123), + }); + + let mut data = ContainerEnum::Foo; + + assert_eq!(ContainerEnum::Foo, data); + data.apply(&patch); + assert_eq!( + ContainerEnum::Bar { + their_type: external_crate::TheirType::Tuple(123) + }, + data + ); + } + #[test] fn should_reflect_nested_remote_type() { mod external_crate { @@ -2567,16 +2629,12 @@ bevy_reflect::tests::Test { } #[reflect_remote(external_crate::TheirOuter)] - // TODO: Remove - #[reflect(from_reflect = false)] struct MyOuter { #[reflect(remote = "MyInner")] pub inner: external_crate::TheirInner, } #[reflect_remote(external_crate::TheirInner)] - // TODO: Remove - #[reflect(from_reflect = false)] struct MyInner(usize); let mut patch = DynamicStruct::default(); From 6004560f941eed48f5b157682b91dc9439950652 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Tue, 20 Sep 2022 21:05:50 -0700 Subject: [PATCH 05/31] Change wording Changed methods from `is_remote` to `is_remote_wrapper` to better indicate that they are NOT the remote types themselves. --- crates/bevy_reflect/derive/src/derive_data.rs | 4 ++-- crates/bevy_reflect/derive/src/from_reflect.rs | 2 +- crates/bevy_reflect/derive/src/impls/enums.rs | 2 +- crates/bevy_reflect/derive/src/struct_utility.rs | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/bevy_reflect/derive/src/derive_data.rs b/crates/bevy_reflect/derive/src/derive_data.rs index 71dd636580fbd..c7f05c057f349 100644 --- a/crates/bevy_reflect/derive/src/derive_data.rs +++ b/crates/bevy_reflect/derive/src/derive_data.rs @@ -575,7 +575,7 @@ impl<'a> ReflectStruct<'a> { } /// Whether this reflected struct represents a remote type or not. - pub fn is_remote(&self) -> bool { + pub fn is_remote_wrapper(&self) -> bool { self.remote_ty.is_some() } @@ -685,7 +685,7 @@ impl<'a> ReflectEnum<'a> { } /// Whether this reflected enum represents a remote type or not. - pub fn is_remote(&self) -> bool { + pub fn is_remote_wrapper(&self) -> bool { self.remote_ty.is_some() } diff --git a/crates/bevy_reflect/derive/src/from_reflect.rs b/crates/bevy_reflect/derive/src/from_reflect.rs index 64ae7839505e4..a73281767cb7c 100644 --- a/crates/bevy_reflect/derive/src/from_reflect.rs +++ b/crates/bevy_reflect/derive/src/from_reflect.rs @@ -52,7 +52,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream .. } = FromReflectVariantBuilder::new(reflect_enum).build(&ref_value); - let match_branches = if reflect_enum.is_remote() { + let match_branches = if reflect_enum.is_remote_wrapper() { quote! { #(#variant_names => #fqoption::Some(Self(#variant_constructors)),)* } diff --git a/crates/bevy_reflect/derive/src/impls/enums.rs b/crates/bevy_reflect/derive/src/impls/enums.rs index 87f60bd7e7635..16df7cc9f464a 100644 --- a/crates/bevy_reflect/derive/src/impls/enums.rs +++ b/crates/bevy_reflect/derive/src/impls/enums.rs @@ -9,7 +9,7 @@ use syn::Fields; pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream { let bevy_reflect_path = reflect_enum.meta().bevy_reflect_path(); let enum_path = reflect_enum.meta().type_path(); - let is_remote = reflect_enum.is_remote(); + let is_remote = reflect_enum.is_remote_wrapper(); // For `match self` expressions where self is a reference let match_this = if is_remote { diff --git a/crates/bevy_reflect/derive/src/struct_utility.rs b/crates/bevy_reflect/derive/src/struct_utility.rs index bc7f27adc371b..b227058796619 100644 --- a/crates/bevy_reflect/derive/src/struct_utility.rs +++ b/crates/bevy_reflect/derive/src/struct_utility.rs @@ -73,7 +73,7 @@ impl FieldAccessors { where F: FnMut(&StructField, proc_macro2::TokenStream) -> proc_macro2::TokenStream, { - let is_remote = reflect_struct.is_remote(); + let is_remote = reflect_struct.is_remote_wrapper(); reflect_struct .active_fields() .map(|field| { From 5e208581c0cc6cf53ecf3c7f7479d3bc8d92ff90 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Tue, 20 Sep 2022 21:13:03 -0700 Subject: [PATCH 06/31] Add test for taking remote type --- crates/bevy_reflect/src/lib.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 0eb8e37c34f52..83ebd9bb9c718 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -2650,6 +2650,36 @@ bevy_reflect::tests::Test { assert_eq!(321, data.0.inner.0); } + #[test] + fn should_take_remote_type() { + mod external_crate { + #[derive(Debug, Default, PartialEq, Eq)] + pub struct TheirType { + pub value: String, + } + } + + // === Remote Wrapper === // + #[reflect_remote(external_crate::TheirType)] + #[derive(Debug, Default)] + #[reflect(Debug, Default)] + struct MyType { + pub value: String, + } + + let input: Box = Box::new(MyType(external_crate::TheirType { + value: "Hello".to_string(), + })); + + let output: MyType = input.take().expect("data should be of type `MyType`"); + assert_eq!( + external_crate::TheirType { + value: "Hello".to_string(), + }, + output.0 + ); + } + #[cfg(feature = "glam")] mod glam { use super::*; From d4886d903f6d549c4eb2642b934076521f3fbc70 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Tue, 20 Sep 2022 21:31:04 -0700 Subject: [PATCH 07/31] Alter Reflect::*_any methods to return remote type --- .../bevy_reflect/derive/src/impls/common.rs | 28 ++++++++++- crates/bevy_reflect/derive/src/impls/enums.rs | 6 ++- .../bevy_reflect/derive/src/impls/structs.rs | 6 ++- .../derive/src/impls/tuple_structs.rs | 6 ++- .../bevy_reflect/derive/src/impls/values.rs | 2 +- crates/bevy_reflect/src/lib.rs | 40 ++++++++++++++-- crates/bevy_reflect/src/reflect.rs | 46 ++++++++++++++----- 7 files changed, 113 insertions(+), 21 deletions(-) diff --git a/crates/bevy_reflect/derive/src/impls/common.rs b/crates/bevy_reflect/derive/src/impls/common.rs index 00c573152cc14..8dd9b2f90485d 100644 --- a/crates/bevy_reflect/derive/src/impls/common.rs +++ b/crates/bevy_reflect/derive/src/impls/common.rs @@ -7,6 +7,7 @@ use crate::{derive_data::ReflectMeta, utility::WhereClauseOptions}; pub fn impl_full_reflect( meta: &ReflectMeta, where_clause_options: &WhereClauseOptions, + is_remote_wrapper: bool, ) -> proc_macro2::TokenStream { let bevy_reflect_path = meta.bevy_reflect_path(); let type_path = meta.type_path(); @@ -14,8 +15,25 @@ pub fn impl_full_reflect( let (impl_generics, ty_generics, where_clause) = type_path.generics().split_for_impl(); let where_reflect_clause = where_clause_options.extend_where_clause(where_clause); - quote! { - impl #impl_generics #bevy_reflect_path::Reflect for #type_path #ty_generics #where_reflect_clause { + let any_impls = if is_remote_wrapper { + quote! { + #[inline] + fn into_any(self: #FQBox) -> #FQBox { + #FQBox::new(self.0) + } + + #[inline] + fn as_any(&self) -> &dyn #FQAny { + &self.0 + } + + #[inline] + fn as_any_mut(&mut self) -> &mut dyn #FQAny { + &mut self.0 + } + } + } else { + quote! { #[inline] fn into_any(self: #FQBox) -> #FQBox { self @@ -30,6 +48,12 @@ pub fn impl_full_reflect( fn as_any_mut(&mut self) -> &mut dyn #FQAny { self } + } + }; + + quote! { + impl #impl_generics #bevy_reflect_path::Reflect for #type_path #ty_generics #where_reflect_clause { + #any_impls #[inline] fn into_reflect(self: #FQBox) -> #FQBox { diff --git a/crates/bevy_reflect/derive/src/impls/enums.rs b/crates/bevy_reflect/derive/src/impls/enums.rs index 16df7cc9f464a..d1a1ad855dd8c 100644 --- a/crates/bevy_reflect/derive/src/impls/enums.rs +++ b/crates/bevy_reflect/derive/src/impls/enums.rs @@ -62,7 +62,11 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream ); let type_path_impl = impl_type_path(reflect_enum.meta()); - let full_reflect_impl = impl_full_reflect(reflect_enum.meta(), &where_clause_options); + let full_reflect_impl = impl_full_reflect( + reflect_enum.meta(), + &where_clause_options, + reflect_enum.is_remote_wrapper(), + ); let common_methods = common_partial_reflect_methods( reflect_enum.meta(), || Some(quote!(#bevy_reflect_path::enum_partial_eq)), diff --git a/crates/bevy_reflect/derive/src/impls/structs.rs b/crates/bevy_reflect/derive/src/impls/structs.rs index 397dabec9b77f..42b1daee69001 100644 --- a/crates/bevy_reflect/derive/src/impls/structs.rs +++ b/crates/bevy_reflect/derive/src/impls/structs.rs @@ -39,7 +39,11 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> proc_macro2::TokenS ); let type_path_impl = impl_type_path(reflect_struct.meta()); - let full_reflect_impl = impl_full_reflect(reflect_struct.meta(), &where_clause_options); + let full_reflect_impl = impl_full_reflect( + reflect_struct.meta(), + &where_clause_options, + reflect_struct.is_remote_wrapper(), + ); let common_methods = common_partial_reflect_methods( reflect_struct.meta(), || Some(quote!(#bevy_reflect_path::struct_partial_eq)), diff --git a/crates/bevy_reflect/derive/src/impls/tuple_structs.rs b/crates/bevy_reflect/derive/src/impls/tuple_structs.rs index 115ad872eae0d..176e53c722f10 100644 --- a/crates/bevy_reflect/derive/src/impls/tuple_structs.rs +++ b/crates/bevy_reflect/derive/src/impls/tuple_structs.rs @@ -29,7 +29,11 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> proc_macro2:: ); let type_path_impl = impl_type_path(reflect_struct.meta()); - let full_reflect_impl = impl_full_reflect(reflect_struct.meta(), &where_clause_options); + let full_reflect_impl = impl_full_reflect( + reflect_struct.meta(), + &where_clause_options, + reflect_struct.is_remote_wrapper(), + ); let common_methods = common_partial_reflect_methods( reflect_struct.meta(), || Some(quote!(#bevy_reflect_path::tuple_struct_partial_eq)), diff --git a/crates/bevy_reflect/derive/src/impls/values.rs b/crates/bevy_reflect/derive/src/impls/values.rs index e50e0a756ad4e..b3f28ce5d3b27 100644 --- a/crates/bevy_reflect/derive/src/impls/values.rs +++ b/crates/bevy_reflect/derive/src/impls/values.rs @@ -28,7 +28,7 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> proc_macro2::TokenStream { ); let type_path_impl = impl_type_path(meta); - let full_reflect_impl = impl_full_reflect(meta, &where_clause_options); + let full_reflect_impl = impl_full_reflect(meta, &where_clause_options, false); let common_methods = common_partial_reflect_methods(meta, || None, || None); #[cfg(not(feature = "functions"))] diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 83ebd9bb9c718..98c38cace5d76 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -2476,7 +2476,7 @@ bevy_reflect::tests::Test { assert_impl_all!(TupleStruct: Reflect); assert_impl_all!(Enum: Reflect); } - + #[test] fn should_reflect_remote_type() { mod external_crate { @@ -2671,12 +2671,46 @@ bevy_reflect::tests::Test { value: "Hello".to_string(), })); - let output: MyType = input.take().expect("data should be of type `MyType`"); + let output: external_crate::TheirType = input + .take() + .expect("should downcast to `external_crate::TheirType`"); + assert_eq!( + external_crate::TheirType { + value: "Hello".to_string(), + }, + output + ); + } + + #[test] + fn should_try_take_remote_type() { + mod external_crate { + #[derive(Debug, Default, PartialEq, Eq)] + pub struct TheirType { + pub value: String, + } + } + + // === Remote Wrapper === // + #[reflect_remote(external_crate::TheirType)] + #[derive(Debug, Default)] + #[reflect(Debug, Default)] + struct MyType { + pub value: String, + } + + let input: Box = Box::new(MyType(external_crate::TheirType { + value: "Hello".to_string(), + })); + + let output: external_crate::TheirType = input + .try_take() + .expect("should downcast to `external_crate::TheirType`"); assert_eq!( external_crate::TheirType { value: "Hello".to_string(), }, - output.0 + output ); } diff --git a/crates/bevy_reflect/src/reflect.rs b/crates/bevy_reflect/src/reflect.rs index 59b8a81d54e56..c7922f7842f9e 100644 --- a/crates/bevy_reflect/src/reflect.rs +++ b/crates/bevy_reflect/src/reflect.rs @@ -410,12 +410,18 @@ where )] pub trait Reflect: PartialReflect + Any { /// Returns the value as a [`Box`][std::any::Any]. + /// + /// For remote wrapper types, this will return the remote type instead. fn into_any(self: Box) -> Box; /// Returns the value as a [`&dyn Any`][std::any::Any]. + /// + /// For remote wrapper types, this will return the remote type instead. fn as_any(&self) -> &dyn Any; /// Returns the value as a [`&mut dyn Any`][std::any::Any]. + /// + /// For remote wrapper types, this will return the remote type instead. fn as_any_mut(&mut self) -> &mut dyn Any; /// Casts this type to a boxed, fully-reflected value. @@ -450,7 +456,9 @@ impl dyn PartialReflect { /// /// If the underlying value does not implement [`Reflect`] /// or is not of type `T`, returns `Err(self)`. - pub fn try_downcast( + /// + /// For remote types, `T` should be the type itself rather than the wraper type. + pub fn try_downcast( self: Box, ) -> Result, Box> { self.try_into_reflect()? @@ -462,9 +470,9 @@ impl dyn PartialReflect { /// /// If the underlying value does not implement [`Reflect`] /// or is not of type `T`, returns `Err(self)`. - pub fn try_take( - self: Box, - ) -> Result> { + /// + /// For remote types, `T` should be the type itself rather than the wraper type. + pub fn try_take(self: Box) -> Result> { self.try_downcast().map(|value| *value) } @@ -472,7 +480,9 @@ impl dyn PartialReflect { /// /// If the underlying value does not implement [`Reflect`] /// or is not of type `T`, returns [`None`]. - pub fn try_downcast_ref(&self) -> Option<&T> { + /// + /// For remote types, `T` should be the type itself rather than the wraper type. + pub fn try_downcast_ref(&self) -> Option<&T> { self.try_as_reflect()?.downcast_ref() } @@ -480,7 +490,9 @@ impl dyn PartialReflect { /// /// If the underlying value does not implement [`Reflect`] /// or is not of type `T`, returns [`None`]. - pub fn try_downcast_mut(&mut self) -> Option<&mut T> { + /// + /// For remote types, `T` should be the type itself rather than the wraper type. + pub fn try_downcast_mut(&mut self) -> Option<&mut T> { self.try_as_reflect_mut()?.downcast_mut() } } @@ -508,7 +520,9 @@ impl dyn Reflect { /// Downcasts the value to type `T`, consuming the trait object. /// /// If the underlying value is not of type `T`, returns `Err(self)`. - pub fn downcast(self: Box) -> Result, Box> { + /// + /// For remote types, `T` should be the type itself rather than the wraper type. + pub fn downcast(self: Box) -> Result, Box> { if self.is::() { Ok(self.into_any().downcast().unwrap()) } else { @@ -519,7 +533,9 @@ impl dyn Reflect { /// Downcasts the value to type `T`, unboxing and consuming the trait object. /// /// If the underlying value is not of type `T`, returns `Err(self)`. - pub fn take(self: Box) -> Result> { + /// + /// For remote types, `T` should be the type itself rather than the wraper type. + pub fn take(self: Box) -> Result> { self.downcast::().map(|value| *value) } @@ -532,25 +548,31 @@ impl dyn Reflect { /// to determine what type they represent. Represented types cannot be downcasted /// to, but you can use [`FromReflect`] to create a value of the represented type from them. /// + /// For remote types, `T` should be the type itself rather than the wraper type. + /// /// [`FromReflect`]: crate::FromReflect #[inline] - pub fn is(&self) -> bool { - self.type_id() == TypeId::of::() + pub fn is(&self) -> bool { + self.as_any().type_id() == TypeId::of::() } /// Downcasts the value to type `T` by reference. /// /// If the underlying value is not of type `T`, returns `None`. + /// + /// For remote types, `T` should be the type itself rather than the wraper type. #[inline] - pub fn downcast_ref(&self) -> Option<&T> { + pub fn downcast_ref(&self) -> Option<&T> { self.as_any().downcast_ref::() } /// Downcasts the value to type `T` by mutable reference. /// /// If the underlying value is not of type `T`, returns `None`. + /// + /// For remote types, `T` should be the type itself rather than the wraper type. #[inline] - pub fn downcast_mut(&mut self) -> Option<&mut T> { + pub fn downcast_mut(&mut self) -> Option<&mut T> { self.as_any_mut().downcast_mut::() } } From c10e58fd803e31a61e019476f54bc73315a4f23d Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Tue, 20 Sep 2022 23:40:35 -0700 Subject: [PATCH 08/31] Better safety documentation --- .../bevy_reflect/derive/src/enum_utility.rs | 2 +- crates/bevy_reflect/derive/src/impls/enums.rs | 1 + crates/bevy_reflect/derive/src/lib.rs | 23 ++++++++++++++++++- .../bevy_reflect/derive/src/struct_utility.rs | 3 +++ 4 files changed, 27 insertions(+), 2 deletions(-) diff --git a/crates/bevy_reflect/derive/src/enum_utility.rs b/crates/bevy_reflect/derive/src/enum_utility.rs index a67c4f35d18ae..f6745c35164fe 100644 --- a/crates/bevy_reflect/derive/src/enum_utility.rs +++ b/crates/bevy_reflect/derive/src/enum_utility.rs @@ -113,7 +113,7 @@ pub(crate) trait VariantBuilder: Sized { if field.field.attrs().remote.is_some() { quote! { - // SAFETY: The remote type should always be a `#[repr(transparent)]` for the actual field type + // SAFE: The wrapper type should be repr(transparent) over the remote type unsafe { ::core::mem::transmute(#construction) } diff --git a/crates/bevy_reflect/derive/src/impls/enums.rs b/crates/bevy_reflect/derive/src/impls/enums.rs index d1a1ad855dd8c..bc39ba76707b0 100644 --- a/crates/bevy_reflect/derive/src/impls/enums.rs +++ b/crates/bevy_reflect/derive/src/impls/enums.rs @@ -349,6 +349,7 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden .as_ref() .map(|ty| { quote! { + // SAFE: The wrapper type should be repr(transparent) over the remote type unsafe { ::core::mem::transmute::<#ref_token _, #ref_token #ty>(#ident) } } }) diff --git a/crates/bevy_reflect/derive/src/lib.rs b/crates/bevy_reflect/derive/src/lib.rs index 37f83e72f9264..17f95c263dfec 100644 --- a/crates/bevy_reflect/derive/src/lib.rs +++ b/crates/bevy_reflect/derive/src/lib.rs @@ -516,7 +516,7 @@ pub fn reflect_trait(args: TokenStream, input: TokenStream) -> TokenStream { /// Generates a wrapper type that can be used to "derive `Reflect`" for remote types. /// /// This works by wrapping the remote type in a generated wrapper that has the `#[repr(transparent)]` attribute. -/// This allows the two types to be safely transmuted back-and-forth. +/// This allows the two types to be safely [transmuted] back-and-forth. /// /// # Defining the Wrapper /// @@ -572,6 +572,26 @@ pub fn reflect_trait(args: TokenStream, input: TokenStream) -> TokenStream { /// pub struct Wrapper(RemoteType); /// ``` /// +/// # Usage as a Field +/// +/// You can tell `Reflect` to use a remote type's wrapper internally on fields of a struct or enum. +/// This allows the real type to be used as usual while `Reflect` handles everything internally. +/// To do this, add the `#[reflect(remote = "...")]` attribute to your field: +/// +/// ```ignore +/// #[derive(Reflect)] +/// struct SomeStruct { +/// #[reflect(remote = "RemoteTypeWrapper")] +/// data: RemoteType +/// } +/// ``` +/// +/// ## Safety +/// +/// When using the `#[reflect(remote = "...")]` field attribute, be sure you are defining the correct wrapper type. +/// Internally, this field will be unsafely [transmuted], and is only sound if using a wrapper generated for the remote type. +/// This also means keeping your wrapper definitions up-to-date with the remote types. +/// /// # `FromReflect` /// /// Because of the way this code modifies the item it's defined on, it is not possible to implement `FromReflect` @@ -588,6 +608,7 @@ pub fn reflect_trait(args: TokenStream, input: TokenStream) -> TokenStream { /// This is the _only_ trait this works with. You cannot derive any other traits using this method. /// For those, use regular derive macros below this one. /// +/// [transmuted]: std::mem::transmute #[proc_macro_attribute] pub fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStream { remote::reflect_remote(args, input) diff --git a/crates/bevy_reflect/derive/src/struct_utility.rs b/crates/bevy_reflect/derive/src/struct_utility.rs index b227058796619..0875be6c0150d 100644 --- a/crates/bevy_reflect/derive/src/struct_utility.rs +++ b/crates/bevy_reflect/derive/src/struct_utility.rs @@ -27,6 +27,7 @@ impl FieldAccessors { match &field.attrs.remote { Some(wrapper_ty) => { quote! { + // SAFE: The wrapper type should be repr(transparent) over the remote type unsafe { ::core::mem::transmute_copy::<_, #wrapper_ty>(&#accessor) } } } @@ -37,6 +38,7 @@ impl FieldAccessors { match &field.attrs.remote { Some(wrapper_ty) => { quote! { + // SAFE: The wrapper type should be repr(transparent) over the remote type unsafe { ::core::mem::transmute::<&_, &#wrapper_ty>(&#accessor) } } } @@ -47,6 +49,7 @@ impl FieldAccessors { match &field.attrs.remote { Some(wrapper_ty) => { quote! { + // SAFE: The wrapper type should be repr(transparent) over the remote type unsafe { ::core::mem::transmute::<&mut _, &mut #wrapper_ty>(&mut #accessor) } } } From 5dd3b5b9d65a290e77e9b4c29bec472d43944740 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Wed, 21 Sep 2022 00:18:22 -0700 Subject: [PATCH 09/31] Add nested generics to remote type test --- crates/bevy_reflect/src/lib.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 98c38cace5d76..c64e70b97eb75 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -2622,20 +2622,20 @@ bevy_reflect::tests::Test { #[test] fn should_reflect_nested_remote_type() { mod external_crate { - pub struct TheirOuter { - pub inner: TheirInner, + pub struct TheirOuter { + pub inner: TheirInner, } - pub struct TheirInner(pub usize); + pub struct TheirInner(pub T); } - #[reflect_remote(external_crate::TheirOuter)] - struct MyOuter { - #[reflect(remote = "MyInner")] - pub inner: external_crate::TheirInner, + #[reflect_remote(external_crate::TheirOuter)] + struct MyOuter { + #[reflect(remote = "MyInner")] + pub inner: external_crate::TheirInner, } - #[reflect_remote(external_crate::TheirInner)] - struct MyInner(usize); + #[reflect_remote(external_crate::TheirInner)] + struct MyInner(pub T); let mut patch = DynamicStruct::default(); patch.set_represented_type(Some(MyOuter::type_info())); From da4edb67570cf52e2ea3a204d1b3988993a1a64f Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Tue, 20 Dec 2022 23:49:28 -0800 Subject: [PATCH 10/31] Fix reflect_remote not working with generics --- crates/bevy_reflect/derive/src/derive_data.rs | 18 ++-- .../bevy_reflect/derive/src/enum_utility.rs | 17 ++- .../derive/src/field_attributes.rs | 17 +++ .../bevy_reflect/derive/src/from_reflect.rs | 35 ++++-- crates/bevy_reflect/derive/src/remote.rs | 53 ++++++++- crates/bevy_reflect/src/lib.rs | 101 +++++++++++++++++- 6 files changed, 217 insertions(+), 24 deletions(-) diff --git a/crates/bevy_reflect/derive/src/derive_data.rs b/crates/bevy_reflect/derive/src/derive_data.rs index c7f05c057f349..56efeab010871 100644 --- a/crates/bevy_reflect/derive/src/derive_data.rs +++ b/crates/bevy_reflect/derive/src/derive_data.rs @@ -8,6 +8,7 @@ use crate::utility::{StringExpr, WhereClauseOptions}; use quote::{quote, ToTokens}; use syn::token::Comma; +use crate::remote::RemoteType; use crate::serialization::SerializationDataDef; use crate::{ utility, REFLECT_ATTRIBUTE_NAME, REFLECT_VALUE_ATTRIBUTE_NAME, TYPE_NAME_ATTRIBUTE_NAME, @@ -17,7 +18,7 @@ use syn::punctuated::Punctuated; use syn::spanned::Spanned; use syn::{ parse_str, Data, DeriveInput, Field, Fields, GenericParam, Generics, Ident, LitStr, Meta, Path, - PathSegment, Type, TypeParam, TypePath, Variant, + PathSegment, Type, TypeParam, Variant, }; pub(crate) enum ReflectDerive<'a> { @@ -70,7 +71,7 @@ pub(crate) struct ReflectStruct<'a> { meta: ReflectMeta<'a>, serialization_data: Option, fields: Vec>, - remote_ty: Option<&'a TypePath>, + remote_ty: Option>, } /// Enum data used by derive macros for `Reflect` and `FromReflect`. @@ -89,7 +90,7 @@ pub(crate) struct ReflectStruct<'a> { pub(crate) struct ReflectEnum<'a> { meta: ReflectMeta<'a>, variants: Vec>, - remote_ty: Option<&'a TypePath>, + remote_ty: Option>, } /// Represents a field on a struct or tuple struct. @@ -361,7 +362,7 @@ impl<'a> ReflectDerive<'a> { /// # Panics /// /// Panics when called on [`ReflectDerive::Value`]. - pub fn set_remote(&mut self, remote_ty: Option<&'a TypePath>) { + pub fn set_remote(&mut self, remote_ty: Option>) { match self { Self::Struct(data) | Self::TupleStruct(data) | Self::UnitStruct(data) => { data.remote_ty = remote_ty; @@ -581,7 +582,7 @@ impl<'a> ReflectStruct<'a> { #[allow(dead_code)] /// Get the remote type path, if any. - pub fn remote_ty(&self) -> Option<&'a TypePath> { + pub fn remote_ty(&self) -> Option> { self.remote_ty } @@ -691,7 +692,7 @@ impl<'a> ReflectEnum<'a> { #[allow(dead_code)] /// Get the remote type path, if any. - pub fn remote_ty(&self) -> Option<&'a TypePath> { + pub fn remote_ty(&self) -> Option> { self.remote_ty } @@ -701,7 +702,10 @@ impl<'a> ReflectEnum<'a> { pub fn get_unit(&self, variant: &Ident) -> proc_macro2::TokenStream { let name = self .remote_ty - .map(|path| path.to_token_stream()) + .map(|path| match path.as_expr_path() { + Ok(path) => path.to_token_stream(), + Err(err) => err.into_compile_error(), + }) .unwrap_or_else(|| self.meta.type_path().to_token_stream()); quote! { diff --git a/crates/bevy_reflect/derive/src/enum_utility.rs b/crates/bevy_reflect/derive/src/enum_utility.rs index f6745c35164fe..16a8d0ed350e4 100644 --- a/crates/bevy_reflect/derive/src/enum_utility.rs +++ b/crates/bevy_reflect/derive/src/enum_utility.rs @@ -112,10 +112,19 @@ pub(crate) trait VariantBuilder: Sized { }; if field.field.attrs().remote.is_some() { - quote! { - // SAFE: The wrapper type should be repr(transparent) over the remote type - unsafe { - ::core::mem::transmute(#construction) + if field.field.attrs().is_remote_generic().unwrap_or_default() { + quote! { + // SAFE: The wrapper type should be repr(transparent) over the remote type + unsafe { + ::core::mem::transmute_copy(&#construction) + } + } + } else { + quote! { + // SAFE: The wrapper type should be repr(transparent) over the remote type + unsafe { + ::core::mem::transmute(#construction) + } } } } else { diff --git a/crates/bevy_reflect/derive/src/field_attributes.rs b/crates/bevy_reflect/derive/src/field_attributes.rs index e04788cb522a6..c496c74257e05 100644 --- a/crates/bevy_reflect/derive/src/field_attributes.rs +++ b/crates/bevy_reflect/derive/src/field_attributes.rs @@ -217,4 +217,21 @@ impl FieldAttributes { Ok(()) } + + /// Returns `Some(true)` if the field has a generic remote type. + /// + /// If the remote type is not generic, returns `Some(false)`. + /// + /// If the field does not have a remote type, returns `None`. + pub fn is_remote_generic(&self) -> Option { + if let Type::Path(type_path) = self.remote.as_ref()? { + type_path + .path + .segments + .last() + .map(|segment| !segment.arguments.is_empty()) + } else { + Some(false) + } + } } diff --git a/crates/bevy_reflect/derive/src/from_reflect.rs b/crates/bevy_reflect/derive/src/from_reflect.rs index a73281767cb7c..a7e1dfcf562a4 100644 --- a/crates/bevy_reflect/derive/src/from_reflect.rs +++ b/crates/bevy_reflect/derive/src/from_reflect.rs @@ -122,10 +122,20 @@ fn impl_struct_internal( let __this = Ident::new("__this", Span::call_site()); // The reflected type: either `Self` or a remote type - let (reflect_ty, retval) = if let Some(remote_ty) = reflect_struct.remote_ty() { - (quote!(#remote_ty), quote!(Self(#__this))) + let (reflect_ty, constructor, retval) = if let Some(remote_ty) = reflect_struct.remote_ty() { + let constructor = match remote_ty.as_expr_path() { + Ok(path) => path, + Err(err) => return err.into_compile_error(), + }; + let remote_ty = remote_ty.type_path(); + + ( + quote!(#remote_ty), + quote!(#constructor), + quote!(Self(#__this)), + ) } else { - (quote!(Self), quote!(#__this)) + (quote!(Self), quote!(Self), quote!(#__this)) }; let constructor = if is_defaultable { @@ -143,7 +153,7 @@ fn impl_struct_internal( let MemberValuePair(ignored_members, ignored_values) = get_ignored_fields(reflect_struct); quote! { - let #__this = #reflect_ty { + let #__this = #constructor { #(#active_members: #active_values()?,)* #(#ignored_members: #ignored_values,)* }; @@ -222,19 +232,30 @@ fn get_active_fields( is_tuple, ); let ty = field.reflected_type().clone(); - let remote_ty = &field.data.ty; + let real_ty = &field.data.ty; let get_field = quote! { #bevy_reflect_path::#struct_type::field(#dyn_struct_name, #accessor) }; let into_remote = |value: proc_macro2::TokenStream| { - if field.attrs().remote.is_some() { + if field.attrs.is_remote_generic().unwrap_or_default() { quote! { #FQOption::Some( // SAFETY: The remote type should always be a `#[repr(transparent)]` for the actual field type unsafe { - ::core::mem::transmute::<#ty, #remote_ty>(#value?) + ::core::mem::transmute_copy::<#ty, #real_ty>( + &::core::mem::ManuallyDrop::new(#value?) + ) + } + ) + } + } else if field.attrs().remote.is_some() { + quote! { + #FQOption::Some( + // SAFETY: The remote type should always be a `#[repr(transparent)]` for the actual field type + unsafe { + ::core::mem::transmute::<#ty, #real_ty>(#value?) } ) } diff --git a/crates/bevy_reflect/derive/src/remote.rs b/crates/bevy_reflect/derive/src/remote.rs index ec14e4317b395..0488bd10dd628 100644 --- a/crates/bevy_reflect/derive/src/remote.rs +++ b/crates/bevy_reflect/derive/src/remote.rs @@ -4,7 +4,8 @@ use proc_macro::TokenStream; use quote::quote; use syn::parse::{Parse, ParseStream}; use syn::spanned::Spanned; -use syn::{parse_macro_input, DeriveInput, TypePath}; +use syn::token::PathSep; +use syn::{parse_macro_input, DeriveInput, ExprPath, PathArguments, TypePath}; /// Generates the remote wrapper type and implements all the necessary traits. pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStream { @@ -29,7 +30,7 @@ pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStre Err(err) => return err.into_compile_error().into(), }; - derive_data.set_remote(Some(&remote_ty)); + derive_data.set_remote(Some(RemoteType::new(&remote_ty))); let (reflect_impls, from_reflect_impl) = match derive_data { ReflectDerive::Struct(struct_data) | ReflectDerive::UnitStruct(struct_data) => ( @@ -101,6 +102,54 @@ fn generate_remote_wrapper(input: &DeriveInput, remote_ty: &TypePath) -> proc_ma } } +/// A reflected type's remote type. +/// +/// This is a wrapper around [`TypePath`] that allows it to be paired with other remote-specific logic. +#[derive(Copy, Clone)] +pub(crate) struct RemoteType<'a> { + path: &'a TypePath, +} + +impl<'a> RemoteType<'a> { + pub fn new(path: &'a TypePath) -> Self { + Self { path } + } + + /// Returns the [type path](TypePath) of this remote type. + pub fn type_path(&self) -> &'a TypePath { + self.path + } + + /// Attempts to convert the [type path](TypePath) of this remote type into an [expression path](ExprPath). + /// + /// For example, this would convert `foo::Bar` into `foo::Bar::` to be used as part of an expression. + /// + /// This will return an error for types that are parenthesized, such as in `Fn() -> Foo`. + pub fn as_expr_path(&self) -> Result { + let mut expr_path = self.path.clone(); + if let Some(segment) = expr_path.path.segments.last_mut() { + match &mut segment.arguments { + PathArguments::None => {} + PathArguments::AngleBracketed(arg) => { + arg.colon2_token = Some(PathSep::default()); + } + PathArguments::Parenthesized(arg) => { + return Err(syn::Error::new( + arg.span(), + "cannot use parenthesized type as remote type", + )) + } + } + } + + Ok(ExprPath { + path: expr_path.path, + qself: expr_path.qself, + attrs: Vec::new(), + }) + } +} + /// Metadata from the arguments defined in the `reflect_remote` attribute. /// /// The syntax for the arguments is: `#[reflect_remote(REMOTE_TYPE_PATH)]`. diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index c64e70b97eb75..0cdcd48ab0b1a 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -2629,20 +2629,20 @@ bevy_reflect::tests::Test { } #[reflect_remote(external_crate::TheirOuter)] - struct MyOuter { + struct MyOuter { #[reflect(remote = "MyInner")] pub inner: external_crate::TheirInner, } #[reflect_remote(external_crate::TheirInner)] - struct MyInner(pub T); + struct MyInner(pub T); let mut patch = DynamicStruct::default(); - patch.set_represented_type(Some(MyOuter::type_info())); + patch.set_represented_type(Some(MyOuter::::type_info())); patch.insert("inner", MyInner(external_crate::TheirInner(321))); let mut data = MyOuter(external_crate::TheirOuter { - inner: external_crate::TheirInner(123), + inner: external_crate::TheirInner(123_i32), }); assert_eq!(123, data.0.inner.0); @@ -2650,6 +2650,64 @@ bevy_reflect::tests::Test { assert_eq!(321, data.0.inner.0); } + #[test] + fn should_reflect_nested_remote_enum() { + mod external_crate { + use std::fmt::Debug; + + #[derive(Debug)] + pub enum TheirOuter { + Unit, + Tuple(TheirInner), + Struct { value: TheirInner }, + } + #[derive(Debug)] + pub enum TheirInner { + Unit, + Tuple(T), + Struct { value: T }, + } + } + + #[reflect_remote(external_crate::TheirOuter)] + #[derive(Debug)] + enum MyOuter { + Unit, + Tuple(#[reflect(remote = "MyInner")] external_crate::TheirInner), + Struct { + #[reflect(remote = "MyInner")] + value: external_crate::TheirInner, + }, + } + + #[reflect_remote(external_crate::TheirInner)] + #[derive(Debug)] + enum MyInner { + Unit, + Tuple(T), + Struct { value: T }, + } + + let mut patch = DynamicEnum::default(); + let mut value = DynamicStruct::default(); + value.insert("value", MyInner(external_crate::TheirInner::Tuple(123))); + patch.set_variant("Struct", value); + + let mut data = MyOuter(external_crate::TheirOuter::::Unit); + + assert!(matches!( + data, + MyOuter(external_crate::TheirOuter::::Unit) + )); + data.apply(&patch); + assert!(matches!( + data, + MyOuter(external_crate::TheirOuter::Struct { + value: external_crate::TheirInner::Tuple(123) + }) + )); + } + #[test] fn should_take_remote_type() { mod external_crate { @@ -2710,6 +2768,41 @@ bevy_reflect::tests::Test { external_crate::TheirType { value: "Hello".to_string(), }, + output, + ) + } + + #[test] + fn should_take_nested_remote_type() { + mod external_crate { + #[derive(PartialEq, Eq, Debug)] + pub struct TheirOuter { + pub inner: TheirInner, + } + #[derive(PartialEq, Eq, Debug)] + pub struct TheirInner(pub T); + } + + #[reflect_remote(external_crate::TheirOuter)] + struct MyOuter { + #[reflect(remote = "MyInner")] + pub inner: external_crate::TheirInner, + } + + #[reflect_remote(external_crate::TheirInner)] + struct MyInner(pub T); + + let input: Box = Box::new(MyOuter(external_crate::TheirOuter { + inner: external_crate::TheirInner(123), + })); + + let output: external_crate::TheirOuter = input + .take() + .expect("should downcast to `external_crate::TheirOuter`"); + assert_eq!( + external_crate::TheirOuter { + inner: external_crate::TheirInner(123), + }, output ); } From 9ae61e1db75f9bbcc2db808d76a725447d9ea5b0 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Mon, 2 Jan 2023 17:00:18 -0800 Subject: [PATCH 11/31] Add compile fail tests for reflect_remote --- crates/bevy_reflect/compile_fail/Cargo.toml | 4 ++ .../tests/reflect_remote/macro_order_fail.rs | 19 +++++++++ .../tests/reflect_remote/macro_order_pass.rs | 18 +++++++++ .../tests/reflect_remote/nested_fail.rs | 39 +++++++++++++++++++ .../tests/reflect_remote/nested_pass.rs | 20 ++++++++++ .../bevy_reflect/compile_fail/tests/remote.rs | 3 ++ 6 files changed, 103 insertions(+) create mode 100644 crates/bevy_reflect/compile_fail/tests/reflect_remote/macro_order_fail.rs create mode 100644 crates/bevy_reflect/compile_fail/tests/reflect_remote/macro_order_pass.rs create mode 100644 crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs create mode 100644 crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_pass.rs create mode 100644 crates/bevy_reflect/compile_fail/tests/remote.rs diff --git a/crates/bevy_reflect/compile_fail/Cargo.toml b/crates/bevy_reflect/compile_fail/Cargo.toml index f8a8a41d67e87..14e5eb2264f82 100644 --- a/crates/bevy_reflect/compile_fail/Cargo.toml +++ b/crates/bevy_reflect/compile_fail/Cargo.toml @@ -20,3 +20,7 @@ harness = false [[test]] name = "func" harness = false + +[[test]] +name = "remote" +harness = false diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/macro_order_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/macro_order_fail.rs new file mode 100644 index 0000000000000..2114482816b29 --- /dev/null +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/macro_order_fail.rs @@ -0,0 +1,19 @@ +use bevy_reflect::{reflect_remote, std_traits::ReflectDefault}; + +mod external_crate { + #[derive(Debug, Default)] + pub struct TheirType { + pub value: String, + } +} + +#[derive(Debug, Default)] +#[reflect_remote(external_crate::TheirType)] +#[reflect(Debug, Default)] +struct MyType { + pub value: String, + //~^ ERROR: no field `value` on type `&MyType` + //~| ERROR: struct `MyType` has no field named `value` +} + +fn main() {} diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/macro_order_pass.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/macro_order_pass.rs new file mode 100644 index 0000000000000..58fcb56aaf8d3 --- /dev/null +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/macro_order_pass.rs @@ -0,0 +1,18 @@ +//@check-pass +use bevy_reflect::{reflect_remote, std_traits::ReflectDefault}; + +mod external_crate { + #[derive(Debug, Default)] + pub struct TheirType { + pub value: String, + } +} + +#[reflect_remote(external_crate::TheirType)] +#[derive(Debug, Default)] +#[reflect(Debug, Default)] +struct MyType { + pub value: String, +} + +fn main() {} diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs new file mode 100644 index 0000000000000..14b28ec54b322 --- /dev/null +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs @@ -0,0 +1,39 @@ +mod external_crate { + pub struct TheirOuter { + pub inner: TheirInner, + } + pub struct TheirInner(pub T); +} + +mod missing_attribute { + use bevy_reflect::{reflect_remote, Reflect}; + + #[reflect_remote(super::external_crate::TheirOuter)] + struct MyOuter { + // Reason: Missing `#[reflect(remote = "...")]` attribute + pub inner: super::external_crate::TheirInner, + } + + #[reflect_remote(super::external_crate::TheirInner)] + struct MyInner(pub T); +} + +mod incorrect_inner_type { + use bevy_reflect::{reflect_remote, Reflect}; + + #[reflect_remote(super::external_crate::TheirOuter)] + //~^ ERROR: `TheirInner` can not be reflected + //~| ERROR: `TheirInner` can not be reflected + //~| ERROR: `TheirInner` can not be reflected + //~| ERROR: `TheirInner` can not be used as a dynamic type path + //~| ERROR: `?` operator has incompatible types + struct MyOuter { + // Reason: Should not use `MyInner` directly + pub inner: MyInner, + } + + #[reflect_remote(super::external_crate::TheirInner)] + struct MyInner(pub T); +} + +fn main() {} diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_pass.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_pass.rs new file mode 100644 index 0000000000000..1bff345f392a5 --- /dev/null +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_pass.rs @@ -0,0 +1,20 @@ +//@check-pass +use bevy_reflect::{reflect_remote, Reflect}; + +mod external_crate { + pub struct TheirOuter { + pub inner: TheirInner, + } + pub struct TheirInner(pub T); +} + +#[reflect_remote(external_crate::TheirOuter)] +struct MyOuter { + #[reflect(remote = "MyInner")] + pub inner: external_crate::TheirInner, +} + +#[reflect_remote(external_crate::TheirInner)] +struct MyInner(pub T); + +fn main() {} diff --git a/crates/bevy_reflect/compile_fail/tests/remote.rs b/crates/bevy_reflect/compile_fail/tests/remote.rs new file mode 100644 index 0000000000000..3c843853a1aa7 --- /dev/null +++ b/crates/bevy_reflect/compile_fail/tests/remote.rs @@ -0,0 +1,3 @@ +fn main() -> compile_fail_utils::ui_test::Result<()> { + compile_fail_utils::test("reflect_remote", "tests/reflect_remote") +} From 77bc8199c7271443bd26c3c14e7d590a2013284b Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Mon, 2 Jan 2023 18:18:10 -0800 Subject: [PATCH 12/31] Added remote field assertions --- .../tests/reflect_remote/nested_fail.rs | 15 +++ crates/bevy_reflect/derive/src/remote.rs | 106 +++++++++++++++++- 2 files changed, 117 insertions(+), 4 deletions(-) diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs index 14b28ec54b322..8fe5d8d49f5d2 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs @@ -36,4 +36,19 @@ mod incorrect_inner_type { struct MyInner(pub T); } +mod mismatched_remote_type { + use bevy_reflect::{reflect_remote, Reflect}; + + #[reflect_remote(super::external_crate::TheirOuter)] + //~^ ERROR: mismatched types + struct MyOuter { + // Reason: Should be `MyInner` + #[reflect(remote = "MyOuter")] + pub inner: super::external_crate::TheirInner, + } + + #[reflect_remote(super::external_crate::TheirInner)] + struct MyInner(pub T); +} + fn main() {} diff --git a/crates/bevy_reflect/derive/src/remote.rs b/crates/bevy_reflect/derive/src/remote.rs index 0488bd10dd628..d3627c7324b01 100644 --- a/crates/bevy_reflect/derive/src/remote.rs +++ b/crates/bevy_reflect/derive/src/remote.rs @@ -1,11 +1,15 @@ -use crate::derive_data::{ReflectImplSource, ReflectProvenance, ReflectTraitToImpl}; +use crate::derive_data::{ + EnumVariantFields, ReflectImplSource, ReflectProvenance, ReflectTraitToImpl, StructField, +}; +use crate::utility::ident_or_index; use crate::{from_reflect, impls, ReflectDerive, REFLECT_ATTRIBUTE_NAME}; use proc_macro::TokenStream; -use quote::quote; +use proc_macro2::Ident; +use quote::{format_ident, quote}; use syn::parse::{Parse, ParseStream}; use syn::spanned::Spanned; use syn::token::PathSep; -use syn::{parse_macro_input, DeriveInput, ExprPath, PathArguments, TypePath}; +use syn::{parse_macro_input, DeriveInput, ExprPath, Generics, PathArguments, TypePath}; /// Generates the remote wrapper type and implements all the necessary traits. pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStream { @@ -32,7 +36,7 @@ pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStre derive_data.set_remote(Some(RemoteType::new(&remote_ty))); - let (reflect_impls, from_reflect_impl) = match derive_data { + let (reflect_impls, from_reflect_impl, assertions) = match derive_data { ReflectDerive::Struct(struct_data) | ReflectDerive::UnitStruct(struct_data) => ( impls::impl_struct(&struct_data), if struct_data.meta().from_reflect().should_auto_derive() { @@ -40,6 +44,11 @@ pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStre } else { None }, + Some(generate_remote_field_assertions( + struct_data.fields(), + None, + struct_data.meta().type_path().generics(), + )), ), ReflectDerive::TupleStruct(struct_data) => ( impls::impl_tuple_struct(&struct_data), @@ -48,6 +57,11 @@ pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStre } else { None }, + Some(generate_remote_field_assertions( + struct_data.fields(), + None, + struct_data.meta().type_path().generics(), + )), ), ReflectDerive::Enum(enum_data) => ( impls::impl_enum(&enum_data), @@ -56,6 +70,20 @@ pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStre } else { None }, + enum_data + .variants() + .iter() + .map(|variant| match &variant.fields { + EnumVariantFields::Named(fields) | EnumVariantFields::Unnamed(fields) => { + Some(generate_remote_field_assertions( + fields, + Some(&variant.data.ident), + enum_data.meta().type_path().generics(), + )) + } + EnumVariantFields::Unit => None, + }) + .collect(), ), _ => { return syn::Error::new(ast.span(), "cannot reflect a remote value type") @@ -70,6 +98,8 @@ pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStre #reflect_impls #from_reflect_impl + + #assertions }) } @@ -102,6 +132,74 @@ fn generate_remote_wrapper(input: &DeriveInput, remote_ty: &TypePath) -> proc_ma } } +/// Generates compile-time assertions for remote fields. +/// +/// # Example +/// +/// The following would fail to compile due to an incorrect `#[reflect(remote = "...")]` value. +/// +/// ```ignore +/// mod external_crate { +/// pub struct TheirOuter { +/// pub inner: TheirInner, +/// } +/// pub struct TheirInner(pub String); +/// } +/// +/// #[reflect_remote(external_crate::TheirOuter)] +/// struct MyOuter { +/// #[reflect(remote = "MyOuter")] // <- Note the mismatched type (it should be `MyInner`) +/// pub inner: external_crate::TheirInner, +/// } +/// +/// #[reflect_remote(external_crate::TheirInner)] +/// struct MyInner(pub String); +/// ``` +fn generate_remote_field_assertions( + fields: &[StructField<'_>], + variant: Option<&Ident>, + generics: &Generics, +) -> proc_macro2::TokenStream { + fields + .iter() + .filter(|field| field.attrs.remote.is_some()) + .map(|field| { + let ident = if let Some(variant) = variant { + format_ident!( + "{}__{}", + variant, + ident_or_index(field.data.ident.as_ref(), field.declaration_index) + ) + } else { + field + .data + .ident + .as_ref() + .cloned() + .unwrap_or_else(|| format_ident!("field_{}", field.declaration_index)) + }; + + let (impl_generics, _, where_clause) = generics.split_for_impl(); + let field_ty = &field.data.ty; + let remote_ty = field.attrs.remote.as_ref().unwrap(); + let assertion_ident = format_ident!("assert__{}__is_valid_remote", ident); + + quote! { + const _: () = { + struct RemoteFieldAssertions; + + impl RemoteFieldAssertions { + #[allow(non_snake_case)] + fn #assertion_ident #impl_generics (#ident: #remote_ty) #where_clause { + let _: #field_ty = #ident.0; + } + } + }; + } + }) + .collect() +} + /// A reflected type's remote type. /// /// This is a wrapper around [`TypePath`] that allows it to be paired with other remote-specific logic. From 58d1ba13c7106523f5fc9033bcfdb6c33fc310be Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Mon, 2 Jan 2023 21:23:24 -0800 Subject: [PATCH 13/31] Test concrete remote generic --- .../tests/reflect_remote/nested_fail.rs | 15 ++++++++++++ crates/bevy_reflect/src/lib.rs | 23 ++++++++++++------- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs index 8fe5d8d49f5d2..e18f58aea3018 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs @@ -51,4 +51,19 @@ mod mismatched_remote_type { struct MyInner(pub T); } +mod mismatched_remote_generic { + use bevy_reflect::{reflect_remote, Reflect}; + + #[reflect_remote(super::external_crate::TheirOuter)] + //~^ ERROR: `?` operator has incompatible types + struct MyOuter { + // Reason: `TheirOuter::inner` is not defined as `TheirInner` + #[reflect(remote = "MyInner")] + pub inner: super::external_crate::TheirInner, + } + + #[reflect_remote(super::external_crate::TheirInner)] + struct MyInner(pub T); +} + fn main() {} diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 0cdcd48ab0b1a..2200f32278e64 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -2623,31 +2623,38 @@ bevy_reflect::tests::Test { fn should_reflect_nested_remote_type() { mod external_crate { pub struct TheirOuter { - pub inner: TheirInner, + pub a: TheirInner, + pub b: TheirInner, } pub struct TheirInner(pub T); } #[reflect_remote(external_crate::TheirOuter)] - struct MyOuter { + struct MyOuter { #[reflect(remote = "MyInner")] - pub inner: external_crate::TheirInner, + pub a: external_crate::TheirInner, + #[reflect(remote = "MyInner")] + pub b: external_crate::TheirInner, } #[reflect_remote(external_crate::TheirInner)] - struct MyInner(pub T); + struct MyInner(pub T); let mut patch = DynamicStruct::default(); patch.set_represented_type(Some(MyOuter::::type_info())); - patch.insert("inner", MyInner(external_crate::TheirInner(321))); + patch.insert("a", MyInner(external_crate::TheirInner(321_i32))); + patch.insert("b", MyInner(external_crate::TheirInner(true))); let mut data = MyOuter(external_crate::TheirOuter { - inner: external_crate::TheirInner(123_i32), + a: external_crate::TheirInner(123_i32), + b: external_crate::TheirInner(false), }); - assert_eq!(123, data.0.inner.0); + assert_eq!(123, data.0.a.0); + assert!(!data.0.b.0); data.apply(&patch); - assert_eq!(321, data.0.inner.0); + assert_eq!(321, data.0.a.0); + assert!(data.0.b.0); } #[test] From d1a6277885cadf2db46ae63a791f2d0a59ac8fc1 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Thu, 16 Mar 2023 13:14:42 -0700 Subject: [PATCH 14/31] Improved remote reflect assertions --- .../reflect_remote/type_mismatch_fail.rs | 91 +++++++++ .../reflect_remote/type_mismatch_pass.rs | 79 ++++++++ .../derive/src/impls/assertions.rs | 14 ++ crates/bevy_reflect/derive/src/impls/mod.rs | 2 + crates/bevy_reflect/derive/src/lib.rs | 5 + crates/bevy_reflect/derive/src/remote.rs | 177 ++++++++++-------- crates/bevy_reflect/src/lib.rs | 4 +- 7 files changed, 294 insertions(+), 78 deletions(-) create mode 100644 crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs create mode 100644 crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_pass.rs create mode 100644 crates/bevy_reflect/derive/src/impls/assertions.rs diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs new file mode 100644 index 0000000000000..59b1b02877ff8 --- /dev/null +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs @@ -0,0 +1,91 @@ +//@no-rustfix + +mod structs { + use bevy_reflect::{reflect_remote, Reflect}; + + mod external_crate { + pub struct TheirFoo { + pub value: u32, + } + pub struct TheirBar { + pub value: i32, + } + } + + #[reflect_remote(external_crate::TheirFoo)] + struct MyFoo { + pub value: u32, + } + #[reflect_remote(external_crate::TheirBar)] + struct MyBar { + pub value: i32, + } + + #[derive(Reflect)] + //~^ ERROR: mismatched types + struct MyStruct { + // Reason: Should use `MyFoo` + #[reflect(remote = "MyBar")] + foo: external_crate::TheirFoo, + } +} + +mod tuple_structs { + use bevy_reflect::{reflect_remote, Reflect}; + + mod external_crate { + pub struct TheirFoo(pub u32); + + pub struct TheirBar(pub i32); + } + + #[reflect_remote(external_crate::TheirFoo)] + struct MyFoo(pub u32); + + #[reflect_remote(external_crate::TheirBar)] + struct MyBar(pub i32); + + #[derive(Reflect)] + //~^ ERROR: mismatched types + struct MyStruct( + // Reason: Should use `MyFoo` + #[reflect(remote = "MyBar")] external_crate::TheirFoo, + ); +} + +mod enums { + use bevy_reflect::{reflect_remote, Reflect}; + + mod external_crate { + pub enum TheirFoo { + Value(u32), + } + + pub enum TheirBar { + Value(i32), + } + } + + #[reflect_remote(external_crate::TheirFoo)] + enum MyFoo { + Value(u32), + } + + #[reflect_remote(external_crate::TheirBar)] + //~^ ERROR: `?` operator has incompatible types + enum MyBar { + // Reason: Should use `i32` + Value(u32), + } + + #[derive(Reflect)] + //~^ ERROR: mismatched types + enum MyStruct { + Value( + // Reason: Should use `MyFoo` + #[reflect(remote = "MyBar")] external_crate::TheirFoo, + ), + } +} + +fn main() {} diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_pass.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_pass.rs new file mode 100644 index 0000000000000..19707c11c0d9a --- /dev/null +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_pass.rs @@ -0,0 +1,79 @@ +//@check-pass + +mod structs { + use bevy_reflect::{reflect_remote, Reflect}; + + mod external_crate { + pub struct TheirFoo { + pub value: u32, + } + pub struct TheirBar { + pub value: i32, + } + } + + #[reflect_remote(external_crate::TheirFoo)] + struct MyFoo { + pub value: u32, + } + #[reflect_remote(external_crate::TheirBar)] + struct MyBar { + pub value: i32, + } + + #[derive(Reflect)] + struct MyStruct { + #[reflect(remote = "MyFoo")] + foo: external_crate::TheirFoo, + } +} + +mod tuple_structs { + use bevy_reflect::{reflect_remote, Reflect}; + + mod external_crate { + pub struct TheirFoo(pub u32); + + pub struct TheirBar(pub i32); + } + + #[reflect_remote(external_crate::TheirFoo)] + struct MyFoo(pub u32); + + #[reflect_remote(external_crate::TheirBar)] + struct MyBar(pub i32); + + #[derive(Reflect)] + struct MyStruct(#[reflect(remote = "MyFoo")] external_crate::TheirFoo); +} + +mod enums { + use bevy_reflect::{reflect_remote, Reflect}; + + mod external_crate { + pub enum TheirFoo { + Value(u32), + } + + pub enum TheirBar { + Value(i32), + } + } + + #[reflect_remote(external_crate::TheirFoo)] + enum MyFoo { + Value(u32), + } + + #[reflect_remote(external_crate::TheirBar)] + enum MyBar { + Value(i32), + } + + #[derive(Reflect)] + enum MyStruct { + Value(#[reflect(remote = "MyFoo")] external_crate::TheirFoo), + } +} + +fn main() {} diff --git a/crates/bevy_reflect/derive/src/impls/assertions.rs b/crates/bevy_reflect/derive/src/impls/assertions.rs new file mode 100644 index 0000000000000..06d835e6cf9d4 --- /dev/null +++ b/crates/bevy_reflect/derive/src/impls/assertions.rs @@ -0,0 +1,14 @@ +use crate::derive_data::ReflectDerive; +use crate::remote::generate_remote_assertions; +use quote::quote; + +/// Generates an anonymous block containing compile-time assertions. +pub(crate) fn impl_assertions(derive_data: &ReflectDerive) -> proc_macro2::TokenStream { + let mut output = quote!(); + + if let Some(assertions) = generate_remote_assertions(derive_data) { + output.extend(assertions); + } + + output +} diff --git a/crates/bevy_reflect/derive/src/impls/mod.rs b/crates/bevy_reflect/derive/src/impls/mod.rs index 5974d85d9f3da..0905d43bd260e 100644 --- a/crates/bevy_reflect/derive/src/impls/mod.rs +++ b/crates/bevy_reflect/derive/src/impls/mod.rs @@ -1,3 +1,4 @@ +mod assertions; mod common; mod enums; #[cfg(feature = "functions")] @@ -7,6 +8,7 @@ mod tuple_structs; mod typed; mod values; +pub(crate) use assertions::impl_assertions; pub(crate) use common::{common_partial_reflect_methods, impl_full_reflect}; pub(crate) use enums::impl_enum; #[cfg(feature = "functions")] diff --git a/crates/bevy_reflect/derive/src/lib.rs b/crates/bevy_reflect/derive/src/lib.rs index 17f95c263dfec..c4f1d98150a14 100644 --- a/crates/bevy_reflect/derive/src/lib.rs +++ b/crates/bevy_reflect/derive/src/lib.rs @@ -64,6 +64,8 @@ fn match_reflect_impls(ast: DeriveInput, source: ReflectImplSource) -> TokenStre Err(err) => return err.into_compile_error().into(), }; + let assertions = impls::impl_assertions(&derive_data); + let (reflect_impls, from_reflect_impl) = match derive_data { ReflectDerive::Struct(struct_data) | ReflectDerive::UnitStruct(struct_data) => ( impls::impl_struct(&struct_data), @@ -102,7 +104,10 @@ fn match_reflect_impls(ast: DeriveInput, source: ReflectImplSource) -> TokenStre TokenStream::from(quote! { const _: () = { #reflect_impls + #from_reflect_impl + + #assertions }; }) } diff --git a/crates/bevy_reflect/derive/src/remote.rs b/crates/bevy_reflect/derive/src/remote.rs index d3627c7324b01..4528c5d679cab 100644 --- a/crates/bevy_reflect/derive/src/remote.rs +++ b/crates/bevy_reflect/derive/src/remote.rs @@ -1,6 +1,5 @@ -use crate::derive_data::{ - EnumVariantFields, ReflectImplSource, ReflectProvenance, ReflectTraitToImpl, StructField, -}; +use crate::derive_data::{ReflectImplSource, ReflectProvenance, ReflectTraitToImpl}; +use crate::impls::impl_assertions; use crate::utility::ident_or_index; use crate::{from_reflect, impls, ReflectDerive, REFLECT_ATTRIBUTE_NAME}; use proc_macro::TokenStream; @@ -9,7 +8,9 @@ use quote::{format_ident, quote}; use syn::parse::{Parse, ParseStream}; use syn::spanned::Spanned; use syn::token::PathSep; -use syn::{parse_macro_input, DeriveInput, ExprPath, Generics, PathArguments, TypePath}; +use syn::{ + parse_macro_input, DeriveInput, ExprPath, Generics, Member, PathArguments, Type, TypePath, +}; /// Generates the remote wrapper type and implements all the necessary traits. pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStream { @@ -36,7 +37,9 @@ pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStre derive_data.set_remote(Some(RemoteType::new(&remote_ty))); - let (reflect_impls, from_reflect_impl, assertions) = match derive_data { + let assertions = impl_assertions(&derive_data); + + let (reflect_impls, from_reflect_impl) = match derive_data { ReflectDerive::Struct(struct_data) | ReflectDerive::UnitStruct(struct_data) => ( impls::impl_struct(&struct_data), if struct_data.meta().from_reflect().should_auto_derive() { @@ -44,11 +47,6 @@ pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStre } else { None }, - Some(generate_remote_field_assertions( - struct_data.fields(), - None, - struct_data.meta().type_path().generics(), - )), ), ReflectDerive::TupleStruct(struct_data) => ( impls::impl_tuple_struct(&struct_data), @@ -57,11 +55,6 @@ pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStre } else { None }, - Some(generate_remote_field_assertions( - struct_data.fields(), - None, - struct_data.meta().type_path().generics(), - )), ), ReflectDerive::Enum(enum_data) => ( impls::impl_enum(&enum_data), @@ -70,20 +63,6 @@ pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStre } else { None }, - enum_data - .variants() - .iter() - .map(|variant| match &variant.fields { - EnumVariantFields::Named(fields) | EnumVariantFields::Unnamed(fields) => { - Some(generate_remote_field_assertions( - fields, - Some(&variant.data.ident), - enum_data.meta().type_path().generics(), - )) - } - EnumVariantFields::Unit => None, - }) - .collect(), ), _ => { return syn::Error::new(ast.span(), "cannot reflect a remote value type") @@ -95,11 +74,13 @@ pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStre TokenStream::from(quote! { #wrapper_definition - #reflect_impls + const _: () = { + #reflect_impls - #from_reflect_impl + #from_reflect_impl - #assertions + #assertions + }; }) } @@ -134,70 +115,114 @@ fn generate_remote_wrapper(input: &DeriveInput, remote_ty: &TypePath) -> proc_ma /// Generates compile-time assertions for remote fields. /// +/// This prevents types from using an invalid remote type. +/// this works by generating a struct, `RemoteFieldAssertions`, with methods that +/// will result in compile-time failure if types are mismatched. +/// The output of this function is best placed within an anonymous context to maintain hygiene. +/// /// # Example /// /// The following would fail to compile due to an incorrect `#[reflect(remote = "...")]` value. /// /// ```ignore /// mod external_crate { -/// pub struct TheirOuter { -/// pub inner: TheirInner, -/// } -/// pub struct TheirInner(pub String); +/// pub struct TheirFoo(pub u32); +/// pub struct TheirBar(pub i32); /// } /// -/// #[reflect_remote(external_crate::TheirOuter)] -/// struct MyOuter { -/// #[reflect(remote = "MyOuter")] // <- Note the mismatched type (it should be `MyInner`) -/// pub inner: external_crate::TheirInner, -/// } +/// #[reflect_remote(external_crate::TheirFoo)] +/// struct MyFoo(pub u32); +/// #[reflect_remote(external_crate::TheirBar)] +/// struct MyBar(pub i32); /// -/// #[reflect_remote(external_crate::TheirInner)] -/// struct MyInner(pub String); +/// #[derive(Reflect)] +/// struct MyStruct { +/// #[reflect(remote = "MyBar")] // ERROR: expected type `TheirFoo` but found struct `TheirBar` +/// foo: external_crate::TheirFoo +/// } /// ``` -fn generate_remote_field_assertions( - fields: &[StructField<'_>], - variant: Option<&Ident>, - generics: &Generics, -) -> proc_macro2::TokenStream { - fields - .iter() - .filter(|field| field.attrs.remote.is_some()) - .map(|field| { - let ident = if let Some(variant) = variant { - format_ident!( - "{}__{}", - variant, - ident_or_index(field.data.ident.as_ref(), field.declaration_index) - ) - } else { +pub(crate) fn generate_remote_assertions( + derive_data: &ReflectDerive, +) -> Option { + struct RemoteAssertionData<'a> { + ident: Member, + variant: Option<&'a Ident>, + ty: &'a Type, + generics: &'a Generics, + remote_ty: &'a Type, + } + + let fields: Box> = match derive_data { + ReflectDerive::Struct(data) + | ReflectDerive::TupleStruct(data) + | ReflectDerive::UnitStruct(data) => Box::new(data.active_fields().filter_map(|field| { + field + .attrs + .remote + .as_ref() + .map(|remote_ty| RemoteAssertionData { + ident: ident_or_index(field.data.ident.as_ref(), field.declaration_index), + variant: None, + ty: &field.data.ty, + generics: data.meta().type_path().generics(), + remote_ty, + }) + })), + ReflectDerive::Enum(data) => Box::new(data.variants().iter().flat_map(|variant| { + variant.active_fields().filter_map(|field| { field - .data - .ident + .attrs + .remote .as_ref() - .cloned() - .unwrap_or_else(|| format_ident!("field_{}", field.declaration_index)) + .map(|remote_ty| RemoteAssertionData { + ident: ident_or_index(field.data.ident.as_ref(), field.declaration_index), + variant: Some(&variant.data.ident), + ty: &field.data.ty, + generics: data.meta().type_path().generics(), + remote_ty, + }) + }) + })), + + _ => return None, + }; + + let assertions = fields + .map(move |field| { + let ident = if let Some(variant) = field.variant { + format_ident!("{}__{}", variant, field.ident) + } else { + match field.ident { + Member::Named(ident) => ident, + Member::Unnamed(index) => format_ident!("field_{}", index), + } }; + let (impl_generics, _, where_clause) = field.generics.split_for_impl(); - let (impl_generics, _, where_clause) = generics.split_for_impl(); - let field_ty = &field.data.ty; - let remote_ty = field.attrs.remote.as_ref().unwrap(); + let ty = &field.ty; + let remote_ty = field.remote_ty; let assertion_ident = format_ident!("assert__{}__is_valid_remote", ident); quote! { - const _: () = { - struct RemoteFieldAssertions; + #[allow(non_snake_case)] + fn #assertion_ident #impl_generics (#ident: #remote_ty) #where_clause { + let _: #ty = #ident.0; + } + } + }) + .collect::(); - impl RemoteFieldAssertions { - #[allow(non_snake_case)] - fn #assertion_ident #impl_generics (#ident: #remote_ty) #where_clause { - let _: #field_ty = #ident.0; - } - } - }; + if assertions.is_empty() { + None + } else { + Some(quote! { + struct RemoteFieldAssertions; + + impl RemoteFieldAssertions { + #assertions } }) - .collect() + } } /// A reflected type's remote type. diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 2200f32278e64..e5b52f90390e3 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -2630,7 +2630,7 @@ bevy_reflect::tests::Test { } #[reflect_remote(external_crate::TheirOuter)] - struct MyOuter { + struct MyOuter { #[reflect(remote = "MyInner")] pub a: external_crate::TheirInner, #[reflect(remote = "MyInner")] @@ -2638,7 +2638,7 @@ bevy_reflect::tests::Test { } #[reflect_remote(external_crate::TheirInner)] - struct MyInner(pub T); + struct MyInner(pub T); let mut patch = DynamicStruct::default(); patch.set_represented_type(Some(MyOuter::::type_info())); From 396290a6775db5c1b5330ebad3df70899f2c31ca Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Fri, 17 Mar 2023 01:17:59 -0700 Subject: [PATCH 15/31] Add more compile-time assertions for remote types Add assertion to verify wrapper field types match remote field types. --- .../reflect_remote/invalid_definition_fail.rs | 63 +++++++++++++++ .../reflect_remote/invalid_definition_pass.rs | 48 +++++++++++ .../tests/reflect_remote/nested_fail.rs | 2 + .../reflect_remote/type_mismatch_fail.rs | 1 + crates/bevy_reflect/derive/src/derive_data.rs | 22 +++-- crates/bevy_reflect/derive/src/remote.rs | 81 +++++++++++++++++++ 6 files changed, 211 insertions(+), 6 deletions(-) create mode 100644 crates/bevy_reflect/compile_fail/tests/reflect_remote/invalid_definition_fail.rs create mode 100644 crates/bevy_reflect/compile_fail/tests/reflect_remote/invalid_definition_pass.rs diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/invalid_definition_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/invalid_definition_fail.rs new file mode 100644 index 0000000000000..7a08ce8eef205 --- /dev/null +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/invalid_definition_fail.rs @@ -0,0 +1,63 @@ +mod structs { + use bevy_reflect::reflect_remote; + + mod external_crate { + pub struct TheirStruct { + pub value: u32, + } + } + + #[reflect_remote(external_crate::TheirStruct)] + //~^ ERROR: mismatched types + //~| ERROR: `?` operator has incompatible types + struct MyStruct { + // Reason: Should be `u32` + pub value: bool, + } +} + +mod tuple_structs { + use bevy_reflect::reflect_remote; + + mod external_crate { + pub struct TheirStruct(pub u32); + } + + #[reflect_remote(external_crate::TheirStruct)] + //~^ ERROR: mismatched types + //~| ERROR: `?` operator has incompatible types + struct MyStruct( + // Reason: Should be `u32` + pub bool, + ); +} + +mod enums { + use bevy_reflect::reflect_remote; + + mod external_crate { + pub enum TheirStruct { + Unit, + Tuple(u32), + Struct { value: usize }, + } + } + + #[reflect_remote(external_crate::TheirStruct)] + //~^ ERROR: mismatched types + //~| ERROR: mismatched types + //~| ERROR: variant `enums::external_crate::TheirStruct::Unit` does not have a field named `0` + //~| ERROR: variant `enums::external_crate::TheirStruct::Unit` has no field named `0` + //~| ERROR: `?` operator has incompatible types + //~| ERROR: `?` operator has incompatible types + enum MyStruct { + // Reason: Should be unit variant + Unit(i32), + // Reason: Should be `u32` + Tuple(bool), + // Reason: Should be `usize` + Struct { value: String }, + } +} + +fn main() {} diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/invalid_definition_pass.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/invalid_definition_pass.rs new file mode 100644 index 0000000000000..7f2fdb73dd6ef --- /dev/null +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/invalid_definition_pass.rs @@ -0,0 +1,48 @@ +//@check-pass + +mod structs { + use bevy_reflect::reflect_remote; + + mod external_crate { + pub struct TheirStruct { + pub value: u32, + } + } + + #[reflect_remote(external_crate::TheirStruct)] + struct MyStruct { + pub value: u32, + } +} + +mod tuple_structs { + use bevy_reflect::reflect_remote; + + mod external_crate { + pub struct TheirStruct(pub u32); + } + + #[reflect_remote(external_crate::TheirStruct)] + struct MyStruct(pub u32); +} + +mod enums { + use bevy_reflect::reflect_remote; + + mod external_crate { + pub enum TheirStruct { + Unit, + Tuple(u32), + Struct { value: usize }, + } + } + + #[reflect_remote(external_crate::TheirStruct)] + enum MyStruct { + Unit, + Tuple(u32), + Struct { value: usize }, + } +} + +fn main() {} diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs index e18f58aea3018..784795297bf8f 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs @@ -27,6 +27,7 @@ mod incorrect_inner_type { //~| ERROR: `TheirInner` can not be reflected //~| ERROR: `TheirInner` can not be used as a dynamic type path //~| ERROR: `?` operator has incompatible types + //~| ERROR: mismatched types struct MyOuter { // Reason: Should not use `MyInner` directly pub inner: MyInner, @@ -56,6 +57,7 @@ mod mismatched_remote_generic { #[reflect_remote(super::external_crate::TheirOuter)] //~^ ERROR: `?` operator has incompatible types + //~| ERROR: mismatched types struct MyOuter { // Reason: `TheirOuter::inner` is not defined as `TheirInner` #[reflect(remote = "MyInner")] diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs index 59b1b02877ff8..182fccd7361cb 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs @@ -73,6 +73,7 @@ mod enums { #[reflect_remote(external_crate::TheirBar)] //~^ ERROR: `?` operator has incompatible types + //~| ERROR: mismatched types enum MyBar { // Reason: Should use `i32` Value(u32), diff --git a/crates/bevy_reflect/derive/src/derive_data.rs b/crates/bevy_reflect/derive/src/derive_data.rs index 56efeab010871..509e4d0601c2e 100644 --- a/crates/bevy_reflect/derive/src/derive_data.rs +++ b/crates/bevy_reflect/derive/src/derive_data.rs @@ -374,13 +374,23 @@ impl<'a> ReflectDerive<'a> { } } - pub fn meta(&self) -> &ReflectMeta<'a> { + /// Get the remote type path, if any. + pub fn remote_ty(&self) -> Option { + match self { + Self::Struct(data) | Self::TupleStruct(data) | Self::UnitStruct(data) => { + data.remote_ty() + } + Self::Enum(data) => data.remote_ty(), + Self::Value(_) => None, + } + } + + /// Get the [`ReflectMeta`] for this derived type. + pub fn meta(&self) -> &ReflectMeta { match self { - ReflectDerive::Struct(data) - | ReflectDerive::TupleStruct(data) - | ReflectDerive::UnitStruct(data) => data.meta(), - ReflectDerive::Enum(data) => data.meta(), - ReflectDerive::Value(meta) => meta, + Self::Struct(data) | Self::TupleStruct(data) | Self::UnitStruct(data) => data.meta(), + Self::Enum(data) => data.meta(), + Self::Value(meta) => meta, } } diff --git a/crates/bevy_reflect/derive/src/remote.rs b/crates/bevy_reflect/derive/src/remote.rs index 4528c5d679cab..6f8b7ef233368 100644 --- a/crates/bevy_reflect/derive/src/remote.rs +++ b/crates/bevy_reflect/derive/src/remote.rs @@ -38,6 +38,7 @@ pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStre derive_data.set_remote(Some(RemoteType::new(&remote_ty))); let assertions = impl_assertions(&derive_data); + let definition_assertions = generate_remote_definition_assertions(&derive_data); let (reflect_impls, from_reflect_impl) = match derive_data { ReflectDerive::Struct(struct_data) | ReflectDerive::UnitStruct(struct_data) => ( @@ -79,6 +80,8 @@ pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStre #from_reflect_impl + #definition_assertions + #assertions }; }) @@ -225,6 +228,84 @@ pub(crate) fn generate_remote_assertions( } } +/// Generates compile-time assertions that ensure a remote wrapper definition matches up with the +/// remote type it's wrapping. +/// +/// Note: This currently results in "backwards" error messages like: +/// +/// ```ignore +/// expected: +/// found: +/// ``` +/// +/// Ideally it would be the other way around, but there's no easy way of doing this without +/// generating a copy of the struct/enum definition and using that as the base instead of the remote type. +fn generate_remote_definition_assertions(derive_data: &ReflectDerive) -> proc_macro2::TokenStream { + let meta = derive_data.meta(); + let self_ident = format_ident!("__remote__"); + let self_ty = derive_data.remote_ty().unwrap().type_path(); + let self_expr_path = derive_data.remote_ty().unwrap().as_expr_path().unwrap(); + let (impl_generics, _, where_clause) = meta.type_path().generics().split_for_impl(); + + let assertions = match derive_data { + ReflectDerive::Struct(data) + | ReflectDerive::TupleStruct(data) + | ReflectDerive::UnitStruct(data) => { + let field_member = data + .fields() + .iter() + .map(|field| ident_or_index(field.data.ident.as_ref(), field.declaration_index)); + let field_ty = data.fields().iter().map(|field| &field.data.ty); + quote! { + #(let _: #field_ty = #self_ident.#field_member;)* + } + } + ReflectDerive::Enum(data) => { + let variants = data.variants().iter().map(|variant| { + let ident = &variant.data.ident; + + let field_member = variant.fields().iter().map(|field| { + ident_or_index(field.data.ident.as_ref(), field.declaration_index) + }); + let field_ident = variant + .fields() + .iter() + .map(|field| format_ident!("field_{}", field.declaration_index)) + .collect::>(); + let field_ty = variant.fields().iter().map(|field| &field.data.ty); + + quote! { + #self_expr_path::#ident {#(#field_member: #field_ident),*} => { + #(let _: #field_ty = #field_ident;)* + } + } + }); + + quote! { + match #self_ident { + #(#variants)* + } + } + } + ReflectDerive::Value(meta) => { + return syn::Error::new( + meta.type_path().span(), + "cannot reflect a remote value type", + ) + .into_compile_error() + } + }; + + quote! { + const _: () = { + #[allow(non_snake_case)] + fn assert_wrapper_definition_matches_remote_type #impl_generics (#self_ident: #self_ty) #where_clause { + #assertions + } + }; + } +} + /// A reflected type's remote type. /// /// This is a wrapper around [`TypePath`] that allows it to be paired with other remote-specific logic. From e0c977a349eff501751300196e39895508aee685 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Mon, 29 May 2023 17:52:06 -0700 Subject: [PATCH 16/31] Fix assertion type mismatch order Flipped the order of the types when using reflect_remote with mismatched types. Before this was displaying "expected MyType, found TheirType" when it makes more sense to say "expected TheirType, found MyType". --- .../reflect_remote/invalid_definition_fail.rs | 14 ++-- .../tests/reflect_remote/nested_fail.rs | 4 +- .../reflect_remote/type_mismatch_fail.rs | 2 +- crates/bevy_reflect/derive/src/remote.rs | 72 ++++++++++++------- 4 files changed, 58 insertions(+), 34 deletions(-) diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/invalid_definition_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/invalid_definition_fail.rs index 7a08ce8eef205..d691c824ccac9 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/invalid_definition_fail.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/invalid_definition_fail.rs @@ -8,11 +8,11 @@ mod structs { } #[reflect_remote(external_crate::TheirStruct)] - //~^ ERROR: mismatched types - //~| ERROR: `?` operator has incompatible types + //~^ ERROR: `?` operator has incompatible types struct MyStruct { // Reason: Should be `u32` pub value: bool, + //~^ ERROR: mismatched types } } @@ -24,11 +24,11 @@ mod tuple_structs { } #[reflect_remote(external_crate::TheirStruct)] - //~^ ERROR: mismatched types - //~| ERROR: `?` operator has incompatible types + //~^ ERROR: `?` operator has incompatible types struct MyStruct( // Reason: Should be `u32` pub bool, + //~^ ERROR: mismatched types ); } @@ -44,9 +44,7 @@ mod enums { } #[reflect_remote(external_crate::TheirStruct)] - //~^ ERROR: mismatched types - //~| ERROR: mismatched types - //~| ERROR: variant `enums::external_crate::TheirStruct::Unit` does not have a field named `0` + //~^ ERROR: variant `enums::external_crate::TheirStruct::Unit` does not have a field named `0` //~| ERROR: variant `enums::external_crate::TheirStruct::Unit` has no field named `0` //~| ERROR: `?` operator has incompatible types //~| ERROR: `?` operator has incompatible types @@ -55,8 +53,10 @@ mod enums { Unit(i32), // Reason: Should be `u32` Tuple(bool), + //~^ ERROR: mismatched types // Reason: Should be `usize` Struct { value: String }, + //~^ ERROR: mismatched types } } diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs index 784795297bf8f..b7f8db0b60ae4 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs @@ -27,10 +27,10 @@ mod incorrect_inner_type { //~| ERROR: `TheirInner` can not be reflected //~| ERROR: `TheirInner` can not be used as a dynamic type path //~| ERROR: `?` operator has incompatible types - //~| ERROR: mismatched types struct MyOuter { // Reason: Should not use `MyInner` directly pub inner: MyInner, + //~^ ERROR: mismatched types } #[reflect_remote(super::external_crate::TheirInner)] @@ -57,11 +57,11 @@ mod mismatched_remote_generic { #[reflect_remote(super::external_crate::TheirOuter)] //~^ ERROR: `?` operator has incompatible types - //~| ERROR: mismatched types struct MyOuter { // Reason: `TheirOuter::inner` is not defined as `TheirInner` #[reflect(remote = "MyInner")] pub inner: super::external_crate::TheirInner, + //~^ ERROR: mismatched types } #[reflect_remote(super::external_crate::TheirInner)] diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs index 182fccd7361cb..73a1e1bd9c7ee 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs @@ -73,10 +73,10 @@ mod enums { #[reflect_remote(external_crate::TheirBar)] //~^ ERROR: `?` operator has incompatible types - //~| ERROR: mismatched types enum MyBar { // Reason: Should use `i32` Value(u32), + //~^ ERROR: mismatched types } #[derive(Reflect)] diff --git a/crates/bevy_reflect/derive/src/remote.rs b/crates/bevy_reflect/derive/src/remote.rs index 6f8b7ef233368..a9c51812ca77c 100644 --- a/crates/bevy_reflect/derive/src/remote.rs +++ b/crates/bevy_reflect/derive/src/remote.rs @@ -2,9 +2,10 @@ use crate::derive_data::{ReflectImplSource, ReflectProvenance, ReflectTraitToImp use crate::impls::impl_assertions; use crate::utility::ident_or_index; use crate::{from_reflect, impls, ReflectDerive, REFLECT_ATTRIBUTE_NAME}; +use bevy_macro_utils::fq_std::FQOption; use proc_macro::TokenStream; -use proc_macro2::Ident; -use quote::{format_ident, quote}; +use proc_macro2::{Ident, Span}; +use quote::{format_ident, quote, quote_spanned}; use syn::parse::{Parse, ParseStream}; use syn::spanned::Spanned; use syn::token::PathSep; @@ -247,38 +248,58 @@ fn generate_remote_definition_assertions(derive_data: &ReflectDerive) -> proc_ma let self_expr_path = derive_data.remote_ty().unwrap().as_expr_path().unwrap(); let (impl_generics, _, where_clause) = meta.type_path().generics().split_for_impl(); + /// Creates a span located around the given one, but resolves to the assertion's context. + /// + /// This should allow the compiler to point back to the line and column in the user's code, + /// while still attributing the error to the `reflect_remote` macro. + fn create_assertion_span(span: Span) -> Span { + Span::call_site().located_at(span) + } + let assertions = match derive_data { ReflectDerive::Struct(data) | ReflectDerive::TupleStruct(data) | ReflectDerive::UnitStruct(data) => { - let field_member = data - .fields() - .iter() - .map(|field| ident_or_index(field.data.ident.as_ref(), field.declaration_index)); - let field_ty = data.fields().iter().map(|field| &field.data.ty); - quote! { - #(let _: #field_ty = #self_ident.#field_member;)* + let mut output = proc_macro2::TokenStream::new(); + + for field in data.fields() { + let field_member = + ident_or_index(field.data.ident.as_ref(), field.declaration_index); + let field_ty = &field.data.ty; + let span = create_assertion_span(field_ty.span()); + + output.extend(quote_spanned! {span=> + #self_ident.#field_member = (|| -> #FQOption<#field_ty> {None})().unwrap(); + }); } + + output } ReflectDerive::Enum(data) => { let variants = data.variants().iter().map(|variant| { let ident = &variant.data.ident; - let field_member = variant.fields().iter().map(|field| { - ident_or_index(field.data.ident.as_ref(), field.declaration_index) - }); - let field_ident = variant - .fields() - .iter() - .map(|field| format_ident!("field_{}", field.declaration_index)) - .collect::>(); - let field_ty = variant.fields().iter().map(|field| &field.data.ty); - - quote! { - #self_expr_path::#ident {#(#field_member: #field_ident),*} => { - #(let _: #field_ty = #field_ident;)* - } + let mut output = proc_macro2::TokenStream::new(); + + if variant.fields().is_empty() { + return quote!(#self_expr_path::#ident => {}); } + + for field in variant.fields() { + let field_member = + ident_or_index(field.data.ident.as_ref(), field.declaration_index); + let field_ident = format_ident!("field_{}", field_member); + let field_ty = &field.data.ty; + let span = create_assertion_span(field_ty.span()); + + output.extend(quote_spanned! {span=> + #self_expr_path::#ident {#field_member: mut #field_ident, ..} => { + #field_ident = (|| -> #FQOption<#field_ty> {None})().unwrap(); + } + }); + } + + output }); quote! { @@ -299,7 +320,10 @@ fn generate_remote_definition_assertions(derive_data: &ReflectDerive) -> proc_ma quote! { const _: () = { #[allow(non_snake_case)] - fn assert_wrapper_definition_matches_remote_type #impl_generics (#self_ident: #self_ty) #where_clause { + #[allow(unused_variables)] + #[allow(unused_assignments)] + #[allow(unreachable_patterns)] + fn assert_wrapper_definition_matches_remote_type #impl_generics (mut #self_ident: #self_ty) #where_clause { #assertions } }; From c9328003bab0cb04d38838f920c6791aff10fd91 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Tue, 30 May 2023 14:38:13 -0700 Subject: [PATCH 17/31] Add the ReflectRemote trait --- .../reflect_remote/incorrect_wrapper_fail.rs | 23 ++++++++ .../reflect_remote/incorrect_wrapper_pass.rs | 21 +++++++ .../tests/reflect_remote/nested_fail.rs | 2 +- .../tests/reflect_remote/nested_pass.rs | 4 +- .../reflect_remote/type_mismatch_fail.rs | 6 +- crates/bevy_reflect/derive/src/derive_data.rs | 32 ++++++++++ .../bevy_reflect/derive/src/from_reflect.rs | 2 +- crates/bevy_reflect/derive/src/remote.rs | 59 +++++++++++++++---- crates/bevy_reflect/derive/src/utility.rs | 9 +-- crates/bevy_reflect/src/lib.rs | 9 ++- crates/bevy_reflect/src/remote.rs | 51 ++++++++++++++++ 11 files changed, 193 insertions(+), 25 deletions(-) create mode 100644 crates/bevy_reflect/compile_fail/tests/reflect_remote/incorrect_wrapper_fail.rs create mode 100644 crates/bevy_reflect/compile_fail/tests/reflect_remote/incorrect_wrapper_pass.rs create mode 100644 crates/bevy_reflect/src/remote.rs diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/incorrect_wrapper_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/incorrect_wrapper_fail.rs new file mode 100644 index 0000000000000..1a3f0a2d764b4 --- /dev/null +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/incorrect_wrapper_fail.rs @@ -0,0 +1,23 @@ +use bevy_reflect::Reflect; + +mod external_crate { + pub struct TheirFoo { + pub value: u32, + } +} + +#[repr(transparent)] +#[derive(Reflect)] +#[reflect(from_reflect = false)] +struct MyFoo(#[reflect(ignore)] pub external_crate::TheirFoo); + +#[derive(Reflect)] +#[reflect(from_reflect = false)] +struct MyStruct { + // Reason: `MyFoo` does not implement `ReflectRemote` (from `#[reflect_remote]` attribute) + #[reflect(remote = "MyFoo")] + //~^ ERROR: the trait bound `MyFoo: ReflectRemote` is not satisfied + foo: external_crate::TheirFoo, +} + +fn main() {} diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/incorrect_wrapper_pass.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/incorrect_wrapper_pass.rs new file mode 100644 index 0000000000000..1b98340d8eab1 --- /dev/null +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/incorrect_wrapper_pass.rs @@ -0,0 +1,21 @@ +//@check-pass +use bevy_reflect::{reflect_remote, Reflect}; + +mod external_crate { + pub struct TheirFoo { + pub value: u32, + } +} + +#[reflect_remote(external_crate::TheirFoo)] +struct MyFoo { + pub value: u32, +} + +#[derive(Reflect)] +struct MyStruct { + #[reflect(remote = "MyFoo")] + foo: external_crate::TheirFoo, +} + +fn main() {} diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs index b7f8db0b60ae4..7790cbce55702 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs @@ -41,10 +41,10 @@ mod mismatched_remote_type { use bevy_reflect::{reflect_remote, Reflect}; #[reflect_remote(super::external_crate::TheirOuter)] - //~^ ERROR: mismatched types struct MyOuter { // Reason: Should be `MyInner` #[reflect(remote = "MyOuter")] + //~^ ERROR: mismatched types pub inner: super::external_crate::TheirInner, } diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_pass.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_pass.rs index 1bff345f392a5..8794b08507bc1 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_pass.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_pass.rs @@ -1,5 +1,5 @@ //@check-pass -use bevy_reflect::{reflect_remote, Reflect}; +use bevy_reflect::{FromReflect, GetTypeRegistration, reflect_remote, Reflect, Typed}; mod external_crate { pub struct TheirOuter { @@ -9,7 +9,7 @@ mod external_crate { } #[reflect_remote(external_crate::TheirOuter)] -struct MyOuter { +struct MyOuter { #[reflect(remote = "MyInner")] pub inner: external_crate::TheirInner, } diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs index 73a1e1bd9c7ee..465bcb30d87e4 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs @@ -22,10 +22,10 @@ mod structs { } #[derive(Reflect)] - //~^ ERROR: mismatched types struct MyStruct { // Reason: Should use `MyFoo` #[reflect(remote = "MyBar")] + //~^ ERROR: mismatched types foo: external_crate::TheirFoo, } } @@ -46,10 +46,10 @@ mod tuple_structs { struct MyBar(pub i32); #[derive(Reflect)] - //~^ ERROR: mismatched types struct MyStruct( // Reason: Should use `MyFoo` #[reflect(remote = "MyBar")] external_crate::TheirFoo, + //~^ ERROR: mismatched types ); } @@ -80,11 +80,11 @@ mod enums { } #[derive(Reflect)] - //~^ ERROR: mismatched types enum MyStruct { Value( // Reason: Should use `MyFoo` #[reflect(remote = "MyBar")] external_crate::TheirFoo, + //~^ ERROR: mismatched types ), } } diff --git a/crates/bevy_reflect/derive/src/derive_data.rs b/crates/bevy_reflect/derive/src/derive_data.rs index 509e4d0601c2e..d856b438c5dc6 100644 --- a/crates/bevy_reflect/derive/src/derive_data.rs +++ b/crates/bevy_reflect/derive/src/derive_data.rs @@ -394,6 +394,16 @@ impl<'a> ReflectDerive<'a> { } } + pub fn where_clause_options(&self) -> WhereClauseOptions { + match self { + Self::Struct(data) | Self::TupleStruct(data) | Self::UnitStruct(data) => { + data.where_clause_options() + } + Self::Enum(data) => data.where_clause_options(), + Self::Value(meta) => WhereClauseOptions::new(meta), + } + } + fn collect_struct_fields(fields: &'a Fields) -> Result>, syn::Error> { let mut active_index = 0; let sifter: utility::ResultSifter> = fields @@ -1206,6 +1216,28 @@ impl<'a> ReflectTypePath<'a> { pub fn type_ident(&self) -> Option { self.get_ident().map(StringExpr::from) } + + /// Returns the true type regardless of whether a custom path is specified. + /// + /// To get the custom path if there is one, use [`Self::get_path`]. + /// + /// For example, the type `Foo` would return `Foo`. + pub fn true_type(&self) -> proc_macro2::TokenStream { + match self { + Self::Primitive(ident) => quote!(#ident), + Self::Internal { + ident, generics, .. + } => { + let (_, ty_generics, _) = generics.split_for_impl(); + quote!(#ident #ty_generics) + } + Self::External { path, generics, .. } => { + let (_, ty_generics, _) = generics.split_for_impl(); + quote!(#path #ty_generics) + } + Self::Anonymous { qualified_type, .. } => qualified_type.to_token_stream(), + } + } } impl<'a> ToTokens for ReflectTypePath<'a> { diff --git a/crates/bevy_reflect/derive/src/from_reflect.rs b/crates/bevy_reflect/derive/src/from_reflect.rs index a7e1dfcf562a4..83b1802151385 100644 --- a/crates/bevy_reflect/derive/src/from_reflect.rs +++ b/crates/bevy_reflect/derive/src/from_reflect.rs @@ -115,7 +115,7 @@ fn impl_struct_internal( let MemberValuePair(active_members, active_values) = get_active_fields(reflect_struct, &ref_struct, &ref_struct_type, is_tuple); - + let is_defaultable = reflect_struct.meta().attrs().contains(REFLECT_DEFAULT); // The constructed "Self" ident diff --git a/crates/bevy_reflect/derive/src/remote.rs b/crates/bevy_reflect/derive/src/remote.rs index a9c51812ca77c..01e9d4ac1fa92 100644 --- a/crates/bevy_reflect/derive/src/remote.rs +++ b/crates/bevy_reflect/derive/src/remote.rs @@ -41,6 +41,8 @@ pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStre let assertions = impl_assertions(&derive_data); let definition_assertions = generate_remote_definition_assertions(&derive_data); + let reflect_remote_impl = impl_reflect_remote(&derive_data, &remote_ty); + let (reflect_impls, from_reflect_impl) = match derive_data { ReflectDerive::Struct(struct_data) | ReflectDerive::UnitStruct(struct_data) => ( impls::impl_struct(&struct_data), @@ -77,6 +79,8 @@ pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStre #wrapper_definition const _: () = { + #reflect_remote_impl + #reflect_impls #from_reflect_impl @@ -117,6 +121,25 @@ fn generate_remote_wrapper(input: &DeriveInput, remote_ty: &TypePath) -> proc_ma } } +fn impl_reflect_remote(input: &ReflectDerive, remote_ty: &TypePath) -> proc_macro2::TokenStream { + let bevy_reflect_path = input.meta().bevy_reflect_path(); + + let type_path = input.meta().type_path(); + let (impl_generics, ty_generics, where_clause) = + input.meta().type_path().generics().split_for_impl(); + + let where_reflect_clause = input + .where_clause_options() + .extend_where_clause(where_clause); + + quote! { + // SAFE: The generated wrapper type is guaranteed to be valid and repr(transparent) over the remote type. + unsafe impl #impl_generics #bevy_reflect_path::ReflectRemote for #type_path #ty_generics #where_reflect_clause { + type Remote = #remote_ty; + } + } +} + /// Generates compile-time assertions for remote fields. /// /// This prevents types from using an invalid remote type. @@ -156,6 +179,8 @@ pub(crate) fn generate_remote_assertions( remote_ty: &'a Type, } + let bevy_reflect_path = derive_data.meta().bevy_reflect_path(); + let fields: Box> = match derive_data { ReflectDerive::Struct(data) | ReflectDerive::TupleStruct(data) @@ -203,14 +228,22 @@ pub(crate) fn generate_remote_assertions( }; let (impl_generics, _, where_clause) = field.generics.split_for_impl(); + let where_reflect_clause = derive_data + .where_clause_options() + .extend_where_clause(where_clause); + let ty = &field.ty; let remote_ty = field.remote_ty; let assertion_ident = format_ident!("assert__{}__is_valid_remote", ident); - quote! { + let span = create_assertion_span(remote_ty.span()); + + quote_spanned! {span=> #[allow(non_snake_case)] - fn #assertion_ident #impl_generics (#ident: #remote_ty) #where_clause { - let _: #ty = #ident.0; + fn #assertion_ident #impl_generics () #where_reflect_clause { + let _: <#remote_ty as #bevy_reflect_path::ReflectRemote>::Remote = (|| -> #FQOption<#ty> { + None + })().unwrap(); } } }) @@ -248,13 +281,9 @@ fn generate_remote_definition_assertions(derive_data: &ReflectDerive) -> proc_ma let self_expr_path = derive_data.remote_ty().unwrap().as_expr_path().unwrap(); let (impl_generics, _, where_clause) = meta.type_path().generics().split_for_impl(); - /// Creates a span located around the given one, but resolves to the assertion's context. - /// - /// This should allow the compiler to point back to the line and column in the user's code, - /// while still attributing the error to the `reflect_remote` macro. - fn create_assertion_span(span: Span) -> Span { - Span::call_site().located_at(span) - } + let where_reflect_clause = derive_data + .where_clause_options() + .extend_where_clause(where_clause); let assertions = match derive_data { ReflectDerive::Struct(data) @@ -323,13 +352,21 @@ fn generate_remote_definition_assertions(derive_data: &ReflectDerive) -> proc_ma #[allow(unused_variables)] #[allow(unused_assignments)] #[allow(unreachable_patterns)] - fn assert_wrapper_definition_matches_remote_type #impl_generics (mut #self_ident: #self_ty) #where_clause { + fn assert_wrapper_definition_matches_remote_type #impl_generics (mut #self_ident: #self_ty) #where_reflect_clause { #assertions } }; } } +/// Creates a span located around the given one, but resolves to the assertion's context. +/// +/// This should allow the compiler to point back to the line and column in the user's code, +/// while still attributing the error to the macro. +fn create_assertion_span(span: Span) -> Span { + Span::call_site().located_at(span) +} + /// A reflected type's remote type. /// /// This is a wrapper around [`TypePath`] that allows it to be paired with other remote-specific logic. diff --git a/crates/bevy_reflect/derive/src/utility.rs b/crates/bevy_reflect/derive/src/utility.rs index 0670c0fe012e2..231dc18f2bbbc 100644 --- a/crates/bevy_reflect/derive/src/utility.rs +++ b/crates/bevy_reflect/derive/src/utility.rs @@ -154,17 +154,18 @@ impl<'a, 'b> WhereClauseOptions<'a, 'b> { &self, where_clause: Option<&WhereClause>, ) -> proc_macro2::TokenStream { - let type_path = self.meta.type_path(); - let (_, ty_generics, _) = self.meta.type_path().generics().split_for_impl(); + // We would normally just use `Self`, but that won't work for generating things like assertion functions + // and trait impls for a type's reference (e.g. `impl FromArg for &MyType`) + let this = self.meta.type_path().true_type(); let required_bounds = self.required_bounds(); // Maintain existing where clause, if any. let mut generic_where_clause = if let Some(where_clause) = where_clause { let predicates = where_clause.predicates.iter(); - quote! {where #type_path #ty_generics: #required_bounds, #(#predicates,)*} + quote! {where #this: #required_bounds, #(#predicates,)*} } else { - quote!(where #type_path #ty_generics: #required_bounds,) + quote!(where #this: #required_bounds,) }; // Add additional reflection trait bounds diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index e5b52f90390e3..88077fe1d92c7 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -532,6 +532,7 @@ mod list; mod map; mod path; mod reflect; +mod remote; mod set; mod struct_trait; mod tuple; @@ -582,6 +583,7 @@ pub use list::*; pub use map::*; pub use path::*; pub use reflect::*; +pub use remote::*; pub use set::*; pub use struct_trait::*; pub use tuple::*; @@ -2626,11 +2628,12 @@ bevy_reflect::tests::Test { pub a: TheirInner, pub b: TheirInner, } + pub struct TheirInner(pub T); } #[reflect_remote(external_crate::TheirOuter)] - struct MyOuter { + struct MyOuter { #[reflect(remote = "MyInner")] pub a: external_crate::TheirInner, #[reflect(remote = "MyInner")] @@ -2678,7 +2681,7 @@ bevy_reflect::tests::Test { #[reflect_remote(external_crate::TheirOuter)] #[derive(Debug)] - enum MyOuter { + enum MyOuter { Unit, Tuple(#[reflect(remote = "MyInner")] external_crate::TheirInner), Struct { @@ -2791,7 +2794,7 @@ bevy_reflect::tests::Test { } #[reflect_remote(external_crate::TheirOuter)] - struct MyOuter { + struct MyOuter { #[reflect(remote = "MyInner")] pub inner: external_crate::TheirInner, } diff --git a/crates/bevy_reflect/src/remote.rs b/crates/bevy_reflect/src/remote.rs new file mode 100644 index 0000000000000..79de568bfd54e --- /dev/null +++ b/crates/bevy_reflect/src/remote.rs @@ -0,0 +1,51 @@ +use crate::Reflect; + +/// Marks a type as a [reflectable] wrapper for a remote type. +/// +/// This allows types from external libraries (remote types) to be included in reflection. +/// +/// # Safety +/// +/// Remote reflection uses [`transmute`] internally, which is [very unsafe]. +/// To ensure proper safety, it is recommended that this trait not be manually implemented. +/// Instead, use the [`#[reflect_remote]`](crate::reflect_remote) attribute macro. +/// +/// The macro will ensure that the following safety requirements are met: +/// - `Self` is a single-field tuple struct (i.e. a newtype). +/// - `Self` is `#[repr(transparent)]` over the remote type. +/// +/// Additionally, the macro will generate [`Reflect`] and [`FromReflect`] implementations +/// that make safe use of `transmute`. +/// +/// # Example +/// +/// ``` +/// use bevy_reflect_derive::{reflect_remote, Reflect}; +/// +/// mod some_lib { +/// pub struct TheirType { +/// pub value: u32 +/// } +/// } +/// +/// #[reflect_remote(some_lib::TheirType)] +/// struct MyType { +/// pub value: u32 +/// } +/// +/// #[derive(Reflect)] +/// struct MyStruct { +/// #[reflect(remote = "MyType")] +/// data: some_lib::TheirType, +/// } +/// ``` +/// +/// [reflectable]: Reflect +/// [`transmute`]: core::mem::transmute +/// [highly unsafe]: https://doc.rust-lang.org/nomicon/transmutes.html +/// [`FromReflect`]: crate::FromReflect +#[allow(unsafe_code)] +pub unsafe trait ReflectRemote: Reflect { + /// The remote type this type represents via reflection. + type Remote; +} From 7476757947fbbc005c3fad06cf09c6d2f60ed682 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Thu, 13 Jul 2023 17:54:18 -0700 Subject: [PATCH 18/31] Move transmutations into ReflectRemote --- .../bevy_reflect/derive/src/enum_utility.rs | 18 ++------ crates/bevy_reflect/derive/src/impls/enums.rs | 25 ++++++----- .../bevy_reflect/derive/src/impls/structs.rs | 4 +- .../derive/src/impls/tuple_structs.rs | 4 +- crates/bevy_reflect/derive/src/remote.rs | 45 ++++++++++++++++++- .../bevy_reflect/derive/src/struct_utility.rs | 23 ++-------- crates/bevy_reflect/src/remote.rs | 31 +++++++++---- 7 files changed, 91 insertions(+), 59 deletions(-) diff --git a/crates/bevy_reflect/derive/src/enum_utility.rs b/crates/bevy_reflect/derive/src/enum_utility.rs index 16a8d0ed350e4..a279a82237500 100644 --- a/crates/bevy_reflect/derive/src/enum_utility.rs +++ b/crates/bevy_reflect/derive/src/enum_utility.rs @@ -79,9 +79,11 @@ pub(crate) trait VariantBuilder: Sized { /// * `this`: The identifier of the enum /// * `field`: The field to access fn on_active_field(&self, this: &Ident, field: VariantField) -> TokenStream { + let bevy_reflect_path = self.reflect_enum().meta().bevy_reflect_path(); let field_accessor = self.access_field(this, field); let alias = field.alias; + let field_ty = field.field.reflected_type(); let field_constructor = self.construct_field(field); let construction = match &field.field.attrs.default { @@ -112,20 +114,8 @@ pub(crate) trait VariantBuilder: Sized { }; if field.field.attrs().remote.is_some() { - if field.field.attrs().is_remote_generic().unwrap_or_default() { - quote! { - // SAFE: The wrapper type should be repr(transparent) over the remote type - unsafe { - ::core::mem::transmute_copy(&#construction) - } - } - } else { - quote! { - // SAFE: The wrapper type should be repr(transparent) over the remote type - unsafe { - ::core::mem::transmute(#construction) - } - } + quote! { + <#field_ty as #bevy_reflect_path::ReflectRemote>::into_remote(#construction) } } else { construction diff --git a/crates/bevy_reflect/derive/src/impls/enums.rs b/crates/bevy_reflect/derive/src/impls/enums.rs index bc39ba76707b0..4516a7d01ff83 100644 --- a/crates/bevy_reflect/derive/src/impls/enums.rs +++ b/crates/bevy_reflect/derive/src/impls/enums.rs @@ -4,7 +4,7 @@ use crate::impls::{common_partial_reflect_methods, impl_full_reflect, impl_type_ use bevy_macro_utils::fq_std::{FQBox, FQOption, FQResult}; use proc_macro2::{Ident, Span}; use quote::quote; -use syn::Fields; +use syn::{Fields, Path}; pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream { let bevy_reflect_path = reflect_enum.meta().bevy_reflect_path(); @@ -341,18 +341,19 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden ident: &Ident, field: &StructField, is_mutable: bool, + bevy_reflect_path: &Path, ) -> proc_macro2::TokenStream { - let ref_token = if is_mutable { quote!(&mut) } else { quote!(&) }; + let method = if is_mutable { + quote!(as_wrapper_mut) + } else { + quote!(as_wrapper) + }; + field .attrs .remote .as_ref() - .map(|ty| { - quote! { - // SAFE: The wrapper type should be repr(transparent) over the remote type - unsafe { ::core::mem::transmute::<#ref_token _, #ref_token #ty>(#ident) } - } - }) + .map(|ty| quote!(<#ty as #bevy_reflect_path::ReflectRemote>::#method(#ident))) .unwrap_or_else(|| quote!(#ident)) } @@ -373,8 +374,8 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden let declare_field = syn::Index::from(field.declaration_index); let __value = Ident::new("__value", Span::call_site()); - let value_ref = process_field_value(&__value, field, false); - let value_mut = process_field_value(&__value, field, true); + let value_ref = process_field_value(&__value, field, false, bevy_reflect_path); + let value_mut = process_field_value(&__value, field, true, bevy_reflect_path); enum_field_at.push(quote! { #unit { #declare_field : #__value, .. } if #ref_index == #reflection_index => #FQOption::Some(#value_ref) @@ -397,8 +398,8 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden .expect("reflection index should exist for active field"); let __value = Ident::new("__value", Span::call_site()); - let value_ref = process_field_value(&__value, field, false); - let value_mut = process_field_value(&__value, field, true); + let value_ref = process_field_value(&__value, field, false, bevy_reflect_path); + let value_mut = process_field_value(&__value, field, true, bevy_reflect_path); enum_field.push(quote! { #unit{ #field_ident: #__value, .. } if #ref_name == #field_name => #FQOption::Some(#value_ref) diff --git a/crates/bevy_reflect/derive/src/impls/structs.rs b/crates/bevy_reflect/derive/src/impls/structs.rs index 42b1daee69001..b4b0334d0f0d5 100644 --- a/crates/bevy_reflect/derive/src/impls/structs.rs +++ b/crates/bevy_reflect/derive/src/impls/structs.rs @@ -24,11 +24,11 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> proc_macro2::TokenS .collect::>(); let FieldAccessors { - fields, fields_ref, fields_mut, field_indices, field_count, + .. } = FieldAccessors::new(reflect_struct); let where_clause_options = reflect_struct.where_clause_options(); @@ -124,7 +124,7 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> proc_macro2::TokenS fn clone_dynamic(&self) -> #bevy_reflect_path::DynamicStruct { let mut dynamic: #bevy_reflect_path::DynamicStruct = #FQDefault::default(); dynamic.set_represented_type(#bevy_reflect_path::PartialReflect::get_represented_type_info(self)); - #(dynamic.insert_boxed(#field_names, #bevy_reflect_path::PartialReflect::clone_value(&#fields));)* + #(dynamic.insert_boxed(#field_names, #bevy_reflect_path::PartialReflect::clone_value(#fields_ref));)* dynamic } } diff --git a/crates/bevy_reflect/derive/src/impls/tuple_structs.rs b/crates/bevy_reflect/derive/src/impls/tuple_structs.rs index 176e53c722f10..44d8fadc5368e 100644 --- a/crates/bevy_reflect/derive/src/impls/tuple_structs.rs +++ b/crates/bevy_reflect/derive/src/impls/tuple_structs.rs @@ -12,11 +12,11 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> proc_macro2:: let struct_path = reflect_struct.meta().type_path(); let FieldAccessors { - fields, fields_ref, fields_mut, field_indices, field_count, + .. } = FieldAccessors::new(reflect_struct); let where_clause_options = reflect_struct.where_clause_options(); @@ -91,7 +91,7 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> proc_macro2:: fn clone_dynamic(&self) -> #bevy_reflect_path::DynamicTupleStruct { let mut dynamic: #bevy_reflect_path::DynamicTupleStruct = #FQDefault::default(); dynamic.set_represented_type(#bevy_reflect_path::PartialReflect::get_represented_type_info(self)); - #(dynamic.insert_boxed(#bevy_reflect_path::PartialReflect::clone_value(&#fields));)* + #(dynamic.insert_boxed(#bevy_reflect_path::PartialReflect::clone_value(#fields_ref));)* dynamic } } diff --git a/crates/bevy_reflect/derive/src/remote.rs b/crates/bevy_reflect/derive/src/remote.rs index 01e9d4ac1fa92..53d5f59b9b0eb 100644 --- a/crates/bevy_reflect/derive/src/remote.rs +++ b/crates/bevy_reflect/derive/src/remote.rs @@ -121,6 +121,13 @@ fn generate_remote_wrapper(input: &DeriveInput, remote_ty: &TypePath) -> proc_ma } } +/// Generates the implementation of the `ReflectRemote` trait for the given derive data and remote type. +/// +/// # Note +/// +/// The `ReflectRemote` trait could likely be made with default method implementations. +/// However, this makes it really easy for a user to accidentally implement this trait in an unsafe way. +/// To prevent this, we instead generate the implementation through a macro using this function. fn impl_reflect_remote(input: &ReflectDerive, remote_ty: &TypePath) -> proc_macro2::TokenStream { let bevy_reflect_path = input.meta().bevy_reflect_path(); @@ -134,8 +141,44 @@ fn impl_reflect_remote(input: &ReflectDerive, remote_ty: &TypePath) -> proc_macr quote! { // SAFE: The generated wrapper type is guaranteed to be valid and repr(transparent) over the remote type. - unsafe impl #impl_generics #bevy_reflect_path::ReflectRemote for #type_path #ty_generics #where_reflect_clause { + impl #impl_generics #bevy_reflect_path::ReflectRemote for #type_path #ty_generics #where_reflect_clause { type Remote = #remote_ty; + + fn as_remote(&self) -> &Self::Remote { + &self.0 + } + fn as_remote_mut(&mut self) -> &mut Self::Remote { + &mut self.0 + } + fn into_remote(self) -> Self::Remote + { + // SAFE: The wrapper type should be repr(transparent) over the remote type + unsafe { + ::core::mem::transmute_copy::( + // `ManuallyDrop` is used to prevent double-dropping `self` + &::core::mem::ManuallyDrop::new(self) + ) + } + } + + fn as_wrapper(remote: &Self::Remote) -> &Self { + // SAFE: The wrapper type should be repr(transparent) over the remote type + unsafe { ::core::mem::transmute::<&Self::Remote, &Self>(remote) } + } + fn as_wrapper_mut(remote: &mut Self::Remote) -> &mut Self { + // SAFE: The wrapper type should be repr(transparent) over the remote type + unsafe { ::core::mem::transmute::<&mut Self::Remote, &mut Self>(remote) } + } + fn into_wrapper(remote: Self::Remote) -> Self + { + // SAFE: The wrapper type should be repr(transparent) over the remote type + unsafe { + ::core::mem::transmute_copy::( + // `ManuallyDrop` is used to prevent double-dropping `self` + &::core::mem::ManuallyDrop::new(remote) + ) + } + } } } } diff --git a/crates/bevy_reflect/derive/src/struct_utility.rs b/crates/bevy_reflect/derive/src/struct_utility.rs index 0875be6c0150d..4b492393610fd 100644 --- a/crates/bevy_reflect/derive/src/struct_utility.rs +++ b/crates/bevy_reflect/derive/src/struct_utility.rs @@ -9,8 +9,6 @@ use quote::quote; /// /// [`transmute`]: core::mem::transmute pub(crate) struct FieldAccessors { - /// The owned field accessors, such as `self.foo`. - pub fields: Vec, /// The referenced field accessors, such as `&self.foo`. pub fields_ref: Vec, /// The mutably referenced field accessors, such as `&mut self.foo`. @@ -23,23 +21,12 @@ pub(crate) struct FieldAccessors { impl FieldAccessors { pub fn new(reflect_struct: &ReflectStruct) -> Self { - let fields = Self::get_fields(reflect_struct, |field, accessor| { - match &field.attrs.remote { - Some(wrapper_ty) => { - quote! { - // SAFE: The wrapper type should be repr(transparent) over the remote type - unsafe { ::core::mem::transmute_copy::<_, #wrapper_ty>(&#accessor) } - } - } - None => accessor, - } - }); + let bevy_reflect_path = reflect_struct.meta().bevy_reflect_path(); let fields_ref = Self::get_fields(reflect_struct, |field, accessor| { match &field.attrs.remote { Some(wrapper_ty) => { quote! { - // SAFE: The wrapper type should be repr(transparent) over the remote type - unsafe { ::core::mem::transmute::<&_, &#wrapper_ty>(&#accessor) } + <#wrapper_ty as #bevy_reflect_path::ReflectRemote>::as_wrapper(&#accessor) } } None => quote!(& #accessor), @@ -49,19 +36,17 @@ impl FieldAccessors { match &field.attrs.remote { Some(wrapper_ty) => { quote! { - // SAFE: The wrapper type should be repr(transparent) over the remote type - unsafe { ::core::mem::transmute::<&mut _, &mut #wrapper_ty>(&mut #accessor) } + <#wrapper_ty as #bevy_reflect_path::ReflectRemote>::as_wrapper_mut(&mut #accessor) } } None => quote!(&mut #accessor), } }); - let field_count = fields.len(); + let field_count = fields_ref.len(); let field_indices = (0..field_count).collect(); Self { - fields, fields_ref, fields_mut, field_indices, diff --git a/crates/bevy_reflect/src/remote.rs b/crates/bevy_reflect/src/remote.rs index 79de568bfd54e..47d7fe59a0d9e 100644 --- a/crates/bevy_reflect/src/remote.rs +++ b/crates/bevy_reflect/src/remote.rs @@ -6,16 +6,16 @@ use crate::Reflect; /// /// # Safety /// -/// Remote reflection uses [`transmute`] internally, which is [very unsafe]. -/// To ensure proper safety, it is recommended that this trait not be manually implemented. -/// Instead, use the [`#[reflect_remote]`](crate::reflect_remote) attribute macro. +/// It is highly recommended to avoid implementing this trait manually and instead use the +/// [`#[reflect_remote]`](crate::reflect_remote) attribute macro. +/// This is because the trait tends to rely on [`transmute`], which is [very unsafe]. /// /// The macro will ensure that the following safety requirements are met: -/// - `Self` is a single-field tuple struct (i.e. a newtype). +/// - `Self` is a single-field tuple struct (i.e. a newtype) containing the remote type. /// - `Self` is `#[repr(transparent)]` over the remote type. /// -/// Additionally, the macro will generate [`Reflect`] and [`FromReflect`] implementations -/// that make safe use of `transmute`. +/// Additionally, the macro will automatically generate [`Reflect`] and [`FromReflect`] implementations, +/// along with compile-time assertions to validate that the safety requirements have been met. /// /// # Example /// @@ -42,10 +42,23 @@ use crate::Reflect; /// /// [reflectable]: Reflect /// [`transmute`]: core::mem::transmute -/// [highly unsafe]: https://doc.rust-lang.org/nomicon/transmutes.html +/// [very unsafe]: https://doc.rust-lang.org/nomicon/transmutes.html /// [`FromReflect`]: crate::FromReflect -#[allow(unsafe_code)] -pub unsafe trait ReflectRemote: Reflect { +pub trait ReflectRemote: Reflect { /// The remote type this type represents via reflection. type Remote; + + /// Converts a reference of this wrapper to a reference of its remote type. + fn as_remote(&self) -> &Self::Remote; + /// Converts a mutable reference of this wrapper to a mutable reference of its remote type. + fn as_remote_mut(&mut self) -> &mut Self::Remote; + /// Converts this wrapper into its remote type. + fn into_remote(self) -> Self::Remote; + + /// Converts a reference of the remote type to a reference of this wrapper. + fn as_wrapper(remote: &Self::Remote) -> &Self; + /// Converts a mutable reference of the remote type to a mutable reference of this wrapper. + fn as_wrapper_mut(remote: &mut Self::Remote) -> &mut Self; + /// Converts the remote type into this wrapper. + fn into_wrapper(remote: Self::Remote) -> Self; } From e0af6a25c62259c9a3b68d318ebe4c1ebbe7629f Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sun, 30 Jul 2023 21:47:27 -0700 Subject: [PATCH 19/31] Fix compile tests --- .../tests/reflect_remote/incorrect_wrapper_fail.rs | 1 + .../compile_fail/tests/reflect_remote/nested_fail.rs | 4 ++++ .../tests/reflect_remote/type_mismatch_fail.rs | 7 +++++++ 3 files changed, 12 insertions(+) diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/incorrect_wrapper_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/incorrect_wrapper_fail.rs index 1a3f0a2d764b4..7f4b08f39fa16 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/incorrect_wrapper_fail.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/incorrect_wrapper_fail.rs @@ -12,6 +12,7 @@ mod external_crate { struct MyFoo(#[reflect(ignore)] pub external_crate::TheirFoo); #[derive(Reflect)] +//~^ ERROR: the trait bound `MyFoo: ReflectRemote` is not satisfied #[reflect(from_reflect = false)] struct MyStruct { // Reason: `MyFoo` does not implement `ReflectRemote` (from `#[reflect_remote]` attribute) diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs index 7790cbce55702..70f6167cd3ba0 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs @@ -41,6 +41,8 @@ mod mismatched_remote_type { use bevy_reflect::{reflect_remote, Reflect}; #[reflect_remote(super::external_crate::TheirOuter)] + //~^ ERROR: mismatched types + //~| ERROR: mismatched types struct MyOuter { // Reason: Should be `MyInner` #[reflect(remote = "MyOuter")] @@ -57,6 +59,8 @@ mod mismatched_remote_generic { #[reflect_remote(super::external_crate::TheirOuter)] //~^ ERROR: `?` operator has incompatible types + //~| ERROR: mismatched types + //~| ERROR: mismatched types struct MyOuter { // Reason: `TheirOuter::inner` is not defined as `TheirInner` #[reflect(remote = "MyInner")] diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs index 465bcb30d87e4..93c57f7bfa3a8 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs @@ -22,6 +22,8 @@ mod structs { } #[derive(Reflect)] + //~^ ERROR: mismatched types + //~| ERROR: mismatched types struct MyStruct { // Reason: Should use `MyFoo` #[reflect(remote = "MyBar")] @@ -46,6 +48,8 @@ mod tuple_structs { struct MyBar(pub i32); #[derive(Reflect)] + //~^ ERROR: mismatched types + //~| ERROR: mismatched types struct MyStruct( // Reason: Should use `MyFoo` #[reflect(remote = "MyBar")] external_crate::TheirFoo, @@ -80,6 +84,9 @@ mod enums { } #[derive(Reflect)] + //~^ ERROR: mismatched types + //~| ERROR: mismatched types + //~| ERROR: mismatched types enum MyStruct { Value( // Reason: Should use `MyFoo` From 7aba2a65eaaec77bd7ff43d2e1d08d925edc2720 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sun, 30 Jul 2023 21:51:07 -0700 Subject: [PATCH 20/31] Use permalink --- crates/bevy_reflect/src/remote.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/src/remote.rs b/crates/bevy_reflect/src/remote.rs index 47d7fe59a0d9e..bdc10363c8e84 100644 --- a/crates/bevy_reflect/src/remote.rs +++ b/crates/bevy_reflect/src/remote.rs @@ -42,7 +42,7 @@ use crate::Reflect; /// /// [reflectable]: Reflect /// [`transmute`]: core::mem::transmute -/// [very unsafe]: https://doc.rust-lang.org/nomicon/transmutes.html +/// [very unsafe]: https://doc.rust-lang.org/1.71.0/nomicon/transmutes.html /// [`FromReflect`]: crate::FromReflect pub trait ReflectRemote: Reflect { /// The remote type this type represents via reflection. From cf5d5f27239a039a4403e56319faaa2eca04ba55 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Fri, 3 Nov 2023 12:20:46 -0700 Subject: [PATCH 21/31] Update comment --- crates/bevy_reflect/derive/src/remote.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/derive/src/remote.rs b/crates/bevy_reflect/derive/src/remote.rs index 53d5f59b9b0eb..4efdf075ad186 100644 --- a/crates/bevy_reflect/derive/src/remote.rs +++ b/crates/bevy_reflect/derive/src/remote.rs @@ -123,7 +123,7 @@ fn generate_remote_wrapper(input: &DeriveInput, remote_ty: &TypePath) -> proc_ma /// Generates the implementation of the `ReflectRemote` trait for the given derive data and remote type. /// -/// # Note +/// # Note to Developers /// /// The `ReflectRemote` trait could likely be made with default method implementations. /// However, this makes it really easy for a user to accidentally implement this trait in an unsafe way. From 4c6bad16a891350baf921f9a2fe33a510f874339 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Tue, 12 Mar 2024 14:42:45 -0700 Subject: [PATCH 22/31] Fix compile fail tests --- .../tests/reflect_remote/nested_fail.rs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs index 70f6167cd3ba0..eaab9a3d972cc 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs @@ -6,20 +6,20 @@ mod external_crate { } mod missing_attribute { - use bevy_reflect::{reflect_remote, Reflect}; + use bevy_reflect::{FromReflect, GetTypeRegistration, reflect_remote}; #[reflect_remote(super::external_crate::TheirOuter)] - struct MyOuter { + struct MyOuter { // Reason: Missing `#[reflect(remote = "...")]` attribute pub inner: super::external_crate::TheirInner, } #[reflect_remote(super::external_crate::TheirInner)] - struct MyInner(pub T); + struct MyInner(pub T); } mod incorrect_inner_type { - use bevy_reflect::{reflect_remote, Reflect}; + use bevy_reflect::{FromReflect, GetTypeRegistration, reflect_remote}; #[reflect_remote(super::external_crate::TheirOuter)] //~^ ERROR: `TheirInner` can not be reflected @@ -27,23 +27,23 @@ mod incorrect_inner_type { //~| ERROR: `TheirInner` can not be reflected //~| ERROR: `TheirInner` can not be used as a dynamic type path //~| ERROR: `?` operator has incompatible types - struct MyOuter { + struct MyOuter { // Reason: Should not use `MyInner` directly pub inner: MyInner, //~^ ERROR: mismatched types } #[reflect_remote(super::external_crate::TheirInner)] - struct MyInner(pub T); + struct MyInner(pub T); } mod mismatched_remote_type { - use bevy_reflect::{reflect_remote, Reflect}; + use bevy_reflect::{FromReflect, GetTypeRegistration, reflect_remote}; #[reflect_remote(super::external_crate::TheirOuter)] //~^ ERROR: mismatched types //~| ERROR: mismatched types - struct MyOuter { + struct MyOuter { // Reason: Should be `MyInner` #[reflect(remote = "MyOuter")] //~^ ERROR: mismatched types @@ -51,17 +51,17 @@ mod mismatched_remote_type { } #[reflect_remote(super::external_crate::TheirInner)] - struct MyInner(pub T); + struct MyInner(pub T); } mod mismatched_remote_generic { - use bevy_reflect::{reflect_remote, Reflect}; + use bevy_reflect::{FromReflect, GetTypeRegistration, reflect_remote}; #[reflect_remote(super::external_crate::TheirOuter)] //~^ ERROR: `?` operator has incompatible types //~| ERROR: mismatched types //~| ERROR: mismatched types - struct MyOuter { + struct MyOuter { // Reason: `TheirOuter::inner` is not defined as `TheirInner` #[reflect(remote = "MyInner")] pub inner: super::external_crate::TheirInner, @@ -69,7 +69,7 @@ mod mismatched_remote_generic { } #[reflect_remote(super::external_crate::TheirInner)] - struct MyInner(pub T); + struct MyInner(pub T); } fn main() {} From 57ebad322dd740eaf32d9a6da86ebfa97c371e6c Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sun, 11 Aug 2024 19:51:10 -0700 Subject: [PATCH 23/31] Remove outdated doc --- crates/bevy_reflect/derive/src/lib.rs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/crates/bevy_reflect/derive/src/lib.rs b/crates/bevy_reflect/derive/src/lib.rs index c4f1d98150a14..a840885c07cab 100644 --- a/crates/bevy_reflect/derive/src/lib.rs +++ b/crates/bevy_reflect/derive/src/lib.rs @@ -597,22 +597,6 @@ pub fn reflect_trait(args: TokenStream, input: TokenStream) -> TokenStream { /// Internally, this field will be unsafely [transmuted], and is only sound if using a wrapper generated for the remote type. /// This also means keeping your wrapper definitions up-to-date with the remote types. /// -/// # `FromReflect` -/// -/// Because of the way this code modifies the item it's defined on, it is not possible to implement `FromReflect` -/// using a simple derive macro. -/// Instead, you will need to opt-in to to it by adding `FromReflect` -/// (just the identifier, no need to import or qualify the actual type) -/// as the last item in the attribute's argument list: -/// -/// ```ignore -/// #[reflect_remote(foo::Foo, FromReflect)] -/// struct FooWrapper; -/// ``` -/// -/// This is the _only_ trait this works with. You cannot derive any other traits using this method. -/// For those, use regular derive macros below this one. -/// /// [transmuted]: std::mem::transmute #[proc_macro_attribute] pub fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStream { From 957160783fed969dfa30784ea1517243324d610c Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sun, 11 Aug 2024 19:55:04 -0700 Subject: [PATCH 24/31] Change remote field attribute to take type path directly --- .../reflect_remote/incorrect_wrapper_fail.rs | 2 +- .../reflect_remote/incorrect_wrapper_pass.rs | 2 +- .../tests/reflect_remote/nested_fail.rs | 6 +++--- .../tests/reflect_remote/nested_pass.rs | 2 +- .../tests/reflect_remote/type_mismatch_fail.rs | 6 +++--- .../tests/reflect_remote/type_mismatch_pass.rs | 6 +++--- .../bevy_reflect/derive/src/field_attributes.rs | 5 ++--- crates/bevy_reflect/derive/src/lib.rs | 6 +++--- crates/bevy_reflect/derive/src/remote.rs | 4 ++-- crates/bevy_reflect/src/lib.rs | 16 ++++++++-------- crates/bevy_reflect/src/remote.rs | 2 +- 11 files changed, 28 insertions(+), 29 deletions(-) diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/incorrect_wrapper_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/incorrect_wrapper_fail.rs index 7f4b08f39fa16..f25f3fd740f0c 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/incorrect_wrapper_fail.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/incorrect_wrapper_fail.rs @@ -16,7 +16,7 @@ struct MyFoo(#[reflect(ignore)] pub external_crate::TheirFoo); #[reflect(from_reflect = false)] struct MyStruct { // Reason: `MyFoo` does not implement `ReflectRemote` (from `#[reflect_remote]` attribute) - #[reflect(remote = "MyFoo")] + #[reflect(remote = MyFoo)] //~^ ERROR: the trait bound `MyFoo: ReflectRemote` is not satisfied foo: external_crate::TheirFoo, } diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/incorrect_wrapper_pass.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/incorrect_wrapper_pass.rs index 1b98340d8eab1..798511a85aea2 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/incorrect_wrapper_pass.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/incorrect_wrapper_pass.rs @@ -14,7 +14,7 @@ struct MyFoo { #[derive(Reflect)] struct MyStruct { - #[reflect(remote = "MyFoo")] + #[reflect(remote = MyFoo)] foo: external_crate::TheirFoo, } diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs index eaab9a3d972cc..c699af78e9e0a 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs @@ -10,7 +10,7 @@ mod missing_attribute { #[reflect_remote(super::external_crate::TheirOuter)] struct MyOuter { - // Reason: Missing `#[reflect(remote = "...")]` attribute + // Reason: Missing `#[reflect(remote = ...)]` attribute pub inner: super::external_crate::TheirInner, } @@ -45,7 +45,7 @@ mod mismatched_remote_type { //~| ERROR: mismatched types struct MyOuter { // Reason: Should be `MyInner` - #[reflect(remote = "MyOuter")] + #[reflect(remote = MyOuter)] //~^ ERROR: mismatched types pub inner: super::external_crate::TheirInner, } @@ -63,7 +63,7 @@ mod mismatched_remote_generic { //~| ERROR: mismatched types struct MyOuter { // Reason: `TheirOuter::inner` is not defined as `TheirInner` - #[reflect(remote = "MyInner")] + #[reflect(remote = MyInner)] pub inner: super::external_crate::TheirInner, //~^ ERROR: mismatched types } diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_pass.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_pass.rs index 8794b08507bc1..cd79ede57ab5d 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_pass.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_pass.rs @@ -10,7 +10,7 @@ mod external_crate { #[reflect_remote(external_crate::TheirOuter)] struct MyOuter { - #[reflect(remote = "MyInner")] + #[reflect(remote = MyInner)] pub inner: external_crate::TheirInner, } diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs index 93c57f7bfa3a8..1983442ab7d3a 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs @@ -26,7 +26,7 @@ mod structs { //~| ERROR: mismatched types struct MyStruct { // Reason: Should use `MyFoo` - #[reflect(remote = "MyBar")] + #[reflect(remote = MyBar)] //~^ ERROR: mismatched types foo: external_crate::TheirFoo, } @@ -52,7 +52,7 @@ mod tuple_structs { //~| ERROR: mismatched types struct MyStruct( // Reason: Should use `MyFoo` - #[reflect(remote = "MyBar")] external_crate::TheirFoo, + #[reflect(remote = MyBar)] external_crate::TheirFoo, //~^ ERROR: mismatched types ); } @@ -90,7 +90,7 @@ mod enums { enum MyStruct { Value( // Reason: Should use `MyFoo` - #[reflect(remote = "MyBar")] external_crate::TheirFoo, + #[reflect(remote = MyBar)] external_crate::TheirFoo, //~^ ERROR: mismatched types ), } diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_pass.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_pass.rs index 19707c11c0d9a..78f9d52c31526 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_pass.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_pass.rs @@ -23,7 +23,7 @@ mod structs { #[derive(Reflect)] struct MyStruct { - #[reflect(remote = "MyFoo")] + #[reflect(remote = MyFoo)] foo: external_crate::TheirFoo, } } @@ -44,7 +44,7 @@ mod tuple_structs { struct MyBar(pub i32); #[derive(Reflect)] - struct MyStruct(#[reflect(remote = "MyFoo")] external_crate::TheirFoo); + struct MyStruct(#[reflect(remote = MyFoo)] external_crate::TheirFoo); } mod enums { @@ -72,7 +72,7 @@ mod enums { #[derive(Reflect)] enum MyStruct { - Value(#[reflect(remote = "MyFoo")] external_crate::TheirFoo), + Value(#[reflect(remote = MyFoo)] external_crate::TheirFoo), } } diff --git a/crates/bevy_reflect/derive/src/field_attributes.rs b/crates/bevy_reflect/derive/src/field_attributes.rs index c496c74257e05..5228f80a67cdf 100644 --- a/crates/bevy_reflect/derive/src/field_attributes.rs +++ b/crates/bevy_reflect/derive/src/field_attributes.rs @@ -200,7 +200,7 @@ impl FieldAttributes { /// Parse `remote` attribute. /// /// Examples: - /// - `#[reflect(remote = "path::to::RemoteType")]` + /// - `#[reflect(remote = path::to::RemoteType)]` fn parse_remote(&mut self, input: ParseStream) -> syn::Result<()> { if let Some(remote) = self.remote.as_ref() { return Err(input.error(format!( @@ -212,8 +212,7 @@ impl FieldAttributes { input.parse::()?; input.parse::()?; - let lit = input.parse::()?; - self.remote = Some(lit.parse()?); + self.remote = Some(input.parse()?); Ok(()) } diff --git a/crates/bevy_reflect/derive/src/lib.rs b/crates/bevy_reflect/derive/src/lib.rs index a840885c07cab..2fdb055b60040 100644 --- a/crates/bevy_reflect/derive/src/lib.rs +++ b/crates/bevy_reflect/derive/src/lib.rs @@ -581,19 +581,19 @@ pub fn reflect_trait(args: TokenStream, input: TokenStream) -> TokenStream { /// /// You can tell `Reflect` to use a remote type's wrapper internally on fields of a struct or enum. /// This allows the real type to be used as usual while `Reflect` handles everything internally. -/// To do this, add the `#[reflect(remote = "...")]` attribute to your field: +/// To do this, add the `#[reflect(remote = path::to::MyType)]` attribute to your field: /// /// ```ignore /// #[derive(Reflect)] /// struct SomeStruct { -/// #[reflect(remote = "RemoteTypeWrapper")] +/// #[reflect(remote = RemoteTypeWrapper)] /// data: RemoteType /// } /// ``` /// /// ## Safety /// -/// When using the `#[reflect(remote = "...")]` field attribute, be sure you are defining the correct wrapper type. +/// When using the `#[reflect(remote = path::to::MyType)]` field attribute, be sure you are defining the correct wrapper type. /// Internally, this field will be unsafely [transmuted], and is only sound if using a wrapper generated for the remote type. /// This also means keeping your wrapper definitions up-to-date with the remote types. /// diff --git a/crates/bevy_reflect/derive/src/remote.rs b/crates/bevy_reflect/derive/src/remote.rs index 4efdf075ad186..35eee73ed4bf1 100644 --- a/crates/bevy_reflect/derive/src/remote.rs +++ b/crates/bevy_reflect/derive/src/remote.rs @@ -192,7 +192,7 @@ fn impl_reflect_remote(input: &ReflectDerive, remote_ty: &TypePath) -> proc_macr /// /// # Example /// -/// The following would fail to compile due to an incorrect `#[reflect(remote = "...")]` value. +/// The following would fail to compile due to an incorrect `#[reflect(remote = ...)]` value. /// /// ```ignore /// mod external_crate { @@ -207,7 +207,7 @@ fn impl_reflect_remote(input: &ReflectDerive, remote_ty: &TypePath) -> proc_macr /// /// #[derive(Reflect)] /// struct MyStruct { -/// #[reflect(remote = "MyBar")] // ERROR: expected type `TheirFoo` but found struct `TheirBar` +/// #[reflect(remote = MyBar)] // ERROR: expected type `TheirFoo` but found struct `TheirBar` /// foo: external_crate::TheirFoo /// } /// ``` diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 88077fe1d92c7..76bfc6f193a86 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -2512,7 +2512,7 @@ bevy_reflect::tests::Test { #[derive(Reflect, Debug)] #[reflect(from_reflect = false)] struct ContainerStruct { - #[reflect(remote = "MyType")] + #[reflect(remote = MyType)] their_type: external_crate::TheirType, } @@ -2537,7 +2537,7 @@ bevy_reflect::tests::Test { // === Tuple Struct Container === // #[derive(Reflect, Debug)] - struct ContainerTupleStruct(#[reflect(remote = "MyType")] external_crate::TheirType); + struct ContainerTupleStruct(#[reflect(remote = MyType)] external_crate::TheirType); let mut patch = DynamicTupleStruct::default(); patch.set_represented_type(Some(ContainerTupleStruct::type_info())); @@ -2600,7 +2600,7 @@ bevy_reflect::tests::Test { enum ContainerEnum { Foo, Bar { - #[reflect(remote = "MyType")] + #[reflect(remote = MyType)] their_type: external_crate::TheirType, }, } @@ -2634,9 +2634,9 @@ bevy_reflect::tests::Test { #[reflect_remote(external_crate::TheirOuter)] struct MyOuter { - #[reflect(remote = "MyInner")] + #[reflect(remote = MyInner)] pub a: external_crate::TheirInner, - #[reflect(remote = "MyInner")] + #[reflect(remote = MyInner)] pub b: external_crate::TheirInner, } @@ -2683,9 +2683,9 @@ bevy_reflect::tests::Test { #[derive(Debug)] enum MyOuter { Unit, - Tuple(#[reflect(remote = "MyInner")] external_crate::TheirInner), + Tuple(#[reflect(remote = MyInner)] external_crate::TheirInner), Struct { - #[reflect(remote = "MyInner")] + #[reflect(remote = MyInner)] value: external_crate::TheirInner, }, } @@ -2795,7 +2795,7 @@ bevy_reflect::tests::Test { #[reflect_remote(external_crate::TheirOuter)] struct MyOuter { - #[reflect(remote = "MyInner")] + #[reflect(remote = MyInner)] pub inner: external_crate::TheirInner, } diff --git a/crates/bevy_reflect/src/remote.rs b/crates/bevy_reflect/src/remote.rs index bdc10363c8e84..a19d3bdb148d6 100644 --- a/crates/bevy_reflect/src/remote.rs +++ b/crates/bevy_reflect/src/remote.rs @@ -35,7 +35,7 @@ use crate::Reflect; /// /// #[derive(Reflect)] /// struct MyStruct { -/// #[reflect(remote = "MyType")] +/// #[reflect(remote = MyType)] /// data: some_lib::TheirType, /// } /// ``` From 8e0a99df136343787d99e594e9e2ddcc1765423f Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sun, 11 Aug 2024 20:05:24 -0700 Subject: [PATCH 25/31] Add clarifying comment for transmute_copy usage --- crates/bevy_reflect/derive/src/remote.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/crates/bevy_reflect/derive/src/remote.rs b/crates/bevy_reflect/derive/src/remote.rs index 35eee73ed4bf1..be92426242d04 100644 --- a/crates/bevy_reflect/derive/src/remote.rs +++ b/crates/bevy_reflect/derive/src/remote.rs @@ -154,6 +154,16 @@ fn impl_reflect_remote(input: &ReflectDerive, remote_ty: &TypePath) -> proc_macr { // SAFE: The wrapper type should be repr(transparent) over the remote type unsafe { + // Unfortunately, we have to use `transmute_copy` to avoid a compiler error: + // ``` + // error[E0512]: cannot transmute between types of different sizes, or dependently-sized types + // | + // | std::mem::transmute::(a) + // | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + // | + // = note: source type: `A` (this type does not have a fixed size) + // = note: target type: `B` (this type does not have a fixed size) + // ``` ::core::mem::transmute_copy::( // `ManuallyDrop` is used to prevent double-dropping `self` &::core::mem::ManuallyDrop::new(self) @@ -173,6 +183,16 @@ fn impl_reflect_remote(input: &ReflectDerive, remote_ty: &TypePath) -> proc_macr { // SAFE: The wrapper type should be repr(transparent) over the remote type unsafe { + // Unfortunately, we have to use `transmute_copy` to avoid a compiler error: + // ``` + // error[E0512]: cannot transmute between types of different sizes, or dependently-sized types + // | + // | std::mem::transmute::(a) + // | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + // | + // = note: source type: `A` (this type does not have a fixed size) + // = note: target type: `B` (this type does not have a fixed size) + // ``` ::core::mem::transmute_copy::( // `ManuallyDrop` is used to prevent double-dropping `self` &::core::mem::ManuallyDrop::new(remote) From 924363f117eece127da1c7c322d92bf6e51eddb7 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sun, 11 Aug 2024 20:07:16 -0700 Subject: [PATCH 26/31] Remove vis on generated remote type Also hide it from docs (in the case of --document-private-items) --- crates/bevy_reflect/derive/src/remote.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_reflect/derive/src/remote.rs b/crates/bevy_reflect/derive/src/remote.rs index be92426242d04..ed870eb587141 100644 --- a/crates/bevy_reflect/derive/src/remote.rs +++ b/crates/bevy_reflect/derive/src/remote.rs @@ -106,7 +106,6 @@ pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStre /// ``` fn generate_remote_wrapper(input: &DeriveInput, remote_ty: &TypePath) -> proc_macro2::TokenStream { let ident = &input.ident; - let vis = &input.vis; let ty_generics = &input.generics; let where_clause = &input.generics.where_clause; let attrs = input @@ -117,7 +116,8 @@ fn generate_remote_wrapper(input: &DeriveInput, remote_ty: &TypePath) -> proc_ma quote! { #(#attrs)* #[repr(transparent)] - #vis struct #ident #ty_generics (pub #remote_ty) #where_clause; + #[doc(hidden)] + struct #ident #ty_generics (pub #remote_ty) #where_clause; } } From f898f0bd7298ce1ea90a4de6b29b742a098f2f61 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sun, 11 Aug 2024 20:15:27 -0700 Subject: [PATCH 27/31] Silence clippy::multiple_bound_locations warning --- crates/bevy_reflect/derive/src/remote.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/bevy_reflect/derive/src/remote.rs b/crates/bevy_reflect/derive/src/remote.rs index ed870eb587141..c55eeb5592db3 100644 --- a/crates/bevy_reflect/derive/src/remote.rs +++ b/crates/bevy_reflect/derive/src/remote.rs @@ -303,6 +303,7 @@ pub(crate) fn generate_remote_assertions( quote_spanned! {span=> #[allow(non_snake_case)] + #[allow(clippy::multiple_bound_locations)] fn #assertion_ident #impl_generics () #where_reflect_clause { let _: <#remote_ty as #bevy_reflect_path::ReflectRemote>::Remote = (|| -> #FQOption<#ty> { None @@ -415,6 +416,7 @@ fn generate_remote_definition_assertions(derive_data: &ReflectDerive) -> proc_ma #[allow(unused_variables)] #[allow(unused_assignments)] #[allow(unreachable_patterns)] + #[allow(clippy::multiple_bound_locations)] fn assert_wrapper_definition_matches_remote_type #impl_generics (mut #self_ident: #self_ty) #where_reflect_clause { #assertions } From c1496d2d9587b22c9fa56f9eaa7088897bd41769 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sun, 11 Aug 2024 20:48:57 -0700 Subject: [PATCH 28/31] Undo vis removal for generated wrapper type We need to provide a way for users to reference their wrapper type for use in #[reflect(remote = ...)] attributes --- crates/bevy_reflect/derive/src/remote.rs | 3 ++- crates/bevy_reflect/src/lib.rs | 28 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/crates/bevy_reflect/derive/src/remote.rs b/crates/bevy_reflect/derive/src/remote.rs index c55eeb5592db3..0681d8496e5ff 100644 --- a/crates/bevy_reflect/derive/src/remote.rs +++ b/crates/bevy_reflect/derive/src/remote.rs @@ -106,6 +106,7 @@ pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStre /// ``` fn generate_remote_wrapper(input: &DeriveInput, remote_ty: &TypePath) -> proc_macro2::TokenStream { let ident = &input.ident; + let vis = &input.vis; let ty_generics = &input.generics; let where_clause = &input.generics.where_clause; let attrs = input @@ -117,7 +118,7 @@ fn generate_remote_wrapper(input: &DeriveInput, remote_ty: &TypePath) -> proc_ma #(#attrs)* #[repr(transparent)] #[doc(hidden)] - struct #ident #ty_generics (pub #remote_ty) #where_clause; + #vis struct #ident #ty_generics (pub #remote_ty) #where_clause; } } diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 76bfc6f193a86..a37dbd13172b1 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -2554,6 +2554,34 @@ bevy_reflect::tests::Test { assert_eq!("Goodbye", data.0.value); } + #[test] + fn should_reflect_remote_type_from_module() { + mod wrapper { + use super::*; + + // We have to place this module internally to this one to get around the following error: + // ``` + // error[E0433]: failed to resolve: use of undeclared crate or module `external_crate` + // ``` + pub mod external_crate { + pub struct TheirType { + pub value: String, + } + } + + #[reflect_remote(external_crate::TheirType)] + pub struct MyType { + pub value: String, + } + } + + #[derive(Reflect)] + struct ContainerStruct { + #[reflect(remote = wrapper::MyType)] + their_type: wrapper::external_crate::TheirType, + } + } + #[test] fn should_reflect_remote_enum() { mod external_crate { From d17ea00749e46ccca57eebd7f29ffb9c2ab1ca0a Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sun, 11 Aug 2024 20:57:34 -0700 Subject: [PATCH 29/31] Add test for remote reflecting an actual external type --- crates/bevy_reflect/src/lib.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index a37dbd13172b1..05f4abf85b6f3 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -2845,6 +2845,26 @@ bevy_reflect::tests::Test { ); } + #[test] + fn should_reflect_external_crate_type() { + // This test relies on the external type not implementing `Reflect`, + // so let's just double-check that it does not + assert_not_impl_all!(std::collections::Bound: Reflect); + + #[reflect_remote(std::collections::Bound)] + enum MyBound { + Included(T), + Excluded(T), + Unbounded, + } + + #[derive(Reflect)] + struct MyType { + #[reflect(remote = MyBound)] + bound: std::collections::Bound, + } + } + #[cfg(feature = "glam")] mod glam { use super::*; From 0103072105d5548cf94feb53a28dc74dcd103d34 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Mon, 12 Aug 2024 11:32:36 -0700 Subject: [PATCH 30/31] Add support for remote reflection of value types --- crates/bevy_reflect/derive/src/derive_data.rs | 59 ++++++--------- .../bevy_reflect/derive/src/from_reflect.rs | 5 +- .../bevy_reflect/derive/src/impls/common.rs | 3 +- crates/bevy_reflect/derive/src/impls/enums.rs | 8 +- .../bevy_reflect/derive/src/impls/structs.rs | 6 +- .../derive/src/impls/tuple_structs.rs | 6 +- .../bevy_reflect/derive/src/impls/values.rs | 38 +++++++--- crates/bevy_reflect/derive/src/remote.rs | 34 +++++---- .../bevy_reflect/derive/src/struct_utility.rs | 2 +- crates/bevy_reflect/src/lib.rs | 75 +++++++++++++++++++ 10 files changed, 151 insertions(+), 85 deletions(-) diff --git a/crates/bevy_reflect/derive/src/derive_data.rs b/crates/bevy_reflect/derive/src/derive_data.rs index d856b438c5dc6..edb11f3ef3253 100644 --- a/crates/bevy_reflect/derive/src/derive_data.rs +++ b/crates/bevy_reflect/derive/src/derive_data.rs @@ -47,6 +47,8 @@ pub(crate) struct ReflectMeta<'a> { attrs: ContainerAttributes, /// The path to this type. type_path: ReflectTypePath<'a>, + /// The optional remote type to use instead of the actual type. + remote_ty: Option>, /// A cached instance of the path to the `bevy_reflect` crate. bevy_reflect_path: Path, /// The documentation for this type, if any @@ -71,7 +73,6 @@ pub(crate) struct ReflectStruct<'a> { meta: ReflectMeta<'a>, serialization_data: Option, fields: Vec>, - remote_ty: Option>, } /// Enum data used by derive macros for `Reflect` and `FromReflect`. @@ -90,7 +91,6 @@ pub(crate) struct ReflectStruct<'a> { pub(crate) struct ReflectEnum<'a> { meta: ReflectMeta<'a>, variants: Vec>, - remote_ty: Option>, } /// Represents a field on a struct or tuple struct. @@ -331,7 +331,6 @@ impl<'a> ReflectDerive<'a> { meta, serialization_data: SerializationDataDef::new(&fields)?, fields, - remote_ty: None, }; match data.fields { @@ -343,11 +342,7 @@ impl<'a> ReflectDerive<'a> { Data::Enum(data) => { let variants = Self::collect_enum_variants(&data.variants)?; - let reflect_enum = ReflectEnum { - meta, - variants, - remote_ty: None, - }; + let reflect_enum = ReflectEnum { meta, variants }; Ok(Self::Enum(reflect_enum)) } Data::Union(..) => Err(syn::Error::new( @@ -365,12 +360,14 @@ impl<'a> ReflectDerive<'a> { pub fn set_remote(&mut self, remote_ty: Option>) { match self { Self::Struct(data) | Self::TupleStruct(data) | Self::UnitStruct(data) => { - data.remote_ty = remote_ty; + data.meta.remote_ty = remote_ty; } Self::Enum(data) => { - data.remote_ty = remote_ty; + data.meta.remote_ty = remote_ty; + } + Self::Value(meta) => { + meta.remote_ty = remote_ty; } - _ => panic!("cannot create a reflected value type for a remote type"), } } @@ -378,10 +375,10 @@ impl<'a> ReflectDerive<'a> { pub fn remote_ty(&self) -> Option { match self { Self::Struct(data) | Self::TupleStruct(data) | Self::UnitStruct(data) => { - data.remote_ty() + data.meta.remote_ty() } - Self::Enum(data) => data.remote_ty(), - Self::Value(_) => None, + Self::Enum(data) => data.meta.remote_ty(), + Self::Value(meta) => meta.remote_ty(), } } @@ -475,6 +472,7 @@ impl<'a> ReflectMeta<'a> { Self { attrs, type_path, + remote_ty: None, bevy_reflect_path: utility::get_bevy_reflect_path(), #[cfg(feature = "documentation")] docs: Default::default(), @@ -508,6 +506,16 @@ impl<'a> ReflectMeta<'a> { &self.type_path } + /// Get the remote type path, if any. + pub fn remote_ty(&self) -> Option { + self.remote_ty + } + + /// Whether this reflected type represents a remote type or not. + pub fn is_remote_wrapper(&self) -> bool { + self.remote_ty.is_some() + } + /// The cached `bevy_reflect` path. pub fn bevy_reflect_path(&self) -> &Path { &self.bevy_reflect_path @@ -595,17 +603,6 @@ impl<'a> ReflectStruct<'a> { self.serialization_data.as_ref() } - /// Whether this reflected struct represents a remote type or not. - pub fn is_remote_wrapper(&self) -> bool { - self.remote_ty.is_some() - } - - #[allow(dead_code)] - /// Get the remote type path, if any. - pub fn remote_ty(&self) -> Option> { - self.remote_ty - } - /// Returns the `GetTypeRegistration` impl as a `TokenStream`. /// /// Returns a specific implementation for structs and this method should be preferred over the generic [`get_type_registration`](ReflectMeta) method @@ -705,22 +702,12 @@ impl<'a> ReflectEnum<'a> { &self.meta } - /// Whether this reflected enum represents a remote type or not. - pub fn is_remote_wrapper(&self) -> bool { - self.remote_ty.is_some() - } - - #[allow(dead_code)] - /// Get the remote type path, if any. - pub fn remote_ty(&self) -> Option> { - self.remote_ty - } - /// Returns the given ident as a qualified unit variant of this enum. /// /// This takes into account the remote type, if any. pub fn get_unit(&self, variant: &Ident) -> proc_macro2::TokenStream { let name = self + .meta .remote_ty .map(|path| match path.as_expr_path() { Ok(path) => path.to_token_stream(), diff --git a/crates/bevy_reflect/derive/src/from_reflect.rs b/crates/bevy_reflect/derive/src/from_reflect.rs index 83b1802151385..9ed0d1fcaa435 100644 --- a/crates/bevy_reflect/derive/src/from_reflect.rs +++ b/crates/bevy_reflect/derive/src/from_reflect.rs @@ -52,7 +52,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream .. } = FromReflectVariantBuilder::new(reflect_enum).build(&ref_value); - let match_branches = if reflect_enum.is_remote_wrapper() { + let match_branches = if reflect_enum.meta().is_remote_wrapper() { quote! { #(#variant_names => #fqoption::Some(Self(#variant_constructors)),)* } @@ -104,6 +104,7 @@ fn impl_struct_internal( let fqoption = FQOption.into_token_stream(); let struct_path = reflect_struct.meta().type_path(); + let remote_ty = reflect_struct.meta().remote_ty(); let bevy_reflect_path = reflect_struct.meta().bevy_reflect_path(); let ref_struct = Ident::new("__ref_struct", Span::call_site()); @@ -122,7 +123,7 @@ fn impl_struct_internal( let __this = Ident::new("__this", Span::call_site()); // The reflected type: either `Self` or a remote type - let (reflect_ty, constructor, retval) = if let Some(remote_ty) = reflect_struct.remote_ty() { + let (reflect_ty, constructor, retval) = if let Some(remote_ty) = remote_ty { let constructor = match remote_ty.as_expr_path() { Ok(path) => path, Err(err) => return err.into_compile_error(), diff --git a/crates/bevy_reflect/derive/src/impls/common.rs b/crates/bevy_reflect/derive/src/impls/common.rs index 8dd9b2f90485d..f01fd96e044cc 100644 --- a/crates/bevy_reflect/derive/src/impls/common.rs +++ b/crates/bevy_reflect/derive/src/impls/common.rs @@ -7,7 +7,6 @@ use crate::{derive_data::ReflectMeta, utility::WhereClauseOptions}; pub fn impl_full_reflect( meta: &ReflectMeta, where_clause_options: &WhereClauseOptions, - is_remote_wrapper: bool, ) -> proc_macro2::TokenStream { let bevy_reflect_path = meta.bevy_reflect_path(); let type_path = meta.type_path(); @@ -15,7 +14,7 @@ pub fn impl_full_reflect( let (impl_generics, ty_generics, where_clause) = type_path.generics().split_for_impl(); let where_reflect_clause = where_clause_options.extend_where_clause(where_clause); - let any_impls = if is_remote_wrapper { + let any_impls = if meta.is_remote_wrapper() { quote! { #[inline] fn into_any(self: #FQBox) -> #FQBox { diff --git a/crates/bevy_reflect/derive/src/impls/enums.rs b/crates/bevy_reflect/derive/src/impls/enums.rs index 4516a7d01ff83..15d5c01ffa796 100644 --- a/crates/bevy_reflect/derive/src/impls/enums.rs +++ b/crates/bevy_reflect/derive/src/impls/enums.rs @@ -9,7 +9,7 @@ use syn::{Fields, Path}; pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream { let bevy_reflect_path = reflect_enum.meta().bevy_reflect_path(); let enum_path = reflect_enum.meta().type_path(); - let is_remote = reflect_enum.is_remote_wrapper(); + let is_remote = reflect_enum.meta().is_remote_wrapper(); // For `match self` expressions where self is a reference let match_this = if is_remote { @@ -62,11 +62,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream ); let type_path_impl = impl_type_path(reflect_enum.meta()); - let full_reflect_impl = impl_full_reflect( - reflect_enum.meta(), - &where_clause_options, - reflect_enum.is_remote_wrapper(), - ); + let full_reflect_impl = impl_full_reflect(reflect_enum.meta(), &where_clause_options); let common_methods = common_partial_reflect_methods( reflect_enum.meta(), || Some(quote!(#bevy_reflect_path::enum_partial_eq)), diff --git a/crates/bevy_reflect/derive/src/impls/structs.rs b/crates/bevy_reflect/derive/src/impls/structs.rs index b4b0334d0f0d5..15b11d4dd97a1 100644 --- a/crates/bevy_reflect/derive/src/impls/structs.rs +++ b/crates/bevy_reflect/derive/src/impls/structs.rs @@ -39,11 +39,7 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> proc_macro2::TokenS ); let type_path_impl = impl_type_path(reflect_struct.meta()); - let full_reflect_impl = impl_full_reflect( - reflect_struct.meta(), - &where_clause_options, - reflect_struct.is_remote_wrapper(), - ); + let full_reflect_impl = impl_full_reflect(reflect_struct.meta(), &where_clause_options); let common_methods = common_partial_reflect_methods( reflect_struct.meta(), || Some(quote!(#bevy_reflect_path::struct_partial_eq)), diff --git a/crates/bevy_reflect/derive/src/impls/tuple_structs.rs b/crates/bevy_reflect/derive/src/impls/tuple_structs.rs index 44d8fadc5368e..fc2228e70e4c8 100644 --- a/crates/bevy_reflect/derive/src/impls/tuple_structs.rs +++ b/crates/bevy_reflect/derive/src/impls/tuple_structs.rs @@ -29,11 +29,7 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> proc_macro2:: ); let type_path_impl = impl_type_path(reflect_struct.meta()); - let full_reflect_impl = impl_full_reflect( - reflect_struct.meta(), - &where_clause_options, - reflect_struct.is_remote_wrapper(), - ); + let full_reflect_impl = impl_full_reflect(reflect_struct.meta(), &where_clause_options); let common_methods = common_partial_reflect_methods( reflect_struct.meta(), || Some(quote!(#bevy_reflect_path::tuple_struct_partial_eq)), diff --git a/crates/bevy_reflect/derive/src/impls/values.rs b/crates/bevy_reflect/derive/src/impls/values.rs index b3f28ce5d3b27..3a804f3bed5a2 100644 --- a/crates/bevy_reflect/derive/src/impls/values.rs +++ b/crates/bevy_reflect/derive/src/impls/values.rs @@ -28,9 +28,26 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> proc_macro2::TokenStream { ); let type_path_impl = impl_type_path(meta); - let full_reflect_impl = impl_full_reflect(meta, &where_clause_options, false); + let full_reflect_impl = impl_full_reflect(meta, &where_clause_options); let common_methods = common_partial_reflect_methods(meta, || None, || None); + let apply_impl = if let Some(remote_ty) = meta.remote_ty() { + let ty = remote_ty.type_path(); + quote! { + if let #FQOption::Some(value) = ::try_downcast_ref::<#ty>(value) { + *self = Self(#FQClone::clone(value)); + return #FQResult::Ok(()); + } + } + } else { + quote! { + if let #FQOption::Some(value) = ::try_downcast_ref::(value) { + *self = #FQClone::clone(value); + return #FQResult::Ok(()); + } + } + }; + #[cfg(not(feature = "functions"))] let function_impls = None::; #[cfg(feature = "functions")] @@ -67,17 +84,14 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> proc_macro2::TokenStream { &mut self, value: &dyn #bevy_reflect_path::PartialReflect ) -> #FQResult<(), #bevy_reflect_path::ApplyError> { - if let #FQOption::Some(value) = ::try_downcast_ref::(value) { - *self = #FQClone::clone(value); - } else { - return #FQResult::Err( - #bevy_reflect_path::ApplyError::MismatchedTypes { - from_type: ::core::convert::Into::into(#bevy_reflect_path::DynamicTypePath::reflect_type_path(value)), - to_type: ::core::convert::Into::into(::type_path()), - } - ); - } - #FQResult::Ok(()) + #apply_impl + + #FQResult::Err( + #bevy_reflect_path::ApplyError::MismatchedTypes { + from_type: ::core::convert::Into::into(#bevy_reflect_path::DynamicTypePath::reflect_type_path(value)), + to_type: ::core::convert::Into::into(::type_path()), + } + ) } #[inline] diff --git a/crates/bevy_reflect/derive/src/remote.rs b/crates/bevy_reflect/derive/src/remote.rs index 0681d8496e5ff..9a60f930cf992 100644 --- a/crates/bevy_reflect/derive/src/remote.rs +++ b/crates/bevy_reflect/derive/src/remote.rs @@ -1,7 +1,9 @@ use crate::derive_data::{ReflectImplSource, ReflectProvenance, ReflectTraitToImpl}; use crate::impls::impl_assertions; use crate::utility::ident_or_index; -use crate::{from_reflect, impls, ReflectDerive, REFLECT_ATTRIBUTE_NAME}; +use crate::{ + from_reflect, impls, ReflectDerive, REFLECT_ATTRIBUTE_NAME, REFLECT_VALUE_ATTRIBUTE_NAME, +}; use bevy_macro_utils::fq_std::FQOption; use proc_macro::TokenStream; use proc_macro2::{Ident, Span}; @@ -68,11 +70,14 @@ pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStre None }, ), - _ => { - return syn::Error::new(ast.span(), "cannot reflect a remote value type") - .into_compile_error() - .into() - } + ReflectDerive::Value(meta) => ( + impls::impl_value(&meta), + if meta.from_reflect().should_auto_derive() { + Some(from_reflect::impl_value(&meta)) + } else { + None + }, + ), }; TokenStream::from(quote! { @@ -109,10 +114,10 @@ fn generate_remote_wrapper(input: &DeriveInput, remote_ty: &TypePath) -> proc_ma let vis = &input.vis; let ty_generics = &input.generics; let where_clause = &input.generics.where_clause; - let attrs = input - .attrs - .iter() - .filter(|attr| !attr.path().is_ident(REFLECT_ATTRIBUTE_NAME)); + let attrs = input.attrs.iter().filter(|attr| { + !attr.path().is_ident(REFLECT_ATTRIBUTE_NAME) + && !attr.path().is_ident(REFLECT_VALUE_ATTRIBUTE_NAME) + }); quote! { #(#attrs)* @@ -402,12 +407,9 @@ fn generate_remote_definition_assertions(derive_data: &ReflectDerive) -> proc_ma } } } - ReflectDerive::Value(meta) => { - return syn::Error::new( - meta.type_path().span(), - "cannot reflect a remote value type", - ) - .into_compile_error() + ReflectDerive::Value(_) => { + // No assertions needed since there are no fields to check + proc_macro2::TokenStream::new() } }; diff --git a/crates/bevy_reflect/derive/src/struct_utility.rs b/crates/bevy_reflect/derive/src/struct_utility.rs index 4b492393610fd..2320e8b86f5b5 100644 --- a/crates/bevy_reflect/derive/src/struct_utility.rs +++ b/crates/bevy_reflect/derive/src/struct_utility.rs @@ -61,7 +61,7 @@ impl FieldAccessors { where F: FnMut(&StructField, proc_macro2::TokenStream) -> proc_macro2::TokenStream, { - let is_remote = reflect_struct.is_remote_wrapper(); + let is_remote = reflect_struct.meta().is_remote_wrapper(); reflect_struct .active_fields() .map(|field| { diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 05f4abf85b6f3..1b535161787d4 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -2554,6 +2554,81 @@ bevy_reflect::tests::Test { assert_eq!("Goodbye", data.0.value); } + #[test] + fn should_reflect_remote_value_type() { + mod external_crate { + #[derive(Clone, Debug, Default)] + pub struct TheirType { + pub value: String, + } + } + + // === Remote Wrapper === // + #[reflect_remote(external_crate::TheirType)] + #[derive(Clone, Debug, Default)] + #[reflect_value(Debug, Default)] + struct MyType { + pub value: String, + } + + let mut data = MyType(external_crate::TheirType { + value: "Hello".to_string(), + }); + + let patch = MyType(external_crate::TheirType { + value: "Goodbye".to_string(), + }); + + assert_eq!("Hello", data.0.value); + data.apply(&patch); + assert_eq!("Goodbye", data.0.value); + + // === Struct Container === // + #[derive(Reflect, Debug)] + #[reflect(from_reflect = false)] + struct ContainerStruct { + #[reflect(remote = MyType)] + their_type: external_crate::TheirType, + } + + let mut patch = DynamicStruct::default(); + patch.set_represented_type(Some(ContainerStruct::type_info())); + patch.insert( + "their_type", + MyType(external_crate::TheirType { + value: "Goodbye".to_string(), + }), + ); + + let mut data = ContainerStruct { + their_type: external_crate::TheirType { + value: "Hello".to_string(), + }, + }; + + assert_eq!("Hello", data.their_type.value); + data.apply(&patch); + assert_eq!("Goodbye", data.their_type.value); + + // === Tuple Struct Container === // + #[derive(Reflect, Debug)] + struct ContainerTupleStruct(#[reflect(remote = MyType)] external_crate::TheirType); + + let mut patch = DynamicTupleStruct::default(); + patch.set_represented_type(Some(ContainerTupleStruct::type_info())); + patch.insert(MyType(external_crate::TheirType { + value: "Goodbye".to_string(), + })); + + let mut data = ContainerTupleStruct(external_crate::TheirType { + value: "Hello".to_string(), + }); + + assert_eq!("Hello", data.0.value); + data.apply(&patch); + assert_eq!("Goodbye", data.0.value); + } + #[test] fn should_reflect_remote_type_from_module() { mod wrapper { From d8f7d9b6a3baa9022bbf55a8e5e8295e19bdf415 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Mon, 12 Aug 2024 11:43:42 -0700 Subject: [PATCH 31/31] Fix CI errors --- .../tests/reflect_remote/nested_fail.rs | 6 +++--- crates/bevy_reflect/src/lib.rs | 2 +- crates/bevy_reflect/src/reflect.rs | 18 +++++++++--------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs index c699af78e9e0a..c9f85c000187d 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs @@ -22,9 +22,9 @@ mod incorrect_inner_type { use bevy_reflect::{FromReflect, GetTypeRegistration, reflect_remote}; #[reflect_remote(super::external_crate::TheirOuter)] - //~^ ERROR: `TheirInner` can not be reflected - //~| ERROR: `TheirInner` can not be reflected - //~| ERROR: `TheirInner` can not be reflected + //~^ ERROR: `TheirInner` does not implement `PartialReflect` so cannot be introspected + //~| ERROR: `TheirInner` does not implement `PartialReflect` so cannot be introspected + //~| ERROR: `TheirInner` does not implement `PartialReflect` so cannot be introspected //~| ERROR: `TheirInner` can not be used as a dynamic type path //~| ERROR: `?` operator has incompatible types struct MyOuter { diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 1b535161787d4..4d2e0fb72511d 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -2882,7 +2882,7 @@ bevy_reflect::tests::Test { value: "Hello".to_string(), }, output, - ) + ); } #[test] diff --git a/crates/bevy_reflect/src/reflect.rs b/crates/bevy_reflect/src/reflect.rs index c7922f7842f9e..0456d209c7ad9 100644 --- a/crates/bevy_reflect/src/reflect.rs +++ b/crates/bevy_reflect/src/reflect.rs @@ -457,7 +457,7 @@ impl dyn PartialReflect { /// If the underlying value does not implement [`Reflect`] /// or is not of type `T`, returns `Err(self)`. /// - /// For remote types, `T` should be the type itself rather than the wraper type. + /// For remote types, `T` should be the type itself rather than the wrapper type. pub fn try_downcast( self: Box, ) -> Result, Box> { @@ -471,7 +471,7 @@ impl dyn PartialReflect { /// If the underlying value does not implement [`Reflect`] /// or is not of type `T`, returns `Err(self)`. /// - /// For remote types, `T` should be the type itself rather than the wraper type. + /// For remote types, `T` should be the type itself rather than the wrapper type. pub fn try_take(self: Box) -> Result> { self.try_downcast().map(|value| *value) } @@ -481,7 +481,7 @@ impl dyn PartialReflect { /// If the underlying value does not implement [`Reflect`] /// or is not of type `T`, returns [`None`]. /// - /// For remote types, `T` should be the type itself rather than the wraper type. + /// For remote types, `T` should be the type itself rather than the wrapper type. pub fn try_downcast_ref(&self) -> Option<&T> { self.try_as_reflect()?.downcast_ref() } @@ -491,7 +491,7 @@ impl dyn PartialReflect { /// If the underlying value does not implement [`Reflect`] /// or is not of type `T`, returns [`None`]. /// - /// For remote types, `T` should be the type itself rather than the wraper type. + /// For remote types, `T` should be the type itself rather than the wrapper type. pub fn try_downcast_mut(&mut self) -> Option<&mut T> { self.try_as_reflect_mut()?.downcast_mut() } @@ -521,7 +521,7 @@ impl dyn Reflect { /// /// If the underlying value is not of type `T`, returns `Err(self)`. /// - /// For remote types, `T` should be the type itself rather than the wraper type. + /// For remote types, `T` should be the type itself rather than the wrapper type. pub fn downcast(self: Box) -> Result, Box> { if self.is::() { Ok(self.into_any().downcast().unwrap()) @@ -534,7 +534,7 @@ impl dyn Reflect { /// /// If the underlying value is not of type `T`, returns `Err(self)`. /// - /// For remote types, `T` should be the type itself rather than the wraper type. + /// For remote types, `T` should be the type itself rather than the wrapper type. pub fn take(self: Box) -> Result> { self.downcast::().map(|value| *value) } @@ -548,7 +548,7 @@ impl dyn Reflect { /// to determine what type they represent. Represented types cannot be downcasted /// to, but you can use [`FromReflect`] to create a value of the represented type from them. /// - /// For remote types, `T` should be the type itself rather than the wraper type. + /// For remote types, `T` should be the type itself rather than the wrapper type. /// /// [`FromReflect`]: crate::FromReflect #[inline] @@ -560,7 +560,7 @@ impl dyn Reflect { /// /// If the underlying value is not of type `T`, returns `None`. /// - /// For remote types, `T` should be the type itself rather than the wraper type. + /// For remote types, `T` should be the type itself rather than the wrapper type. #[inline] pub fn downcast_ref(&self) -> Option<&T> { self.as_any().downcast_ref::() @@ -570,7 +570,7 @@ impl dyn Reflect { /// /// If the underlying value is not of type `T`, returns `None`. /// - /// For remote types, `T` should be the type itself rather than the wraper type. + /// For remote types, `T` should be the type itself rather than the wrapper type. #[inline] pub fn downcast_mut(&mut self) -> Option<&mut T> { self.as_any_mut().downcast_mut::()