From a954f3e150d8365f2847486894398b474bfe4562 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Thu, 30 Mar 2023 19:02:25 -0400 Subject: [PATCH] Remove `#[system_param(ignore)]` and `#[world_query(ignore)]` (#8265) # Objective Follow-up to #8030. Now that `SystemParam` and `WorldQuery` are implemented for `PhantomData`, the `ignore` attributes are now unnecessary. --- ## Changelog - Removed the attributes `#[system_param(ignore)]` and `#[world_query(ignore)]`. ## Migration Guide The attributes `#[system_param(ignore)]` and `#[world_query]` ignore have been removed. If you were using either of these with `PhantomData` fields, you can simply remove the attribute: ```rust #[derive(SystemParam)] struct MyParam<'w, 's, Marker> { ... // Before: #[system_param(ignore) _marker: PhantomData, // After: _marker: PhantomData, } #[derive(WorldQuery)] struct MyQuery { ... // Before: #[world_query(ignore) _marker: PhantomData, // After: _marker: PhantomData, } ``` If you were using this for another type that implements `Default`, consider wrapping that type in `Local<>` (this only works for `SystemParam`): ```rust #[derive(SystemParam)] struct MyParam<'w, 's> { // Before: #[system_param(ignore)] value: MyDefaultType, // This will be initialized using `Default` each time `MyParam` is created. // After: value: Local, // This will be initialized using `Default` the first time `MyParam` is created. } ``` If you are implementing either trait and need to preserve the exact behavior of the old `ignore` attributes, consider manually implementing `SystemParam` or `WorldQuery` for a wrapper struct that uses the `Default` trait: ```rust // Before: #[derive(WorldQuery) struct MyQuery { #[world_query(ignore)] str: String, } // After: #[derive(WorldQuery) struct MyQuery { str: DefaultQuery, } pub struct DefaultQuery(pub T); unsafe impl WorldQuery for DefaultQuery { type Item<'w> = Self; ... unsafe fn fetch<'w>(...) -> Self::Item<'w> { Self(T::default()) } } ``` --- crates/bevy_ecs/macros/src/fetch.rs | 98 ++++++---------------- crates/bevy_ecs/macros/src/lib.rs | 75 +++-------------- crates/bevy_ecs/src/query/fetch.rs | 10 +-- crates/bevy_ecs/src/system/system_param.rs | 20 ++--- 4 files changed, 45 insertions(+), 158 deletions(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index dcef1cfbd8..97db95af2d 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -129,10 +129,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { _ => panic!("Expected a struct with named fields"), }; - let mut ignored_field_attrs = Vec::new(); - let mut ignored_field_visibilities = Vec::new(); - let mut ignored_field_idents = Vec::new(); - let mut ignored_field_types = Vec::new(); let mut field_attrs = Vec::new(); let mut field_visibilities = Vec::new(); let mut field_idents = Vec::new(); @@ -140,22 +136,17 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { let mut read_only_field_types = Vec::new(); for field in fields { - let WorldQueryFieldInfo { is_ignored, attrs } = read_world_query_field_info(field); + let attrs = match read_world_query_field_info(field) { + Ok(WorldQueryFieldInfo { attrs }) => attrs, + Err(e) => return e.into_compile_error().into(), + }; - let field_ident = field.ident.as_ref().unwrap().clone(); - if is_ignored { - ignored_field_attrs.push(attrs); - ignored_field_visibilities.push(field.vis.clone()); - ignored_field_idents.push(field_ident.clone()); - ignored_field_types.push(field.ty.clone()); - } else { - field_attrs.push(attrs); - field_visibilities.push(field.vis.clone()); - field_idents.push(field_ident.clone()); - let field_ty = field.ty.clone(); - field_types.push(quote!(#field_ty)); - read_only_field_types.push(quote!(<#field_ty as #path::query::WorldQuery>::ReadOnly)); - } + field_attrs.push(attrs); + field_visibilities.push(field.vis.clone()); + field_idents.push(field.ident.as_ref().unwrap().clone()); + let field_ty = field.ty.clone(); + field_types.push(quote!(#field_ty)); + read_only_field_types.push(quote!(<#field_ty as #path::query::WorldQuery>::ReadOnly)); } let derive_args = &fetch_struct_attributes.derive_args; @@ -193,7 +184,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { #[automatically_derived] #visibility struct #item_struct_name #user_impl_generics_with_world #user_where_clauses_with_world { #(#(#field_attrs)* #field_visibilities #field_idents: <#field_types as #path::query::WorldQuery>::Item<'__w>,)* - #(#(#ignored_field_attrs)* #ignored_field_visibilities #ignored_field_idents: #ignored_field_types,)* } }; @@ -205,7 +195,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { #[automatically_derived] #visibility struct #fetch_struct_name #user_impl_generics_with_world #user_where_clauses_with_world { #(#field_idents: <#field_types as #path::query::WorldQuery>::Fetch<'__w>,)* - #(#ignored_field_idents: #ignored_field_types,)* } // SAFETY: `update_component_access` and `update_archetype_component_access` are called on every field @@ -224,9 +213,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { #( #field_idents: <#field_types>::shrink(item.#field_idents), )* - #( - #ignored_field_idents: item.#ignored_field_idents, - )* } } @@ -245,7 +231,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { _this_run, ), )* - #(#ignored_field_idents: Default::default(),)* } } @@ -256,9 +241,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { #( #field_idents: <#field_types>::clone_fetch(& _fetch. #field_idents), )* - #( - #ignored_field_idents: Default::default(), - )* } } @@ -296,7 +278,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { ) -> ::Item<'__w> { Self::Item { #(#field_idents: <#field_types>::fetch(&mut _fetch.#field_idents, _entity, _table_row),)* - #(#ignored_field_idents: Default::default(),)* } } @@ -327,7 +308,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { fn init_state(world: &mut #path::world::World) -> #state_struct_name #user_ty_generics { #state_struct_name { #(#field_idents: <#field_types>::init_state(world),)* - #(#ignored_field_idents: Default::default(),)* } } @@ -349,7 +329,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { #[automatically_derived] #visibility struct #read_only_struct_name #user_impl_generics #user_where_clauses { #( #field_visibilities #field_idents: #read_only_field_types, )* - #(#(#ignored_field_attrs)* #ignored_field_visibilities #ignored_field_idents: #ignored_field_types,)* } #readonly_state @@ -396,7 +375,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { #[automatically_derived] #visibility struct #state_struct_name #user_impl_generics #user_where_clauses { #(#field_idents: <#field_types as #path::query::WorldQuery>::State,)* - #(#ignored_field_idents: ::std::marker::PhantomData #ignored_field_types>,)* } #mutable_impl @@ -428,56 +406,32 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { q2: #read_only_struct_name #user_ty_generics ) #user_where_clauses { #(q.#field_idents;)* - #(q.#ignored_field_idents;)* #(q2.#field_idents;)* - #(q2.#ignored_field_idents;)* } }; }) } struct WorldQueryFieldInfo { - /// Has the `#[world_query(ignore)]` attribute. - is_ignored: bool, /// All field attributes except for `world_query` ones. attrs: Vec, } -fn read_world_query_field_info(field: &Field) -> WorldQueryFieldInfo { - let is_ignored = field - .attrs - .iter() - .find(|attr| { - attr.path - .get_ident() - .map_or(false, |ident| ident == WORLD_QUERY_ATTRIBUTE_NAME) - }) - .map_or(false, |attr| { - let mut is_ignored = false; - attr.parse_args_with(|input: ParseStream| { - if input - .parse::>()? - .is_some() - { - is_ignored = true; - } - Ok(()) - }) - .unwrap_or_else(|_| panic!("Invalid `{WORLD_QUERY_ATTRIBUTE_NAME}` attribute format")); +fn read_world_query_field_info(field: &Field) -> syn::Result { + let mut attrs = Vec::new(); + for attr in &field.attrs { + if attr + .path + .get_ident() + .map_or(false, |ident| ident == WORLD_QUERY_ATTRIBUTE_NAME) + { + return Err(syn::Error::new_spanned( + attr, + "#[derive(WorldQuery)] does not support field attributes.", + )); + } + attrs.push(attr.clone()); + } - is_ignored - }); - - let attrs = field - .attrs - .iter() - .filter(|attr| { - attr.path - .get_ident() - .map_or(true, |ident| ident != WORLD_QUERY_ATTRIBUTE_NAME) - }) - .cloned() - .collect(); - - WorldQueryFieldInfo { is_ignored, attrs } + Ok(WorldQueryFieldInfo { attrs }) } diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 45a0a38d3d..c2c411a8de 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -13,9 +13,8 @@ use proc_macro::TokenStream; use proc_macro2::Span; use quote::{format_ident, quote}; use syn::{ - parse::ParseStream, parse_macro_input, parse_quote, punctuated::Punctuated, spanned::Spanned, - ConstParam, DeriveInput, GenericParam, Ident, Index, Meta, MetaList, NestedMeta, Token, - TypeParam, + parse_macro_input, parse_quote, punctuated::Punctuated, spanned::Spanned, ConstParam, + DeriveInput, GenericParam, Ident, Index, Meta, MetaList, NestedMeta, Token, TypeParam, }; enum BundleFieldKind { @@ -252,13 +251,6 @@ pub fn impl_param_set(_input: TokenStream) -> TokenStream { tokens } -#[derive(Default)] -struct SystemParamFieldAttributes { - pub ignore: bool, -} - -static SYSTEM_PARAM_ATTRIBUTE_NAME: &str = "system_param"; - /// Implement `SystemParam` to use a struct as a parameter in a system #[proc_macro_derive(SystemParam, attributes(system_param))] pub fn derive_system_param(input: TokenStream) -> TokenStream { @@ -271,53 +263,20 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { }; let path = bevy_ecs_path(); - let field_attributes = field_definitions - .iter() - .map(|field| { - ( - field, - field - .attrs - .iter() - .find(|a| *a.path.get_ident().as_ref().unwrap() == SYSTEM_PARAM_ATTRIBUTE_NAME) - .map_or_else(SystemParamFieldAttributes::default, |a| { - syn::custom_keyword!(ignore); - let mut attributes = SystemParamFieldAttributes::default(); - a.parse_args_with(|input: ParseStream| { - if input.parse::>()?.is_some() { - attributes.ignore = true; - } - Ok(()) - }) - .expect("Invalid 'system_param' attribute format."); - - attributes - }), - ) - }) - .collect::>(); - let mut field_locals = Vec::new(); let mut fields = Vec::new(); let mut field_types = Vec::new(); - let mut ignored_fields = Vec::new(); - let mut ignored_field_types = Vec::new(); - for (i, (field, attrs)) in field_attributes.iter().enumerate() { - if attrs.ignore { - ignored_fields.push(field.ident.as_ref().unwrap()); - ignored_field_types.push(&field.ty); - } else { - field_locals.push(format_ident!("f{i}")); - let i = Index::from(i); - fields.push( - field - .ident - .as_ref() - .map(|f| quote! { #f }) - .unwrap_or_else(|| quote! { #i }), - ); - field_types.push(&field.ty); - } + for (i, field) in field_definitions.iter().enumerate() { + field_locals.push(format_ident!("f{i}")); + let i = Index::from(i); + fields.push( + field + .ident + .as_ref() + .map(|f| quote! { #f }) + .unwrap_or_else(|| quote! { #i }), + ); + field_types.push(&field.ty); } let generics = ast.generics; @@ -383,13 +342,6 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { let mut tuple_types: Vec<_> = field_types.iter().map(|x| quote! { #x }).collect(); let mut tuple_patterns: Vec<_> = field_locals.iter().map(|x| quote! { #x }).collect(); - tuple_types.extend( - ignored_field_types - .iter() - .map(|ty| parse_quote!(::std::marker::PhantomData::<#ty>)), - ); - tuple_patterns.extend(ignored_field_types.iter().map(|_| parse_quote!(_))); - // If the number of fields exceeds the 16-parameter limit, // fold the fields into tuples of tuples until we are below the limit. const LIMIT: usize = 16; @@ -463,7 +415,6 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { >::get_param(&mut state.state, system_meta, world, change_tick); #struct_name { #(#fields: #field_locals,)* - #(#ignored_fields: std::default::Default::default(),)* } } } diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 7d3a8921fd..9eabc6ba1d 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -55,13 +55,7 @@ use std::{cell::UnsafeCell, marker::PhantomData}; /// - Methods can be implemented for the query items. /// - There is no hardcoded limit on the number of elements. /// -/// This trait can only be derived if each field either -/// -/// * also implements `WorldQuery`, or -/// * is marked with `#[world_query(ignore)]`. Fields decorated with this attribute -/// must implement [`Default`] and will be initialized to the default value as defined -/// by the trait. -/// +/// This trait can only be derived if each field also implements `WorldQuery`. /// The derive macro only supports regular structs (structs with named fields). /// /// ``` @@ -1485,9 +1479,7 @@ mod tests { #[derive(WorldQuery)] pub struct IgnoredQuery { id: Entity, - #[world_query(ignore)] _marker: PhantomData, - _marker2: PhantomData, } fn ignored_system(_: Query>) {} diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 94b01e4900..9ea7f5ed3f 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -36,12 +36,10 @@ use std::{ /// Derived `SystemParam` structs may have two lifetimes: `'w` for data stored in the [`World`], /// and `'s` for data stored in the parameter's state. /// -/// ## Attributes +/// ## `PhantomData` /// -/// `#[system_param(ignore)]`: -/// Can be added to any field in the struct. Fields decorated with this attribute -/// will be created with the default value upon realisation. -/// This is most useful for `PhantomData` fields, such as markers for generic types. +/// [`PhantomData`] is a special type of `SystemParam` that does nothing. +/// This is useful for constraining generic types or lifetimes. /// /// # Example /// @@ -55,7 +53,6 @@ use std::{ /// #[derive(SystemParam)] /// struct MyParam<'w, Marker: 'static> { /// foo: Res<'w, SomeResource>, -/// #[system_param(ignore)] /// marker: PhantomData, /// } /// @@ -66,11 +63,6 @@ use std::{ /// # bevy_ecs::system::assert_is_system(my_system::<()>); /// ``` /// -/// ## `PhantomData` -/// -/// [`PhantomData`] is a special type of `SystemParam` that does nothing. -/// This is useful for constraining generic types or lifetimes. -/// /// # Generic `SystemParam`s /// /// When using the derive macro, you may see an error in the form of: @@ -1652,14 +1644,12 @@ mod tests { #[test] fn system_param_phantom_data() { #[derive(SystemParam)] - struct IgnoredParam<'w, T: Resource, Marker: 'static> { + struct PhantomParam<'w, T: Resource, Marker: 'static> { _foo: Res<'w, T>, - #[system_param(ignore)] marker: PhantomData<&'w Marker>, - marker2: PhantomData<&'w Marker>, } - fn my_system(_: IgnoredParam, ()>) {} + fn my_system(_: PhantomParam, ()>) {} assert_is_system(my_system); }