Make #[system_param(ignore)] and #[world_query(ignore)] unnecessary (#8030)

When using `PhantomData` fields with the `#[derive(SystemParam)]` or
`#[derive(WorldQuery)]` macros, the user is required to add the
`#[system_param(ignore)]` attribute so that the macro knows to treat
that field specially. This is undesirable, since it makes the macro more
fragile and less consistent.

Implement `SystemParam` and `WorldQuery` for `PhantomData`. This makes
the `ignore` attributes unnecessary.

Some internal changes make the derive macro compatible with types that
have invariant lifetimes, which fixes #8192. From what I can tell, this
fix requires `PhantomData` to implement `SystemParam` in order to ensure
that all of a type's generic parameters are always constrained.

---

+ Implemented `SystemParam` and `WorldQuery` for `PhantomData<T>`.
+ Fixed a miscompilation caused when invariant lifetimes were used with
the `SystemParam` macro.

Author: joseph-gio

crates/bevy_ecs/macros/src/fetch.rs

@@ -378,7 +378,7 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> 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: #ignored_field_types,)*
+            #(#ignored_field_idents: ::std::marker::PhantomData<fn() -> #ignored_field_types>,)*
         }
 
         /// SAFETY: we assert fields are readonly below
@@ -419,7 +419,7 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream {
 }
 
 struct WorldQueryFieldInfo {
-    /// Has `#[fetch(ignore)]` or `#[filter_fetch(ignore)]` attribute.
+    /// Has the `#[world_query(ignore)]` attribute.
     is_ignored: bool,
     /// All field attributes except for `world_query` ones.
     attrs: Vec<Attribute>,

crates/bevy_ecs/macros/src/lib.rs

@@ -12,7 +12,7 @@ use proc_macro2::Span;
 use quote::{format_ident, quote};
 use syn::{
     parse::ParseStream, parse_macro_input, parse_quote, punctuated::Punctuated, spanned::Spanned,
-    ConstParam, DeriveInput, Field, GenericParam, Ident, Index, Meta, MetaList, NestedMeta, Token,
+    ConstParam, DeriveInput, GenericParam, Ident, Index, Meta, MetaList, NestedMeta, Token,
     TypeParam,
 };
 
@@ -259,7 +259,7 @@ static SYSTEM_PARAM_ATTRIBUTE_NAME: &str = "system_param";
 #[proc_macro_derive(SystemParam, attributes(system_param))]
 pub fn derive_system_param(input: TokenStream) -> TokenStream {
     let ast = parse_macro_input!(input as DeriveInput);
-    let syn::Data::Struct(syn::DataStruct { fields: field_definitions, ..}) = ast.data else {
+    let syn::Data::Struct(syn::DataStruct { fields: field_definitions, .. }) = ast.data else {
         return syn::Error::new(ast.span(), "Invalid `SystemParam` type: expected a `struct`")
             .into_compile_error()
             .into();
@@ -290,7 +290,8 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
                     }),
             )
         })
-        .collect::<Vec<(&Field, SystemParamFieldAttributes)>>();
+        .collect::<Vec<_>>();
+
     let mut field_locals = Vec::new();
     let mut fields = Vec::new();
     let mut field_types = Vec::new();
@@ -341,11 +342,7 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
         .filter(|g| !matches!(g, GenericParam::Lifetime(_)))
         .collect();
 
