Skip to content

Commit

Permalink
Enable deriving Reflect on structs with generic types (#7364)
Browse files Browse the repository at this point in the history
# Objective

I recently had an issue, where I have a struct:
```
struct Property {
   inner: T
}
```
that I use as a wrapper for internal purposes.
I don't want to update my struct definition to 
```
struct Property<T: Reflect>{
   inner: T
}
```
because I still want to be able to build `Property<T>` for types `T` that are not `Reflect`. (and also because I don't want to update my whole code base with `<T: Reflect>` bounds)

I still wanted to have reflection on it (for `bevy_inspector_egui`), but adding `derive(Reflect)` fails with the error:
`T cannot be sent between threads safely. T needs to implement Sync.`

I believe that `bevy_reflect` should adopt the model of other derives in the case of generics, which is to add the `Reflect` implementation only if the generics also implement `Reflect`. (That is the behaviour of other macros such as `derive(Clone)` or `derive(Debug)`.

It's also the current behavior of `derive(FromReflect)`.

Basically doing something like:
```
impl<T> Reflect for Foo<T>
where T: Reflect
```


## Solution

- I updated the derive macros for `Structs` and `TupleStructs` to add extra `where` bounds.
   -  Every type that is reflected will need a `T: Reflect` bound
   -  Ignored types will need a `T: 'static + Send + Sync` bound. Here's the reason. For cases like this:
```
#[derive(Reflect)]
struct Foo<T, U>{
   a: T
   #[reflect(ignore)]
   b: U
}
```
I had to add the bound `'static + Send + Sync` to ignored generics like `U`.
The reason is that we want `Foo<T, U>` to be `Reflect: 'static + Send + Sync`, so `Foo<T, U>` must be able to implement those auto-traits. `Foo<T, U>` will only implement those auto-traits if every generic type implements them, including ignored types.
This means that the previously compile-fail case now compiles:
```
#[derive(Reflect)]
struct Foo<'a> {
    #[reflect(ignore)]
    value: &'a str,
}
```
But `Foo<'a>` will only be useable in the cases where `'a: 'static` and panic if we don't have `'a: 'static`, which is what we want (nice bonus from this PR ;) )



---

## Changelog

> This section is optional. If this was a trivial fix, or has no externally-visible impact, you can delete this section.

### Added
Possibility to add `derive(Reflect)` to structs and enums that contain generic types, like so:
```
#[derive(Reflect)]
struct Foo<T>{
   a: T
}
```
Reflection will only be available if the generic type T also implements `Reflect`.
(previously, this would just return a compiler error)
  • Loading branch information
cbournhonesque-sc committed Jan 28, 2023
1 parent 9ffba9b commit cbb4c26
Show file tree
Hide file tree
Showing 14 changed files with 312 additions and 44 deletions.
107 changes: 98 additions & 9 deletions crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::container_attributes::ReflectTraits;
use crate::field_attributes::{parse_field_attrs, ReflectFieldAttr};
use crate::utility::members_to_serialization_denylist;
use crate::fq_std::{FQAny, FQDefault, FQSend, FQSync};
use crate::utility::{members_to_serialization_denylist, WhereClauseOptions};
use bit_set::BitSet;
use quote::quote;

Expand Down Expand Up @@ -316,12 +317,16 @@ impl<'a> ReflectMeta<'a> {
}

/// Returns the `GetTypeRegistration` impl as a `TokenStream`.
pub fn get_type_registration(&self) -> proc_macro2::TokenStream {
pub fn get_type_registration(
&self,
where_clause_options: &WhereClauseOptions,
) -> proc_macro2::TokenStream {
crate::registration::impl_get_type_registration(
self.type_name,
&self.bevy_reflect_path,
self.traits.idents(),
self.generics,
where_clause_options,
None,
)
}
Expand Down Expand Up @@ -350,46 +355,65 @@ impl<'a> ReflectStruct<'a> {
/// 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`](crate::ReflectMeta) method
pub fn get_type_registration(&self) -> proc_macro2::TokenStream {
pub fn get_type_registration(
&self,
where_clause_options: &WhereClauseOptions,
) -> proc_macro2::TokenStream {
let reflect_path = self.meta.bevy_reflect_path();

crate::registration::impl_get_type_registration(
self.meta.type_name(),
reflect_path,
self.meta.traits().idents(),
self.meta.generics(),
where_clause_options,
Some(&self.serialization_denylist),
)
}

/// Get a collection of types which are exposed to the reflection API
pub fn active_types(&self) -> Vec<syn::Type> {
self.fields
.iter()
.filter(move |field| field.attrs.ignore.is_active())
self.active_fields()
.map(|field| field.data.ty.clone())
.collect::<Vec<_>>()
.collect()
}

/// Get an iterator of fields which are exposed to the reflection API
pub fn active_fields(&self) -> impl Iterator<Item = &StructField<'a>> {
self.fields
.iter()
.filter(move |field| field.attrs.ignore.is_active())
.filter(|field| field.attrs.ignore.is_active())
}

/// Get a collection of types which are ignored by the reflection API
pub fn ignored_types(&self) -> Vec<syn::Type> {
self.ignored_fields()
.map(|field| field.data.ty.clone())
.collect()
}

/// Get an iterator of fields which are ignored by the reflection API
pub fn ignored_fields(&self) -> impl Iterator<Item = &StructField<'a>> {
self.fields
.iter()
.filter(move |field| field.attrs.ignore.is_ignored())
.filter(|field| field.attrs.ignore.is_ignored())
}

/// The complete set of fields in this struct.
#[allow(dead_code)]
pub fn fields(&self) -> &[StructField<'a>] {
&self.fields
}

pub fn where_clause_options(&self) -> WhereClauseOptions {
let bevy_reflect_path = &self.meta().bevy_reflect_path;
WhereClauseOptions {
active_types: self.active_types().into(),
active_trait_bounds: quote! { #bevy_reflect_path::Reflect },
ignored_types: self.ignored_types().into(),
ignored_trait_bounds: quote! { #FQAny + #FQSend + #FQSync },
}
}
}

impl<'a> ReflectEnum<'a> {
Expand All @@ -410,4 +434,69 @@ impl<'a> ReflectEnum<'a> {
pub fn variants(&self) -> &[EnumVariant<'a>] {
&self.variants
}

/// Get an iterator of fields which are exposed to the reflection API
pub fn active_fields(&self) -> impl Iterator<Item = &StructField<'a>> {
self.variants()
.iter()
.flat_map(|variant| variant.active_fields())
}

/// Get a collection of types which are exposed to the reflection API
pub fn active_types(&self) -> Vec<syn::Type> {
self.active_fields()
.map(|field| field.data.ty.clone())
.collect()
}

/// Get an iterator of fields which are ignored by the reflection API
pub fn ignored_fields(&self) -> impl Iterator<Item = &StructField<'a>> {
self.variants()
.iter()
.flat_map(|variant| variant.ignored_fields())
}

/// Get a collection of types which are ignored to the reflection API
pub fn ignored_types(&self) -> Vec<syn::Type> {
self.ignored_fields()
.map(|field| field.data.ty.clone())
.collect()
}

pub fn where_clause_options(&self) -> WhereClauseOptions {
let bevy_reflect_path = &self.meta().bevy_reflect_path;
WhereClauseOptions {
active_types: self.active_types().into(),
active_trait_bounds: quote! { #bevy_reflect_path::FromReflect },
ignored_types: self.ignored_types().into(),
ignored_trait_bounds: quote! { #FQAny + #FQSend + #FQSync + #FQDefault },
}
}
}

impl<'a> EnumVariant<'a> {
/// Get an iterator of fields which are exposed to the reflection API
#[allow(dead_code)]
pub fn active_fields(&self) -> impl Iterator<Item = &StructField<'a>> {
self.fields()
.iter()
.filter(|field| field.attrs.ignore.is_active())
}

/// Get an iterator of fields which are ignored by the reflection API
#[allow(dead_code)]
pub fn ignored_fields(&self) -> impl Iterator<Item = &StructField<'a>> {
self.fields()
.iter()
.filter(|field| field.attrs.ignore.is_ignored())
}

/// The complete set of fields in this variant.
#[allow(dead_code)]
pub fn fields(&self) -> &[StructField<'a>] {
match &self.fields {
EnumVariantFields::Named(fields) | EnumVariantFields::Unnamed(fields) => fields,
EnumVariantFields::Unit => &[],
}
}
}
14 changes: 14 additions & 0 deletions crates/bevy_reflect/bevy_reflect_derive/src/fq_std.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ pub(crate) struct FQClone;
pub(crate) struct FQDefault;
pub(crate) struct FQOption;
pub(crate) struct FQResult;
pub(crate) struct FQSend;
pub(crate) struct FQSync;

impl ToTokens for FQAny {
fn to_tokens(&self, tokens: &mut TokenStream) {
Expand Down Expand Up @@ -75,3 +77,15 @@ impl ToTokens for FQResult {
quote!(::core::result::Result).to_tokens(tokens);
}
}

impl ToTokens for FQSend {
fn to_tokens(&self, tokens: &mut TokenStream) {
quote!(::core::marker::Send).to_tokens(tokens);
}
}

impl ToTokens for FQSync {
fn to_tokens(&self, tokens: &mut TokenStream) {
quote!(::core::marker::Sync).to_tokens(tokens);
}
}
14 changes: 11 additions & 3 deletions crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::derive_data::{EnumVariant, EnumVariantFields, ReflectEnum, StructFiel
use crate::enum_utility::{get_variant_constructors, EnumVariantConstructors};
use crate::fq_std::{FQAny, FQBox, FQOption, FQResult};
use crate::impls::impl_typed;
use crate::utility::extend_where_clause;
use proc_macro::TokenStream;
use proc_macro2::{Ident, Span};
use quote::quote;
Expand All @@ -15,6 +16,8 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream {
let ref_index = Ident::new("__index_param", Span::call_site());
let ref_value = Ident::new("__value_param", Span::call_site());

let where_clause_options = reflect_enum.where_clause_options();

let EnumImpls {
variant_info,
enum_field,
Expand Down Expand Up @@ -76,6 +79,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream {
let typed_impl = impl_typed(
enum_name,
reflect_enum.meta().generics(),
&where_clause_options,
quote! {
let variants = [#(#variant_info),*];
let info = #info_generator;
Expand All @@ -84,16 +88,20 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream {
bevy_reflect_path,
);

let get_type_registration_impl = reflect_enum.meta().get_type_registration();
let get_type_registration_impl = reflect_enum
.meta()
.get_type_registration(&where_clause_options);
let (impl_generics, ty_generics, where_clause) =
reflect_enum.meta().generics().split_for_impl();

let where_reflect_clause = extend_where_clause(where_clause, &where_clause_options);

TokenStream::from(quote! {
#get_type_registration_impl

#typed_impl

impl #impl_generics #bevy_reflect_path::Enum for #enum_name #ty_generics #where_clause {
impl #impl_generics #bevy_reflect_path::Enum for #enum_name #ty_generics #where_reflect_clause {
fn field(&self, #ref_name: &str) -> #FQOption<&dyn #bevy_reflect_path::Reflect> {
match self {
#(#enum_field,)*
Expand Down Expand Up @@ -177,7 +185,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream {
}
}

impl #impl_generics #bevy_reflect_path::Reflect for #enum_name #ty_generics #where_clause {
impl #impl_generics #bevy_reflect_path::Reflect for #enum_name #ty_generics #where_reflect_clause {
#[inline]
fn type_name(&self) -> &str {
::core::any::type_name::<Self>()
Expand Down
11 changes: 8 additions & 3 deletions crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::fq_std::{FQAny, FQBox, FQDefault, FQOption, FQResult};
use crate::impls::impl_typed;
use crate::utility::extend_where_clause;
use crate::ReflectStruct;
use proc_macro::TokenStream;
use quote::{quote, ToTokens};
Expand Down Expand Up @@ -88,9 +89,11 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream {
}
};

let where_clause_options = reflect_struct.where_clause_options();
let typed_impl = impl_typed(
struct_name,
reflect_struct.meta().generics(),
&where_clause_options,
quote! {
let fields = [#field_generator];
let info = #info_generator;
Expand All @@ -99,16 +102,18 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream {
bevy_reflect_path,
);

let get_type_registration_impl = reflect_struct.get_type_registration();
let get_type_registration_impl = reflect_struct.get_type_registration(&where_clause_options);
let (impl_generics, ty_generics, where_clause) =
reflect_struct.meta().generics().split_for_impl();

let where_reflect_clause = extend_where_clause(where_clause, &where_clause_options);

TokenStream::from(quote! {
#get_type_registration_impl

#typed_impl

impl #impl_generics #bevy_reflect_path::Struct for #struct_name #ty_generics #where_clause {
impl #impl_generics #bevy_reflect_path::Struct for #struct_name #ty_generics #where_reflect_clause {
fn field(&self, name: &str) -> #FQOption<&dyn #bevy_reflect_path::Reflect> {
match name {
#(#field_names => #fqoption::Some(&self.#field_idents),)*
Expand Down Expand Up @@ -160,7 +165,7 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream {
}
}

impl #impl_generics #bevy_reflect_path::Reflect for #struct_name #ty_generics #where_clause {
impl #impl_generics #bevy_reflect_path::Reflect for #struct_name #ty_generics #where_reflect_clause {
#[inline]
fn type_name(&self) -> &str {
::core::any::type_name::<Self>()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::fq_std::{FQAny, FQBox, FQDefault, FQOption, FQResult};
use crate::impls::impl_typed;
use crate::utility::extend_where_clause;
use crate::ReflectStruct;
use proc_macro::TokenStream;
use quote::{quote, ToTokens};
Expand All @@ -11,7 +12,6 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream {

let bevy_reflect_path = reflect_struct.meta().bevy_reflect_path();
let struct_name = reflect_struct.meta().type_name();
let get_type_registration_impl = reflect_struct.get_type_registration();

let field_idents = reflect_struct
.active_fields()
Expand All @@ -21,6 +21,9 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream {
let field_count = field_idents.len();
let field_indices = (0..field_count).collect::<Vec<usize>>();

let where_clause_options = reflect_struct.where_clause_options();
let get_type_registration_impl = reflect_struct.get_type_registration(&where_clause_options);

let hash_fn = reflect_struct
.meta()
.traits()
Expand Down Expand Up @@ -75,6 +78,7 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream {
let typed_impl = impl_typed(
struct_name,
reflect_struct.meta().generics(),
&where_clause_options,
quote! {
let fields = [#field_generator];
let info = #info_generator;
Expand All @@ -86,12 +90,14 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream {
let (impl_generics, ty_generics, where_clause) =
reflect_struct.meta().generics().split_for_impl();

let where_reflect_clause = extend_where_clause(where_clause, &where_clause_options);

TokenStream::from(quote! {
#get_type_registration_impl

#typed_impl

impl #impl_generics #bevy_reflect_path::TupleStruct for #struct_name #ty_generics #where_clause {
impl #impl_generics #bevy_reflect_path::TupleStruct for #struct_name #ty_generics #where_reflect_clause {
fn field(&self, index: usize) -> #FQOption<&dyn #bevy_reflect_path::Reflect> {
match index {
#(#field_indices => #fqoption::Some(&self.#field_idents),)*
Expand Down Expand Up @@ -122,7 +128,7 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream {
}
}

impl #impl_generics #bevy_reflect_path::Reflect for #struct_name #ty_generics #where_clause {
impl #impl_generics #bevy_reflect_path::Reflect for #struct_name #ty_generics #where_reflect_clause {
#[inline]
fn type_name(&self) -> &str {
::core::any::type_name::<Self>()
Expand Down
8 changes: 7 additions & 1 deletion crates/bevy_reflect/bevy_reflect_derive/src/impls/typed.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
use crate::utility::{extend_where_clause, WhereClauseOptions};
use proc_macro2::Ident;
use quote::quote;
use syn::{Generics, Path};

#[allow(clippy::too_many_arguments)]
pub(crate) fn impl_typed(
type_name: &Ident,
generics: &Generics,
where_clause_options: &WhereClauseOptions,
generator: proc_macro2::TokenStream,
bevy_reflect_path: &Path,
) -> proc_macro2::TokenStream {
Expand All @@ -28,8 +31,11 @@ pub(crate) fn impl_typed(

let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();

// Add Typed bound for each active field
let where_reflect_clause = extend_where_clause(where_clause, where_clause_options);

quote! {
impl #impl_generics #bevy_reflect_path::Typed for #type_name #ty_generics #where_clause {
impl #impl_generics #bevy_reflect_path::Typed for #type_name #ty_generics #where_reflect_clause {
fn type_info() -> &'static #bevy_reflect_path::TypeInfo {
#static_generator
}
Expand Down
Loading

0 comments on commit cbb4c26

Please sign in to comment.