Skip to content

Commit 59f68b9

Browse files
MrGVSVMrGVSV
committed
Fix reflect_remote not working with generics
1 parent d1f0286 commit 59f68b9

File tree

6 files changed

+229
-28
lines changed

6 files changed

+229
-28
lines changed

‎crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,11 @@ use crate::utility::members_to_serialization_denylist;
44
use bit_set::BitSet;
55
use quote::{quote, ToTokens};
66

7+
use crate::remote::RemoteType;
78
use crate::{utility, REFLECT_ATTRIBUTE_NAME, REFLECT_VALUE_ATTRIBUTE_NAME};
89
use syn::punctuated::Punctuated;
910
use syn::spanned::Spanned;
10-
use syn::{
11-
Data, DeriveInput, Field, Fields, Generics, Ident, Meta, Path, Token, Type, TypePath, Variant,
12-
};
11+
use syn::{Data, DeriveInput, Field, Fields, Generics, Ident, Meta, Path, Token, Type, Variant};
1312

1413
pub(crate) enum ReflectDerive<'a> {
1514
Struct(ReflectStruct<'a>),
@@ -63,7 +62,7 @@ pub(crate) struct ReflectStruct<'a> {
6362
meta: ReflectMeta<'a>,
6463
serialization_denylist: BitSet<u32>,
6564
fields: Vec<StructField<'a>>,
66-
remote_ty: Option<&'a TypePath>,
65+
remote_ty: Option<RemoteType<'a>>,
6766
}
6867

6968
/// Enum data used by derive macros for `Reflect` and `FromReflect`.
@@ -82,7 +81,7 @@ pub(crate) struct ReflectStruct<'a> {
8281
pub(crate) struct ReflectEnum<'a> {
8382
meta: ReflectMeta<'a>,
8483
variants: Vec<EnumVariant<'a>>,
85-
remote_ty: Option<&'a TypePath>,
84+
remote_ty: Option<RemoteType<'a>>,
8685
}
8786

