Skip to content

Commit

Permalink
Fix field ordering issues with #[reflect(skip_serializing)]
Browse files Browse the repository at this point in the history
  • Loading branch information
MrGVSV committed Apr 23, 2023
1 parent 1118109 commit 113851b
Show file tree
Hide file tree
Showing 16 changed files with 560 additions and 268 deletions.
1 change: 0 additions & 1 deletion crates/bevy_reflect/bevy_reflect_derive/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,3 @@ syn = { version = "1.0", features = ["full"] }
proc-macro2 = "1.0"
quote = "1.0"
uuid = { version = "1.1", features = ["v4"] }
bit-set = "0.5.2"
66 changes: 66 additions & 0 deletions crates/bevy_reflect/bevy_reflect_derive/src/associated_data.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
use crate::field_attributes::{DefaultBehavior, ReflectFieldAttr, ReflectIgnoreBehavior};
use crate::fq_std::{FQBox, FQDefault};
use crate::utility::ident_or_index;
use proc_macro2::{Ident, TokenStream};
use quote::{format_ident, ToTokens};
use syn::{parse_quote, Field, ItemFn, Path};

/// Associated data to generate alongside derived trait implementations.
///
/// It's important these are generated within the context of an [_unnamed const_]
/// in order to avoid conflicts and keep the macro hygenic.
///
/// [_unnamed const_]: https://doc.rust-lang.org/stable/reference/items/constant-items.html#unnamed-constant
pub(crate) struct AssociatedData {
default_fn: Option<ItemFn>,
}

impl AssociatedData {
/// Generates a new `AssociatedData` for a given field.
pub fn new(
field: &Field,
index: usize,
attrs: &ReflectFieldAttr,
qualifier: &Ident,
bevy_reflect_path: &Path,
) -> Self {
let field_ident = ident_or_index(field.ident.as_ref(), index);
let field_ty = &field.ty;

let default_fn = match attrs.ignore {
ReflectIgnoreBehavior::IgnoreSerialization => {
let ident = format_ident!("get_default__{}__{}", qualifier, field_ident);
match &attrs.default {
DefaultBehavior::Required | DefaultBehavior::Default => Some(parse_quote! {
#[allow(non_snake_case)]
fn #ident() -> #FQBox<dyn #bevy_reflect_path::Reflect> {
#FQBox::new(<#field_ty as #FQDefault>::default())
}
}),
DefaultBehavior::Func(func) => Some(parse_quote! {
#[allow(non_snake_case)]
fn #ident() -> #FQBox<dyn #bevy_reflect_path::Reflect> {
#FQBox::new(#func() as #field_ty)
}
}),
}
}
_ => None,
};

Self { default_fn }
}

/// Returns the function used to generate a default instance of a field.
///
/// Returns `None` if the field does not have or need such a function.
pub fn default_fn(&self) -> Option<&ItemFn> {
self.default_fn.as_ref()
}
}

impl ToTokens for AssociatedData {
fn to_tokens(&self, tokens: &mut TokenStream) {
self.default_fn.to_tokens(tokens);
}
}
79 changes: 60 additions & 19 deletions crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use crate::associated_data::AssociatedData;
use crate::container_attributes::ReflectTraits;
use crate::field_attributes::{parse_field_attrs, ReflectFieldAttr};
use crate::fq_std::{FQAny, FQDefault, FQSend, FQSync};
use crate::utility::{members_to_serialization_denylist, WhereClauseOptions};
use bit_set::BitSet;
use quote::quote;

use crate::serialization::SerializationDataDef;
use crate::utility::WhereClauseOptions;
use crate::{utility, REFLECT_ATTRIBUTE_NAME, REFLECT_VALUE_ATTRIBUTE_NAME};
use quote::quote;
use syn::punctuated::Punctuated;
use syn::spanned::Spanned;
use syn::{Data, DeriveInput, Field, Fields, Generics, Ident, Meta, Path, Token, Variant};
Expand Down Expand Up @@ -60,7 +60,7 @@ pub(crate) struct ReflectMeta<'a> {
/// ```
pub(crate) struct ReflectStruct<'a> {
meta: ReflectMeta<'a>,
serialization_denylist: BitSet<u32>,
serialization_data: Option<SerializationDataDef>,
fields: Vec<StructField<'a>>,
}

