Skip to content

Commit

Permalink
Replace impl_param_set proc macro with a macro_rules macro (#16847)
Browse files Browse the repository at this point in the history 
# Objective

Simplify the code by using `macro_rules` instead of a proc macro where
possible.

## Solution

Replace `impl_param_set` proc macro with a `macro_rules` macro.
  • Loading branch information
@chescock
chescock authored Dec 18, 2024
1 parent b9123e7 commit 3ef99cf
Show file tree
Hide file tree
Showing 7 changed files with 107 additions and 140 deletions.
2 changes: 1 addition & 1 deletion crates/bevy_app/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ bevy_tasks = { path = "../bevy_tasks", version = "0.15.0-dev" }
# other
downcast-rs = "1.2.0"
thiserror = { version = "2", default-features = false }
variadics_please = "1.0"
variadics_please = "1.1"

[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
ctrlc = "3.4.4"
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ nonmax = { version = "0.5", default-features = false }
arrayvec = { version = "0.7.4", default-features = false, optional = true }
smallvec = { version = "1", features = ["union"] }
indexmap = { version = "2.5.0", default-features = false }
variadics_please = { version = "1.0", default-features = false }
variadics_please = { version = "1.1", default-features = false }
critical-section = { version = "1.2.0", optional = true }
portable-atomic = { version = "1", default-features = false, features = [
"fallback",
Expand Down
133 changes: 1 addition & 132 deletions crates/bevy_ecs/macros/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@ mod world_query;
use crate::{query_data::derive_query_data_impl, query_filter::derive_query_filter_impl};
use bevy_macro_utils::{derive_label, ensure_no_collision, get_struct_fields, BevyManifest};
use proc_macro::TokenStream;
use proc_macro2::Span;
use proc_macro2::TokenStream as TokenStream2;
use quote::{format_ident, quote};
use syn::{
parse_macro_input, parse_quote, punctuated::Punctuated, spanned::Spanned, token::Comma,
ConstParam, DeriveInput, GenericParam, Ident, Index, TypeParam,
ConstParam, DeriveInput, GenericParam, Index, TypeParam,
};

enum BundleFieldKind {
Expand Down Expand Up @@ -284,136 +283,6 @@ pub fn derive_visit_entities(input: TokenStream) -> TokenStream {
})
}

fn get_idents(fmt_string: fn(usize) -> String, count: usize) -> Vec<Ident> {
(0..count)
.map(|i| Ident::new(&fmt_string(i), Span::call_site()))
.collect::<Vec<Ident>>()
}

#[proc_macro]
pub fn impl_param_set(_input: TokenStream) -> TokenStream {
let mut tokens = TokenStream::new();
let max_params = 8;
let params = get_idents(|i| format!("P{i}"), max_params);
let metas = get_idents(|i| format!("m{i}"), max_params);
let mut param_fn_muts = Vec::new();
for (i, param) in params.iter().enumerate() {
let fn_name = Ident::new(&format!("p{i}"), Span::call_site());
let index = Index::from(i);
let ordinal = match i {
1 => "1st".to_owned(),
2 => "2nd".to_owned(),
3 => "3rd".to_owned(),
x => format!("{x}th"),
};
let comment =
format!("Gets exclusive access to the {ordinal} parameter in this [`ParamSet`].");
param_fn_muts.push(quote! {
#[doc = #comment]
/// No other parameters may be accessed while this one is active.
pub fn #fn_name<'a>(&'a mut self) -> SystemParamItem<'a, 'a, #param> {
// SAFETY: systems run without conflicts with other systems.
// Conflicting params in ParamSet are not accessible at the same time
// ParamSets are guaranteed to not conflict with other SystemParams
unsafe {
#param::get_param(&mut self.param_states.#index, &self.system_meta, self.world, self.change_tick)
}
}
});
}

for param_count in 1..=max_params {
let param = &params[0..param_count];
let meta = &metas[0..param_count];
let param_fn_mut = &param_fn_muts[0..param_count];
tokens.extend(TokenStream::from(quote! {
// SAFETY: All parameters are constrained to ReadOnlySystemParam, so World is only read
unsafe impl<'w, 's, #(#param,)*> ReadOnlySystemParam for ParamSet<'w, 's, (#(#param,)*)>
where #(#param: ReadOnlySystemParam,)*
{ }

// SAFETY: Relevant parameter ComponentId and ArchetypeComponentId access is applied to SystemMeta. If any ParamState conflicts
// with any prior access, a panic will occur.
unsafe impl<'_w, '_s, #(#param: SystemParam,)*> SystemParam for ParamSet<'_w, '_s, (#(#param,)*)>
{
type State = (#(#param::State,)*);
type Item<'w, 's> = ParamSet<'w, 's, (#(#param,)*)>;

// Note: We allow non snake case so the compiler don't complain about the creation of non_snake_case variables
#[allow(non_snake_case)]
fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State {
#(
// Pretend to add each param to the system alone, see if it conflicts
let mut #meta = system_meta.clone();
#meta.component_access_set.clear();
#meta.archetype_component_access.clear();
#param::init_state(world, &mut #meta);
// The variable is being defined with non_snake_case here
let #param = #param::init_state(world, &mut system_meta.clone());
)*
// Make the ParamSet non-send if any of its parameters are non-send.
if false #(|| !#meta.is_send())* {
system_meta.set_non_send();
}
#(
system_meta
.component_access_set
.extend(#meta.component_access_set);
system_meta
.archetype_component_access
.extend(&#meta.archetype_component_access);
)*
(#(#param,)*)
}

unsafe fn new_archetype(state: &mut Self::State, archetype: &Archetype, system_meta: &mut SystemMeta) {
// SAFETY: The caller ensures that `archetype` is from the World the state was initialized from in `init_state`.
unsafe { <(#(#param,)*) as SystemParam>::new_archetype(state, archetype, system_meta); }
}

fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) {
<(#(#param,)*) as SystemParam>::apply(state, system_meta, world);
}

fn queue(state: &mut Self::State, system_meta: &SystemMeta, mut world: DeferredWorld) {
<(#(#param,)*) as SystemParam>::queue(state, system_meta, world.reborrow());
}

#[inline]
unsafe fn validate_param<'w, 's>(
state: &'s Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
) -> bool {
<(#(#param,)*) as SystemParam>::validate_param(state, system_meta, world)
}

#[inline]
unsafe fn get_param<'w, 's>(
state: &'s mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
change_tick: Tick,
) -> Self::Item<'w, 's> {
ParamSet {
param_states: state,
system_meta: system_meta.clone(),
world,
change_tick,
}
}
}

impl<'w, 's, #(#param: SystemParam,)*> ParamSet<'w, 's, (#(#param,)*)>
{
#(#param_fn_mut)*
}
}));
}

tokens
}

/// 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 {
Expand Down
104 changes: 101 additions & 3 deletions crates/bevy_ecs/src/system/system_param.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use crate::{
},
};
use alloc::{borrow::ToOwned, boxed::Box, vec::Vec};
use bevy_ecs_macros::impl_param_set;
pub use bevy_ecs_macros::{Resource, SystemParam};
use bevy_ptr::UnsafeCellDeref;
use bevy_utils::synccell::SyncCell;
Expand All @@ -32,7 +31,7 @@ use core::{
use disqualified::ShortName;

use super::Populated;
use variadics_please::all_tuples;
use variadics_please::{all_tuples, all_tuples_enumerated};

/// A parameter that can be used in a [`System`](super::System).
///
Expand Down Expand Up @@ -674,7 +673,106 @@ pub struct ParamSet<'w, 's, T: SystemParam> {
change_tick: Tick,
}

impl_param_set!();
macro_rules! impl_param_set {
($(($index: tt, $param: ident, $system_meta: ident, $fn_name: ident)),*) => {
// SAFETY: All parameters are constrained to ReadOnlySystemParam, so World is only read
unsafe impl<'w, 's, $($param,)*> ReadOnlySystemParam for ParamSet<'w, 's, ($($param,)*)>
where $($param: ReadOnlySystemParam,)*
{ }

// SAFETY: Relevant parameter ComponentId and ArchetypeComponentId access is applied to SystemMeta. If any ParamState conflicts
// with any prior access, a panic will occur.
unsafe impl<'_w, '_s, $($param: SystemParam,)*> SystemParam for ParamSet<'_w, '_s, ($($param,)*)>
{
type State = ($($param::State,)*);
type Item<'w, 's> = ParamSet<'w, 's, ($($param,)*)>;

// Note: We allow non snake case so the compiler don't complain about the creation of non_snake_case variables
#[allow(non_snake_case)]
fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State {
$(
// Pretend to add each param to the system alone, see if it conflicts
let mut $system_meta = system_meta.clone();
$system_meta.component_access_set.clear();
$system_meta.archetype_component_access.clear();
$param::init_state(world, &mut $system_meta);
// The variable is being defined with non_snake_case here
let $param = $param::init_state(world, &mut system_meta.clone());
)*
// Make the ParamSet non-send if any of its parameters are non-send.
if false $(|| !$system_meta.is_send())* {
system_meta.set_non_send();
}
$(
system_meta
.component_access_set
.extend($system_meta.component_access_set);
system_meta
.archetype_component_access
.extend(&$system_meta.archetype_component_access);
)*
($($param,)*)
}

unsafe fn new_archetype(state: &mut Self::State, archetype: &Archetype, system_meta: &mut SystemMeta) {
// SAFETY: The caller ensures that `archetype` is from the World the state was initialized from in `init_state`.
unsafe { <($($param,)*) as SystemParam>::new_archetype(state, archetype, system_meta); }
}

fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) {
<($($param,)*) as SystemParam>::apply(state, system_meta, world);
}

fn queue(state: &mut Self::State, system_meta: &SystemMeta, mut world: DeferredWorld) {
<($($param,)*) as SystemParam>::queue(state, system_meta, world.reborrow());
}

#[inline]
unsafe fn validate_param<'w, 's>(
state: &'s Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
) -> bool {
<($($param,)*) as SystemParam>::validate_param(state, system_meta, world)
}

#[inline]
unsafe fn get_param<'w, 's>(
state: &'s mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
change_tick: Tick,
) -> Self::Item<'w, 's> {
ParamSet {
param_states: state,
system_meta: system_meta.clone(),
world,
change_tick,
}
}
}

impl<'w, 's, $($param: SystemParam,)*> ParamSet<'w, 's, ($($param,)*)>
{
$(
/// Gets exclusive access to the parameter at index
#[doc = stringify!($index)]
/// in this [`ParamSet`].
/// No other parameters may be accessed while this one is active.
pub fn $fn_name<'a>(&'a mut self) -> SystemParamItem<'a, 'a, $param> {
// SAFETY: systems run without conflicts with other systems.
// Conflicting params in ParamSet are not accessible at the same time
// ParamSets are guaranteed to not conflict with other SystemParams
unsafe {
$param::get_param(&mut self.param_states.$index, &self.system_meta, self.world, self.change_tick)
}
}
)*
}
}
}

all_tuples_enumerated!(impl_param_set, 1, 8, P, m, p);

/// A type that can be inserted into a [`World`] as a singleton.
///
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_reflect/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ uuid = { version = "1.0", default-features = false, optional = true, features =
"v4",
"serde",
] }
variadics_please = "1.0"
variadics_please = "1.1"
wgpu-types = { version = "23", features = ["serde"], optional = true }

[dev-dependencies]
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_render/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ async-channel = "2.3.0"
nonmax = "0.5"
smallvec = { version = "1.11", features = ["const_new"] }
offset-allocator = "0.2"
variadics_please = "1.0"
variadics_please = "1.1"

[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
# Omit the `glsl` feature in non-WebAssembly by default.
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_state/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ bevy_utils = { path = "../bevy_utils", version = "0.15.0-dev" }
bevy_reflect = { path = "../bevy_reflect", version = "0.15.0-dev", optional = true }
bevy_app = { path = "../bevy_app", version = "0.15.0-dev", optional = true }
bevy_hierarchy = { path = "../bevy_hierarchy", version = "0.15.0-dev", optional = true }
variadics_please = "1.0"
variadics_please = "1.1"

[lints]
workspace = true
Expand Down

0 comments on commit 3ef99cf

Please sign in to comment.