From faf003fc9dd4c61380cc07b8868c3803a2aea048 Mon Sep 17 00:00:00 2001 From: Gino Valente <49806985+MrGVSV@users.noreply.github.com> Date: Wed, 22 May 2024 14:18:57 -0700 Subject: [PATCH] bevy_reflect: `enum_utility` cleanup (#13424) # Objective The `enum_utility` module contains the `get_variant_constructors` function, which is used to generate token streams for constructing enums. It's used for the `FromReflect::from_reflect` implementation and the `Reflect::try_apply` implementation. Due to the complexity of enums, this function is understandably a little messy and difficult to extend. ## Solution Clean up the `enum_utility` module. Now "clean" is a bit subjective. I believe my solution is "cleaner" in that the logic to generate the tokens are strictly coupled with the intended usage. Because of this, `try_apply` is also no longer strictly coupled with `from_reflect`. This makes it easier to extend with new functionality, which is something I'm doing in a future unrelated PR that I have based off this one. ## Testing There shouldn't be any testing required other than ensuring that the project still builds and that CI passes. --- .../bevy_reflect/derive/src/enum_utility.rs | 350 ++++++++++++------ .../bevy_reflect/derive/src/from_reflect.rs | 8 +- crates/bevy_reflect/derive/src/impls/enums.rs | 7 +- 3 files changed, 256 insertions(+), 109 deletions(-) diff --git a/crates/bevy_reflect/derive/src/enum_utility.rs b/crates/bevy_reflect/derive/src/enum_utility.rs index c69a4ec4e2..9f7a12aa70 100644 --- a/crates/bevy_reflect/derive/src/enum_utility.rs +++ b/crates/bevy_reflect/derive/src/enum_utility.rs @@ -1,122 +1,266 @@ use crate::derive_data::StructField; use crate::field_attributes::DefaultBehavior; -use crate::{ - derive_data::{EnumVariantFields, ReflectEnum}, - utility::ident_or_index, -}; +use crate::{derive_data::ReflectEnum, utility::ident_or_index}; use bevy_macro_utils::fq_std::{FQDefault, FQOption}; -use proc_macro2::Ident; -use quote::{quote, ToTokens}; -use syn::Member; +use proc_macro2::{Ident, TokenStream}; +use quote::{format_ident, quote}; -/// Contains all data needed to construct all variants within an enum. -pub(crate) struct EnumVariantConstructors { +pub(crate) struct EnumVariantOutputData { /// The names of each variant as a string. + /// + /// For example, `Some` and `None` for the `Option` enum. pub variant_names: Vec, - /// The stream of tokens that will construct each variant. - pub variant_constructors: Vec, + /// The constructor portion of each variant. + /// + /// For example, `Option::Some { 0: value }` and `Option::None {}` for the `Option` enum. + pub variant_constructors: Vec, } -/// Gets the constructors for all variants in the given enum. -pub(crate) fn get_variant_constructors( - reflect_enum: &ReflectEnum, - ref_value: &Ident, - return_apply_error: bool, -) -> EnumVariantConstructors { - let bevy_reflect_path = reflect_enum.meta().bevy_reflect_path(); - let variant_count = reflect_enum.variants().len(); - let mut variant_names = Vec::with_capacity(variant_count); - let mut variant_constructors = Vec::with_capacity(variant_count); +#[derive(Copy, Clone)] +pub(crate) struct VariantField<'a, 'b> { + /// The alias for the field. + /// + /// This should be used whenever the field needs to be referenced in a token stream. + pub alias: &'a Ident, + /// The name of the variant that contains the field. + pub variant_name: &'a str, + /// The field data. + pub field: &'a StructField<'b>, +} - for variant in reflect_enum.variants() { - let ident = &variant.data.ident; - let name = ident.to_string(); - let variant_constructor = reflect_enum.get_unit(ident); +/// Trait used to control how enum variants are built. +pub(crate) trait VariantBuilder: Sized { + /// Returns the enum data. + fn reflect_enum(&self) -> &ReflectEnum; - let fields: &[StructField] = match &variant.fields { - EnumVariantFields::Unit => &[], - EnumVariantFields::Named(fields) | EnumVariantFields::Unnamed(fields) => { - fields.as_slice() + /// Returns a token stream that accesses a field of a variant as an `Option`. + /// + /// The default implementation of this method will return a token stream + /// which gets the field dynamically so as to support `dyn Enum`. + /// + /// # Parameters + /// * `this`: The identifier of the enum + /// * `field`: The field to access + fn access_field(&self, this: &Ident, field: VariantField) -> TokenStream { + match &field.field.data.ident { + Some(field_ident) => { + let name = field_ident.to_string(); + quote!(#this.field(#name)) } - }; - let mut reflect_index: usize = 0; - let constructor_fields = fields.iter().enumerate().map(|(declare_index, field)| { - let field_ident = ident_or_index(field.data.ident.as_ref(), declare_index); - let field_ty = &field.data.ty; - - let field_value = if field.attrs.ignore.is_ignored() { - match &field.attrs.default { - DefaultBehavior::Func(path) => quote! { #path() }, - _ => quote! { #FQDefault::default() } - } - } else { - let field_accessor = match &field.data.ident { - Some(ident) => { - let name = ident.to_string(); - quote!(#ref_value.field(#name)) - } - None => quote!(#ref_value.field_at(#reflect_index)), - }; - reflect_index += 1; - - let (resolve_error, resolve_missing) = if return_apply_error { - let field_ref_str = match &field_ident { - Member::Named(ident) => format!("{ident}"), - Member::Unnamed(index) => format!(".{}", index.index) - }; - let ty = field.data.ty.to_token_stream(); - - ( - quote!(.ok_or(#bevy_reflect_path::ApplyError::MismatchedTypes { - // The unwrap won't panic. By this point the #field_accessor would have been invoked once and any failure to - // access the given field handled by the `resolve_missing` code bellow. - from_type: ::core::convert::Into::into( - #bevy_reflect_path::DynamicTypePath::reflect_type_path(#FQOption::unwrap(#field_accessor)) - ), - to_type: ::core::convert::Into::into(<#ty as #bevy_reflect_path::TypePath>::type_path()) - })?), - quote!(.ok_or(#bevy_reflect_path::ApplyError::MissingEnumField { - variant_name: ::core::convert::Into::into(#name), - field_name: ::core::convert::Into::into(#field_ref_str) - })?) - ) + None => { + if let Some(field_index) = field.field.reflection_index { + quote!(#this.field_at(#field_index)) } else { - (quote!(?), quote!(?)) + quote!(::core::compile_error!( + "internal bevy_reflect error: field should be active" + )) + } + } + } + } + + /// Returns a token stream that unwraps a field of a variant as a `&dyn Reflect` + /// (from an `Option`). + /// + /// # Parameters + /// * `field`: The field to access + fn unwrap_field(&self, field: VariantField) -> TokenStream; + + /// Returns a token stream that constructs a field of a variant as a concrete type + /// (from a `&dyn Reflect`). + /// + /// # Parameters + /// * `field`: The field to access + fn construct_field(&self, field: VariantField) -> TokenStream; + + /// Returns a token stream that constructs an instance of an active field. + /// + /// # Parameters + /// * `this`: The identifier of the enum + /// * `field`: The field to access + fn on_active_field(&self, this: &Ident, field: VariantField) -> TokenStream { + let field_accessor = self.access_field(this, field); + + let alias = field.alias; + let field_constructor = self.construct_field(field); + + match &field.field.attrs.default { + DefaultBehavior::Func(path) => quote! { + if let #FQOption::Some(#alias) = #field_accessor { + #field_constructor + } else { + #path() + } + }, + DefaultBehavior::Default => quote! { + if let #FQOption::Some(#alias) = #field_accessor { + #field_constructor + } else { + #FQDefault::default() + } + }, + DefaultBehavior::Required => { + let field_unwrapper = self.unwrap_field(field); + + quote! {{ + // `#alias` is used by both the unwrapper and constructor + let #alias = #field_accessor; + let #alias = #field_unwrapper; + #field_constructor + }} + } + } + } + + /// Returns a token stream that constructs an instance of an ignored field. + /// + /// # Parameters + /// * `field`: The field to access + fn on_ignored_field(&self, field: VariantField) -> TokenStream { + match &field.field.attrs.default { + DefaultBehavior::Func(path) => quote! { #path() }, + _ => quote! { #FQDefault::default() }, + } + } + + /// Builds the enum variant output data. + fn build(&self, this: &Ident) -> EnumVariantOutputData { + let variants = self.reflect_enum().variants(); + + let mut variant_names = Vec::with_capacity(variants.len()); + let mut variant_constructors = Vec::with_capacity(variants.len()); + + for variant in variants { + let variant_ident = &variant.data.ident; + let variant_name = variant_ident.to_string(); + let variant_path = self.reflect_enum().get_unit(variant_ident); + + let fields = variant.fields(); + + let field_constructors = fields.iter().map(|field| { + let member = ident_or_index(field.data.ident.as_ref(), field.declaration_index); + let alias = format_ident!("_{}", member); + + let variant_field = VariantField { + alias: &alias, + variant_name: &variant_name, + field, }; - match &field.attrs.default { - DefaultBehavior::Func(path) => quote! { - if let #FQOption::Some(field) = #field_accessor { - <#field_ty as #bevy_reflect_path::FromReflect>::from_reflect(field) - #resolve_error - } else { - #path() - } - }, - DefaultBehavior::Default => quote! { - if let #FQOption::Some(field) = #field_accessor { - <#field_ty as #bevy_reflect_path::FromReflect>::from_reflect(field) - #resolve_error - } else { - #FQDefault::default() - } - }, - DefaultBehavior::Required => quote! { - <#field_ty as #bevy_reflect_path::FromReflect>::from_reflect(#field_accessor #resolve_missing) - #resolve_error - }, + let value = if field.attrs.ignore.is_ignored() { + self.on_ignored_field(variant_field) + } else { + self.on_active_field(this, variant_field) + }; + + let constructor = quote! { + #member: #value + }; + + constructor + }); + + let constructor = quote! { + #variant_path { + #( #field_constructors ),* } }; - quote! { #field_ident : #field_value } - }); - variant_constructors.push(quote! { - #variant_constructor { #( #constructor_fields ),* } - }); - variant_names.push(name); - } - EnumVariantConstructors { - variant_names, - variant_constructors, + variant_names.push(variant_name); + variant_constructors.push(constructor); + } + + EnumVariantOutputData { + variant_names, + variant_constructors, + } + } +} + +/// Generates the enum variant output data needed to build the `FromReflect::from_reflect` implementation. +pub(crate) struct FromReflectVariantBuilder<'a> { + reflect_enum: &'a ReflectEnum<'a>, +} + +impl<'a> FromReflectVariantBuilder<'a> { + pub fn new(reflect_enum: &'a ReflectEnum) -> Self { + Self { reflect_enum } + } +} + +impl<'a> VariantBuilder for FromReflectVariantBuilder<'a> { + fn reflect_enum(&self) -> &ReflectEnum { + self.reflect_enum + } + + fn unwrap_field(&self, field: VariantField) -> TokenStream { + let alias = field.alias; + quote!(#alias?) + } + + 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 alias = field.alias; + + quote! { + <#field_ty as #bevy_reflect_path::FromReflect>::from_reflect(#alias)? + } + } +} + +/// Generates the enum variant output data needed to build the `Reflect::try_apply` implementation. +pub(crate) struct TryApplyVariantBuilder<'a> { + reflect_enum: &'a ReflectEnum<'a>, +} + +impl<'a> TryApplyVariantBuilder<'a> { + pub fn new(reflect_enum: &'a ReflectEnum) -> Self { + Self { reflect_enum } + } +} + +impl<'a> VariantBuilder for TryApplyVariantBuilder<'a> { + fn reflect_enum(&self) -> &ReflectEnum { + self.reflect_enum + } + + fn unwrap_field(&self, field: VariantField) -> TokenStream { + let VariantField { + alias, + variant_name, + field, + .. + } = field; + + let bevy_reflect_path = self.reflect_enum.meta().bevy_reflect_path(); + + let field_name = match &field.data.ident { + Some(ident) => format!("{ident}"), + None => format!(".{}", field.declaration_index), + }; + + quote! { + #alias.ok_or(#bevy_reflect_path::ApplyError::MissingEnumField { + variant_name: ::core::convert::Into::into(#variant_name), + field_name: ::core::convert::Into::into(#field_name) + })? + } + } + + 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; + + quote! { + <#field_ty as #bevy_reflect_path::FromReflect>::from_reflect(#alias) + .ok_or(#bevy_reflect_path::ApplyError::MismatchedTypes { + from_type: ::core::convert::Into::into( + #bevy_reflect_path::DynamicTypePath::reflect_type_path(#alias) + ), + to_type: ::core::convert::Into::into(<#field_ty as #bevy_reflect_path::TypePath>::type_path()) + })? + } } } diff --git a/crates/bevy_reflect/derive/src/from_reflect.rs b/crates/bevy_reflect/derive/src/from_reflect.rs index a4559cf81c..abc7fb8929 100644 --- a/crates/bevy_reflect/derive/src/from_reflect.rs +++ b/crates/bevy_reflect/derive/src/from_reflect.rs @@ -1,6 +1,6 @@ use crate::container_attributes::REFLECT_DEFAULT; use crate::derive_data::ReflectEnum; -use crate::enum_utility::{get_variant_constructors, EnumVariantConstructors}; +use crate::enum_utility::{EnumVariantOutputData, FromReflectVariantBuilder, VariantBuilder}; use crate::field_attributes::DefaultBehavior; use crate::utility::{ident_or_index, WhereClauseOptions}; use crate::{ReflectMeta, ReflectStruct}; @@ -41,10 +41,12 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream let bevy_reflect_path = reflect_enum.meta().bevy_reflect_path(); let ref_value = Ident::new("__param0", Span::call_site()); - let EnumVariantConstructors { + + let EnumVariantOutputData { variant_names, variant_constructors, - } = get_variant_constructors(reflect_enum, &ref_value, false); + .. + } = FromReflectVariantBuilder::new(reflect_enum).build(&ref_value); let (impl_generics, ty_generics, where_clause) = enum_path.generics().split_for_impl(); diff --git a/crates/bevy_reflect/derive/src/impls/enums.rs b/crates/bevy_reflect/derive/src/impls/enums.rs index ab72539a71..42162717a4 100644 --- a/crates/bevy_reflect/derive/src/impls/enums.rs +++ b/crates/bevy_reflect/derive/src/impls/enums.rs @@ -1,5 +1,5 @@ use crate::derive_data::{EnumVariantFields, ReflectEnum, StructField}; -use crate::enum_utility::{get_variant_constructors, EnumVariantConstructors}; +use crate::enum_utility::{EnumVariantOutputData, TryApplyVariantBuilder, VariantBuilder}; use crate::impls::{impl_type_path, impl_typed}; use bevy_macro_utils::fq_std::{FQAny, FQBox, FQOption, FQResult}; use proc_macro2::{Ident, Span}; @@ -27,10 +27,11 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream enum_variant_type, } = generate_impls(reflect_enum, &ref_index, &ref_name); - let EnumVariantConstructors { + let EnumVariantOutputData { variant_names, variant_constructors, - } = get_variant_constructors(reflect_enum, &ref_value, true); + .. + } = TryApplyVariantBuilder::new(reflect_enum).build(&ref_value); let hash_fn = reflect_enum .meta()