Expand Down Expand Up @@ -88,6 +88,8 @@ pub(crate) struct StructField<'a> {
pub data: &'a Field,
/// The reflection-based attributes on the field.
pub attrs: ReflectFieldAttr,
/// The associated data to be generated on behalf of this field.
pub associated_data: AssociatedData,
/// The index of this field within the struct.
pub declaration_index: usize,
/// The index of this field as seen by the reflection API.
Expand Down Expand Up @@ -203,12 +205,14 @@ impl<'a> ReflectDerive<'a> {

return match &input.data {
Data::Struct(data) => {
let fields = Self::collect_struct_fields(&data.fields)?;
let fields = Self::collect_struct_fields(
&data.fields,
meta.type_name,
meta.bevy_reflect_path(),
)?;
let reflect_struct = ReflectStruct {
meta,
serialization_denylist: members_to_serialization_denylist(
fields.iter().map(|v| v.attrs.ignore),
),
serialization_data: SerializationDataDef::new(&fields)?,
fields,
};

Expand All @@ -219,7 +223,8 @@ impl<'a> ReflectDerive<'a> {
}
}
Data::Enum(data) => {
let variants = Self::collect_enum_variants(&data.variants)?;
let variants =
Self::collect_enum_variants(&data.variants, meta.bevy_reflect_path())?;

let reflect_enum = ReflectEnum { meta, variants };
Ok(Self::Enum(reflect_enum))
Expand All @@ -231,14 +236,25 @@ impl<'a> ReflectDerive<'a> {
};
}

fn collect_struct_fields(fields: &'a Fields) -> Result<Vec<StructField<'a>>, syn::Error> {
fn collect_struct_fields(
fields: &'a Fields,
qualifier: &Ident,
bevy_reflect_path: &Path,
) -> Result<Vec<StructField<'a>>, syn::Error> {
let mut active_index = 0;
let sifter: utility::ResultSifter<StructField<'a>> = fields
.iter()
.enumerate()
.map(
|(declaration_index, field)| -> Result<StructField, syn::Error> {
let attrs = parse_field_attrs(&field.attrs)?;
let associated_data = AssociatedData::new(
field,
declaration_index,
&attrs,
qualifier,
bevy_reflect_path,
);

let reflection_index = if attrs.ignore.is_ignored() {
None
Expand All @@ -251,6 +267,7 @@ impl<'a> ReflectDerive<'a> {
declaration_index,
reflection_index,
attrs,
associated_data,
data: field,
#[cfg(feature = "documentation")]
doc: crate::documentation::Documentation::from_attributes(&field.attrs),
Expand All @@ -267,12 +284,17 @@ impl<'a> ReflectDerive<'a> {

fn collect_enum_variants(
variants: &'a Punctuated<Variant, Token![,]>,
bevy_reflect_path: &Path,
) -> Result<Vec<EnumVariant<'a>>, syn::Error> {
let sifter: utility::ResultSifter<EnumVariant<'a>> = variants
.iter()
.enumerate()
.map(|(index, variant)| -> Result<EnumVariant, syn::Error> {
let fields = Self::collect_struct_fields(&variant.fields)?;
let fields = Self::collect_struct_fields(
&variant.fields,
&variant.ident,
bevy_reflect_path,
)?;

let fields = match variant.fields {
Fields::Named(..) => EnumVariantFields::Named(fields),
Expand All @@ -295,6 +317,23 @@ impl<'a> ReflectDerive<'a> {

sifter.finish()
}

/// The complete set of fields in this item.
pub fn fields(&self) -> Box<dyn Iterator<Item = &StructField<'a>> + '_> {
match self {
ReflectDerive::Struct(reflect_struct)
| ReflectDerive::TupleStruct(reflect_struct)
| ReflectDerive::UnitStruct(reflect_struct) => Box::new(reflect_struct.fields.iter()),
ReflectDerive::Enum(reflect_enum) => Box::new(reflect_enum.fields()),
ReflectDerive::Value(_) => Box::new(core::iter::empty()),
}
}

/// Generate all [associated data](AssociatedData) into a `TokenStream`.
pub fn associated_data(&self) -> proc_macro2::TokenStream {
let associated_data = self.fields().map(|field| &field.associated_data);
quote!(#(#associated_data)*)
}
}

impl<'a> ReflectMeta<'a> {
Expand Down Expand Up @@ -363,12 +402,9 @@ impl<'a> ReflectStruct<'a> {
&self.meta
}

/// Access the data about which fields should be ignored during serialization.
///
/// The returned bitset is a collection of indices obtained from the [`members_to_serialization_denylist`](crate::utility::members_to_serialization_denylist) function.
#[allow(dead_code)]
pub fn serialization_denylist(&self) -> &BitSet<u32> {
&self.serialization_denylist
/// Returns the [`SerializationDataDef`] for this struct.
pub fn serialization_data(&self) -> Option<&SerializationDataDef> {
self.serialization_data.as_ref()
}

/// Returns the `GetTypeRegistration` impl as a `TokenStream`.
Expand All @@ -386,7 +422,7 @@ impl<'a> ReflectStruct<'a> {
self.meta.traits().idents(),
self.meta.generics(),
where_clause_options,
Some(&self.serialization_denylist),
self.serialization_data(),
)
}

Expand Down Expand Up @@ -454,6 +490,11 @@ impl<'a> ReflectEnum<'a> {
&self.variants
}

/// The complete set of fields in this enum.
pub fn fields(&self) -> impl Iterator<Item = &StructField<'a>> {
self.variants().iter().flat_map(|variant| variant.fields())
}

/// Get an iterator of fields which are exposed to the reflection API
pub fn active_fields(&self) -> impl Iterator<Item = &StructField<'a>> {
self.variants()
Expand Down
13 changes: 8 additions & 5 deletions crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@ 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;
use syn::Fields;

pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream {
pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream {
let bevy_reflect_path = reflect_enum.meta().bevy_reflect_path();
let enum_name = reflect_enum.meta().type_name();

Expand Down Expand Up @@ -96,7 +95,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream {

let where_reflect_clause = extend_where_clause(where_clause, &where_clause_options);

TokenStream::from(quote! {
quote! {
#get_type_registration_impl

#typed_impl
Expand Down Expand Up @@ -288,7 +287,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream {

#debug_fn
}
})
}
}

struct EnumImpls {
Expand Down Expand Up @@ -351,7 +350,11 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden
// Ignored field
continue;
}
constructor_argument.push(generate_for_field(reflect_idx, field.declaration_index, field));
constructor_argument.push(generate_for_field(
reflect_idx,
field.declaration_index,
field,
));
reflect_idx += 1;
}
constructor_argument
Expand Down
7 changes: 3 additions & 4 deletions crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@ use crate::fq_std::{FQAny, FQBox, FQDefault, FQOption, FQResult};
use crate::impls::impl_typed;
use crate::utility::{extend_where_clause, ident_or_index};
use crate::ReflectStruct;
use proc_macro::TokenStream;
use quote::{quote, ToTokens};

/// Implements `Struct`, `GetTypeRegistration`, and `Reflect` for the given derive data.
pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream {
pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> proc_macro2::TokenStream {
let fqoption = FQOption.into_token_stream();

let bevy_reflect_path = reflect_struct.meta().bevy_reflect_path();
Expand Down Expand Up @@ -100,7 +99,7 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream {

let where_reflect_clause = extend_where_clause(where_clause, &where_clause_options);

TokenStream::from(quote! {
quote! {
#get_type_registration_impl

#typed_impl
Expand Down Expand Up @@ -239,5 +238,5 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream {

#debug_fn
}
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@ 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};
use syn::{Index, Member};

/// Implements `TupleStruct`, `GetTypeRegistration`, and `Reflect` for the given derive data.
pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream {
pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> proc_macro2::TokenStream {
let fqoption = FQOption.into_token_stream();

let bevy_reflect_path = reflect_struct.meta().bevy_reflect_path();
Expand Down Expand Up @@ -92,7 +91,7 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream {

let where_reflect_clause = extend_where_clause(where_clause, &where_clause_options);

TokenStream::from(quote! {
quote! {
#get_type_registration_impl

#typed_impl
Expand Down Expand Up @@ -209,5 +208,5 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream {

#debug_fn
}
})
}
}
7 changes: 3 additions & 4 deletions crates/bevy_reflect/bevy_reflect_derive/src/impls/values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@ use crate::fq_std::{FQAny, FQBox, FQClone, FQOption, FQResult};
use crate::impls::impl_typed;
use crate::utility::WhereClauseOptions;
use crate::ReflectMeta;
use proc_macro::TokenStream;
use quote::quote;

/// Implements `GetTypeRegistration` and `Reflect` for the given type data.
pub(crate) fn impl_value(meta: &ReflectMeta) -> TokenStream {
pub(crate) fn impl_value(meta: &ReflectMeta) -> proc_macro2::TokenStream {
let bevy_reflect_path = meta.bevy_reflect_path();
let type_name = meta.type_name();

Expand Down Expand Up @@ -37,7 +36,7 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> TokenStream {
let (impl_generics, ty_generics, where_clause) = meta.generics().split_for_impl();
let get_type_registration_impl = meta.get_type_registration(&where_clause_options);

TokenStream::from(quote! {
quote! {
#get_type_registration_impl

#typed_impl
Expand Down Expand Up @@ -122,5 +121,5 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> TokenStream {

#debug_fn
}
})
}
}
Loading

0 comments on commit 113851b

Please sign in to comment.