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 Jun 9, 2023
1 parent 0c13ca5 commit 4ce56a6
Show file tree
Hide file tree
Showing 16 changed files with 564 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 = "2.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);
}
}
78 changes: 61 additions & 17 deletions crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
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::type_path::parse_path_no_leading_colon;
use crate::utility::{members_to_serialization_denylist, StringExpr, WhereClauseOptions};
use bit_set::BitSet;
use crate::utility::{StringExpr, WhereClauseOptions};
use quote::{quote, ToTokens};
use syn::token::Comma;

use crate::serialization::SerializationDataDef;
use crate::{
utility, REFLECT_ATTRIBUTE_NAME, REFLECT_VALUE_ATTRIBUTE_NAME, TYPE_NAME_ATTRIBUTE_NAME,
TYPE_PATH_ATTRIBUTE_NAME,
Expand Down Expand Up @@ -66,7 +67,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 @@ -94,6 +95,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 @@ -269,12 +272,16 @@ 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_path()
.get_ident()
.expect("structs should never be anonymous"),
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 @@ -285,7 +292,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 @@ -307,14 +315,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 @@ -327,6 +346,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 @@ -343,12 +363,17 @@ impl<'a> ReflectDerive<'a> {

fn collect_enum_variants(
variants: &'a Punctuated<Variant, Comma>,
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 @@ -371,6 +396,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 @@ -426,12 +468,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 @@ -444,7 +483,7 @@ impl<'a> ReflectStruct<'a> {
crate::registration::impl_get_type_registration(
self.meta(),
where_clause_options,
Some(&self.serialization_denylist),
self.serialization_data(),
)
}

Expand Down Expand Up @@ -513,6 +552,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_type_path, 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_path = reflect_enum.meta().type_path();

Expand Down Expand Up @@ -97,7 +96,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 @@ -291,7 +290,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream {

#debug_fn
}
})
}
}

struct EnumImpls {
Expand Down Expand Up @@ -354,7 +353,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_type_path, 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 @@ -104,7 +103,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 @@ -245,5 +244,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_type_path, 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 @@ -95,7 +94,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 @@ -214,5 +213,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_type_path, impl_typed};
use crate::utility::{extend_where_clause, 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_path = meta.type_path();

Expand Down Expand Up @@ -38,7 +37,7 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> TokenStream {
let where_reflect_clause = extend_where_clause(where_clause, &where_clause_options);
let get_type_registration_impl = meta.get_type_registration(&where_clause_options);

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

#type_path_impl
Expand Down Expand Up @@ -125,5 +124,5 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> TokenStream {

#debug_fn
}
})
}
}
Loading

0 comments on commit 4ce56a6

Please sign in to comment.