-    let mut shadowed_lifetimes: Vec<_> = generics.lifetimes().map(|x| x.lifetime.clone()).collect();
-    for lifetime in &mut shadowed_lifetimes {
-        let shadowed_ident = format_ident!("_{}", lifetime.ident);
-        lifetime.ident = shadowed_ident;
-    }
+    let shadowed_lifetimes: Vec<_> = generics.lifetimes().map(|_| quote!('_)).collect();
 
     let mut punctuated_generics = Punctuated::<_, Token![,]>::new();
     punctuated_generics.extend(lifetimeless_generics.iter().map(|g| match g {
@@ -367,9 +364,27 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
         _ => unreachable!(),
     }));
 
+    let punctuated_generics_no_bounds: Punctuated<_, Token![,]> = lifetimeless_generics
+        .iter()
+        .map(|&g| match g.clone() {
+            GenericParam::Type(mut g) => {
+                g.bounds.clear();
+                GenericParam::Type(g)
+            }
+            g => g,
+        })
+        .collect();
+
     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;
@@ -380,6 +395,7 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
         let end = Vec::from_iter(tuple_patterns.drain(..LIMIT));
         tuple_patterns.push(parse_quote!( (#(#end,)*) ));
     }
+
     // Create a where clause for the `ReadOnlySystemParam` impl.
     // Ensure that each field implements `ReadOnlySystemParam`.
     let mut read_only_generics = generics.clone();
@@ -390,6 +406,8 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
             .push(syn::parse_quote!(#field_type: #path::system::ReadOnlySystemParam));
     }
 
+    let fields_alias = format_ident!("__StructFieldsAlias");
+
     let struct_name = &ast.ident;
     let state_struct_visibility = &ast.vis;
 
@@ -398,41 +416,41 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
         // The struct can still be accessed via SystemParam::State, e.g. EventReaderState can be accessed via
         // <EventReader<'static, 'static, T> as SystemParam>::State
         const _: () = {
+            // Allows rebinding the lifetimes of each field type.
+            type #fields_alias <'w, 's, #punctuated_generics_no_bounds> = (#(#tuple_types,)*);
+
             #[doc(hidden)]
-            #state_struct_visibility struct FetchState <'w, 's, #(#lifetimeless_generics,)*>
+            #state_struct_visibility struct FetchState <#(#lifetimeless_generics,)*>
             #where_clause {
-                state: (#(<#tuple_types as #path::system::SystemParam>::State,)*),
-                marker: std::marker::PhantomData<(
-                    <#path::prelude::Query<'w, 's, ()> as #path::system::SystemParam>::State,
-                    #(fn() -> #ignored_field_types,)*
-                )>,
+                state: <#fields_alias::<'static, 'static, #punctuated_generic_idents> as #path::system::SystemParam>::State,
             }
 
-            unsafe impl<'w, 's, #punctuated_generics> #path::system::SystemParam for #struct_name #ty_generics #where_clause {
-                type State = FetchState<'static, 'static, #punctuated_generic_idents>;
-                type Item<'_w, '_s> = #struct_name <#(#shadowed_lifetimes,)* #punctuated_generic_idents>;
+            unsafe impl<#punctuated_generics> #path::system::SystemParam for
+                #struct_name <#(#shadowed_lifetimes,)* #punctuated_generic_idents> #where_clause
+            {
+                type State = FetchState<#punctuated_generic_idents>;
+                type Item<'w, 's> = #struct_name #ty_generics;
 
                 fn init_state(world: &mut #path::world::World, system_meta: &mut #path::system::SystemMeta) -> Self::State {
                     FetchState {
-                        state: <(#(#tuple_types,)*) as #path::system::SystemParam>::init_state(world, system_meta),
-                        marker: std::marker::PhantomData,
+                        state: <#fields_alias::<'_, '_, #punctuated_generic_idents> as #path::system::SystemParam>::init_state(world, system_meta),
                     }
                 }
 
                 fn new_archetype(state: &mut Self::State, archetype: &#path::archetype::Archetype, system_meta: &mut #path::system::SystemMeta) {
-                    <(#(#tuple_types,)*) as #path::system::SystemParam>::new_archetype(&mut state.state, archetype, system_meta)
+                    <#fields_alias::<'_, '_, #punctuated_generic_idents> as #path::system::SystemParam>::new_archetype(&mut state.state, archetype, system_meta)
                 }
 
                 fn apply(state: &mut Self::State, system_meta: &#path::system::SystemMeta, world: &mut #path::world::World) {
-                    <(#(#tuple_types,)*) as #path::system::SystemParam>::apply(&mut state.state, system_meta, world);
+                    <#fields_alias::<'_, '_, #punctuated_generic_idents> as #path::system::SystemParam>::apply(&mut state.state, system_meta, world);
                 }
 
-                unsafe fn get_param<'w2, 's2>(
-                    state: &'s2 mut Self::State,
+                unsafe fn get_param<'w, 's>(
+                    state: &'s mut Self::State,
                     system_meta: &#path::system::SystemMeta,
-                    world: &'w2 #path::world::World,
+                    world: &'w #path::world::World,
                     change_tick: u32,
-                ) -> Self::Item<'w2, 's2> {
+                ) -> Self::Item<'w, 's> {
                     let (#(#tuple_patterns,)*) = <
                         (#(#tuple_types,)*) as #path::system::SystemParam
                     >::get_param(&mut state.state, system_meta, world, change_tick);

crates/bevy_ecs/src/query/fetch.rs

@@ -281,6 +281,24 @@ use std::{cell::UnsafeCell, marker::PhantomData};
 /// # bevy_ecs::system::assert_is_system(my_system);
 /// ```
 ///
+/// # Generic Queries
+///
+/// When writing generic code, it is often necessary to use [`PhantomData`]
+/// to constrain type parameters. Since `WorldQuery` is implemented for all
+/// `PhantomData<T>` types, this pattern can be used with this macro.
+///
+/// ```
+/// # use bevy_ecs::{prelude::*, query::WorldQuery};
+/// # use std::marker::PhantomData;
+/// #[derive(WorldQuery)]
+/// pub struct GenericQuery<T> {
+///     id: Entity,
+///     marker: PhantomData<T>,
+/// }
+/// # fn my_system(q: Query<GenericQuery<()>>) {}
+/// # bevy_ecs::system::assert_is_system(my_system);
+/// ```
+///
 /// # Safety
 ///
 /// Component access of `Self::ReadOnly` must be a subset of `Self`
@@ -1563,7 +1581,6 @@ macro_rules! impl_anytuple_fetch {
 
         /// SAFETY: each item in the tuple is read only
         unsafe impl<$($name: ReadOnlyWorldQuery),*> ReadOnlyWorldQuery for AnyOf<($($name,)*)> {}
-
     };
 }
 
@@ -1643,6 +1660,71 @@ unsafe impl<Q: WorldQuery> WorldQuery for NopWorldQuery<Q> {
 /// SAFETY: `NopFetch` never accesses any data
 unsafe impl<Q: WorldQuery> ReadOnlyWorldQuery for NopWorldQuery<Q> {}
 
+/// SAFETY: `PhantomData` never accesses any world data.
+unsafe impl<T: ?Sized> WorldQuery for PhantomData<T> {
+    type Item<'a> = ();
+    type Fetch<'a> = ();
+    type ReadOnly = Self;
+    type State = ();
+
+    fn shrink<'wlong: 'wshort, 'wshort>(_item: Self::Item<'wlong>) -> Self::Item<'wshort> {}
+
+    unsafe fn init_fetch<'w>(
+        _world: &'w World,
+        _state: &Self::State,
+        _last_change_tick: u32,
+        _change_tick: u32,
+    ) -> Self::Fetch<'w> {
+    }
+
+    unsafe fn clone_fetch<'w>(_fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {}
+
+    // `PhantomData` does not match any components, so all components it matches
+    // are stored in a Table (vacuous truth).
+    const IS_DENSE: bool = true;
+    // `PhantomData` matches every entity in each archetype.
+    const IS_ARCHETYPAL: bool = true;
+
+    unsafe fn set_archetype<'w>(
+        _fetch: &mut Self::Fetch<'w>,
+        _state: &Self::State,
+        _archetype: &'w Archetype,
+        _table: &'w Table,
+    ) {
+    }
+
+    unsafe fn set_table<'w>(_fetch: &mut Self::Fetch<'w>, _state: &Self::State, _table: &'w Table) {
+    }
+
+    unsafe fn fetch<'w>(
+        _fetch: &mut Self::Fetch<'w>,
+        _entity: Entity,
+        _table_row: TableRow,
+    ) -> Self::Item<'w> {
+    }
+
+    fn update_component_access(_state: &Self::State, _access: &mut FilteredAccess<ComponentId>) {}
+
+    fn update_archetype_component_access(
+        _state: &Self::State,
+        _archetype: &Archetype,
+        _access: &mut Access<ArchetypeComponentId>,
+    ) {
+    }
+
+    fn init_state(_world: &mut World) -> Self::State {}
+
+    fn matches_component_set(
+        _state: &Self::State,
+        _set_contains_id: &impl Fn(ComponentId) -> bool,
+    ) -> bool {
+        true
+    }
+}
+
+/// SAFETY: `PhantomData` never accesses any world data.
+unsafe impl<T: ?Sized> ReadOnlyWorldQuery for PhantomData<T> {}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -1651,6 +1733,22 @@ mod tests {
     #[derive(Component)]
     pub struct A;
 
+    // Compile test for https://github.com/bevyengine/bevy/pull/8030.
+    #[test]
+    fn world_query_phantom_data() {
+        #[derive(WorldQuery)]
+        pub struct IgnoredQuery<Marker> {
+            id: Entity,
+            #[world_query(ignore)]
+            _marker: PhantomData<Marker>,
+            _marker2: PhantomData<Marker>,
+        }
+
+        fn ignored_system(_: Query<IgnoredQuery<()>>) {}
+
+        crate::system::assert_is_system(ignored_system);
+    }
+
     // Ensures that each field of a `WorldQuery` struct's read-only variant
     // has the same visibility as its corresponding mutable field.
     #[test]

crates/bevy_ecs/src/system/system_param.rs

@@ -19,6 +19,7 @@ use bevy_utils::{all_tuples, synccell::SyncCell};
 use std::{
     borrow::Cow,
     fmt::Debug,
+    marker::PhantomData,
     ops::{Deref, DerefMut},
 };
 
@@ -65,6 +66,11 @@ 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:
@@ -1476,7 +1482,6 @@ pub mod lifetimeless {
 /// #[derive(SystemParam)]
 /// struct GenericParam<'w, 's, T: SystemParam> {
 ///     field: T,
-///     #[system_param(ignore)]
 ///     // Use the lifetimes in this type, or they will be unbound.
 ///     phantom: core::marker::PhantomData<&'w &'s ()>
 /// }
@@ -1542,6 +1547,26 @@ unsafe impl<P: SystemParam + 'static> SystemParam for StaticSystemParam<'_, '_,
     }
 }
 
+// SAFETY: No world access.
+unsafe impl<T: ?Sized> SystemParam for PhantomData<T> {
+    type State = ();
+    type Item<'world, 'state> = Self;
+
+    fn init_state(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State {}
+
+    unsafe fn get_param<'world, 'state>(
+        _state: &'state mut Self::State,
+        _system_meta: &SystemMeta,
+        _world: &'world World,
+        _change_tick: u32,
+    ) -> Self::Item<'world, 'state> {
+        PhantomData
+    }
+}
+
+// SAFETY: No world access.
+unsafe impl<T: ?Sized> ReadOnlySystemParam for PhantomData<T> {}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -1616,6 +1641,7 @@ mod tests {
         _foo: Res<'w, T>,
         #[system_param(ignore)]
         marker: PhantomData<&'w Marker>,
+        marker2: PhantomData<&'w Marker>,
     }
 
     // Compile tests for https://github.com/bevyengine/bevy/pull/6957.
@@ -1644,4 +1670,10 @@ mod tests {
     {
         _q: Query<'w, 's, Q, ()>,
     }
+
+    // Regression test for https://github.com/bevyengine/bevy/issues/8192.
+    #[derive(SystemParam)]
+    pub struct InvariantParam<'w, 's> {
+        _set: ParamSet<'w, 's, (Query<'w, 's, ()>,)>,
+    }
 }