8887
/// Represents a field on a struct or tuple struct.
@@ -237,7 +236,7 @@ impl<'a> ReflectDerive<'a> {
237236
/// # Panics
238237
///
239238
/// Panics when called on [`ReflectDerive::Value`].
240-
pub fn set_remote(&mut self, remote_ty: Option<&'a TypePath>) {
239+
pub fn set_remote(&mut self, remote_ty: Option<RemoteType<'a>>) {
241240
match self {
242241
Self::Struct(data) | Self::TupleStruct(data) | Self::UnitStruct(data) => {
243242
data.remote_ty = remote_ty;
@@ -383,7 +382,7 @@ impl<'a> ReflectStruct<'a> {
383382

384383
#[allow(dead_code)]
385384
/// Get the remote type path, if any.
386-
pub fn remote_ty(&self) -> Option<&'a TypePath> {
385+
pub fn remote_ty(&self) -> Option<RemoteType<'a>> {
387386
self.remote_ty
388387
}
389388

@@ -453,7 +452,7 @@ impl<'a> ReflectEnum<'a> {
453452

454453
#[allow(dead_code)]
455454
/// Get the remote type path, if any.
456-
pub fn remote_ty(&self) -> Option<&'a TypePath> {
455+
pub fn remote_ty(&self) -> Option<RemoteType<'a>> {
457456
self.remote_ty
458457
}
459458

@@ -463,7 +462,10 @@ impl<'a> ReflectEnum<'a> {
463462
pub fn get_unit(&self, variant: &Ident) -> proc_macro2::TokenStream {
464463
let name = self
465464
.remote_ty
466-
.map(|path| path.to_token_stream())
465+
.map(|path| match path.as_expr_path() {
466+
Ok(path) => path.to_token_stream(),
467+
Err(err) => err.into_compile_error(),
468+
})
467469
.unwrap_or_else(|| self.meta.type_name.to_token_stream());
468470

469471
quote! {

‎crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,13 +67,25 @@ pub(crate) fn get_variant_constructors(
6767
let accessor = quote!(#field_accessor .expect(#missing_field_err_message));
6868

6969
if let Some(field_ty) = &field.attrs.remote {
70-
quote! {
71-
unsafe {
72-
// SAFE: The wrapper type should be repr(transparent) over the remote type
73-
::std::mem::transmute(
74-
<#field_ty as #bevy_reflect_path::FromReflect>::from_reflect(#ref_value #accessor)
75-
#unwrapper
76-
)
70+
71+
let param = quote! {
72+
<#field_ty as #bevy_reflect_path::FromReflect>::from_reflect(#ref_value #accessor)
73+
#unwrapper
74+
};
75+
76+
if field.attrs.is_remote_generic().unwrap_or_default() {
77+
quote!{
78+
unsafe {
79+
// SAFE: The wrapper type should be repr(transparent) over the remote type
80+
::core::mem::transmute_copy(&#param)
81+
}
82+
}
83+
} else {
84+
quote! {
85+
unsafe {
86+
// SAFE: The wrapper type should be repr(transparent) over the remote type
87+
::core::mem::transmute(#param)
88+
}
7789
}
7890
}
7991
} else {

‎crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,20 @@ pub(crate) struct ReflectFieldAttr {
5757
pub remote: Option<Type>,
5858
}
5959

60+
impl ReflectFieldAttr {
61+
pub fn is_remote_generic(&self) -> Option<bool> {
62+
if let Type::Path(type_path) = self.remote.as_ref()? {
63+
type_path
64+
.path
65+
.segments
66+
.last()
67+
.map(|segment| !segment.arguments.is_empty())
68+
} else {
69+
Some(false)
70+
}
71+
}
72+
}
73+
6074
/// Controls how the default value is determined for a field.
6175
#[derive(Default)]
6276
pub(crate) enum DefaultBehavior {

‎crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs‎

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,20 @@ fn impl_struct_internal(reflect_struct: &ReflectStruct, is_tuple: bool) -> Token
106106
let __this = Ident::new("__this", Span::call_site());
107107

108108
// The reflected type: either `Self` or a remote type
109-
let (reflect_ty, retval) = if let Some(remote_ty) = reflect_struct.remote_ty() {
110-
(quote!(#remote_ty), quote!(Self(#__this)))
109+
let (reflect_ty, constructor, retval) = if let Some(remote_ty) = reflect_struct.remote_ty() {
110+
let constructor = match remote_ty.as_expr_path() {
111+
Ok(path) => path,
112+
Err(err) => return err.into_compile_error().into(),
113+
};
114+
let remote_ty = remote_ty.type_path();
115+
116+
(
117+
quote!(#remote_ty),
118+
quote!(#constructor),
119+
quote!(Self(#__this)),
120+
)
111121
} else {
112-
(quote!(Self), quote!(#__this))
122+
(quote!(Self), quote!(Self), quote!(#__this))
113123
};
114124

115125
let constructor = if reflect_struct.meta().traits().contains(REFLECT_DEFAULT) {
@@ -128,7 +138,7 @@ fn impl_struct_internal(reflect_struct: &ReflectStruct, is_tuple: bool) -> Token
128138
get_ignored_fields(reflect_struct, is_tuple);
129139

130140
quote! {
131-
let #__this = #reflect_ty {
141+
let #__this = #constructor {
132142
#(#active_members: #active_values()?,)*
133143
#(#ignored_members: #ignored_values,)*
134144
};
@@ -204,13 +214,14 @@ fn get_active_fields(
204214
.map(|field| {
205215
let member = get_ident(field.data, field.index, is_tuple);
206216
let accessor = get_field_accessor(field.data, field.index, is_tuple);
207-
let ty = field.data.ty.clone();
217+
let ty = field.reflected_type();
218+
let real_ty = &field.data.ty;
208219

209220
let get_field = quote! {
210221
#bevy_reflect_path::#struct_type::field(#dyn_struct_name, #accessor)
211222
};
212223

213-
let value = match &field.attrs.default {
224+
let mut value = match &field.attrs.default {
214225
DefaultBehavior::Func(path) => quote! {
215226
(||
216227
if let #FQOption::Some(field) = #get_field {
@@ -234,6 +245,27 @@ fn get_active_fields(
234245
},
235246
};
236247

248+
if field.attrs.remote.is_some() {
249+
value = if field.attrs.is_remote_generic().unwrap_or_default() {
250+
quote! {
251+
(|| Some(
252+
unsafe {::core::mem::transmute_copy::<_, #real_ty>(&#value()?)}
253+
))
254+
}
255+
} else {
256+
quote! {
257+
(|| Some(
258+
unsafe {::core::mem::transmute::<_, #real_ty>(#value()?)}
259+
))
260+
}
261+
};
262+
value = quote! {
263+
(|| Some(
264+
unsafe {::core::mem::transmute_copy::<_, #real_ty>(&#value()?)}
265+
))
266+
}
267+
}
268+
237269
(member, value)
238270
})
239271
.unzip(),

‎crates/bevy_reflect/bevy_reflect_derive/src/remote.rs‎

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use proc_macro::TokenStream;
33
use quote::quote;
44
use syn::parse::{Parse, ParseStream};
55
use syn::spanned::Spanned;
6-
use syn::{parse_macro_input, DeriveInput, Token, TypePath};
6+
use syn::{parse_macro_input, DeriveInput, ExprPath, PathArguments, Token, TypePath};
77

88
/// Generates the remote wrapper type and implements all the necessary traits.
99
pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStream {
@@ -23,7 +23,7 @@ pub(crate) fn reflect_remote(args: TokenStream, input: TokenStream) -> TokenStre
2323
Err(err) => return err.into_compile_error().into(),
2424
};
2525

26-
derive_data.set_remote(Some(&remote_ty));
26+
derive_data.set_remote(Some(RemoteType::new(&remote_ty)));
2727

2828
let from_reflect_impl = if is_from_reflect {
2929
Some(from_reflect_remote(&derive_data))
@@ -96,6 +96,54 @@ fn generate_remote_wrapper(input: &DeriveInput, remote_ty: &TypePath) -> proc_ma
9696
}
9797
}
9898

99+
/// A reflected type's remote type.
100+
///
101+
/// This is a wrapper around [`TypePath`] that allows it to be paired with other remote-specific logic.
102+
#[derive(Copy, Clone)]
103+
pub(crate) struct RemoteType<'a> {
104+
path: &'a TypePath,
105+
}
106+
107+
impl<'a> RemoteType<'a> {
108+
pub fn new(path: &'a TypePath) -> Self {
109+
Self { path }
110+
}
111+
112+
/// Returns the [type path](TypePath) of this remote type.
113+
pub fn type_path(&self) -> &'a TypePath {
114+
self.path
115+
}
116+
117+
/// Attempts to convert the [type path](TypePath) of this remote type into an [expression path](ExprPath).
118+
///
119+
/// For example, this would convert `foo::Bar<T>` into `foo::Bar::<T>` to be used as part of an expression.
120+
///
121+
/// This will return an error for types that are parenthesized, such as in `Fn() -> Foo`.
122+
pub fn as_expr_path(&self) -> Result<ExprPath, syn::Error> {
123+
let mut expr_path = self.path.clone();
124+
if let Some(segment) = expr_path.path.segments.last_mut() {
125+
match &mut segment.arguments {
126+
PathArguments::None => {}
127+
PathArguments::AngleBracketed(arg) => {
128+
arg.colon2_token = Some(syn::token::Colon2::default());
129+
}
130+
PathArguments::Parenthesized(arg) => {
131+
return Err(syn::Error::new(
132+
arg.span(),
133+
"cannot use parenthesized type as remote type",
134+
))
135+
}
136+
}
137+
}
138+
139+
Ok(ExprPath {
140+
path: expr_path.path,
141+
qself: expr_path.qself,
142+
attrs: Vec::new(),
143+
})
144+
}
145+
}
146+
99147
/// Metadata from the arguments defined in the `reflect_remote` attribute.
100148
///
101149
/// The syntax for the arguments is: `#[reflect_remote(REMOTE_TYPE_PATH)]`

0 commit comments

Comments
 (0)