Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
c766e7c
impl `SystemParam` for `PhantomData`
joseph-gio Mar 10, 2023
8994d13
ensure state types don't collide for `SystemParam`
joseph-gio Mar 10, 2023
92d7921
add a dedicated method for generating identifiers with no collision
joseph-gio Mar 10, 2023
1c6d01f
add a comment describing the collision algorithm
joseph-gio Mar 10, 2023
a25fb60
use explicit lifetimes in the `SystemParam` derive
joseph-gio Mar 10, 2023
65f332e
remove `#system_param(ignore)`
joseph-gio Mar 10, 2023
43507a6
clean up state struct lifetimes
joseph-gio Mar 10, 2023
6df2d0a
Merge remote-tracking branch 'upstream/main' into system-param-phanto…
joseph-gio Mar 10, 2023
4e5797c
remove unused attributes
joseph-gio Mar 10, 2023
6eac07a
use a descriptive name for a macro variable
joseph-gio Mar 10, 2023
653fc46
implement `WorldQuery` for `PhantomData`
joseph-gio Mar 10, 2023
e18e7af
add a compile test
joseph-gio Mar 10, 2023
715f8ec
remove `#[world_query(ignore)]`
joseph-gio Mar 10, 2023
f60909b
make `world_query(ignore)` an error
joseph-gio Mar 10, 2023
4ee9978
add comments to `IS_DENSE` and `IS_ARCHETYPAL`
joseph-gio Mar 10, 2023
3a55ca1
update a comment
joseph-gio Mar 10, 2023
c4005ac
disallow `#[system_param]` attributes
joseph-gio Mar 11, 2023
136033c
Merge remote-tracking branch 'upstream/main' into system-param-phanto…
joseph-gio Mar 11, 2023
eaddf9a
remove an unused const
joseph-gio Mar 11, 2023
b2f4309
undo an overlapping change
joseph-gio Mar 13, 2023
0ab464c
add an example demonstrating `PhantomData` queries
joseph-gio Mar 17, 2023
978437a
import worldquery
joseph-gio Mar 17, 2023
08d6b60
remove collision code
joseph-gio Mar 18, 2023
244a589
remove needless whitespace
joseph-gio Mar 18, 2023
6f5d446
Merge remote-tracking branch 'upstream/main' into system-param-phanto…
joseph-gio Mar 18, 2023
3f0bd19
remove an unused import
joseph-gio Mar 18, 2023
47040f2
Merge remote-tracking branch 'upstream/main' into system-param-phanto…
joseph-gio Mar 21, 2023
988ec67
fix formatting for a `let ... else` block
joseph-gio Mar 21, 2023
986f10c
Merge remote-tracking branch 'upstream/main' into system-param-phanto…
joseph-gio Mar 22, 2023
aebb03d
Merge remote-tracking branch 'upstream/main' into system-param-phanto…
joseph-gio Mar 22, 2023
589361e
remove a space
joseph-gio Mar 22, 2023
5dfc332
use non-conflicting lifetime names
joseph-gio Mar 23, 2023
93624c3
add a test case for invariant lifetimes
joseph-gio Mar 24, 2023
eb3e17d
re-add `#[system_param(ignore)]`
joseph-gio Mar 28, 2023
f3572b1
re-add `#[world_query(ignore)]`
joseph-gio Mar 28, 2023
bcfee71
restore some docs
joseph-gio Mar 28, 2023
10edef0
cargo fmt
joseph-gio Mar 28, 2023
f8469c5
re-order an example
joseph-gio Mar 28, 2023
2579709
allow unlimited ignored fields
joseph-gio Mar 28, 2023
6a3cfce
reduce churn
joseph-gio Mar 28, 2023
8ff0178
Update crates/bevy_ecs/macros/src/lib.rs
joseph-gio Mar 29, 2023
464e095
deduplicate regression tests
joseph-gio Mar 29, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/bevy_ecs/macros/src/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ 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: #ignored_field_types,)*
#(#ignored_field_idents: ::std::marker::PhantomData<fn() -> #ignored_field_types>,)*
}

#mutable_impl
Expand Down Expand Up @@ -437,7 +437,7 @@ pub fn derive_world_query_impl(input: TokenStream) -> 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>,
Expand Down
69 changes: 44 additions & 25 deletions crates/bevy_ecs/macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,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,
};

Expand Down Expand Up @@ -264,7 +264,7 @@ static SYSTEM_PARAM_ATTRIBUTE_NAME: &str = "system_param";
pub fn derive_system_param(input: TokenStream) -> TokenStream {
let token_stream = input.clone();
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();
Expand Down Expand Up @@ -295,7 +295,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();
Expand Down Expand Up @@ -346,11 +347,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 {
Expand All @@ -372,9 +369,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;
Expand All @@ -385,6 +400,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();
Expand All @@ -395,6 +411,9 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
.push(syn::parse_quote!(#field_type: #path::system::ReadOnlySystemParam));
}

let fields_alias =
ensure_no_collision(format_ident!("__StructFieldsAlias"), token_stream.clone());

let struct_name = &ast.ident;
let state_struct_visibility = &ast.vis;
let state_struct_name = ensure_no_collision(format_ident!("FetchState"), token_stream);
Expand All @@ -404,41 +423,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 #state_struct_name <'w, 's, #(#lifetimeless_generics,)*>
#state_struct_visibility struct #state_struct_name <#(#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 = #state_struct_name<'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 = #state_struct_name<#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 {
#state_struct_name {
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: #path::component::Tick,
) -> 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);
Expand Down
100 changes: 99 additions & 1 deletion crates/bevy_ecs/src/query/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,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`
Expand Down Expand Up @@ -1315,7 +1333,6 @@ macro_rules! impl_anytuple_fetch {

/// SAFETY: each item in the tuple is read only
unsafe impl<$($name: ReadOnlyWorldQuery),*> ReadOnlyWorldQuery for AnyOf<($($name,)*)> {}

};
}

Expand Down Expand Up @@ -1389,6 +1406,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_run: Tick,
_this_run: Tick,
) -> 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::*;
Expand All @@ -1397,6 +1479,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]
Expand Down
34 changes: 33 additions & 1 deletion crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use bevy_utils::{all_tuples, synccell::SyncCell};
use std::{
borrow::Cow,
fmt::Debug,
marker::PhantomData,
ops::{Deref, DerefMut},
};

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -1465,7 +1471,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 ()>
/// }
Expand Down Expand Up @@ -1531,6 +1536,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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is better than just returning () because it's easier to debug.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also necessary, since SystemParams have to be reflexive :)


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: Tick,
) -> Self::Item<'world, 'state> {
PhantomData
}
}

// SAFETY: No world access.
unsafe impl<T: ?Sized> ReadOnlySystemParam for PhantomData<T> {}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -1605,6 +1630,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.
Expand Down Expand Up @@ -1642,4 +1668,10 @@ mod tests {

#[derive(Resource)]
pub struct FetchState;

// Regression test for https://github.com/bevyengine/bevy/issues/8192.
#[derive(SystemParam)]
pub struct InvariantParam<'w, 's> {
_set: ParamSet<'w, 's, (Query<'w, 's, ()>,)>,
}
}