From cf1c27eb4985d3a5ef5fc57f9fd21869f6f6c170 Mon Sep 17 00:00:00 2001 From: Nicola Papale Date: Thu, 23 Jun 2022 11:24:55 +0200 Subject: [PATCH 1/3] Use field access syntax in enum macros Using struct expression and patterns is allowed in rust for tuple structs. As per references. This allows us to simplify some of the macros, and remove `underscores` from the utility module. - https://play.rust-lang.org/?gist=fd4cc2283fff6a6aade5dbe9427e44db - https://doc.rust-lang.org/reference/expressions/struct-expr.html - https://doc.rust-lang.org/reference/patterns.html#struct-patterns --- .../bevy_reflect_derive/src/derive_data.rs | 30 ++-- .../bevy_reflect_derive/src/enum_utility.rs | 151 +++++++----------- .../bevy_reflect_derive/src/impls/enums.rs | 144 +++++++---------- .../bevy_reflect_derive/src/utility.rs | 25 --- 4 files changed, 122 insertions(+), 228 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs index dfbedecafc1fa..3eb9bed751857 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -187,28 +187,18 @@ impl<'a> ReflectDerive<'a> { .iter() .enumerate() .map(|(index, variant)| -> Result { - let attrs = parse_field_attrs(&variant.attrs)?; let fields = Self::collect_struct_fields(&variant.fields)?; - Ok(match variant.fields { - Fields::Named(..) => EnumVariant { - data: variant, - fields: EnumVariantFields::Named(fields), - attrs, - index, - }, - Fields::Unnamed(..) => EnumVariant { - data: variant, - fields: EnumVariantFields::Unnamed(fields), - attrs, - index, - }, - Fields::Unit => EnumVariant { - data: variant, - fields: EnumVariantFields::Unit, - attrs, - index, - }, + let fields = match variant.fields { + Fields::Named(..) => EnumVariantFields::Named(fields), + Fields::Unnamed(..) => EnumVariantFields::Unnamed(fields), + Fields::Unit => EnumVariantFields::Unit, + }; + Ok(EnumVariant { + fields, + attrs: parse_field_attrs(&variant.attrs)?, + data: variant, + index, }) }) .fold( diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs b/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs index 64ee3bbdc8396..f7410e675eacf 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs @@ -1,6 +1,7 @@ use crate::derive_data::{EnumVariantFields, ReflectEnum}; -use proc_macro2::Ident; +use proc_macro2::{Ident, TokenStream}; use quote::{quote, ToTokens}; +use syn::Index; /// Contains all data needed to construct all variants within an enum. pub(crate) struct EnumVariantConstructors { @@ -10,6 +11,13 @@ pub(crate) struct EnumVariantConstructors { pub variant_constructors: Vec, } +fn field_indentifier(i: usize, ident: Option<&Ident>) -> TokenStream { + let tuple_accessor = Index::from(i); + match ident { + Some(ident) => quote!(#ident :), + None => quote!(#tuple_accessor :), + } +} /// Gets the constructors for all variants in the given enum. pub(crate) fn get_variant_constructors( reflect_enum: &ReflectEnum, @@ -17,106 +25,63 @@ pub(crate) fn get_variant_constructors( can_panic: bool, ) -> EnumVariantConstructors { let bevy_reflect_path = reflect_enum.meta().bevy_reflect_path(); - let mut variant_names: Vec = Vec::new(); - let mut variant_constructors: Vec = Vec::new(); + let variant_count = reflect_enum.variants().len(); + let mut variant_names: Vec = Vec::with_capacity(variant_count); + let mut variant_constructors: Vec = Vec::with_capacity(variant_count); for variant in reflect_enum.active_variants() { let ident = &variant.data.ident; let name = ident.to_string(); - let unit = reflect_enum.get_unit(ident); - - match &variant.fields { - EnumVariantFields::Unit => { - variant_constructors.push(quote! { - #unit - }); - } - EnumVariantFields::Unnamed(fields) => { - let mut variant_apply = Vec::new(); - let mut field_idx: usize = 0; - for field in fields.iter() { - if field.attrs.ignore { - // Ignored field -> use default value - variant_apply.push(quote! { - Default::default() - }); - continue; - } - - let field_ty = &field.data.ty; - let expect_field = format!("field at index `{}` should exist", field_idx); + let variant_constructor = reflect_enum.get_unit(ident); + + let fields = match &variant.fields { + EnumVariantFields::Unit => &[], + EnumVariantFields::Named(fields) => fields.as_slice(), + EnumVariantFields::Unnamed(fields) => fields.as_slice(), + }; + let mut reflect_index: usize = 0; + let constructor_fields = fields.iter().enumerate().map(|(declar_index, field)| { + let field_ident = field_indentifier(declar_index, field.data.ident.as_ref()); + let field_value = if field.attrs.ignore { + quote! { Default::default() } + } else { + let error_repr = match (&field.data.ident, reflect_index) { + (None, 0) => "1st".to_owned(), + (None, 1) => "2nd".to_owned(), + (None, 2) => "3rd".to_owned(), + // Assuming we have less than 21 fields + (None, n) => format!("{}th", n + 1), + (Some(name), _) => format!("`{name}`"), + }; + let unwrapper = if can_panic { let expect_type = format!( - "field at index `{}` should be of type `{}`", - field_idx, - field_ty.to_token_stream() + "the {error_repr} field should be of type `{}`", + field.data.ty.to_token_stream() ); - - let unwrapper = if can_panic { - quote!(.expect(#expect_type)) - } else { - quote!(?) - }; - - variant_apply.push(quote! { - #bevy_reflect_path::FromReflect::from_reflect( - #ref_value - .field_at(#field_idx) - .expect(#expect_field) - ) - #unwrapper - }); - - field_idx += 1; - } - - variant_constructors.push(quote! { - #unit( #(#variant_apply),* ) - }); - } - EnumVariantFields::Named(fields) => { - let mut variant_apply = Vec::new(); - for field in fields.iter() { - let field_ident = field.data.ident.as_ref().unwrap(); - - if field.attrs.ignore { - // Ignored field -> use default value - variant_apply.push(quote! { - #field_ident: Default::default() - }); - continue; + quote!(.expect(#expect_type)) + } else { + quote!(?) + }; + let field_accessor = match &field.data.ident { + Some(ident) => { + let name = ident.to_string(); + quote!(.field(#name)) } - - let field_name = field_ident.to_string(); - let field_ty = &field.data.ty; - let expect_field = format!("field with name `{}` should exist", field_name); - let expect_type = format!( - "field with name `{}` should be of type `{}`", - field_name, - field_ty.to_token_stream() - ); - - let unwrapper = if can_panic { - quote!(.expect(#expect_type)) - } else { - quote!(?) - }; - - variant_apply.push(quote! { - #field_ident: #bevy_reflect_path::FromReflect::from_reflect( - #ref_value - .field(#field_name) - .expect(#expect_field) - ) - #unwrapper - }); + None => quote!(.field_at(#reflect_index)), + }; + reflect_index += 1; + let expect_field = format!("the {error_repr} field was not declared"); + let accessor = quote!(#field_accessor .expect(#expect_field)); + quote! { + #bevy_reflect_path::FromReflect::from_reflect(#ref_value #accessor) + #unwrapper } - - variant_constructors.push(quote! { - #unit{ #(#variant_apply),* } - }); - } - } - + }; + quote! { #field_ident #field_value } + }); + variant_constructors.push(quote! { + #variant_constructor { #( #constructor_fields ),* } + }); variant_names.push(name); } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs b/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs index 364c0531bd191..33c9adcd23b51 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs @@ -1,10 +1,9 @@ use crate::derive_data::{EnumVariantFields, ReflectEnum}; use crate::enum_utility::{get_variant_constructors, EnumVariantConstructors}; use crate::impls::impl_typed; -use crate::utility; use proc_macro::TokenStream; use proc_macro2::{Ident, Span}; -use quote::quote; +use quote::{format_ident, quote}; pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream { let bevy_reflect_path = reflect_enum.meta().bevy_reflect_path(); @@ -262,126 +261,91 @@ struct EnumImpls { fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Ident) -> EnumImpls { let bevy_reflect_path = reflect_enum.meta().bevy_reflect_path(); - let mut variant_info: Vec = Vec::new(); - let mut enum_field: Vec = Vec::new(); - let mut enum_field_at: Vec = Vec::new(); - let mut enum_index_of: Vec = Vec::new(); - let mut enum_name_at: Vec = Vec::new(); - let mut enum_field_len: Vec = Vec::new(); - let mut enum_variant_name: Vec = Vec::new(); - let mut enum_variant_type: Vec = Vec::new(); + let mut variant_info = Vec::new(); + let mut enum_field = Vec::new(); + let mut enum_field_at = Vec::new(); + let mut enum_index_of = Vec::new(); + let mut enum_name_at = Vec::new(); + let mut enum_field_len = Vec::new(); + let mut enum_variant_name = Vec::new(); + let mut enum_variant_type = Vec::new(); for variant in reflect_enum.active_variants() { let ident = &variant.data.ident; let name = ident.to_string(); let unit = reflect_enum.get_unit(ident); - match &variant.fields { - EnumVariantFields::Unit => { - variant_info.push(quote! { - #bevy_reflect_path::VariantInfo::Unit( - #bevy_reflect_path::UnitVariantInfo::new(#name) - ) - }); - enum_variant_name.push(quote! { - #unit => #name - }); - enum_variant_type.push(quote! { - #unit => #bevy_reflect_path::VariantType::Unit - }); - } - EnumVariantFields::Unnamed(fields) => { + // This is equivalent to a |fields: &Vec, to_run: |usize, usize, &StructField| -> TokenStream| + // closure. Issue is that the closure cannot itself accept another closure, because closures cannot vary in + // the concrete type they accept, and each closure has a different concrete type. + macro_rules! for_fields { + ($fields:expr, |$reflect_idx:ident, $declar_field:pat, $field:ident| $body:expr) => {{ let mut field_info = Vec::new(); - let mut field_idx: usize = 0; - for field in fields.iter() { - if field.attrs.ignore { + let mut $reflect_idx: usize = 0; + for ($declar_field, $field) in $fields.iter().enumerate() { + if $field.attrs.ignore { // Ignored field continue; } + field_info.push($body); + $reflect_idx += 1; + } + ($reflect_idx, field_info) + }}; + } + let (variant_type, field_len, field_info) = match &variant.fields { + EnumVariantFields::Unit => ("Unit", 0usize, quote!(#name)), - let empties = utility::underscores(field_idx); + EnumVariantFields::Unnamed(fields) => { + let (field_len, field_info) = for_fields!(fields, |reflect_idx, declar, field| { + let declar_field = syn::Index::from(declar); enum_field_at.push(quote! { - #unit( #empties value, .. ) if #ref_index == #field_idx => Some(value) + #unit { #declar_field : value, .. } if #ref_index == #reflect_idx => Some(value) }); - let field_ty = &field.data.ty; - field_info.push(quote! { - #bevy_reflect_path::UnnamedField::new::<#field_ty>(#field_idx) - }); - - field_idx += 1; - } - - let field_len = field_idx; - enum_field_len.push(quote! { - #unit(..) => #field_len - }); - enum_variant_name.push(quote! { - #unit(..) => #name - }); - enum_variant_type.push(quote! { - #unit(..) => #bevy_reflect_path::VariantType::Tuple - }); - variant_info.push(quote! { - #bevy_reflect_path::VariantInfo::Tuple( - #bevy_reflect_path::TupleVariantInfo::new(#name, &[ - #(#field_info),* - ]) - ) + quote! { #bevy_reflect_path::UnnamedField::new::<#field_ty>(#reflect_idx) } }); + ("Tuple", field_len, quote!(#name, &[ #(#field_info),* ])) } EnumVariantFields::Named(fields) => { - let mut field_info = Vec::new(); - let mut field_idx: usize = 0; - for field in fields.iter() { + let (field_len, field_info) = for_fields!(fields, |reflect_idx, _, field| { let field_ident = field.data.ident.as_ref().unwrap(); - - if field.attrs.ignore { - // Ignored field - continue; - } - let field_name = field_ident.to_string(); enum_field.push(quote! { #unit{ #field_ident, .. } if #ref_name == #field_name => Some(#field_ident) }); enum_field_at.push(quote! { - #unit{ #field_ident, .. } if #ref_index == #field_idx => Some(#field_ident) + #unit{ #field_ident, .. } if #ref_index == #reflect_idx => Some(#field_ident) }); enum_index_of.push(quote! { - #unit{ .. } if #ref_name == #field_name => Some(#field_idx) + #unit{ .. } if #ref_name == #field_name => Some(#reflect_idx) }); enum_name_at.push(quote! { - #unit{ .. } if #ref_index == #field_idx => Some(#field_name) + #unit{ .. } if #ref_index == #reflect_idx => Some(#field_name) }); let field_ty = &field.data.ty; - field_info.push(quote! { - #bevy_reflect_path::NamedField::new::<#field_ty, _>(#field_name) - }); - - field_idx += 1; - } - - let field_len = field_idx; - enum_field_len.push(quote! { - #unit{..} => #field_len - }); - enum_variant_name.push(quote! { - #unit{..} => #name - }); - enum_variant_type.push(quote! { - #unit{..} => #bevy_reflect_path::VariantType::Struct - }); - variant_info.push(quote! { - #bevy_reflect_path::VariantInfo::Struct( - #bevy_reflect_path::StructVariantInfo::new(#name, &[ - #(#field_info),* - ]) - ) + quote! { #bevy_reflect_path::NamedField::new::<#field_ty, _>(#field_name) } }); + ("Struct", field_len, quote!(#name, &[ #(#field_info),* ])) } - } + }; + let variant = Ident::new(variant_type, Span::call_site()); + let info_type = format_ident!("{}VariantInfo", variant_type); + variant_info.push(quote! { + #bevy_reflect_path::VariantInfo::#variant( + #bevy_reflect_path::#info_type::new(#field_info) + ) + }); + enum_field_len.push(quote! { + #unit{..} => #field_len + }); + enum_variant_name.push(quote! { + #unit{..} => #name + }); + enum_variant_type.push(quote! { + #unit{..} => #bevy_reflect_path::VariantType::#variant + }); } EnumImpls { diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs index 3e2a88e03caa3..a03541e934e26 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs @@ -2,7 +2,6 @@ use bevy_macro_utils::BevyManifest; use proc_macro2::{Ident, Span}; -use quote::quote; use syn::Path; /// Returns the correct path for `bevy_reflect`. @@ -23,30 +22,6 @@ pub(crate) fn get_reflect_ident(name: &str) -> Ident { Ident::new(&reflected, Span::call_site()) } -/// Returns a token stream of comma-separated underscores for the given count. -/// -/// This is useful for creating tuple field accessors: -/// -/// ```ignore -/// let empties = underscores(2); -/// quote! { -/// let (#empties value, ..) = (10, 20, 30); -/// assert_eq!(30, value); -/// } -/// ``` -/// -/// > Note: This automatically handles the trailing comma. -/// -pub(crate) fn underscores(count: usize) -> proc_macro2::TokenStream { - let mut output = proc_macro2::TokenStream::new(); - for _ in 0..count { - output = quote! { - #output _, - } - } - output -} - /// Helper struct used to process an iterator of `Result, syn::Error>`, /// combining errors into one along the way. pub(crate) struct ResultSifter { From 0cf20290bee1ed5d6cfe0df418cdc24e8f3832e0 Mon Sep 17 00:00:00 2001 From: Nicola Papale Date: Sun, 26 Jun 2022 11:43:43 +0200 Subject: [PATCH 2/3] Clenaup a bit more the bevy_reflect enum code --- .../bevy_reflect_derive/src/enum_utility.rs | 45 +++++------- .../bevy_reflect_derive/src/impls/enums.rs | 70 +++++++++++-------- .../bevy_reflect_derive/src/utility.rs | 9 ++- 3 files changed, 65 insertions(+), 59 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs b/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs index f7410e675eacf..230e0876c6b6c 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs @@ -1,7 +1,9 @@ -use crate::derive_data::{EnumVariantFields, ReflectEnum}; -use proc_macro2::{Ident, TokenStream}; +use crate::{ + derive_data::{EnumVariantFields, ReflectEnum}, + utility::field_ident_or_indexed, +}; +use proc_macro2::Ident; use quote::{quote, ToTokens}; -use syn::Index; /// Contains all data needed to construct all variants within an enum. pub(crate) struct EnumVariantConstructors { @@ -11,13 +13,6 @@ pub(crate) struct EnumVariantConstructors { pub variant_constructors: Vec, } -fn field_indentifier(i: usize, ident: Option<&Ident>) -> TokenStream { - let tuple_accessor = Index::from(i); - match ident { - Some(ident) => quote!(#ident :), - None => quote!(#tuple_accessor :), - } -} /// Gets the constructors for all variants in the given enum. pub(crate) fn get_variant_constructors( reflect_enum: &ReflectEnum, @@ -26,8 +21,8 @@ pub(crate) fn get_variant_constructors( ) -> EnumVariantConstructors { let bevy_reflect_path = reflect_enum.meta().bevy_reflect_path(); let variant_count = reflect_enum.variants().len(); - let mut variant_names: Vec = Vec::with_capacity(variant_count); - let mut variant_constructors: Vec = Vec::with_capacity(variant_count); + let mut variant_names = Vec::with_capacity(variant_count); + let mut variant_constructors = Vec::with_capacity(variant_count); for variant in reflect_enum.active_variants() { let ident = &variant.data.ident; @@ -41,24 +36,20 @@ pub(crate) fn get_variant_constructors( }; let mut reflect_index: usize = 0; let constructor_fields = fields.iter().enumerate().map(|(declar_index, field)| { - let field_ident = field_indentifier(declar_index, field.data.ident.as_ref()); + let field_ident = field_ident_or_indexed(declar_index, field.data.ident.as_ref()); let field_value = if field.attrs.ignore { quote! { Default::default() } } else { - let error_repr = match (&field.data.ident, reflect_index) { - (None, 0) => "1st".to_owned(), - (None, 1) => "2nd".to_owned(), - (None, 2) => "3rd".to_owned(), - // Assuming we have less than 21 fields - (None, n) => format!("{}th", n + 1), - (Some(name), _) => format!("`{name}`"), - }; + let error_repr = field.data.ident.as_ref().map_or_else( + || format!("at index {reflect_index}"), + |name| format!("`{name}`"), + ); let unwrapper = if can_panic { - let expect_type = format!( - "the {error_repr} field should be of type `{}`", + let type_err_message = format!( + "the field {error_repr} should be of type `{}`", field.data.ty.to_token_stream() ); - quote!(.expect(#expect_type)) + quote!(.expect(#type_err_message)) } else { quote!(?) }; @@ -70,14 +61,14 @@ pub(crate) fn get_variant_constructors( None => quote!(.field_at(#reflect_index)), }; reflect_index += 1; - let expect_field = format!("the {error_repr} field was not declared"); - let accessor = quote!(#field_accessor .expect(#expect_field)); + let missing_field_err_message = format!("the field {error_repr} was not declared"); + let accessor = quote!(#field_accessor .expect(#missing_field_err_message)); quote! { #bevy_reflect_path::FromReflect::from_reflect(#ref_value #accessor) #unwrapper } }; - quote! { #field_ident #field_value } + quote! { #field_ident : #field_value } }); variant_constructors.push(quote! { #variant_constructor { #( #constructor_fields ),* } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs b/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs index 33c9adcd23b51..db1e10bb2fcb4 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs @@ -1,9 +1,9 @@ -use crate::derive_data::{EnumVariantFields, ReflectEnum}; +use crate::derive_data::{EnumVariantFields, ReflectEnum, StructField}; use crate::enum_utility::{get_variant_constructors, EnumVariantConstructors}; use crate::impls::impl_typed; use proc_macro::TokenStream; use proc_macro2::{Ident, Span}; -use quote::{format_ident, quote}; +use quote::quote; pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream { let bevy_reflect_path = reflect_enum.meta().bevy_reflect_path(); @@ -275,29 +275,40 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden let name = ident.to_string(); let unit = reflect_enum.get_unit(ident); - // This is equivalent to a |fields: &Vec, to_run: |usize, usize, &StructField| -> TokenStream| - // closure. Issue is that the closure cannot itself accept another closure, because closures cannot vary in - // the concrete type they accept, and each closure has a different concrete type. - macro_rules! for_fields { - ($fields:expr, |$reflect_idx:ident, $declar_field:pat, $field:ident| $body:expr) => {{ - let mut field_info = Vec::new(); - let mut $reflect_idx: usize = 0; - for ($declar_field, $field) in $fields.iter().enumerate() { - if $field.attrs.ignore { - // Ignored field - continue; - } - field_info.push($body); - $reflect_idx += 1; + fn for_fields( + fields: &[StructField], + mut generate_for_field: impl FnMut(usize, usize, &StructField) -> proc_macro2::TokenStream, + ) -> (usize, Vec) { + let mut constructor_argument = Vec::new(); + let mut reflect_idx = 0; + for field in fields.iter() { + if field.attrs.ignore { + // Ignored field + continue; } - ($reflect_idx, field_info) - }}; + constructor_argument.push(generate_for_field(reflect_idx, field.index, field)); + reflect_idx += 1; + } + (reflect_idx, constructor_argument) } - let (variant_type, field_len, field_info) = match &variant.fields { - EnumVariantFields::Unit => ("Unit", 0usize, quote!(#name)), + let mut info_type = |variant, info_type, arguments| { + let variant = Ident::new(variant, Span::call_site()); + let info_type = Ident::new(info_type, Span::call_site()); + variant_info.push(quote! { + #bevy_reflect_path::VariantInfo::#variant( + #bevy_reflect_path::#info_type::new(#arguments) + ) + }); + variant + }; + let (variant, field_len) = match &variant.fields { + EnumVariantFields::Unit => { + let variant = info_type("Unit", "UnitVariantInfo", quote!(#name)); + (variant, 0usize) + } EnumVariantFields::Unnamed(fields) => { - let (field_len, field_info) = for_fields!(fields, |reflect_idx, declar, field| { + let (field_len, argument) = for_fields(fields, |reflect_idx, declar, field| { let declar_field = syn::Index::from(declar); enum_field_at.push(quote! { #unit { #declar_field : value, .. } if #ref_index == #reflect_idx => Some(value) @@ -305,10 +316,12 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden let field_ty = &field.data.ty; quote! { #bevy_reflect_path::UnnamedField::new::<#field_ty>(#reflect_idx) } }); - ("Tuple", field_len, quote!(#name, &[ #(#field_info),* ])) + let arguments = quote!(#name, &[ #(#argument),* ]); + let variant = info_type("Tuple", "TupleVariantInfo", arguments); + (variant, field_len) } EnumVariantFields::Named(fields) => { - let (field_len, field_info) = for_fields!(fields, |reflect_idx, _, field| { + let (field_len, argument) = for_fields(fields, |reflect_idx, _, field| { let field_ident = field.data.ident.as_ref().unwrap(); let field_name = field_ident.to_string(); enum_field.push(quote! { @@ -327,16 +340,11 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden let field_ty = &field.data.ty; quote! { #bevy_reflect_path::NamedField::new::<#field_ty, _>(#field_name) } }); - ("Struct", field_len, quote!(#name, &[ #(#field_info),* ])) + let arguments = quote!(#name, &[ #(#argument),* ]); + let variant = info_type("Struct", "StructVariantInfo", arguments); + (variant, field_len) } }; - let variant = Ident::new(variant_type, Span::call_site()); - let info_type = format_ident!("{}VariantInfo", variant_type); - variant_info.push(quote! { - #bevy_reflect_path::VariantInfo::#variant( - #bevy_reflect_path::#info_type::new(#field_info) - ) - }); enum_field_len.push(quote! { #unit{..} => #field_len }); diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs index a03541e934e26..9c79f40200fc7 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs @@ -2,7 +2,7 @@ use bevy_macro_utils::BevyManifest; use proc_macro2::{Ident, Span}; -use syn::Path; +use syn::{Member, Path}; /// Returns the correct path for `bevy_reflect`. pub(crate) fn get_bevy_reflect_path() -> Path { @@ -29,6 +29,13 @@ pub(crate) struct ResultSifter { errors: Option, } +pub(crate) fn field_ident_or_indexed(index: usize, ident: Option<&Ident>) -> Member { + ident.as_ref().map_or_else( + || Member::Unnamed(index.into()), + |&ident| Member::Named(ident.clone()), + ) +} + impl Default for ResultSifter { fn default() -> Self { Self { From 9f1036017e925269d206af1872d89c71de1d3161 Mon Sep 17 00:00:00 2001 From: Nicola Papale Date: Sun, 26 Jun 2022 19:57:57 +0200 Subject: [PATCH 3/3] Cleanup bevy_reflect a bit more --- .../bevy_reflect_derive/src/enum_utility.rs | 4 +-- .../bevy_reflect_derive/src/impls/enums.rs | 33 ++++++++----------- .../bevy_reflect_derive/src/utility.rs | 27 +++++++++++++-- 3 files changed, 40 insertions(+), 24 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs b/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs index 230e0876c6b6c..af6037f6c563c 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs @@ -1,6 +1,6 @@ use crate::{ derive_data::{EnumVariantFields, ReflectEnum}, - utility::field_ident_or_indexed, + utility::ident_or_index, }; use proc_macro2::Ident; use quote::{quote, ToTokens}; @@ -36,7 +36,7 @@ pub(crate) fn get_variant_constructors( }; let mut reflect_index: usize = 0; let constructor_fields = fields.iter().enumerate().map(|(declar_index, field)| { - let field_ident = field_ident_or_indexed(declar_index, field.data.ident.as_ref()); + let field_ident = ident_or_index(field.data.ident.as_ref(), declar_index); let field_value = if field.attrs.ignore { quote! { Default::default() } } else { diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs b/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs index db1e10bb2fcb4..3c982f55f8c61 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs @@ -291,7 +291,7 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden } (reflect_idx, constructor_argument) } - let mut info_type = |variant, info_type, arguments| { + let mut add_fields_branch = |variant, info_type, arguments, field_len| { let variant = Ident::new(variant, Span::call_site()); let info_type = Ident::new(info_type, Span::call_site()); variant_info.push(quote! { @@ -299,14 +299,20 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden #bevy_reflect_path::#info_type::new(#arguments) ) }); - variant + enum_field_len.push(quote! { + #unit{..} => #field_len + }); + enum_variant_name.push(quote! { + #unit{..} => #name + }); + enum_variant_type.push(quote! { + #unit{..} => #bevy_reflect_path::VariantType::#variant + }); }; - let (variant, field_len) = match &variant.fields { + match &variant.fields { EnumVariantFields::Unit => { - let variant = info_type("Unit", "UnitVariantInfo", quote!(#name)); - (variant, 0usize) + add_fields_branch("Unit", "UnitVariantInfo", quote!(#name), 0usize); } - EnumVariantFields::Unnamed(fields) => { let (field_len, argument) = for_fields(fields, |reflect_idx, declar, field| { let declar_field = syn::Index::from(declar); @@ -317,8 +323,7 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden quote! { #bevy_reflect_path::UnnamedField::new::<#field_ty>(#reflect_idx) } }); let arguments = quote!(#name, &[ #(#argument),* ]); - let variant = info_type("Tuple", "TupleVariantInfo", arguments); - (variant, field_len) + add_fields_branch("Tuple", "TupleVariantInfo", arguments, field_len); } EnumVariantFields::Named(fields) => { let (field_len, argument) = for_fields(fields, |reflect_idx, _, field| { @@ -341,19 +346,9 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden quote! { #bevy_reflect_path::NamedField::new::<#field_ty, _>(#field_name) } }); let arguments = quote!(#name, &[ #(#argument),* ]); - let variant = info_type("Struct", "StructVariantInfo", arguments); - (variant, field_len) + add_fields_branch("Struct", "StructVariantInfo", arguments, field_len); } }; - enum_field_len.push(quote! { - #unit{..} => #field_len - }); - enum_variant_name.push(quote! { - #unit{..} => #name - }); - enum_variant_type.push(quote! { - #unit{..} => #bevy_reflect_path::VariantType::#variant - }); } EnumImpls { diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs index 9c79f40200fc7..f8894689a3858 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs @@ -29,10 +29,31 @@ pub(crate) struct ResultSifter { errors: Option, } -pub(crate) fn field_ident_or_indexed(index: usize, ident: Option<&Ident>) -> Member { - ident.as_ref().map_or_else( +/// Returns a `Member` made of `ident` or `index` if `ident` is None. +/// +/// Rust struct syntax allows for `Struct { foo: "string" }` with explicitly +/// named fields. It allows the `Struct { 0: "string" }` syntax when the struct +/// is declared as a tuple struct. +/// +/// ``` +/// # fn main() { +/// struct Foo { field: &'static str } +/// struct Bar(&'static str); +/// let Foo { field } = Foo { field: "hi" }; +/// let Bar { 0: field } = Bar { 0: "hello" }; +/// let Bar(field) = Bar("hello"); // more common syntax +/// # } +/// ``` +/// +/// This function helps field access in context where you are declaring either +/// a tuple struct or a struct with named fields. If you don't have a field name, +/// it means you need to access the struct through an index. +pub(crate) fn ident_or_index(ident: Option<&Ident>, index: usize) -> Member { + // TODO(Quality) when #4761 is merged, code that does this should be replaced + // by a call to `ident_or_index`. + ident.map_or_else( || Member::Unnamed(index.into()), - |&ident| Member::Named(ident.clone()), + |ident| Member::Named(ident.clone()), ) }