From 431a27eab4626fad901ba93e72d921eaef1335f1 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 22 Jul 2023 15:05:12 +0200 Subject: [PATCH 01/53] Replace boxed labels with interned labels --- crates/bevy_app/src/app.rs | 31 +- crates/bevy_ecs/macros/src/lib.rs | 4 +- crates/bevy_ecs/macros/src/set.rs | 49 ++- crates/bevy_ecs/src/schedule/config.rs | 61 ++-- crates/bevy_ecs/src/schedule/graph_utils.rs | 8 +- crates/bevy_ecs/src/schedule/schedule.rs | 56 ++-- crates/bevy_ecs/src/schedule/set.rs | 80 ++++- crates/bevy_ecs/src/system/combinator.rs | 3 +- .../src/system/exclusive_function_system.rs | 5 +- crates/bevy_ecs/src/system/function_system.rs | 5 +- crates/bevy_ecs/src/system/system.rs | 3 +- crates/bevy_ecs/src/world/error.rs | 4 +- crates/bevy_ecs/src/world/mod.rs | 16 +- crates/bevy_macro_utils/src/lib.rs | 60 +++- crates/bevy_render/src/lib.rs | 2 +- crates/bevy_utils/src/intern.rs | 310 ++++++++++++++++++ crates/bevy_utils/src/label.rs | 88 +++-- crates/bevy_utils/src/lib.rs | 1 + 18 files changed, 644 insertions(+), 142 deletions(-) create mode 100644 crates/bevy_utils/src/intern.rs diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 39ed373a785e2..e8b7a350f026b 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -4,7 +4,7 @@ use bevy_ecs::{ prelude::*, schedule::{ apply_state_transition, common_conditions::run_once as run_once_condition, - run_enter_schedule, BoxedScheduleLabel, IntoSystemConfigs, IntoSystemSetConfigs, + run_enter_schedule, InternedScheduleLabel, IntoSystemConfigs, IntoSystemSetConfigs, ScheduleLabel, }, }; @@ -70,7 +70,7 @@ pub struct App { /// The schedule that runs the main loop of schedule execution. /// /// This is initially set to [`Main`]. - pub main_schedule_label: BoxedScheduleLabel, + pub main_schedule_label: InternedScheduleLabel, sub_apps: HashMap, plugin_registry: Vec>, plugin_name_added: HashSet, @@ -219,7 +219,7 @@ impl App { sub_apps: HashMap::default(), plugin_registry: Vec::default(), plugin_name_added: Default::default(), - main_schedule_label: Box::new(Main), + main_schedule_label: Main.into(), building_plugin_depth: 0, } } @@ -379,9 +379,10 @@ impl App { schedule: impl ScheduleLabel, systems: impl IntoSystemConfigs, ) -> &mut Self { + let schedule: InternedScheduleLabel = (&schedule as &dyn ScheduleLabel).into(); let mut schedules = self.world.resource_mut::(); - if let Some(schedule) = schedules.get_mut(&schedule) { + if let Some(schedule) = schedules.get_mut(schedule) { schedule.add_systems(systems); } else { let mut new_schedule = Schedule::new(); @@ -398,8 +399,9 @@ impl App { schedule: impl ScheduleLabel, set: impl IntoSystemSetConfig, ) -> &mut Self { + let schedule: InternedScheduleLabel = (&schedule as &dyn ScheduleLabel).into(); let mut schedules = self.world.resource_mut::(); - if let Some(schedule) = schedules.get_mut(&schedule) { + if let Some(schedule) = schedules.get_mut(schedule) { schedule.configure_set(set); } else { let mut new_schedule = Schedule::new(); @@ -415,8 +417,9 @@ impl App { schedule: impl ScheduleLabel, sets: impl IntoSystemSetConfigs, ) -> &mut Self { + let schedule: InternedScheduleLabel = (&schedule as &dyn ScheduleLabel).into(); let mut schedules = self.world.resource_mut::(); - if let Some(schedule) = schedules.get_mut(&schedule) { + if let Some(schedule) = schedules.get_mut(schedule) { schedule.configure_sets(sets); } else { let mut new_schedule = Schedule::new(); @@ -798,7 +801,7 @@ impl App { /// To avoid this behavior, use the `init_schedule` method instead. pub fn add_schedule(&mut self, label: impl ScheduleLabel, schedule: Schedule) -> &mut Self { let mut schedules = self.world.resource_mut::(); - schedules.insert(label, schedule); + schedules.insert(&label as &dyn ScheduleLabel, schedule); self } @@ -807,8 +810,9 @@ impl App { /// /// See [`App::add_schedule`] to pass in a pre-constructed schedule. pub fn init_schedule(&mut self, label: impl ScheduleLabel) -> &mut Self { + let label: InternedScheduleLabel = (&label as &dyn ScheduleLabel).into(); let mut schedules = self.world.resource_mut::(); - if !schedules.contains(&label) { + if !schedules.contains(label) { schedules.insert(label, Schedule::new()); } self @@ -817,7 +821,7 @@ impl App { /// Gets read-only access to the [`Schedule`] with the provided `label` if it exists. pub fn get_schedule(&self, label: impl ScheduleLabel) -> Option<&Schedule> { let schedules = self.world.get_resource::()?; - schedules.get(&label) + schedules.get(&label as &dyn ScheduleLabel) } /// Gets read-write access to a [`Schedule`] with the provided `label` if it exists. @@ -825,7 +829,7 @@ impl App { let schedules = self.world.get_resource_mut::()?; // We need to call .into_inner here to satisfy the borrow checker: // it can reason about reborrows using ordinary references but not the `Mut` smart pointer. - schedules.into_inner().get_mut(&label) + schedules.into_inner().get_mut(&label as &dyn ScheduleLabel) } /// Applies the function to the [`Schedule`] associated with `label`. @@ -836,13 +840,14 @@ impl App { label: impl ScheduleLabel, f: impl FnOnce(&mut Schedule), ) -> &mut Self { + let label: InternedScheduleLabel = (&label as &dyn ScheduleLabel).into(); let mut schedules = self.world.resource_mut::(); - if schedules.get(&label).is_none() { - schedules.insert(label.dyn_clone(), Schedule::new()); + if schedules.get(label).is_none() { + schedules.insert(label, Schedule::new()); } - let schedule = schedules.get_mut(&label).unwrap(); + let schedule = schedules.get_mut(label).unwrap(); // Call the function f, passing in the schedule retrieved f(schedule); diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 5bfbb3cfee1e6..5ea4c916dcc2d 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -7,7 +7,7 @@ mod states; use crate::{fetch::derive_world_query_impl, set::derive_set}; use bevy_macro_utils::{ - derive_boxed_label, ensure_no_collision, get_named_struct_fields, BevyManifest, + derive_interned_label, ensure_no_collision, get_named_struct_fields, BevyManifest, }; use proc_macro::TokenStream; use proc_macro2::Span; @@ -442,7 +442,7 @@ pub fn derive_schedule_label(input: TokenStream) -> TokenStream { trait_path .segments .push(format_ident!("ScheduleLabel").into()); - derive_boxed_label(input, &trait_path) + derive_interned_label(input, &trait_path) } /// Derive macro generating an impl of the trait `SystemSet`. diff --git a/crates/bevy_ecs/macros/src/set.rs b/crates/bevy_ecs/macros/src/set.rs index 66bfb601ecf35..2a759f133b054 100644 --- a/crates/bevy_ecs/macros/src/set.rs +++ b/crates/bevy_ecs/macros/src/set.rs @@ -1,5 +1,6 @@ use proc_macro::TokenStream; -use quote::quote; +use quote::{quote, quote_spanned}; +use syn::spanned::Spanned; /// Derive a set trait /// @@ -21,12 +22,56 @@ pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStrea }) .unwrap(), ); - + let dyn_static_ref_impl = match input.data { + syn::Data::Struct(data) => { + if data.fields.is_empty() { + quote! { std::option::Option::Some(&Self) } + } else { + quote! { std::option::Option::None } + } + } + syn::Data::Enum(data) => { + let mut use_fallback_variant = false; + let variants: Vec<_> = data + .variants + .into_iter() + .filter_map(|variant| { + if variant.fields.is_empty() { + let span = variant.span(); + let variant_ident = variant.ident; + Some(quote_spanned! { span => Self::#variant_ident => std::option::Option::Some(&Self::#variant_ident), }) + } else { + use_fallback_variant = true; + None + } + }) + .collect(); + if use_fallback_variant { + quote! { + match self { + #(#variants)* + _ => None + } + } + } else { + quote! { + match self { + #(#variants)* + } + } + } + } + syn::Data::Union(_) => quote! { std::option::Option::None }, + }; (quote! { impl #impl_generics #trait_path for #ident #ty_generics #where_clause { fn dyn_clone(&self) -> std::boxed::Box { std::boxed::Box::new(std::clone::Clone::clone(self)) } + + fn dyn_static_ref(&self) -> std::option::Option<&'static dyn #trait_path> { + #dyn_static_ref_impl + } } }) .into() diff --git a/crates/bevy_ecs/src/schedule/config.rs b/crates/bevy_ecs/src/schedule/config.rs index 708661035ed55..705f37bf587e0 100644 --- a/crates/bevy_ecs/src/schedule/config.rs +++ b/crates/bevy_ecs/src/schedule/config.rs @@ -4,7 +4,7 @@ use crate::{ schedule::{ condition::{BoxedCondition, Condition}, graph_utils::{Ambiguity, Dependency, DependencyKind, GraphInfo}, - set::{BoxedSystemSet, IntoSystemSet, SystemSet}, + set::{InternedSystemSet, IntoSystemSet, SystemSet}, }, system::{BoxedSystem, IntoSystem, System}, }; @@ -20,7 +20,7 @@ fn new_condition(condition: impl Condition) -> BoxedCondition { Box::new(condition_system) } -fn ambiguous_with(graph_info: &mut GraphInfo, set: BoxedSystemSet) { +fn ambiguous_with(graph_info: &mut GraphInfo, set: InternedSystemSet) { match &mut graph_info.ambiguous_with { detection @ Ambiguity::Check => { *detection = Ambiguity::IgnoreWithSet(vec![set]); @@ -83,20 +83,20 @@ impl SystemConfigs { }) } - pub(crate) fn in_set_inner(&mut self, set: BoxedSystemSet) { + pub(crate) fn in_set_inner(&mut self, set: InternedSystemSet) { match self { SystemConfigs::SystemConfig(config) => { config.graph_info.sets.push(set); } SystemConfigs::Configs { configs, .. } => { for config in configs { - config.in_set_inner(set.dyn_clone()); + config.in_set_inner(set); } } } } - fn before_inner(&mut self, set: BoxedSystemSet) { + fn before_inner(&mut self, set: InternedSystemSet) { match self { SystemConfigs::SystemConfig(config) => { config @@ -106,13 +106,13 @@ impl SystemConfigs { } SystemConfigs::Configs { configs, .. } => { for config in configs { - config.before_inner(set.dyn_clone()); + config.before_inner(set); } } } } - fn after_inner(&mut self, set: BoxedSystemSet) { + fn after_inner(&mut self, set: InternedSystemSet) { match self { SystemConfigs::SystemConfig(config) => { config @@ -122,7 +122,7 @@ impl SystemConfigs { } SystemConfigs::Configs { configs, .. } => { for config in configs { - config.after_inner(set.dyn_clone()); + config.after_inner(set); } } } @@ -141,14 +141,14 @@ impl SystemConfigs { } } - fn ambiguous_with_inner(&mut self, set: BoxedSystemSet) { + fn ambiguous_with_inner(&mut self, set: InternedSystemSet) { match self { SystemConfigs::SystemConfig(config) => { ambiguous_with(&mut config.graph_info, set); } SystemConfigs::Configs { configs, .. } => { for config in configs { - config.ambiguous_with_inner(set.dyn_clone()); + config.ambiguous_with_inner(set); } } } @@ -307,20 +307,20 @@ impl IntoSystemConfigs<()> for SystemConfigs { "adding arbitrary systems to a system type set is not allowed" ); - self.in_set_inner(set.dyn_clone()); + self.in_set_inner((&set as &dyn SystemSet).into()); self } fn before(mut self, set: impl IntoSystemSet) -> Self { let set = set.into_system_set(); - self.before_inner(set.dyn_clone()); + self.before_inner((&set as &dyn SystemSet).into()); self } fn after(mut self, set: impl IntoSystemSet) -> Self { let set = set.into_system_set(); - self.after_inner(set.dyn_clone()); + self.after_inner((&set as &dyn SystemSet).into()); self } @@ -331,7 +331,7 @@ impl IntoSystemConfigs<()> for SystemConfigs { fn ambiguous_with(mut self, set: impl IntoSystemSet) -> Self { let set = set.into_system_set(); - self.ambiguous_with_inner(set.dyn_clone()); + self.ambiguous_with_inner((&set as &dyn SystemSet).into()); self } @@ -382,13 +382,13 @@ all_tuples!(impl_system_collection, 1, 20, P, S); /// A [`SystemSet`] with scheduling metadata. pub struct SystemSetConfig { - pub(super) set: BoxedSystemSet, + pub(super) set: InternedSystemSet, pub(super) graph_info: GraphInfo, pub(super) conditions: Vec, } impl SystemSetConfig { - fn new(set: BoxedSystemSet) -> Self { + fn new(set: InternedSystemSet) -> Self { // system type sets are automatically populated // to avoid unintentionally broad changes, they cannot be configured assert!( @@ -445,11 +445,11 @@ pub trait IntoSystemSetConfig: Sized { impl IntoSystemSetConfig for S { fn into_config(self) -> SystemSetConfig { - SystemSetConfig::new(Box::new(self)) + SystemSetConfig::new((&self as &dyn SystemSet).into()) } } -impl IntoSystemSetConfig for BoxedSystemSet { +impl IntoSystemSetConfig for InternedSystemSet { fn into_config(self) -> SystemSetConfig { SystemSetConfig::new(self) } @@ -466,14 +466,14 @@ impl IntoSystemSetConfig for SystemSetConfig { set.system_type().is_none(), "adding arbitrary systems to a system type set is not allowed" ); - self.graph_info.sets.push(Box::new(set)); + self.graph_info.sets.push((&set as &dyn SystemSet).into()); self } fn before(mut self, set: impl IntoSystemSet) -> Self { self.graph_info.dependencies.push(Dependency::new( DependencyKind::Before, - Box::new(set.into_system_set()), + (&set.into_system_set() as &dyn SystemSet).into(), )); self } @@ -481,7 +481,7 @@ impl IntoSystemSetConfig for SystemSetConfig { fn after(mut self, set: impl IntoSystemSet) -> Self { self.graph_info.dependencies.push(Dependency::new( DependencyKind::After, - Box::new(set.into_system_set()), + (&set.into_system_set() as &dyn SystemSet).into(), )); self } @@ -492,7 +492,10 @@ impl IntoSystemSetConfig for SystemSetConfig { } fn ambiguous_with(mut self, set: impl IntoSystemSet) -> Self { - ambiguous_with(&mut self.graph_info, Box::new(set.into_system_set())); + ambiguous_with( + &mut self.graph_info, + (&set.into_system_set() as &dyn SystemSet).into(), + ); self } @@ -566,40 +569,40 @@ impl IntoSystemSetConfigs for SystemSetConfigs { "adding arbitrary systems to a system type set is not allowed" ); for config in &mut self.sets { - config.graph_info.sets.push(set.dyn_clone()); + config.graph_info.sets.push((&set as &dyn SystemSet).into()); } self } fn before(mut self, set: impl IntoSystemSet) -> Self { - let set = set.into_system_set(); + let set = (&set.into_system_set() as &dyn SystemSet).into(); for config in &mut self.sets { config .graph_info .dependencies - .push(Dependency::new(DependencyKind::Before, set.dyn_clone())); + .push(Dependency::new(DependencyKind::Before, set)); } self } fn after(mut self, set: impl IntoSystemSet) -> Self { - let set = set.into_system_set(); + let set = (&set.into_system_set() as &dyn SystemSet).into(); for config in &mut self.sets { config .graph_info .dependencies - .push(Dependency::new(DependencyKind::After, set.dyn_clone())); + .push(Dependency::new(DependencyKind::After, set)); } self } fn ambiguous_with(mut self, set: impl IntoSystemSet) -> Self { - let set = set.into_system_set(); + let set = (&set.into_system_set() as &dyn SystemSet).into(); for config in &mut self.sets { - ambiguous_with(&mut config.graph_info, set.dyn_clone()); + ambiguous_with(&mut config.graph_info, set); } self diff --git a/crates/bevy_ecs/src/schedule/graph_utils.rs b/crates/bevy_ecs/src/schedule/graph_utils.rs index 9209ff05b94b8..dd262877e54ab 100644 --- a/crates/bevy_ecs/src/schedule/graph_utils.rs +++ b/crates/bevy_ecs/src/schedule/graph_utils.rs @@ -51,11 +51,11 @@ pub(crate) enum DependencyKind { #[derive(Clone)] pub(crate) struct Dependency { pub(crate) kind: DependencyKind, - pub(crate) set: BoxedSystemSet, + pub(crate) set: InternedSystemSet, } impl Dependency { - pub fn new(kind: DependencyKind, set: BoxedSystemSet) -> Self { + pub fn new(kind: DependencyKind, set: InternedSystemSet) -> Self { Self { kind, set } } } @@ -66,14 +66,14 @@ pub(crate) enum Ambiguity { #[default] Check, /// Ignore warnings with systems in any of these system sets. May contain duplicates. - IgnoreWithSet(Vec), + IgnoreWithSet(Vec), /// Ignore all warnings. IgnoreAll, } #[derive(Clone, Default)] pub(crate) struct GraphInfo { - pub(crate) sets: Vec, + pub(crate) sets: Vec, pub(crate) dependencies: Vec, pub(crate) ambiguous_with: Ambiguity, } diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index 4f1b90ff9050d..3ed1a32a0711d 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -26,7 +26,7 @@ use crate::{ /// Resource that stores [`Schedule`]s mapped to [`ScheduleLabel`]s. #[derive(Default, Resource)] pub struct Schedules { - inner: HashMap, + inner: HashMap, } impl Schedules { @@ -41,37 +41,40 @@ impl Schedules { /// /// If the map already had an entry for `label`, `schedule` is inserted, /// and the old schedule is returned. Otherwise, `None` is returned. - pub fn insert(&mut self, label: impl ScheduleLabel, schedule: Schedule) -> Option { - let label = label.dyn_clone(); - self.inner.insert(label, schedule) + pub fn insert( + &mut self, + label: impl Into, + schedule: Schedule, + ) -> Option { + self.inner.insert(label.into(), schedule) } /// Removes the schedule corresponding to the `label` from the map, returning it if it existed. - pub fn remove(&mut self, label: &dyn ScheduleLabel) -> Option { - self.inner.remove(label) + pub fn remove(&mut self, label: impl Into) -> Option { + self.inner.remove(&label.into()) } /// Removes the (schedule, label) pair corresponding to the `label` from the map, returning it if it existed. pub fn remove_entry( &mut self, - label: &dyn ScheduleLabel, - ) -> Option<(Box, Schedule)> { - self.inner.remove_entry(label) + label: impl Into, + ) -> Option<(InternedScheduleLabel, Schedule)> { + self.inner.remove_entry(&label.into()) } /// Does a schedule with the provided label already exist? - pub fn contains(&self, label: &dyn ScheduleLabel) -> bool { - self.inner.contains_key(label) + pub fn contains(&self, label: impl Into) -> bool { + self.inner.contains_key(&label.into()) } /// Returns a reference to the schedule associated with `label`, if it exists. - pub fn get(&self, label: &dyn ScheduleLabel) -> Option<&Schedule> { - self.inner.get(label) + pub fn get(&self, label: impl Into) -> Option<&Schedule> { + self.inner.get(&label.into()) } /// Returns a mutable reference to the schedule associated with `label`, if it exists. - pub fn get_mut(&mut self, label: &dyn ScheduleLabel) -> Option<&mut Schedule> { - self.inner.get_mut(label) + pub fn get_mut(&mut self, label: impl Into) -> Option<&mut Schedule> { + self.inner.get_mut(&label.into()) } /// Returns an iterator over all schedules. Iteration order is undefined. @@ -329,11 +332,11 @@ impl Dag { /// A [`SystemSet`] with metadata, stored in a [`ScheduleGraph`]. struct SystemSetNode { - inner: BoxedSystemSet, + inner: InternedSystemSet, } impl SystemSetNode { - pub fn new(set: BoxedSystemSet) -> Self { + pub fn new(set: InternedSystemSet) -> Self { Self { inner: set } } @@ -378,7 +381,7 @@ pub struct ScheduleGraph { system_conditions: Vec>, system_sets: Vec, system_set_conditions: Vec>, - system_set_ids: HashMap, + system_set_ids: HashMap, uninit: Vec<(NodeId, usize)>, hierarchy: Dag, dependency: Dag, @@ -532,7 +535,7 @@ impl ScheduleGraph { if more_than_one_entry { let set = AnonymousSet::new(); for config in &mut configs { - config.in_set_inner(set.dyn_clone()); + config.in_set_inner((&set as &dyn SystemSet).into()); } let mut set_config = set.into_config(); set_config.conditions.extend(collective_conditions); @@ -694,7 +697,7 @@ impl ScheduleGraph { let id = match self.system_set_ids.get(&set) { Some(&id) => id, - None => self.add_set(set.dyn_clone()), + None => self.add_set(set), }; // graph updates are immediate @@ -708,23 +711,24 @@ impl ScheduleGraph { Ok(id) } - fn add_set(&mut self, set: BoxedSystemSet) -> NodeId { + fn add_set(&mut self, set: InternedSystemSet) -> NodeId { let id = NodeId::Set(self.system_sets.len()); - self.system_sets.push(SystemSetNode::new(set.dyn_clone())); + self.system_sets.push(SystemSetNode::new(set)); self.system_set_conditions.push(Vec::new()); self.system_set_ids.insert(set, id); id } fn check_set(&mut self, id: &NodeId, set: &dyn SystemSet) -> Result<(), ScheduleBuildError> { - match self.system_set_ids.get(set) { + let set = set.into(); + match self.system_set_ids.get(&set) { Some(set_id) => { if id == set_id { return Err(ScheduleBuildError::HierarchyLoop(self.get_node_name(id))); } } None => { - self.add_set(set.dyn_clone()); + self.add_set(set); } } @@ -756,7 +760,7 @@ impl ScheduleGraph { } } None => { - self.add_set(set.dyn_clone()); + self.add_set(*set); } } } @@ -764,7 +768,7 @@ impl ScheduleGraph { if let Ambiguity::IgnoreWithSet(ambiguous_with) = &graph_info.ambiguous_with { for set in ambiguous_with { if !self.system_set_ids.contains_key(set) { - self.add_set(set.dyn_clone()); + self.add_set(*set); } } } diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index 6e0caaba0937c..232f1e1924870 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -1,23 +1,26 @@ use std::any::TypeId; +use std::borrow::Borrow; use std::fmt::Debug; use std::hash::{Hash, Hasher}; use std::marker::PhantomData; use std::sync::atomic::{AtomicUsize, Ordering}; pub use bevy_ecs_macros::{ScheduleLabel, SystemSet}; -use bevy_utils::define_boxed_label; +use bevy_utils::define_interned_label; +use bevy_utils::intern::{Interned, Leak, OptimizedInterner, StaticRef}; use bevy_utils::label::DynHash; use crate::system::{ ExclusiveSystemParamFunction, IsExclusiveFunctionSystem, IsFunctionSystem, SystemParamFunction, }; -define_boxed_label!(ScheduleLabel); +define_interned_label!(ScheduleLabel, SCHEDULE_LABEL_INTERNER); -/// A shorthand for `Box`. -pub type BoxedSystemSet = Box; -/// A shorthand for `Box`. -pub type BoxedScheduleLabel = Box; +static SYSTEM_SET_INTERNER: OptimizedInterner = OptimizedInterner::new(); +/// A shorthand for `Interned`. +pub type InternedSystemSet = Interned; +/// A shorthand for `Interned`. +pub type InternedScheduleLabel = Interned; /// Types that identify logical groups of systems. pub trait SystemSet: DynHash + Debug + Send + Sync + 'static { @@ -30,8 +33,55 @@ pub trait SystemSet: DynHash + Debug + Send + Sync + 'static { fn is_anonymous(&self) -> bool { false } + /// Creates a boxed clone of the label corresponding to this system set. fn dyn_clone(&self) -> Box; + + /// Returns a static reference to a value equal to `self`, if possible. + /// This method is used to optimize [interning](bevy_utils::intern). + /// + /// # Invariant + /// + /// The following invariant must be hold: + /// + /// `ptr_eq(a.dyn_static_ref(), b.dyn_static_ref()) == true` if and only if `a.dyn_eq(b)` + /// + /// where `ptr_eq` is defined as : + /// ``` + /// fn ptr_eq(x: Option<&'static T>, y: Option<&'static T>) -> bool { + /// match (x, y) { + /// (Some(x), Some(y)) => std::ptr::eq(x, y), + /// _ => false, + /// } + /// } + /// ``` + /// + /// # Provided implementation + /// + /// The provided implementation always returns `None`. + fn dyn_static_ref(&self) -> Option<&'static dyn SystemSet> { + None + } +} + +impl From<&dyn SystemSet> for Interned { + fn from(value: &dyn SystemSet) -> Interned { + struct LeakHelper<'a>(&'a dyn SystemSet); + + impl<'a> Borrow for LeakHelper<'a> { + fn borrow(&self) -> &dyn SystemSet { + self.0 + } + } + + impl<'a> Leak for LeakHelper<'a> { + fn leak(self) -> &'static dyn SystemSet { + Box::leak(self.0.dyn_clone()) + } + } + + SYSTEM_SET_INTERNER.intern(LeakHelper(value)) + } } impl PartialEq for dyn SystemSet { @@ -48,9 +98,9 @@ impl Hash for dyn SystemSet { } } -impl Clone for Box { - fn clone(&self) -> Self { - self.dyn_clone() +impl StaticRef for dyn SystemSet { + fn static_ref(&self) -> Option<&'static dyn SystemSet> { + self.dyn_static_ref() } } @@ -107,6 +157,10 @@ impl SystemSet for SystemTypeSet { fn dyn_clone(&self) -> Box { Box::new(*self) } + + fn dyn_static_ref(&self) -> Option<&'static dyn SystemSet> { + Some(&Self(PhantomData)) + } } /// A [`SystemSet`] implicitly created when using @@ -187,7 +241,7 @@ mod tests { use super::*; #[test] - fn test_boxed_label() { + fn test_interned_label() { use crate::{self as bevy_ecs, world::World}; #[derive(Resource)] @@ -202,14 +256,14 @@ mod tests { schedule.add_systems(|mut flag: ResMut| flag.0 = true); world.add_schedule(schedule, A); - let boxed: Box = Box::new(A); + let interned: Interned = (&A as &dyn ScheduleLabel).into(); world.insert_resource(Flag(false)); - world.run_schedule(&boxed); + world.run_schedule(interned); assert!(world.resource::().0); world.insert_resource(Flag(false)); - world.run_schedule(boxed); + world.run_schedule(interned); assert!(world.resource::().0); } } diff --git a/crates/bevy_ecs/src/system/combinator.rs b/crates/bevy_ecs/src/system/combinator.rs index 13c2b7869303d..6b064c24547d8 100644 --- a/crates/bevy_ecs/src/system/combinator.rs +++ b/crates/bevy_ecs/src/system/combinator.rs @@ -7,6 +7,7 @@ use crate::{ component::{ComponentId, Tick}, prelude::World, query::Access, + schedule::InternedSystemSet, world::unsafe_world_cell::UnsafeWorldCell, }; @@ -226,7 +227,7 @@ where self.b.set_last_run(last_run); } - fn default_system_sets(&self) -> Vec> { + fn default_system_sets(&self) -> Vec { let mut default_sets = self.a.default_system_sets(); default_sets.append(&mut self.b.default_system_sets()); default_sets diff --git a/crates/bevy_ecs/src/system/exclusive_function_system.rs b/crates/bevy_ecs/src/system/exclusive_function_system.rs index 6d2a31f554201..b3d18fd5bbeda 100644 --- a/crates/bevy_ecs/src/system/exclusive_function_system.rs +++ b/crates/bevy_ecs/src/system/exclusive_function_system.rs @@ -2,6 +2,7 @@ use crate::{ archetype::ArchetypeComponentId, component::{ComponentId, Tick}, query::Access, + schedule::{InternedSystemSet, SystemSet}, system::{ check_system_change_tick, ExclusiveSystemParam, ExclusiveSystemParamItem, In, IntoSystem, System, SystemMeta, @@ -147,9 +148,9 @@ where ); } - fn default_system_sets(&self) -> Vec> { + fn default_system_sets(&self) -> Vec { let set = crate::schedule::SystemTypeSet::::new(); - vec![Box::new(set)] + vec![(&set as &dyn SystemSet).into()] } } diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index e245f93716d78..47d549e5bbbf9 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -3,6 +3,7 @@ use crate::{ component::{ComponentId, Tick}, prelude::FromWorld, query::{Access, FilteredAccessSet}, + schedule::{InternedSystemSet, SystemSet}, system::{check_system_change_tick, ReadOnlySystemParam, System, SystemParam, SystemParamItem}, world::{unsafe_world_cell::UnsafeWorldCell, World, WorldId}, }; @@ -509,9 +510,9 @@ where ); } - fn default_system_sets(&self) -> Vec> { + fn default_system_sets(&self) -> Vec { let set = crate::schedule::SystemTypeSet::::new(); - vec![Box::new(set)] + vec![(&set as &dyn SystemSet).into()] } } diff --git a/crates/bevy_ecs/src/system/system.rs b/crates/bevy_ecs/src/system/system.rs index 462774460bfc7..c1b95ef52ff05 100644 --- a/crates/bevy_ecs/src/system/system.rs +++ b/crates/bevy_ecs/src/system/system.rs @@ -2,6 +2,7 @@ use bevy_utils::tracing::warn; use core::fmt::Debug; use crate::component::Tick; +use crate::schedule::InternedSystemSet; use crate::world::unsafe_world_cell::UnsafeWorldCell; use crate::{archetype::ArchetypeComponentId, component::ComponentId, query::Access, world::World}; @@ -91,7 +92,7 @@ pub trait System: Send + Sync + 'static { fn check_change_tick(&mut self, change_tick: Tick); /// Returns the system's default [system sets](crate::schedule::SystemSet). - fn default_system_sets(&self) -> Vec> { + fn default_system_sets(&self) -> Vec { Vec::new() } diff --git a/crates/bevy_ecs/src/world/error.rs b/crates/bevy_ecs/src/world/error.rs index f3794bb03d61c..326b0310ba15c 100644 --- a/crates/bevy_ecs/src/world/error.rs +++ b/crates/bevy_ecs/src/world/error.rs @@ -2,11 +2,11 @@ use thiserror::Error; -use crate::schedule::BoxedScheduleLabel; +use crate::schedule::InternedScheduleLabel; /// The error type returned by [`World::try_run_schedule`] if the provided schedule does not exist. /// /// [`World::try_run_schedule`]: crate::world::World::try_run_schedule #[derive(Error, Debug)] #[error("The schedule with the label {0:?} was not found.")] -pub struct TryRunScheduleError(pub BoxedScheduleLabel); +pub struct TryRunScheduleError(pub InternedScheduleLabel); diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 0e79b19ce8568..d6c4d71de33c5 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -20,7 +20,7 @@ use crate::{ event::{Event, Events}, query::{DebugCheckedUnwrap, QueryState, ReadOnlyWorldQuery, WorldQuery}, removal_detection::RemovedComponentEvents, - schedule::{Schedule, ScheduleLabel, Schedules}, + schedule::{InternedScheduleLabel, Schedule, ScheduleLabel, Schedules}, storage::{ResourceData, Storages}, system::Resource, world::error::TryRunScheduleError, @@ -1750,7 +1750,7 @@ impl World { /// The `Schedules` resource will be initialized if it does not already exist. pub fn add_schedule(&mut self, schedule: Schedule, label: impl ScheduleLabel) { let mut schedules = self.get_resource_or_insert_with(Schedules::default); - schedules.insert(label, schedule); + schedules.insert(&label as &dyn ScheduleLabel, schedule); } /// Temporarily removes the schedule associated with `label` from the world, @@ -1766,15 +1766,15 @@ impl World { /// For other use cases, see the example on [`World::schedule_scope`]. pub fn try_schedule_scope( &mut self, - label: impl AsRef, + label: impl Into, f: impl FnOnce(&mut World, &mut Schedule) -> R, ) -> Result { - let label = label.as_ref(); + let label = label.into(); let Some((extracted_label, mut schedule)) = self .get_resource_mut::() .and_then(|mut s| s.remove_entry(label)) else { - return Err(TryRunScheduleError(label.dyn_clone())); + return Err(TryRunScheduleError(label)); }; // TODO: move this span to Schedule::run @@ -1831,7 +1831,7 @@ impl World { /// If the requested schedule does not exist. pub fn schedule_scope( &mut self, - label: impl AsRef, + label: impl Into, f: impl FnOnce(&mut World, &mut Schedule) -> R, ) -> R { self.try_schedule_scope(label, f) @@ -1847,7 +1847,7 @@ impl World { /// For simple testing use cases, call [`Schedule::run(&mut world)`](Schedule::run) instead. pub fn try_run_schedule( &mut self, - label: impl AsRef, + label: impl Into, ) -> Result<(), TryRunScheduleError> { self.try_schedule_scope(label, |world, sched| sched.run(world)) } @@ -1862,7 +1862,7 @@ impl World { /// # Panics /// /// If the requested schedule does not exist. - pub fn run_schedule(&mut self, label: impl AsRef) { + pub fn run_schedule(&mut self, label: impl Into) { self.schedule_scope(label, |world, sched| sched.run(world)); } } diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index 59e224d51194d..3dd14375a08e3 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -169,7 +169,7 @@ pub fn ensure_no_collision(value: Ident, haystack: TokenStream) -> Ident { /// /// - `input`: The [`syn::DeriveInput`] for struct that is deriving the label trait /// - `trait_path`: The path [`syn::Path`] to the label trait -pub fn derive_boxed_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStream { +pub fn derive_interned_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStream { let bevy_utils_path = BevyManifest::default().get_path("bevy_utils"); let ident = input.ident; @@ -184,7 +184,47 @@ pub fn derive_boxed_label(input: syn::DeriveInput, trait_path: &syn::Path) -> To }) .unwrap(), ); - + let dyn_static_ref_impl = match input.data { + syn::Data::Struct(data) => { + if data.fields.is_empty() { + quote! { std::option::Option::Some(&Self) } + } else { + quote! { std::option::Option::None } + } + } + syn::Data::Enum(data) => { + let mut use_fallback_variant = false; + let variants: Vec<_> = data + .variants + .into_iter() + .filter_map(|variant| { + if variant.fields.is_empty() { + let span = variant.span(); + let variant_ident = variant.ident; + Some(quote_spanned! { span => Self::#variant_ident => std::option::Option::Some(&Self::#variant_ident), }) + } else { + use_fallback_variant = true; + None + } + }) + .collect(); + if use_fallback_variant { + quote! { + match self { + #(#variants)* + _ => quote! { std::option::Option::None } + } + } + } else { + quote! { + match self { + #(#variants)* + } + } + } + } + syn::Data::Union(_) => quote! { std::option::Option::None }, + }; (quote! { impl #impl_generics #trait_path for #ident #ty_generics #where_clause { fn dyn_clone(&self) -> std::boxed::Box { @@ -200,11 +240,21 @@ pub fn derive_boxed_label(input: syn::DeriveInput, trait_path: &syn::Path) -> To ::std::hash::Hash::hash(&ty_id, &mut state); ::std::hash::Hash::hash(self, &mut state); } + + fn dyn_static_ref(&self) -> std::option::Option<&'static dyn #trait_path> { + #dyn_static_ref_impl + } } - impl #impl_generics ::std::convert::AsRef for #ident #ty_generics #where_clause { - fn as_ref(&self) -> &dyn #trait_path { - self + impl #impl_generics Into<::bevy_utils::intern::Interned> for #ident #ty_generics #where_clause { + fn into(self) -> ::bevy_utils::intern::Interned { + (&self as &dyn #trait_path).into() + } + } + + impl #impl_generics Into<::bevy_utils::intern::Interned> for & #ident #ty_generics #where_clause { + fn into(self) -> ::bevy_utils::intern::Interned { + (self as &dyn #trait_path).into() } } }) diff --git a/crates/bevy_render/src/lib.rs b/crates/bevy_render/src/lib.rs index bab3e4b73cacd..cf5dce491ea1a 100644 --- a/crates/bevy_render/src/lib.rs +++ b/crates/bevy_render/src/lib.rs @@ -270,7 +270,7 @@ impl Plugin for RenderPlugin { app.init_resource::(); let mut render_app = App::empty(); - render_app.main_schedule_label = Box::new(Render); + render_app.main_schedule_label = Render.into(); let mut extract_schedule = Schedule::new(); extract_schedule.set_apply_final_deferred(false); diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs new file mode 100644 index 0000000000000..1863a014ebefe --- /dev/null +++ b/crates/bevy_utils/src/intern.rs @@ -0,0 +1,310 @@ +//! Provides types used to statically intern immutable values. +//! +//! Interning is a pattern used to save memory by deduplicating identical values, +//! speed up code by shrinking the stack size of large types, +//! and make comparisons for any type as fast as integers. + +use std::{ + borrow::{Borrow, Cow}, + fmt::Debug, + hash::Hash, + ops::Deref, + ptr, + sync::{OnceLock, PoisonError, RwLock}, +}; + +use crate::HashSet; + +/// An interned value. Will stay valid until the end of the program and will not drop. +/// +/// For details on interning, see [the module level docs](self). +/// +/// # Implementation details +/// +/// This is just a reference to a value with an `'static` lifetime. +/// +/// This implements [`Copy`], [`Clone`], [`PartialEq`], [`Eq`] and [`Hash`] regardles of what `T` +/// implements, because it only uses the pointer to the value and not the value itself. +/// Therefore it MUST NEVER implement [`Borrow`](`std::borrow::Borrow`). +pub struct Interned(&'static T); + +impl Deref for Interned { + type Target = T; + + fn deref(&self) -> &Self::Target { + self.0 + } +} + +impl Clone for Interned { + fn clone(&self) -> Self { + Self(self.0) + } +} + +impl Copy for Interned {} + +// Two Interned should only be equal if they are clones from the same instance. +// Therefore, we only use the pointer to determine equality. +impl PartialEq for Interned { + fn eq(&self, other: &Self) -> bool { + ptr::eq(self.0, other.0) + } +} + +impl Eq for Interned {} + +// Important: This must be kept in sync with the PartialEq/Eq implementation +impl Hash for Interned { + fn hash(&self, state: &mut H) { + ptr::hash(self.0, state); + } +} + +impl Debug for Interned { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.0.fmt(f) + } +} + +impl From<&Interned> for Interned { + fn from(value: &Interned) -> Self { + *value + } +} + +/// A trait for leaking data. +/// +/// This is used by [`Interner`] to create static references for values that are interned. +/// +/// It is implemented for static references of `T`, [`Box`], and [`Cow<'static, T>`]. +pub trait Leak: Borrow { + /// Creates a static reference to a value equal to `self.borrow()`, possibly leaking memory. + fn leak(self) -> &'static T; +} + +impl Leak for &'static T { + fn leak(self) -> &'static T { + self + } +} + +impl Leak for Box { + fn leak(self) -> &'static T { + Box::leak(self) + } +} + +impl Leak for Cow<'static, T> { + fn leak(self) -> &'static T { + match self { + Cow::Borrowed(value) => value, + Cow::Owned(value) => Box::leak(Box::new(value)), + } + } +} + +/// A thread-safe interner which can be used to create [`Interned`] from `&T` +/// +/// For details on interning, see [the module level docs](self). +/// +/// The implementation ensures that two equal values return two equal [`Interned`] values. +pub struct Interner(OnceLock>>); + +impl Interner { + /// Creates a new empty interner + pub const fn new() -> Self { + Self(OnceLock::new()) + } +} + +impl Interner { + /// Return the [`Interned`] corresponding to `value`. + /// + /// If it is called the first time for `value`, it will possibly leak the value and return an + /// [`Interned`] using the obtained static reference. Subsequent calls for the same `value` + /// will return [`Interned`] using the same static reference. + pub fn intern>(&self, value: Q) -> Interned { + let lock = self.0.get_or_init(Default::default); + { + let set = lock.read().unwrap_or_else(PoisonError::into_inner); + if let Some(value) = set.get(value.borrow()) { + return Interned(*value); + } + } + { + let mut set = lock.write().unwrap_or_else(PoisonError::into_inner); + if let Some(value) = set.get(value.borrow()) { + Interned(*value) + } else { + let leaked = value.leak(); + set.insert(leaked); + Interned(leaked) + } + } + } +} + +impl Default for Interner { + fn default() -> Self { + Self::new() + } +} + +/// A type that can provide static references to equal values. +pub trait StaticRef { + /// Returns a static reference to a value equal to `self`, if possible. + /// This method is used by [`OptimizedInterner::intern`] to optimize the interning process. + /// + /// # Invariant + /// + /// The following invariant must be hold: + /// + /// `ptr_eq(a.static_ref(), b.static_ref()) == true` if and only if `a == b` + /// + /// where `ptr_eq` is defined as : + /// ``` + /// fn ptr_eq(x: Option<&'static T>, y: Option<&'static T>) -> bool { + /// match (x, y) { + /// (Some(x), Some(y)) => std::ptr::eq(x, y), + /// _ => false, + /// } + /// } + /// ``` + fn static_ref(&self) -> Option<&'static Self>; +} + +/// An optimized [`Interner`] for types that implement [`StaticRef`]. +pub struct OptimizedInterner(Interner); + +impl OptimizedInterner { + /// Creates a new empty interner + pub const fn new() -> Self { + Self(Interner::new()) + } +} + +impl OptimizedInterner { + /// Return the [`Interned`] corresponding to `value`. + /// + /// If it is called the first time for `value`, it will possibly leak the value and return an + /// [`Interned`] using the obtained static reference. Subsequent calls for the same `value` + /// will return [`Interned`] using the same static reference. + /// + /// Compared to [`Interner::intern`], this uses [`StaticRef::static_ref`] to short-circuit + /// the interning process. + pub fn intern>(&self, value: Q) -> Interned { + if let Some(value) = value.borrow().static_ref() { + return Interned(value); + } + self.0.intern(value) + } +} + +impl Default for OptimizedInterner { + fn default() -> Self { + Self::new() + } +} + +#[cfg(test)] +mod tests { + use std::{ + collections::hash_map::DefaultHasher, + hash::{Hash, Hasher}, + }; + + use crate::intern::{Interned, Interner, Leak}; + + #[test] + fn zero_sized_type() { + #[derive(PartialEq, Eq, Hash, Debug)] + pub struct A; + + impl Leak for A { + fn leak(self) -> &'static Self { + &A + } + } + + let interner = Interner::default(); + let x = interner.intern(&A); + let y = interner.intern(&A); + assert_eq!(x, y); + } + + #[test] + fn fieldless_enum() { + #[derive(PartialEq, Eq, Hash, Debug)] + pub enum A { + X, + Y, + } + + impl Leak for A { + fn leak(self) -> &'static Self { + match self { + A::X => &A::X, + A::Y => &A::Y, + } + } + } + + let interner = Interner::default(); + let x = interner.intern(A::X); + let y = interner.intern(A::Y); + assert_ne!(x, y); + } + + #[test] + fn static_sub_strings() { + let str = "ABC ABC"; + let a = &str[0..3]; + let b = &str[4..7]; + // Same contents + assert_eq!(a, b); + let x = Interned(a); + let y = Interned(b); + // Different pointers + assert_ne!(x, y); + let interner = Interner::default(); + let x = interner.intern(a); + let y = interner.intern(b); + // Same pointers returned by interner + assert_eq!(x, y); + } + + #[test] + fn same_interned_instance() { + let a = Interned("A"); + let b = a; + + assert_eq!(a, b); + + let mut hasher = DefaultHasher::default(); + a.hash(&mut hasher); + let hash_a = hasher.finish(); + + let mut hasher = DefaultHasher::default(); + b.hash(&mut hasher); + let hash_b = hasher.finish(); + + assert_eq!(hash_a, hash_b); + } + + #[test] + fn same_interned_content() { + let a = Interned(Box::leak(Box::new("A"))); + let b = Interned(Box::leak(Box::new("A"))); + + assert_ne!(a, b); + } + + #[test] + fn different_interned_content() { + let a = Interned(Box::leak(Box::new("A"))); + let b = Interned(Box::leak(Box::new("B"))); + + assert_ne!(a, b); + } +} diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index 6eff79c0bb389..6b1c6aa9264a5 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -65,12 +65,12 @@ where /// # Example /// /// ``` -/// # use bevy_utils::define_boxed_label; -/// define_boxed_label!(MyNewLabelTrait); +/// # use bevy_utils::define_interned_label; +/// define_interned_label!(MyNewLabelTrait, MY_NEW_LABEL_TRAIT_INTERNER); /// ``` #[macro_export] -macro_rules! define_boxed_label { - ($label_trait_name:ident) => { +macro_rules! define_interned_label { + ($label_trait_name:ident, $interner_name:ident) => { /// A strongly-typed label. pub trait $label_trait_name: 'static + Send + Sync + ::std::fmt::Debug { /// Return's the [TypeId] of this label, or the the ID of the @@ -95,6 +95,32 @@ macro_rules! define_boxed_label { /// /// [`Hasher`]: std::hash::Hasher fn dyn_hash(&self, state: &mut dyn ::std::hash::Hasher); + + /// Returns a static reference to a value equal to `self`, if possible. + /// This method is used to optimize [interning](bevy_utils::intern). + /// + /// # Invariant + /// + /// The following invariant must be hold: + /// + /// `ptr_eq(a.dyn_static_ref(), b.dyn_static_ref()) == true` if and only if `a.dyn_eq(b)` + /// + /// where `ptr_eq` is defined as : + /// ``` + /// fn ptr_eq(x: Option<&'static T>, y: Option<&'static T>) -> bool { + /// match (x, y) { + /// (Some(x), Some(y)) => std::ptr::eq(x, y), + /// _ => false, + /// } + /// } + /// ``` + /// + /// # Provided implementation + /// + /// The provided implementation always returns `None`. + fn dyn_static_ref(&self) -> Option<&'static dyn $label_trait_name> { + None + } } impl PartialEq for dyn $label_trait_name { @@ -111,36 +137,36 @@ macro_rules! define_boxed_label { } } - impl ::std::convert::AsRef for dyn $label_trait_name { - #[inline] - fn as_ref(&self) -> &Self { - self + impl ::bevy_utils::intern::StaticRef for dyn $label_trait_name { + fn static_ref(&self) -> std::option::Option<&'static dyn $label_trait_name> { + self.dyn_static_ref() } } - impl Clone for Box { - fn clone(&self) -> Self { - self.dyn_clone() - } - } - - impl $label_trait_name for Box { - fn inner_type_id(&self) -> ::std::any::TypeId { - (**self).inner_type_id() - } - - fn dyn_clone(&self) -> Box { - // Be explicit that we want to use the inner value - // to avoid infinite recursion. - (**self).dyn_clone() - } - - fn as_dyn_eq(&self) -> &dyn ::bevy_utils::label::DynEq { - (**self).as_dyn_eq() - } - - fn dyn_hash(&self, state: &mut dyn ::std::hash::Hasher) { - (**self).dyn_hash(state); + static $interner_name: ::bevy_utils::intern::OptimizedInterner = + ::bevy_utils::intern::OptimizedInterner::new(); + + impl From<&dyn $label_trait_name> + for ::bevy_utils::intern::Interned + { + fn from( + value: &dyn $label_trait_name, + ) -> ::bevy_utils::intern::Interned { + struct LeakHelper<'a>(&'a dyn $label_trait_name); + + impl<'a> ::std::borrow::Borrow for LeakHelper<'a> { + fn borrow(&self) -> &dyn $label_trait_name { + self.0 + } + } + + impl<'a> ::bevy_utils::intern::Leak for LeakHelper<'a> { + fn leak(self) -> &'static dyn $label_trait_name { + Box::leak(self.0.dyn_clone()) + } + } + + $interner_name.intern(LeakHelper(value)) } } }; diff --git a/crates/bevy_utils/src/lib.rs b/crates/bevy_utils/src/lib.rs index 3dc32e81d8904..defe04bccee69 100644 --- a/crates/bevy_utils/src/lib.rs +++ b/crates/bevy_utils/src/lib.rs @@ -20,6 +20,7 @@ pub mod syncunsafecell; mod default; mod float_ord; +pub mod intern; pub use ahash::{AHasher, RandomState}; pub use bevy_utils_proc_macros::*; From 777b39477ee7c6dfa9cbbaef0e47ae891baf0c85 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 22 Jul 2023 15:38:40 +0200 Subject: [PATCH 02/53] Implement `From` instead of `Into` --- crates/bevy_macro_utils/src/lib.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index 3dd14375a08e3..738ed225ba914 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -246,15 +246,15 @@ pub fn derive_interned_label(input: syn::DeriveInput, trait_path: &syn::Path) -> } } - impl #impl_generics Into<::bevy_utils::intern::Interned> for #ident #ty_generics #where_clause { - fn into(self) -> ::bevy_utils::intern::Interned { - (&self as &dyn #trait_path).into() + impl #impl_generics From<#ident #ty_generics> for ::bevy_utils::intern::Interned #where_clause { + fn from(value: #ident #ty_generics) -> Self { + (&value as &dyn #trait_path).into() } } - impl #impl_generics Into<::bevy_utils::intern::Interned> for & #ident #ty_generics #where_clause { - fn into(self) -> ::bevy_utils::intern::Interned { - (self as &dyn #trait_path).into() + impl #impl_generics From<& #ident #ty_generics> for ::bevy_utils::intern::Interned #where_clause { + fn from(value: &#ident #ty_generics) -> Self { + (value as &dyn #trait_path).into() } } }) From 369cf434b282111dd2910c604f8ef093ff050ee1 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sun, 23 Jul 2023 01:02:42 +0200 Subject: [PATCH 03/53] Use `from` instead of `into` --- crates/bevy_app/src/app.rs | 10 +++++----- crates/bevy_ecs/src/schedule/set.rs | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index e8b7a350f026b..3d43e793a0e6d 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -379,7 +379,7 @@ impl App { schedule: impl ScheduleLabel, systems: impl IntoSystemConfigs, ) -> &mut Self { - let schedule: InternedScheduleLabel = (&schedule as &dyn ScheduleLabel).into(); + let schedule = InternedScheduleLabel::from(&schedule as &dyn ScheduleLabel); let mut schedules = self.world.resource_mut::(); if let Some(schedule) = schedules.get_mut(schedule) { @@ -399,7 +399,7 @@ impl App { schedule: impl ScheduleLabel, set: impl IntoSystemSetConfig, ) -> &mut Self { - let schedule: InternedScheduleLabel = (&schedule as &dyn ScheduleLabel).into(); + let schedule = InternedScheduleLabel::from(&schedule as &dyn ScheduleLabel); let mut schedules = self.world.resource_mut::(); if let Some(schedule) = schedules.get_mut(schedule) { schedule.configure_set(set); @@ -417,7 +417,7 @@ impl App { schedule: impl ScheduleLabel, sets: impl IntoSystemSetConfigs, ) -> &mut Self { - let schedule: InternedScheduleLabel = (&schedule as &dyn ScheduleLabel).into(); + let schedule = InternedScheduleLabel::from(&schedule as &dyn ScheduleLabel); let mut schedules = self.world.resource_mut::(); if let Some(schedule) = schedules.get_mut(schedule) { schedule.configure_sets(sets); @@ -810,7 +810,7 @@ impl App { /// /// See [`App::add_schedule`] to pass in a pre-constructed schedule. pub fn init_schedule(&mut self, label: impl ScheduleLabel) -> &mut Self { - let label: InternedScheduleLabel = (&label as &dyn ScheduleLabel).into(); + let label = InternedScheduleLabel::from(&label as &dyn ScheduleLabel); let mut schedules = self.world.resource_mut::(); if !schedules.contains(label) { schedules.insert(label, Schedule::new()); @@ -840,7 +840,7 @@ impl App { label: impl ScheduleLabel, f: impl FnOnce(&mut Schedule), ) -> &mut Self { - let label: InternedScheduleLabel = (&label as &dyn ScheduleLabel).into(); + let label = InternedScheduleLabel::from(&label as &dyn ScheduleLabel); let mut schedules = self.world.resource_mut::(); if schedules.get(label).is_none() { diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index 232f1e1924870..50b33cf1825d1 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -256,7 +256,7 @@ mod tests { schedule.add_systems(|mut flag: ResMut| flag.0 = true); world.add_schedule(schedule, A); - let interned: Interned = (&A as &dyn ScheduleLabel).into(); + let interned = InternedScheduleLabel::from(&A as &dyn ScheduleLabel); world.insert_resource(Flag(false)); world.run_schedule(interned); From 5277f1acc977d072565056bec6428c1896c2b09c Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sun, 23 Jul 2023 01:51:08 +0200 Subject: [PATCH 04/53] Fix invariant descriptions --- crates/bevy_ecs/src/schedule/set.rs | 16 +++++++++++++--- crates/bevy_utils/src/intern.rs | 16 +++++++++++++--- crates/bevy_utils/src/label.rs | 16 +++++++++++++--- 3 files changed, 39 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index 50b33cf1825d1..d4633e82c18e1 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -42,15 +42,25 @@ pub trait SystemSet: DynHash + Debug + Send + Sync + 'static { /// /// # Invariant /// - /// The following invariant must be hold: + /// The following invariants must be hold: /// - /// `ptr_eq(a.dyn_static_ref(), b.dyn_static_ref()) == true` if and only if `a.dyn_eq(b)` + /// `ptr_eq(a.dyn_static_ref(), b.dyn_static_ref())` if `a.dyn_eq(b)` + /// `ptr_neq(a.dyn_static_ref(), b.dyn_static_ref())` if `!a.dyn_eq(b)` /// - /// where `ptr_eq` is defined as : + /// where `ptr_eq` and `ptr_neq` are defined as : /// ``` /// fn ptr_eq(x: Option<&'static T>, y: Option<&'static T>) -> bool { /// match (x, y) { /// (Some(x), Some(y)) => std::ptr::eq(x, y), + /// (None, None) => true, + /// _ => false, + /// } + /// } + /// + /// fn ptr_neq(x: Option<&'static T>, y: Option<&'static T>) -> bool { + /// match (x, y) { + /// (Some(x), Some(y)) => !std::ptr::eq(x, y), + /// (None, None) => true, /// _ => false, /// } /// } diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index 1863a014ebefe..0281b41f3f2e6 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -158,15 +158,25 @@ pub trait StaticRef { /// /// # Invariant /// - /// The following invariant must be hold: + /// The following invariants must be hold: /// - /// `ptr_eq(a.static_ref(), b.static_ref()) == true` if and only if `a == b` + /// `ptr_eq(a.static_ref(), b.static_ref())` if `a == b` + /// `ptr_neq(a.static_ref(), b.static_ref())` if `a != b` /// - /// where `ptr_eq` is defined as : + /// where `ptr_eq` and `ptr_neq` are defined as : /// ``` /// fn ptr_eq(x: Option<&'static T>, y: Option<&'static T>) -> bool { /// match (x, y) { /// (Some(x), Some(y)) => std::ptr::eq(x, y), + /// (None, None) => true, + /// _ => false, + /// } + /// } + /// + /// fn ptr_neq(x: Option<&'static T>, y: Option<&'static T>) -> bool { + /// match (x, y) { + /// (Some(x), Some(y)) => !std::ptr::eq(x, y), + /// (None, None) => true, /// _ => false, /// } /// } diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index 6b1c6aa9264a5..8fe6e75e3e74d 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -101,15 +101,25 @@ macro_rules! define_interned_label { /// /// # Invariant /// - /// The following invariant must be hold: + /// The following invariants must be hold: /// - /// `ptr_eq(a.dyn_static_ref(), b.dyn_static_ref()) == true` if and only if `a.dyn_eq(b)` + /// `ptr_eq(a.dyn_static_ref(), b.dyn_static_ref())` if `a.dyn_eq(b)` + /// `ptr_neq(a.dyn_static_ref(), b.dyn_static_ref())` if `!a.dyn_eq(b)` /// - /// where `ptr_eq` is defined as : + /// where `ptr_eq` and `ptr_neq` are defined as : /// ``` /// fn ptr_eq(x: Option<&'static T>, y: Option<&'static T>) -> bool { /// match (x, y) { /// (Some(x), Some(y)) => std::ptr::eq(x, y), + /// (None, None) => true, + /// _ => false, + /// } + /// } + /// + /// fn ptr_neq(x: Option<&'static T>, y: Option<&'static T>) -> bool { + /// match (x, y) { + /// (Some(x), Some(y)) => !std::ptr::eq(x, y), + /// (None, None) => true, /// _ => false, /// } /// } From 1ac419a22c474c6f1f5e6c2601aa59cb8bba7a72 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sun, 23 Jul 2023 11:50:51 +0200 Subject: [PATCH 05/53] Reuse anonymous set ids between different schedules --- crates/bevy_ecs/src/schedule/schedule.rs | 10 +++++++++- crates/bevy_ecs/src/schedule/set.rs | 7 ++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index 3ed1a32a0711d..4a7f1151ffd62 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -390,6 +390,7 @@ pub struct ScheduleGraph { ambiguous_with_flattened: UnGraphMap, ambiguous_with_all: HashSet, conflicting_systems: Vec<(NodeId, NodeId, Vec)>, + anonymous_sets: usize, changed: bool, settings: ScheduleBuildSettings, } @@ -411,6 +412,7 @@ impl ScheduleGraph { ambiguous_with_flattened: UnGraphMap::new(), ambiguous_with_all: HashSet::new(), conflicting_systems: Vec::new(), + anonymous_sets: 0, changed: false, settings: default(), } @@ -533,7 +535,7 @@ impl ScheduleGraph { let more_than_one_entry = configs.len() > 1; if !collective_conditions.is_empty() { if more_than_one_entry { - let set = AnonymousSet::new(); + let set = self.create_anonymous_set(); for config in &mut configs { config.in_set_inner((&set as &dyn SystemSet).into()); } @@ -735,6 +737,12 @@ impl ScheduleGraph { Ok(()) } + fn create_anonymous_set(&mut self) -> AnonymousSet { + let id = self.anonymous_sets; + self.anonymous_sets += 1; + AnonymousSet::new(id) + } + fn check_sets( &mut self, id: &NodeId, diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index d4633e82c18e1..5dd3ce37a3504 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -3,7 +3,6 @@ use std::borrow::Borrow; use std::fmt::Debug; use std::hash::{Hash, Hasher}; use std::marker::PhantomData; -use std::sync::atomic::{AtomicUsize, Ordering}; pub use bevy_ecs_macros::{ScheduleLabel, SystemSet}; use bevy_utils::define_interned_label; @@ -178,11 +177,9 @@ impl SystemSet for SystemTypeSet { #[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)] pub struct AnonymousSet(usize); -static NEXT_ANONYMOUS_SET_ID: AtomicUsize = AtomicUsize::new(0); - impl AnonymousSet { - pub(crate) fn new() -> Self { - Self(NEXT_ANONYMOUS_SET_ID.fetch_add(1, Ordering::Relaxed)) + pub(crate) fn new(id: usize) -> Self { + Self(id) } } From 92584503de8cc359ebe42a5ca04b93fad3c0dd05 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sun, 23 Jul 2023 01:34:23 +0200 Subject: [PATCH 06/53] Make all labels internable --- crates/bevy_app/src/app.rs | 27 ++++--- crates/bevy_derive/src/lib.rs | 4 +- crates/bevy_ecs/macros/src/lib.rs | 6 +- crates/bevy_ecs/src/schedule/set.rs | 10 ++- crates/bevy_macro_utils/src/lib.rs | 114 +--------------------------- crates/bevy_utils/src/label.rs | 90 ++++------------------ 6 files changed, 41 insertions(+), 210 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 3d43e793a0e6d..b7480aa3fd8eb 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -8,7 +8,7 @@ use bevy_ecs::{ ScheduleLabel, }, }; -use bevy_utils::{tracing::debug, HashMap, HashSet}; +use bevy_utils::{intern::Interned, tracing::debug, HashMap, HashSet}; use std::{ fmt::Debug, panic::{catch_unwind, resume_unwind, AssertUnwindSafe}, @@ -20,10 +20,12 @@ use bevy_utils::tracing::info_span; bevy_utils::define_label!( /// A strongly-typed class of labels used to identify an [`App`]. AppLabel, - /// A strongly-typed identifier for an [`AppLabel`]. - AppLabelId, + APP_LABEL_INTERNER ); +/// A shorthand for `Interned`. +pub type InternedAppLabel = Interned; + pub(crate) enum AppError { DuplicatePlugin { plugin_name: String }, } @@ -71,7 +73,7 @@ pub struct App { /// /// This is initially set to [`Main`]. pub main_schedule_label: InternedScheduleLabel, - sub_apps: HashMap, + sub_apps: HashMap, plugin_registry: Vec>, plugin_name_added: HashSet, /// A private counter to prevent incorrect calls to `App::run()` from `Plugin::build()` @@ -749,16 +751,15 @@ impl App { pub fn sub_app_mut(&mut self, label: impl AppLabel) -> &mut App { match self.get_sub_app_mut(label) { Ok(app) => app, - Err(label) => panic!("Sub-App with label '{:?}' does not exist", label.as_str()), + Err(label) => panic!("Sub-App with label '{:?}' does not exist", label), } } /// Retrieves a `SubApp` inside this [`App`] with the given label, if it exists. Otherwise returns /// an [`Err`] containing the given label. - pub fn get_sub_app_mut(&mut self, label: impl AppLabel) -> Result<&mut App, AppLabelId> { - let label = label.as_label(); + pub fn get_sub_app_mut(&mut self, label: impl AppLabel) -> Result<&mut App, impl AppLabel> { self.sub_apps - .get_mut(&label) + .get_mut(&InternedAppLabel::from(&label as &dyn AppLabel)) .map(|sub_app| &mut sub_app.app) .ok_or(label) } @@ -771,25 +772,27 @@ impl App { pub fn sub_app(&self, label: impl AppLabel) -> &App { match self.get_sub_app(label) { Ok(app) => app, - Err(label) => panic!("Sub-App with label '{:?}' does not exist", label.as_str()), + Err(label) => panic!("Sub-App with label '{:?}' does not exist", label), } } /// Inserts an existing sub app into the app pub fn insert_sub_app(&mut self, label: impl AppLabel, sub_app: SubApp) { - self.sub_apps.insert(label.as_label(), sub_app); + self.sub_apps + .insert((&label as &dyn AppLabel).into(), sub_app); } /// Removes a sub app from the app. Returns [`None`] if the label doesn't exist. pub fn remove_sub_app(&mut self, label: impl AppLabel) -> Option { - self.sub_apps.remove(&label.as_label()) + self.sub_apps + .remove(&InternedAppLabel::from(&label as &dyn AppLabel)) } /// Retrieves a `SubApp` inside this [`App`] with the given label, if it exists. Otherwise returns /// an [`Err`] containing the given label. pub fn get_sub_app(&self, label: impl AppLabel) -> Result<&App, impl AppLabel> { self.sub_apps - .get(&label.as_label()) + .get(&InternedAppLabel::from(&label as &dyn AppLabel)) .map(|sub_app| &sub_app.app) .ok_or(label) } diff --git a/crates/bevy_derive/src/lib.rs b/crates/bevy_derive/src/lib.rs index 9030d8791ba71..293289b81d0a9 100644 --- a/crates/bevy_derive/src/lib.rs +++ b/crates/bevy_derive/src/lib.rs @@ -203,10 +203,10 @@ pub fn derive_enum_variant_meta(input: TokenStream) -> TokenStream { /// /// This works only for unit structs, or enums with only unit variants. /// You may force a struct or variant to behave as if it were fieldless with `#[app_label(ignore_fields)]`. -#[proc_macro_derive(AppLabel, attributes(app_label))] +#[proc_macro_derive(AppLabel)] pub fn derive_app_label(input: TokenStream) -> TokenStream { let input = syn::parse_macro_input!(input as syn::DeriveInput); let mut trait_path = BevyManifest::default().get_path("bevy_app"); trait_path.segments.push(format_ident!("AppLabel").into()); - derive_label(input, &trait_path, "app_label") + derive_label(input, &trait_path) } diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 5ea4c916dcc2d..bb399579cf963 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -6,9 +6,7 @@ mod set; mod states; use crate::{fetch::derive_world_query_impl, set::derive_set}; -use bevy_macro_utils::{ - derive_interned_label, ensure_no_collision, get_named_struct_fields, BevyManifest, -}; +use bevy_macro_utils::{derive_label, ensure_no_collision, get_named_struct_fields, BevyManifest}; use proc_macro::TokenStream; use proc_macro2::Span; use quote::{format_ident, quote}; @@ -442,7 +440,7 @@ pub fn derive_schedule_label(input: TokenStream) -> TokenStream { trait_path .segments .push(format_ident!("ScheduleLabel").into()); - derive_interned_label(input, &trait_path) + derive_label(input, &trait_path) } /// Derive macro generating an impl of the trait `SystemSet`. diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index 5dd3ce37a3504..a1b5fb2a99701 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -5,7 +5,7 @@ use std::hash::{Hash, Hasher}; use std::marker::PhantomData; pub use bevy_ecs_macros::{ScheduleLabel, SystemSet}; -use bevy_utils::define_interned_label; +use bevy_utils::define_label; use bevy_utils::intern::{Interned, Leak, OptimizedInterner, StaticRef}; use bevy_utils::label::DynHash; @@ -13,7 +13,11 @@ use crate::system::{ ExclusiveSystemParamFunction, IsExclusiveFunctionSystem, IsFunctionSystem, SystemParamFunction, }; -define_interned_label!(ScheduleLabel, SCHEDULE_LABEL_INTERNER); +define_label!( + /// A strongly-typed class of labels used to identify an [`Schedule`]. + ScheduleLabel, + SCHEDULE_LABEL_INTERNER +); static SYSTEM_SET_INTERNER: OptimizedInterner = OptimizedInterner::new(); /// A shorthand for `Interned`. @@ -248,7 +252,7 @@ mod tests { use super::*; #[test] - fn test_interned_label() { + fn test_label() { use crate::{self as bevy_ecs, world::World}; #[derive(Resource)] diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index 738ed225ba914..b6bf4cee13285 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -169,7 +169,7 @@ pub fn ensure_no_collision(value: Ident, haystack: TokenStream) -> Ident { /// /// - `input`: The [`syn::DeriveInput`] for struct that is deriving the label trait /// - `trait_path`: The path [`syn::Path`] to the label trait -pub fn derive_interned_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStream { +pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStream { let bevy_utils_path = BevyManifest::default().get_path("bevy_utils"); let ident = input.ident; @@ -260,115 +260,3 @@ pub fn derive_interned_label(input: syn::DeriveInput, trait_path: &syn::Path) -> }) .into() } - -/// Derive a label trait -/// -/// # Args -/// -/// - `input`: The [`syn::DeriveInput`] for struct that is deriving the label trait -/// - `trait_path`: The path [`syn::Path`] to the label trait -pub fn derive_label( - input: syn::DeriveInput, - trait_path: &syn::Path, - attr_name: &str, -) -> TokenStream { - // return true if the variant specified is an `ignore_fields` attribute - fn is_ignore(attr: &syn::Attribute, attr_name: &str) -> bool { - if attr.path().get_ident().as_ref().unwrap() != &attr_name { - return false; - } - - syn::custom_keyword!(ignore_fields); - attr.parse_args_with(|input: syn::parse::ParseStream| { - let ignore = input.parse::>()?.is_some(); - Ok(ignore) - }) - .unwrap() - } - - let ident = input.ident.clone(); - - let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); - let mut where_clause = where_clause.cloned().unwrap_or_else(|| syn::WhereClause { - where_token: Default::default(), - predicates: Default::default(), - }); - where_clause - .predicates - .push(syn::parse2(quote! { Self: 'static }).unwrap()); - - let as_str = match input.data { - syn::Data::Struct(d) => { - // see if the user tried to ignore fields incorrectly - if let Some(attr) = d - .fields - .iter() - .flat_map(|f| &f.attrs) - .find(|a| is_ignore(a, attr_name)) - { - let err_msg = format!("`#[{attr_name}(ignore_fields)]` cannot be applied to fields individually: add it to the struct declaration"); - return quote_spanned! { - attr.span() => compile_error!(#err_msg); - } - .into(); - } - // Structs must either be fieldless, or explicitly ignore the fields. - let ignore_fields = input.attrs.iter().any(|a| is_ignore(a, attr_name)); - if matches!(d.fields, syn::Fields::Unit) || ignore_fields { - let lit = ident.to_string(); - quote! { #lit } - } else { - let err_msg = format!("Labels cannot contain data, unless explicitly ignored with `#[{attr_name}(ignore_fields)]`"); - return quote_spanned! { - d.fields.span() => compile_error!(#err_msg); - } - .into(); - } - } - syn::Data::Enum(d) => { - // check if the user put #[label(ignore_fields)] in the wrong place - if let Some(attr) = input.attrs.iter().find(|a| is_ignore(a, attr_name)) { - let err_msg = format!("`#[{attr_name}(ignore_fields)]` can only be applied to enum variants or struct declarations"); - return quote_spanned! { - attr.span() => compile_error!(#err_msg); - } - .into(); - } - let arms = d.variants.iter().map(|v| { - // Variants must either be fieldless, or explicitly ignore the fields. - let ignore_fields = v.attrs.iter().any(|a| is_ignore(a, attr_name)); - if matches!(v.fields, syn::Fields::Unit) | ignore_fields { - let mut path = syn::Path::from(ident.clone()); - path.segments.push(v.ident.clone().into()); - let lit = format!("{ident}::{}", v.ident.clone()); - quote! { #path { .. } => #lit } - } else { - let err_msg = format!("Label variants cannot contain data, unless explicitly ignored with `#[{attr_name}(ignore_fields)]`"); - quote_spanned! { - v.fields.span() => _ => { compile_error!(#err_msg); } - } - } - }); - quote! { - match self { - #(#arms),* - } - } - } - syn::Data::Union(_) => { - return quote_spanned! { - input.span() => compile_error!("Unions cannot be used as labels."); - } - .into(); - } - }; - - (quote! { - impl #impl_generics #trait_path for #ident #ty_generics #where_clause { - fn as_str(&self) -> &'static str { - #as_str - } - } - }) - .into() -} diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index 8fe6e75e3e74d..1fcecc289f13d 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -65,13 +65,22 @@ where /// # Example /// /// ``` -/// # use bevy_utils::define_interned_label; -/// define_interned_label!(MyNewLabelTrait, MY_NEW_LABEL_TRAIT_INTERNER); +/// # use bevy_utils::define_label; +/// define_label!( +/// /// Documentation of label trait +/// MyNewLabelTrait, +/// MY_NEW_LABEL_TRAIT_INTERNER +/// ); /// ``` #[macro_export] -macro_rules! define_interned_label { - ($label_trait_name:ident, $interner_name:ident) => { - /// A strongly-typed label. +macro_rules! define_label { + ( + $(#[$label_attr:meta])* + $label_trait_name:ident, + $interner_name:ident + ) => { + + $(#[$label_attr])* pub trait $label_trait_name: 'static + Send + Sync + ::std::fmt::Debug { /// Return's the [TypeId] of this label, or the the ID of the /// wrappped label type for `Box { - $(#[$id_attr])* - #[derive(Clone, Copy, PartialEq, Eq, Hash)] - pub struct $id_name(::core::any::TypeId, &'static str); - - impl ::core::fmt::Debug for $id_name { - fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result { - write!(f, "{}", self.1) - } - } - - $(#[$label_attr])* - pub trait $label_name: 'static { - /// Converts this type into an opaque, strongly-typed label. - fn as_label(&self) -> $id_name { - let id = self.type_id(); - let label = self.as_str(); - $id_name(id, label) - } - /// Returns the [`TypeId`] used to differentiate labels. - fn type_id(&self) -> ::core::any::TypeId { - ::core::any::TypeId::of::() - } - /// Returns the representation of this label as a string literal. - /// - /// In cases where you absolutely need a label to be determined at runtime, - /// you can use [`Box::leak`] to get a `'static` reference. - fn as_str(&self) -> &'static str; - } - - impl $label_name for $id_name { - fn as_label(&self) -> Self { - *self - } - fn type_id(&self) -> ::core::any::TypeId { - self.0 - } - fn as_str(&self) -> &'static str { - self.1 - } - } - - impl $label_name for &'static str { - fn as_str(&self) -> Self { - self - } - } - }; -} From 556a8ac0bc759144a250ae0a448440855ff8dfdc Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sun, 23 Jul 2023 13:29:07 +0200 Subject: [PATCH 07/53] Merge `OptimizedInterner` into `Interner` --- crates/bevy_ecs/src/schedule/set.rs | 4 +- crates/bevy_utils/src/intern.rs | 121 ++++++++++++++-------------- crates/bevy_utils/src/label.rs | 4 +- 3 files changed, 63 insertions(+), 66 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index a1b5fb2a99701..0b1a7ed1771b5 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -6,7 +6,7 @@ use std::marker::PhantomData; pub use bevy_ecs_macros::{ScheduleLabel, SystemSet}; use bevy_utils::define_label; -use bevy_utils::intern::{Interned, Leak, OptimizedInterner, StaticRef}; +use bevy_utils::intern::{Interned, Interner, Leak, StaticRef}; use bevy_utils::label::DynHash; use crate::system::{ @@ -19,7 +19,7 @@ define_label!( SCHEDULE_LABEL_INTERNER ); -static SYSTEM_SET_INTERNER: OptimizedInterner = OptimizedInterner::new(); +static SYSTEM_SET_INTERNER: Interner = Interner::new(); /// A shorthand for `Interned`. pub type InternedSystemSet = Interned; /// A shorthand for `Interned`. diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index 0281b41f3f2e6..72b110fd2ee36 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -104,57 +104,10 @@ impl Leak for Cow<'static, T> { } } -/// A thread-safe interner which can be used to create [`Interned`] from `&T` -/// -/// For details on interning, see [the module level docs](self). -/// -/// The implementation ensures that two equal values return two equal [`Interned`] values. -pub struct Interner(OnceLock>>); - -impl Interner { - /// Creates a new empty interner - pub const fn new() -> Self { - Self(OnceLock::new()) - } -} - -impl Interner { - /// Return the [`Interned`] corresponding to `value`. - /// - /// If it is called the first time for `value`, it will possibly leak the value and return an - /// [`Interned`] using the obtained static reference. Subsequent calls for the same `value` - /// will return [`Interned`] using the same static reference. - pub fn intern>(&self, value: Q) -> Interned { - let lock = self.0.get_or_init(Default::default); - { - let set = lock.read().unwrap_or_else(PoisonError::into_inner); - if let Some(value) = set.get(value.borrow()) { - return Interned(*value); - } - } - { - let mut set = lock.write().unwrap_or_else(PoisonError::into_inner); - if let Some(value) = set.get(value.borrow()) { - Interned(*value) - } else { - let leaked = value.leak(); - set.insert(leaked); - Interned(leaked) - } - } - } -} - -impl Default for Interner { - fn default() -> Self { - Self::new() - } -} - /// A type that can provide static references to equal values. pub trait StaticRef { /// Returns a static reference to a value equal to `self`, if possible. - /// This method is used by [`OptimizedInterner::intern`] to optimize the interning process. + /// This method is used by [`Interner::intern`] to optimize the interning process. /// /// # Invariant /// @@ -181,37 +134,66 @@ pub trait StaticRef { /// } /// } /// ``` - fn static_ref(&self) -> Option<&'static Self>; + /// + /// # Provided implementation + /// + /// The provided implementation always returns `None`. + fn static_ref(&self) -> Option<&'static Self> { + None + } } -/// An optimized [`Interner`] for types that implement [`StaticRef`]. -pub struct OptimizedInterner(Interner); +impl StaticRef for str {} + +/// A thread-safe interner which can be used to create [`Interned`] from `&T` +/// +/// For details on interning, see [the module level docs](self). +/// +/// The implementation ensures that two equal values return two equal [`Interned`] values. +/// +/// To use an [`Interner`], `T` must implement [`StaticRef`], [`Hash`] and [`Eq`]. +pub struct Interner(OnceLock>>); -impl OptimizedInterner { +impl Interner { /// Creates a new empty interner pub const fn new() -> Self { - Self(Interner::new()) + Self(OnceLock::new()) } } -impl OptimizedInterner { +impl Interner { /// Return the [`Interned`] corresponding to `value`. /// /// If it is called the first time for `value`, it will possibly leak the value and return an /// [`Interned`] using the obtained static reference. Subsequent calls for the same `value` /// will return [`Interned`] using the same static reference. /// - /// Compared to [`Interner::intern`], this uses [`StaticRef::static_ref`] to short-circuit - /// the interning process. + /// This uses [`StaticRef::static_ref`] to short-circuit the interning process. pub fn intern>(&self, value: Q) -> Interned { if let Some(value) = value.borrow().static_ref() { return Interned(value); } - self.0.intern(value) + let lock = self.0.get_or_init(Default::default); + { + let set = lock.read().unwrap_or_else(PoisonError::into_inner); + if let Some(value) = set.get(value.borrow()) { + return Interned(*value); + } + } + { + let mut set = lock.write().unwrap_or_else(PoisonError::into_inner); + if let Some(value) = set.get(value.borrow()) { + Interned(*value) + } else { + let leaked = value.leak(); + set.insert(leaked); + Interned(leaked) + } + } } } -impl Default for OptimizedInterner { +impl Default for Interner { fn default() -> Self { Self::new() } @@ -224,13 +206,19 @@ mod tests { hash::{Hash, Hasher}, }; - use crate::intern::{Interned, Interner, Leak}; + use crate::intern::{Interned, Interner, Leak, StaticRef}; #[test] fn zero_sized_type() { #[derive(PartialEq, Eq, Hash, Debug)] pub struct A; + impl StaticRef for A { + fn static_ref(&self) -> Option<&'static Self> { + Some(&A) + } + } + impl Leak for A { fn leak(self) -> &'static Self { &A @@ -238,19 +226,28 @@ mod tests { } let interner = Interner::default(); - let x = interner.intern(&A); - let y = interner.intern(&A); + let x = interner.intern(A); + let y = interner.intern(A); assert_eq!(x, y); } #[test] fn fieldless_enum() { - #[derive(PartialEq, Eq, Hash, Debug)] + #[derive(PartialEq, Eq, Hash, Debug, Clone)] pub enum A { X, Y, } + impl StaticRef for A { + fn static_ref(&self) -> Option<&'static Self> { + Some(match self { + A::X => &A::X, + A::Y => &A::Y, + }) + } + } + impl Leak for A { fn leak(self) -> &'static Self { match self { diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index 1fcecc289f13d..94f927efca259 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -162,8 +162,8 @@ macro_rules! define_label { } } - static $interner_name: ::bevy_utils::intern::OptimizedInterner = - ::bevy_utils::intern::OptimizedInterner::new(); + static $interner_name: ::bevy_utils::intern::Interner = + ::bevy_utils::intern::Interner::new(); impl From<&dyn $label_trait_name> for ::bevy_utils::intern::Interned From 0f6596e46c1fe7cb4d9d625d7e048a9d4f23a44f Mon Sep 17 00:00:00 2001 From: Joseph <21144246+JoJoJet@users.noreply.github.com> Date: Thu, 27 Jul 2023 14:04:47 -0700 Subject: [PATCH 08/53] remove `Borrow` from `Leak` --- crates/bevy_ecs/src/schedule/set.rs | 23 ++++-------- crates/bevy_utils/src/intern.rs | 54 ++++++++++------------------- crates/bevy_utils/src/label.rs | 22 ++++-------- 3 files changed, 33 insertions(+), 66 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index 0b1a7ed1771b5..b97d14f836152 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -1,5 +1,4 @@ use std::any::TypeId; -use std::borrow::Borrow; use std::fmt::Debug; use std::hash::{Hash, Hasher}; use std::marker::PhantomData; @@ -79,21 +78,7 @@ pub trait SystemSet: DynHash + Debug + Send + Sync + 'static { impl From<&dyn SystemSet> for Interned { fn from(value: &dyn SystemSet) -> Interned { - struct LeakHelper<'a>(&'a dyn SystemSet); - - impl<'a> Borrow for LeakHelper<'a> { - fn borrow(&self) -> &dyn SystemSet { - self.0 - } - } - - impl<'a> Leak for LeakHelper<'a> { - fn leak(self) -> &'static dyn SystemSet { - Box::leak(self.0.dyn_clone()) - } - } - - SYSTEM_SET_INTERNER.intern(LeakHelper(value)) + SYSTEM_SET_INTERNER.intern(value) } } @@ -111,6 +96,12 @@ impl Hash for dyn SystemSet { } } +impl Leak for dyn SystemSet { + fn leak(&self) -> &'static Self { + Box::leak(self.dyn_clone()) + } +} + impl StaticRef for dyn SystemSet { fn static_ref(&self) -> Option<&'static dyn SystemSet> { self.dyn_static_ref() diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index 72b110fd2ee36..766ca479bdbe6 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -5,7 +5,7 @@ //! and make comparisons for any type as fast as integers. use std::{ - borrow::{Borrow, Cow}, + borrow::Borrow, fmt::Debug, hash::Hash, ops::Deref, @@ -76,31 +76,15 @@ impl From<&Interned> for Interned { /// A trait for leaking data. /// /// This is used by [`Interner`] to create static references for values that are interned. -/// -/// It is implemented for static references of `T`, [`Box`], and [`Cow<'static, T>`]. -pub trait Leak: Borrow { - /// Creates a static reference to a value equal to `self.borrow()`, possibly leaking memory. - fn leak(self) -> &'static T; -} - -impl Leak for &'static T { - fn leak(self) -> &'static T { - self - } +pub trait Leak { + /// Creates a static reference to `self`, possibly leaking memory. + fn leak(&self) -> &'static Self; } -impl Leak for Box { - fn leak(self) -> &'static T { - Box::leak(self) - } -} - -impl Leak for Cow<'static, T> { - fn leak(self) -> &'static T { - match self { - Cow::Borrowed(value) => value, - Cow::Owned(value) => Box::leak(Box::new(value)), - } +impl Leak for str { + fn leak(&self) -> &'static Self { + let str = self.to_owned().into_boxed_str(); + Box::leak(str) } } @@ -161,7 +145,7 @@ impl Interner { } } -impl Interner { +impl Interner { /// Return the [`Interned`] corresponding to `value`. /// /// If it is called the first time for `value`, it will possibly leak the value and return an @@ -169,8 +153,8 @@ impl Interner { /// will return [`Interned`] using the same static reference. /// /// This uses [`StaticRef::static_ref`] to short-circuit the interning process. - pub fn intern>(&self, value: Q) -> Interned { - if let Some(value) = value.borrow().static_ref() { + pub fn intern(&self, value: &T) -> Interned { + if let Some(value) = value.static_ref() { return Interned(value); } let lock = self.0.get_or_init(Default::default); @@ -219,15 +203,15 @@ mod tests { } } - impl Leak for A { - fn leak(self) -> &'static Self { + impl Leak for A { + fn leak(&self) -> &'static Self { &A } } let interner = Interner::default(); - let x = interner.intern(A); - let y = interner.intern(A); + let x = interner.intern(&A); + let y = interner.intern(&A); assert_eq!(x, y); } @@ -248,8 +232,8 @@ mod tests { } } - impl Leak for A { - fn leak(self) -> &'static Self { + impl Leak for A { + fn leak(&self) -> &'static Self { match self { A::X => &A::X, A::Y => &A::Y, @@ -258,8 +242,8 @@ mod tests { } let interner = Interner::default(); - let x = interner.intern(A::X); - let y = interner.intern(A::Y); + let x = interner.intern(&A::X); + let y = interner.intern(&A::Y); assert_ne!(x, y); } diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index 94f927efca259..dd936d60c5b89 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -156,6 +156,12 @@ macro_rules! define_label { } } + impl ::bevy_utils::intern::Leak for dyn $label_trait_name { + fn leak(&self) -> &'static Self { + Box::leak(self.dyn_clone()) + } + } + impl ::bevy_utils::intern::StaticRef for dyn $label_trait_name { fn static_ref(&self) -> std::option::Option<&'static dyn $label_trait_name> { self.dyn_static_ref() @@ -171,21 +177,7 @@ macro_rules! define_label { fn from( value: &dyn $label_trait_name, ) -> ::bevy_utils::intern::Interned { - struct LeakHelper<'a>(&'a dyn $label_trait_name); - - impl<'a> ::std::borrow::Borrow for LeakHelper<'a> { - fn borrow(&self) -> &dyn $label_trait_name { - self.0 - } - } - - impl<'a> ::bevy_utils::intern::Leak for LeakHelper<'a> { - fn leak(self) -> &'static dyn $label_trait_name { - Box::leak(self.0.dyn_clone()) - } - } - - $interner_name.intern(LeakHelper(value)) + $interner_name.intern(value) } } }; From e9e16fb7ff1fd416c1b4f1ac05409f75b11b44c4 Mon Sep 17 00:00:00 2001 From: Joseph <21144246+JoJoJet@users.noreply.github.com> Date: Thu, 27 Jul 2023 14:11:25 -0700 Subject: [PATCH 09/53] merge `StaticRef` with `Leak` --- crates/bevy_ecs/src/schedule/set.rs | 4 +-- crates/bevy_utils/src/intern.rs | 47 ++++++++++++----------------- crates/bevy_utils/src/label.rs | 2 -- 3 files changed, 20 insertions(+), 33 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index b97d14f836152..e18f7dc9e4b21 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -5,7 +5,7 @@ use std::marker::PhantomData; pub use bevy_ecs_macros::{ScheduleLabel, SystemSet}; use bevy_utils::define_label; -use bevy_utils::intern::{Interned, Interner, Leak, StaticRef}; +use bevy_utils::intern::{Interned, Interner, Leak}; use bevy_utils::label::DynHash; use crate::system::{ @@ -100,9 +100,7 @@ impl Leak for dyn SystemSet { fn leak(&self) -> &'static Self { Box::leak(self.dyn_clone()) } -} -impl StaticRef for dyn SystemSet { fn static_ref(&self) -> Option<&'static dyn SystemSet> { self.dyn_static_ref() } diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index 766ca479bdbe6..104a52bec11d5 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -79,17 +79,7 @@ impl From<&Interned> for Interned { pub trait Leak { /// Creates a static reference to `self`, possibly leaking memory. fn leak(&self) -> &'static Self; -} - -impl Leak for str { - fn leak(&self) -> &'static Self { - let str = self.to_owned().into_boxed_str(); - Box::leak(str) - } -} -/// A type that can provide static references to equal values. -pub trait StaticRef { /// Returns a static reference to a value equal to `self`, if possible. /// This method is used by [`Interner::intern`] to optimize the interning process. /// @@ -127,7 +117,12 @@ pub trait StaticRef { } } -impl StaticRef for str {} +impl Leak for str { + fn leak(&self) -> &'static Self { + let str = self.to_owned().into_boxed_str(); + Box::leak(str) + } +} /// A thread-safe interner which can be used to create [`Interned`] from `&T` /// @@ -145,7 +140,7 @@ impl Interner { } } -impl Interner { +impl Interner { /// Return the [`Interned`] corresponding to `value`. /// /// If it is called the first time for `value`, it will possibly leak the value and return an @@ -190,23 +185,21 @@ mod tests { hash::{Hash, Hasher}, }; - use crate::intern::{Interned, Interner, Leak, StaticRef}; + use crate::intern::{Interned, Interner, Leak}; #[test] fn zero_sized_type() { #[derive(PartialEq, Eq, Hash, Debug)] pub struct A; - impl StaticRef for A { - fn static_ref(&self) -> Option<&'static Self> { - Some(&A) - } - } - impl Leak for A { fn leak(&self) -> &'static Self { &A } + + fn static_ref(&self) -> Option<&'static Self> { + Some(&A) + } } let interner = Interner::default(); @@ -223,15 +216,6 @@ mod tests { Y, } - impl StaticRef for A { - fn static_ref(&self) -> Option<&'static Self> { - Some(match self { - A::X => &A::X, - A::Y => &A::Y, - }) - } - } - impl Leak for A { fn leak(&self) -> &'static Self { match self { @@ -239,6 +223,13 @@ mod tests { A::Y => &A::Y, } } + + fn static_ref(&self) -> Option<&'static Self> { + Some(match self { + A::X => &A::X, + A::Y => &A::Y, + }) + } } let interner = Interner::default(); diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index dd936d60c5b89..eb23575787344 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -160,9 +160,7 @@ macro_rules! define_label { fn leak(&self) -> &'static Self { Box::leak(self.dyn_clone()) } - } - impl ::bevy_utils::intern::StaticRef for dyn $label_trait_name { fn static_ref(&self) -> std::option::Option<&'static dyn $label_trait_name> { self.dyn_static_ref() } From 0d9d83b1bec34b0e6c0f5b7f4e37324bb4c5a71a Mon Sep 17 00:00:00 2001 From: Joseph <21144246+JoJoJet@users.noreply.github.com> Date: Thu, 27 Jul 2023 14:11:54 -0700 Subject: [PATCH 10/53] make a note private --- crates/bevy_utils/src/intern.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index 104a52bec11d5..c4e34ddd7e217 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -22,10 +22,10 @@ use crate::HashSet; /// # Implementation details /// /// This is just a reference to a value with an `'static` lifetime. -/// -/// This implements [`Copy`], [`Clone`], [`PartialEq`], [`Eq`] and [`Hash`] regardles of what `T` -/// implements, because it only uses the pointer to the value and not the value itself. -/// Therefore it MUST NEVER implement [`Borrow`](`std::borrow::Borrow`). +// +// This implements [`Copy`], [`Clone`], [`PartialEq`], [`Eq`] and [`Hash`] regardles of what `T` +// implements, because it only uses the pointer to the value and not the value itself. +// Therefore it MUST NEVER implement [`Borrow`](`std::borrow::Borrow`). pub struct Interned(&'static T); impl Deref for Interned { From fff98641703994901a500c26065b490c70608eda Mon Sep 17 00:00:00 2001 From: Joseph <21144246+JoJoJet@users.noreply.github.com> Date: Thu, 27 Jul 2023 14:17:49 -0700 Subject: [PATCH 11/53] interned labels are labels --- crates/bevy_ecs/src/schedule/config.rs | 6 ------ crates/bevy_ecs/src/schedule/set.rs | 18 ++++++++++++++++++ crates/bevy_utils/src/intern.rs | 2 +- crates/bevy_utils/src/label.rs | 23 +++++++++++++++++++++++ 4 files changed, 42 insertions(+), 7 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/config.rs b/crates/bevy_ecs/src/schedule/config.rs index 705f37bf587e0..95cfa3defb4a3 100644 --- a/crates/bevy_ecs/src/schedule/config.rs +++ b/crates/bevy_ecs/src/schedule/config.rs @@ -449,12 +449,6 @@ impl IntoSystemSetConfig for S { } } -impl IntoSystemSetConfig for InternedSystemSet { - fn into_config(self) -> SystemSetConfig { - SystemSetConfig::new(self) - } -} - impl IntoSystemSetConfig for SystemSetConfig { fn into_config(self) -> Self { self diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index e18f7dc9e4b21..4a836c440f872 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -76,6 +76,24 @@ pub trait SystemSet: DynHash + Debug + Send + Sync + 'static { } } +impl SystemSet for Interned { + fn system_type(&self) -> Option { + (**self).system_type() + } + + fn is_anonymous(&self) -> bool { + (**self).is_anonymous() + } + + fn dyn_clone(&self) -> Box { + (**self).dyn_clone() + } + + fn dyn_static_ref(&self) -> Option<&'static dyn SystemSet> { + Some(self.0) + } +} + impl From<&dyn SystemSet> for Interned { fn from(value: &dyn SystemSet) -> Interned { SYSTEM_SET_INTERNER.intern(value) diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index c4e34ddd7e217..040cc1caa7491 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -26,7 +26,7 @@ use crate::HashSet; // This implements [`Copy`], [`Clone`], [`PartialEq`], [`Eq`] and [`Hash`] regardles of what `T` // implements, because it only uses the pointer to the value and not the value itself. // Therefore it MUST NEVER implement [`Borrow`](`std::borrow::Borrow`). -pub struct Interned(&'static T); +pub struct Interned(pub &'static T); impl Deref for Interned { type Target = T; diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index eb23575787344..ab3c6e977da30 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -142,6 +142,29 @@ macro_rules! define_label { } } + impl $label_trait_name for ::bevy_utils::intern::Interned { + fn inner_type_id(&self) -> ::std::any::TypeId { + (**self).inner_type_id() + } + + fn dyn_clone(&self) -> Box { + (**self).dyn_clone() + } + + /// Casts this value to a form where it can be compared with other type-erased values. + fn as_dyn_eq(&self) -> &dyn ::bevy_utils::label::DynEq { + (**self).as_dyn_eq() + } + + fn dyn_hash(&self, state: &mut dyn ::std::hash::Hasher) { + (**self).dyn_hash(state) + } + + fn dyn_static_ref(&self) -> Option<&'static dyn $label_trait_name> { + Some(self.0) + } + } + impl PartialEq for dyn $label_trait_name { fn eq(&self, other: &Self) -> bool { self.as_dyn_eq().dyn_eq(other.as_dyn_eq()) From 803fa9cfa4d3eb95f88d10f72d3e84c00cfe7951 Mon Sep 17 00:00:00 2001 From: Joseph <21144246+JoJoJet@users.noreply.github.com> Date: Thu, 27 Jul 2023 15:03:59 -0700 Subject: [PATCH 12/53] simplify interning APIs --- crates/bevy_app/src/app.rs | 30 ++++++++--------- crates/bevy_app/src/main_schedule.rs | 26 +++++++-------- crates/bevy_ecs/src/schedule/config.rs | 26 +++++++-------- crates/bevy_ecs/src/schedule/schedule.rs | 33 +++++++++---------- crates/bevy_ecs/src/schedule/set.rs | 13 +++++--- .../src/system/exclusive_function_system.rs | 2 +- crates/bevy_ecs/src/system/function_system.rs | 2 +- crates/bevy_ecs/src/world/mod.rs | 14 ++++---- crates/bevy_macro_utils/src/lib.rs | 12 ------- crates/bevy_render/src/lib.rs | 4 +-- crates/bevy_utils/src/label.rs | 16 ++++----- 11 files changed, 82 insertions(+), 96 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index b7480aa3fd8eb..b54308a8dd2c5 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -158,7 +158,7 @@ impl SubApp { /// Runs the [`SubApp`]'s default schedule. pub fn run(&mut self) { - self.app.world.run_schedule(&*self.app.main_schedule_label); + self.app.world.run_schedule(self.app.main_schedule_label); self.app.world.clear_trackers(); } @@ -221,7 +221,7 @@ impl App { sub_apps: HashMap::default(), plugin_registry: Vec::default(), plugin_name_added: Default::default(), - main_schedule_label: Main.into(), + main_schedule_label: Main.intern(), building_plugin_depth: 0, } } @@ -243,7 +243,7 @@ impl App { { #[cfg(feature = "trace")] let _bevy_main_update_span = info_span!("main app").entered(); - self.world.run_schedule(&*self.main_schedule_label); + self.world.run_schedule(self.main_schedule_label); } for (_label, sub_app) in self.sub_apps.iter_mut() { #[cfg(feature = "trace")] @@ -381,7 +381,7 @@ impl App { schedule: impl ScheduleLabel, systems: impl IntoSystemConfigs, ) -> &mut Self { - let schedule = InternedScheduleLabel::from(&schedule as &dyn ScheduleLabel); + let schedule = schedule.intern(); let mut schedules = self.world.resource_mut::(); if let Some(schedule) = schedules.get_mut(schedule) { @@ -401,7 +401,7 @@ impl App { schedule: impl ScheduleLabel, set: impl IntoSystemSetConfig, ) -> &mut Self { - let schedule = InternedScheduleLabel::from(&schedule as &dyn ScheduleLabel); + let schedule = schedule.intern(); let mut schedules = self.world.resource_mut::(); if let Some(schedule) = schedules.get_mut(schedule) { schedule.configure_set(set); @@ -419,7 +419,7 @@ impl App { schedule: impl ScheduleLabel, sets: impl IntoSystemSetConfigs, ) -> &mut Self { - let schedule = InternedScheduleLabel::from(&schedule as &dyn ScheduleLabel); + let schedule = schedule.intern(); let mut schedules = self.world.resource_mut::(); if let Some(schedule) = schedules.get_mut(schedule) { schedule.configure_sets(sets); @@ -759,7 +759,7 @@ impl App { /// an [`Err`] containing the given label. pub fn get_sub_app_mut(&mut self, label: impl AppLabel) -> Result<&mut App, impl AppLabel> { self.sub_apps - .get_mut(&InternedAppLabel::from(&label as &dyn AppLabel)) + .get_mut(&label.intern()) .map(|sub_app| &mut sub_app.app) .ok_or(label) } @@ -779,20 +779,20 @@ impl App { /// Inserts an existing sub app into the app pub fn insert_sub_app(&mut self, label: impl AppLabel, sub_app: SubApp) { self.sub_apps - .insert((&label as &dyn AppLabel).into(), sub_app); + .insert(label.intern(), sub_app); } /// Removes a sub app from the app. Returns [`None`] if the label doesn't exist. pub fn remove_sub_app(&mut self, label: impl AppLabel) -> Option { self.sub_apps - .remove(&InternedAppLabel::from(&label as &dyn AppLabel)) + .remove(&label.intern()) } /// Retrieves a `SubApp` inside this [`App`] with the given label, if it exists. Otherwise returns /// an [`Err`] containing the given label. pub fn get_sub_app(&self, label: impl AppLabel) -> Result<&App, impl AppLabel> { self.sub_apps - .get(&InternedAppLabel::from(&label as &dyn AppLabel)) + .get(&label.intern()) .map(|sub_app| &sub_app.app) .ok_or(label) } @@ -804,7 +804,7 @@ impl App { /// To avoid this behavior, use the `init_schedule` method instead. pub fn add_schedule(&mut self, label: impl ScheduleLabel, schedule: Schedule) -> &mut Self { let mut schedules = self.world.resource_mut::(); - schedules.insert(&label as &dyn ScheduleLabel, schedule); + schedules.insert(label, schedule); self } @@ -813,7 +813,7 @@ impl App { /// /// See [`App::add_schedule`] to pass in a pre-constructed schedule. pub fn init_schedule(&mut self, label: impl ScheduleLabel) -> &mut Self { - let label = InternedScheduleLabel::from(&label as &dyn ScheduleLabel); + let label = label.intern(); let mut schedules = self.world.resource_mut::(); if !schedules.contains(label) { schedules.insert(label, Schedule::new()); @@ -824,7 +824,7 @@ impl App { /// Gets read-only access to the [`Schedule`] with the provided `label` if it exists. pub fn get_schedule(&self, label: impl ScheduleLabel) -> Option<&Schedule> { let schedules = self.world.get_resource::()?; - schedules.get(&label as &dyn ScheduleLabel) + schedules.get(label) } /// Gets read-write access to a [`Schedule`] with the provided `label` if it exists. @@ -832,7 +832,7 @@ impl App { let schedules = self.world.get_resource_mut::()?; // We need to call .into_inner here to satisfy the borrow checker: // it can reason about reborrows using ordinary references but not the `Mut` smart pointer. - schedules.into_inner().get_mut(&label as &dyn ScheduleLabel) + schedules.into_inner().get_mut(label) } /// Applies the function to the [`Schedule`] associated with `label`. @@ -843,7 +843,7 @@ impl App { label: impl ScheduleLabel, f: impl FnOnce(&mut Schedule), ) -> &mut Self { - let label = InternedScheduleLabel::from(&label as &dyn ScheduleLabel); + let label = label.intern(); let mut schedules = self.world.resource_mut::(); if schedules.get(label).is_none() { diff --git a/crates/bevy_app/src/main_schedule.rs b/crates/bevy_app/src/main_schedule.rs index f72ac80fb00cc..b5826cc23abb1 100644 --- a/crates/bevy_app/src/main_schedule.rs +++ b/crates/bevy_app/src/main_schedule.rs @@ -1,6 +1,6 @@ use crate::{App, Plugin}; use bevy_ecs::{ - schedule::{ExecutorKind, Schedule, ScheduleLabel}, + schedule::{ExecutorKind, Schedule, ScheduleLabel, InternedScheduleLabel}, system::{Local, Resource}, world::{Mut, World}, }; @@ -105,21 +105,21 @@ pub struct Last; #[derive(Resource, Debug)] pub struct MainScheduleOrder { /// The labels to run for the [`Main`] schedule (in the order they will be run). - pub labels: Vec>, + pub labels: Vec, } impl Default for MainScheduleOrder { fn default() -> Self { Self { labels: vec![ - Box::new(First), - Box::new(PreUpdate), - Box::new(StateTransition), - Box::new(RunFixedUpdateLoop), - Box::new(Update), - Box::new(SpawnScene), - Box::new(PostUpdate), - Box::new(Last), + First.intern(), + PreUpdate.intern(), + StateTransition.intern(), + RunFixedUpdateLoop.intern(), + Update.intern(), + SpawnScene.intern(), + PostUpdate.intern(), + Last.intern(), ], } } @@ -133,7 +133,7 @@ impl MainScheduleOrder { .iter() .position(|current| (**current).eq(&after)) .unwrap_or_else(|| panic!("Expected {after:?} to exist")); - self.labels.insert(index + 1, Box::new(schedule)); + self.labels.insert(index + 1, schedule.intern()); } } @@ -148,8 +148,8 @@ impl Main { } world.resource_scope(|world, order: Mut| { - for label in &order.labels { - let _ = world.try_run_schedule(&**label); + for &label in &order.labels { + let _ = world.try_run_schedule(label); } }); } diff --git a/crates/bevy_ecs/src/schedule/config.rs b/crates/bevy_ecs/src/schedule/config.rs index 95cfa3defb4a3..8bb4d9aefdaa6 100644 --- a/crates/bevy_ecs/src/schedule/config.rs +++ b/crates/bevy_ecs/src/schedule/config.rs @@ -307,20 +307,20 @@ impl IntoSystemConfigs<()> for SystemConfigs { "adding arbitrary systems to a system type set is not allowed" ); - self.in_set_inner((&set as &dyn SystemSet).into()); + self.in_set_inner(set.intern()); self } fn before(mut self, set: impl IntoSystemSet) -> Self { let set = set.into_system_set(); - self.before_inner((&set as &dyn SystemSet).into()); + self.before_inner(set.intern()); self } fn after(mut self, set: impl IntoSystemSet) -> Self { let set = set.into_system_set(); - self.after_inner((&set as &dyn SystemSet).into()); + self.after_inner(set.intern()); self } @@ -331,7 +331,7 @@ impl IntoSystemConfigs<()> for SystemConfigs { fn ambiguous_with(mut self, set: impl IntoSystemSet) -> Self { let set = set.into_system_set(); - self.ambiguous_with_inner((&set as &dyn SystemSet).into()); + self.ambiguous_with_inner(set.intern()); self } @@ -445,7 +445,7 @@ pub trait IntoSystemSetConfig: Sized { impl IntoSystemSetConfig for S { fn into_config(self) -> SystemSetConfig { - SystemSetConfig::new((&self as &dyn SystemSet).into()) + SystemSetConfig::new(self.intern()) } } @@ -460,14 +460,14 @@ impl IntoSystemSetConfig for SystemSetConfig { set.system_type().is_none(), "adding arbitrary systems to a system type set is not allowed" ); - self.graph_info.sets.push((&set as &dyn SystemSet).into()); + self.graph_info.sets.push(set.intern()); self } fn before(mut self, set: impl IntoSystemSet) -> Self { self.graph_info.dependencies.push(Dependency::new( DependencyKind::Before, - (&set.into_system_set() as &dyn SystemSet).into(), + set.into_system_set().intern(), )); self } @@ -475,7 +475,7 @@ impl IntoSystemSetConfig for SystemSetConfig { fn after(mut self, set: impl IntoSystemSet) -> Self { self.graph_info.dependencies.push(Dependency::new( DependencyKind::After, - (&set.into_system_set() as &dyn SystemSet).into(), + set.into_system_set().intern(), )); self } @@ -488,7 +488,7 @@ impl IntoSystemSetConfig for SystemSetConfig { fn ambiguous_with(mut self, set: impl IntoSystemSet) -> Self { ambiguous_with( &mut self.graph_info, - (&set.into_system_set() as &dyn SystemSet).into(), + set.into_system_set().intern(), ); self } @@ -563,14 +563,14 @@ impl IntoSystemSetConfigs for SystemSetConfigs { "adding arbitrary systems to a system type set is not allowed" ); for config in &mut self.sets { - config.graph_info.sets.push((&set as &dyn SystemSet).into()); + config.graph_info.sets.push(set.intern()); } self } fn before(mut self, set: impl IntoSystemSet) -> Self { - let set = (&set.into_system_set() as &dyn SystemSet).into(); + let set = set.into_system_set().intern(); for config in &mut self.sets { config .graph_info @@ -582,7 +582,7 @@ impl IntoSystemSetConfigs for SystemSetConfigs { } fn after(mut self, set: impl IntoSystemSet) -> Self { - let set = (&set.into_system_set() as &dyn SystemSet).into(); + let set = set.into_system_set().intern(); for config in &mut self.sets { config .graph_info @@ -594,7 +594,7 @@ impl IntoSystemSetConfigs for SystemSetConfigs { } fn ambiguous_with(mut self, set: impl IntoSystemSet) -> Self { - let set = (&set.into_system_set() as &dyn SystemSet).into(); + let set = set.into_system_set().intern(); for config in &mut self.sets { ambiguous_with(&mut config.graph_info, set); } diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index 4a7f1151ffd62..c193371699f1c 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -43,38 +43,38 @@ impl Schedules { /// and the old schedule is returned. Otherwise, `None` is returned. pub fn insert( &mut self, - label: impl Into, + label: impl ScheduleLabel, schedule: Schedule, ) -> Option { - self.inner.insert(label.into(), schedule) + self.inner.insert(label.intern(), schedule) } /// Removes the schedule corresponding to the `label` from the map, returning it if it existed. - pub fn remove(&mut self, label: impl Into) -> Option { - self.inner.remove(&label.into()) + pub fn remove(&mut self, label: impl ScheduleLabel) -> Option { + self.inner.remove(&label.intern ()) } /// Removes the (schedule, label) pair corresponding to the `label` from the map, returning it if it existed. pub fn remove_entry( &mut self, - label: impl Into, + label: impl ScheduleLabel, ) -> Option<(InternedScheduleLabel, Schedule)> { - self.inner.remove_entry(&label.into()) + self.inner.remove_entry(&label.intern()) } /// Does a schedule with the provided label already exist? - pub fn contains(&self, label: impl Into) -> bool { - self.inner.contains_key(&label.into()) + pub fn contains(&self, label: impl ScheduleLabel) -> bool { + self.inner.contains_key(&label.intern()) } /// Returns a reference to the schedule associated with `label`, if it exists. - pub fn get(&self, label: impl Into) -> Option<&Schedule> { - self.inner.get(&label.into()) + pub fn get(&self, label: impl ScheduleLabel) -> Option<&Schedule> { + self.inner.get(&label.intern()) } /// Returns a mutable reference to the schedule associated with `label`, if it exists. - pub fn get_mut(&mut self, label: impl Into) -> Option<&mut Schedule> { - self.inner.get_mut(&label.into()) + pub fn get_mut(&mut self, label: impl ScheduleLabel) -> Option<&mut Schedule> { + self.inner.get_mut(&label.intern()) } /// Returns an iterator over all schedules. Iteration order is undefined. @@ -537,7 +537,7 @@ impl ScheduleGraph { if more_than_one_entry { let set = self.create_anonymous_set(); for config in &mut configs { - config.in_set_inner((&set as &dyn SystemSet).into()); + config.in_set_inner(set.intern()); } let mut set_config = set.into_config(); set_config.conditions.extend(collective_conditions); @@ -721,8 +721,7 @@ impl ScheduleGraph { id } - fn check_set(&mut self, id: &NodeId, set: &dyn SystemSet) -> Result<(), ScheduleBuildError> { - let set = set.into(); + fn check_set(&mut self, id: &NodeId, set: InternedSystemSet) -> Result<(), ScheduleBuildError> { match self.system_set_ids.get(&set) { Some(set_id) => { if id == set_id { @@ -748,8 +747,8 @@ impl ScheduleGraph { id: &NodeId, graph_info: &GraphInfo, ) -> Result<(), ScheduleBuildError> { - for set in &graph_info.sets { - self.check_set(id, &**set)?; + for &set in &graph_info.sets { + self.check_set(id, set)?; } Ok(()) diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index 4a836c440f872..0770fdb5e1f7a 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -74,6 +74,11 @@ pub trait SystemSet: DynHash + Debug + Send + Sync + 'static { fn dyn_static_ref(&self) -> Option<&'static dyn SystemSet> { None } + + /// Returns an [`InternedSystemSet`] corresponding to `self`. + fn intern(&self) -> InternedSystemSet where Self: Sized { + SYSTEM_SET_INTERNER.intern(self) + } } impl SystemSet for Interned { @@ -92,11 +97,9 @@ impl SystemSet for Interned { fn dyn_static_ref(&self) -> Option<&'static dyn SystemSet> { Some(self.0) } -} -impl From<&dyn SystemSet> for Interned { - fn from(value: &dyn SystemSet) -> Interned { - SYSTEM_SET_INTERNER.intern(value) + fn intern(&self) -> InternedSystemSet where Self: Sized { + *self } } @@ -274,7 +277,7 @@ mod tests { schedule.add_systems(|mut flag: ResMut| flag.0 = true); world.add_schedule(schedule, A); - let interned = InternedScheduleLabel::from(&A as &dyn ScheduleLabel); + let interned = A.intern(); world.insert_resource(Flag(false)); world.run_schedule(interned); diff --git a/crates/bevy_ecs/src/system/exclusive_function_system.rs b/crates/bevy_ecs/src/system/exclusive_function_system.rs index b3d18fd5bbeda..403c789831c03 100644 --- a/crates/bevy_ecs/src/system/exclusive_function_system.rs +++ b/crates/bevy_ecs/src/system/exclusive_function_system.rs @@ -150,7 +150,7 @@ where fn default_system_sets(&self) -> Vec { let set = crate::schedule::SystemTypeSet::::new(); - vec![(&set as &dyn SystemSet).into()] + vec![set.intern()] } } diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index 47d549e5bbbf9..c0c05afdfeb3d 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -512,7 +512,7 @@ where fn default_system_sets(&self) -> Vec { let set = crate::schedule::SystemTypeSet::::new(); - vec![(&set as &dyn SystemSet).into()] + vec![set.intern()] } } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index d6c4d71de33c5..c1b67455dddc7 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -20,7 +20,7 @@ use crate::{ event::{Event, Events}, query::{DebugCheckedUnwrap, QueryState, ReadOnlyWorldQuery, WorldQuery}, removal_detection::RemovedComponentEvents, - schedule::{InternedScheduleLabel, Schedule, ScheduleLabel, Schedules}, + schedule::{Schedule, ScheduleLabel, Schedules}, storage::{ResourceData, Storages}, system::Resource, world::error::TryRunScheduleError, @@ -1750,7 +1750,7 @@ impl World { /// The `Schedules` resource will be initialized if it does not already exist. pub fn add_schedule(&mut self, schedule: Schedule, label: impl ScheduleLabel) { let mut schedules = self.get_resource_or_insert_with(Schedules::default); - schedules.insert(&label as &dyn ScheduleLabel, schedule); + schedules.insert(label, schedule); } /// Temporarily removes the schedule associated with `label` from the world, @@ -1766,10 +1766,10 @@ impl World { /// For other use cases, see the example on [`World::schedule_scope`]. pub fn try_schedule_scope( &mut self, - label: impl Into, + label: impl ScheduleLabel, f: impl FnOnce(&mut World, &mut Schedule) -> R, ) -> Result { - let label = label.into(); + let label = label.intern(); let Some((extracted_label, mut schedule)) = self .get_resource_mut::() .and_then(|mut s| s.remove_entry(label)) @@ -1831,7 +1831,7 @@ impl World { /// If the requested schedule does not exist. pub fn schedule_scope( &mut self, - label: impl Into, + label: impl ScheduleLabel, f: impl FnOnce(&mut World, &mut Schedule) -> R, ) -> R { self.try_schedule_scope(label, f) @@ -1847,7 +1847,7 @@ impl World { /// For simple testing use cases, call [`Schedule::run(&mut world)`](Schedule::run) instead. pub fn try_run_schedule( &mut self, - label: impl Into, + label: impl ScheduleLabel, ) -> Result<(), TryRunScheduleError> { self.try_schedule_scope(label, |world, sched| sched.run(world)) } @@ -1862,7 +1862,7 @@ impl World { /// # Panics /// /// If the requested schedule does not exist. - pub fn run_schedule(&mut self, label: impl Into) { + pub fn run_schedule(&mut self, label: impl ScheduleLabel) { self.schedule_scope(label, |world, sched| sched.run(world)); } } diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index b6bf4cee13285..76df2bb1783f6 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -245,18 +245,6 @@ pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStr #dyn_static_ref_impl } } - - impl #impl_generics From<#ident #ty_generics> for ::bevy_utils::intern::Interned #where_clause { - fn from(value: #ident #ty_generics) -> Self { - (&value as &dyn #trait_path).into() - } - } - - impl #impl_generics From<& #ident #ty_generics> for ::bevy_utils::intern::Interned #where_clause { - fn from(value: &#ident #ty_generics) -> Self { - (value as &dyn #trait_path).into() - } - } }) .into() } diff --git a/crates/bevy_render/src/lib.rs b/crates/bevy_render/src/lib.rs index cf5dce491ea1a..9a470bb615c74 100644 --- a/crates/bevy_render/src/lib.rs +++ b/crates/bevy_render/src/lib.rs @@ -270,7 +270,7 @@ impl Plugin for RenderPlugin { app.init_resource::(); let mut render_app = App::empty(); - render_app.main_schedule_label = Render.into(); + render_app.main_schedule_label = Render.intern(); let mut extract_schedule = Schedule::new(); extract_schedule.set_apply_final_deferred(false); @@ -419,7 +419,7 @@ fn extract(main_world: &mut World, render_app: &mut App) { fn apply_extract_commands(render_world: &mut World) { render_world.resource_scope(|render_world, mut schedules: Mut| { schedules - .get_mut(&ExtractSchedule) + .get_mut(ExtractSchedule) .unwrap() .apply_deferred(render_world); }); diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index ab3c6e977da30..cf80103c0201f 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -140,6 +140,12 @@ macro_rules! define_label { fn dyn_static_ref(&self) -> Option<&'static dyn $label_trait_name> { None } + + /// Returns an `interned`(bevy_utils::intern::Interned) value corresponding to `self`. + fn intern(&self) -> ::bevy_utils::intern::Interned + where Self: Sized { + $interner_name.intern(self) + } } impl $label_trait_name for ::bevy_utils::intern::Interned { @@ -191,15 +197,5 @@ macro_rules! define_label { static $interner_name: ::bevy_utils::intern::Interner = ::bevy_utils::intern::Interner::new(); - - impl From<&dyn $label_trait_name> - for ::bevy_utils::intern::Interned - { - fn from( - value: &dyn $label_trait_name, - ) -> ::bevy_utils::intern::Interned { - $interner_name.intern(value) - } - } }; } From 3099a4beffdd1e99e4b748c3e87c20f4f33f5c27 Mon Sep 17 00:00:00 2001 From: Joseph <21144246+JoJoJet@users.noreply.github.com> Date: Thu, 27 Jul 2023 17:42:50 -0700 Subject: [PATCH 13/53] rustfmt --- crates/bevy_app/src/app.rs | 6 ++---- crates/bevy_app/src/main_schedule.rs | 2 +- crates/bevy_ecs/src/schedule/config.rs | 5 +---- crates/bevy_ecs/src/schedule/schedule.rs | 8 ++------ crates/bevy_ecs/src/schedule/set.rs | 10 ++++++++-- 5 files changed, 14 insertions(+), 17 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index b54308a8dd2c5..f47a538058a37 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -778,14 +778,12 @@ impl App { /// Inserts an existing sub app into the app pub fn insert_sub_app(&mut self, label: impl AppLabel, sub_app: SubApp) { - self.sub_apps - .insert(label.intern(), sub_app); + self.sub_apps.insert(label.intern(), sub_app); } /// Removes a sub app from the app. Returns [`None`] if the label doesn't exist. pub fn remove_sub_app(&mut self, label: impl AppLabel) -> Option { - self.sub_apps - .remove(&label.intern()) + self.sub_apps.remove(&label.intern()) } /// Retrieves a `SubApp` inside this [`App`] with the given label, if it exists. Otherwise returns diff --git a/crates/bevy_app/src/main_schedule.rs b/crates/bevy_app/src/main_schedule.rs index b5826cc23abb1..a6a9527f914e4 100644 --- a/crates/bevy_app/src/main_schedule.rs +++ b/crates/bevy_app/src/main_schedule.rs @@ -1,6 +1,6 @@ use crate::{App, Plugin}; use bevy_ecs::{ - schedule::{ExecutorKind, Schedule, ScheduleLabel, InternedScheduleLabel}, + schedule::{ExecutorKind, InternedScheduleLabel, Schedule, ScheduleLabel}, system::{Local, Resource}, world::{Mut, World}, }; diff --git a/crates/bevy_ecs/src/schedule/config.rs b/crates/bevy_ecs/src/schedule/config.rs index 8bb4d9aefdaa6..c0dc3a771869e 100644 --- a/crates/bevy_ecs/src/schedule/config.rs +++ b/crates/bevy_ecs/src/schedule/config.rs @@ -486,10 +486,7 @@ impl IntoSystemSetConfig for SystemSetConfig { } fn ambiguous_with(mut self, set: impl IntoSystemSet) -> Self { - ambiguous_with( - &mut self.graph_info, - set.into_system_set().intern(), - ); + ambiguous_with(&mut self.graph_info, set.into_system_set().intern()); self } diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index c193371699f1c..f438cbc0e6184 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -41,17 +41,13 @@ impl Schedules { /// /// If the map already had an entry for `label`, `schedule` is inserted, /// and the old schedule is returned. Otherwise, `None` is returned. - pub fn insert( - &mut self, - label: impl ScheduleLabel, - schedule: Schedule, - ) -> Option { + pub fn insert(&mut self, label: impl ScheduleLabel, schedule: Schedule) -> Option { self.inner.insert(label.intern(), schedule) } /// Removes the schedule corresponding to the `label` from the map, returning it if it existed. pub fn remove(&mut self, label: impl ScheduleLabel) -> Option { - self.inner.remove(&label.intern ()) + self.inner.remove(&label.intern()) } /// Removes the (schedule, label) pair corresponding to the `label` from the map, returning it if it existed. diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index 0770fdb5e1f7a..e213db4502d01 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -76,7 +76,10 @@ pub trait SystemSet: DynHash + Debug + Send + Sync + 'static { } /// Returns an [`InternedSystemSet`] corresponding to `self`. - fn intern(&self) -> InternedSystemSet where Self: Sized { + fn intern(&self) -> InternedSystemSet + where + Self: Sized, + { SYSTEM_SET_INTERNER.intern(self) } } @@ -98,7 +101,10 @@ impl SystemSet for Interned { Some(self.0) } - fn intern(&self) -> InternedSystemSet where Self: Sized { + fn intern(&self) -> InternedSystemSet + where + Self: Sized, + { *self } } From 815b52c5f550cd2c31f80cb4588e7b2eae72b57c Mon Sep 17 00:00:00 2001 From: Joseph <21144246+JoJoJet@users.noreply.github.com> Date: Thu, 27 Jul 2023 17:44:28 -0700 Subject: [PATCH 14/53] special-case interned system sets --- crates/bevy_ecs/macros/src/set.rs | 12 +++++++++ crates/bevy_ecs/src/schedule/set.rs | 38 ++++++++++++++++++++++++++--- crates/bevy_transform/Cargo.toml | 1 + 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/macros/src/set.rs b/crates/bevy_ecs/macros/src/set.rs index 2a759f133b054..0b3fcbc782bd5 100644 --- a/crates/bevy_ecs/macros/src/set.rs +++ b/crates/bevy_ecs/macros/src/set.rs @@ -1,3 +1,4 @@ +use bevy_macro_utils::BevyManifest; use proc_macro::TokenStream; use quote::{quote, quote_spanned}; use syn::spanned::Spanned; @@ -9,6 +10,8 @@ use syn::spanned::Spanned; /// - `input`: The [`syn::DeriveInput`] for the struct that we want to derive the set trait for /// - `trait_path`: The [`syn::Path`] to the set trait pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStream { + let bevy_utils_path = BevyManifest::default().get_path("bevy_utils"); + let ident = input.ident; let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); @@ -69,6 +72,15 @@ pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStrea std::boxed::Box::new(std::clone::Clone::clone(self)) } + fn as_dyn_eq(&self) -> &dyn #bevy_utils_path::label::DynEq { + self + } + + fn dyn_hash(&self, mut state: &mut dyn ::std::hash::Hasher) { + std::hash::Hash::hash(&std::any::TypeId::of::(), &mut state); + std::hash::Hash::hash(self, &mut state); + } + fn dyn_static_ref(&self) -> std::option::Option<&'static dyn #trait_path> { #dyn_static_ref_impl } diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index e213db4502d01..9028aacf73439 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -6,7 +6,7 @@ use std::marker::PhantomData; pub use bevy_ecs_macros::{ScheduleLabel, SystemSet}; use bevy_utils::define_label; use bevy_utils::intern::{Interned, Interner, Leak}; -use bevy_utils::label::DynHash; +use bevy_utils::label::DynEq; use crate::system::{ ExclusiveSystemParamFunction, IsExclusiveFunctionSystem, IsFunctionSystem, SystemParamFunction, @@ -25,7 +25,7 @@ pub type InternedSystemSet = Interned; pub type InternedScheduleLabel = Interned; /// Types that identify logical groups of systems. -pub trait SystemSet: DynHash + Debug + Send + Sync + 'static { +pub trait SystemSet: Debug + Send + Sync + 'static { /// Returns `Some` if this system set is a [`SystemTypeSet`]. fn system_type(&self) -> Option { None @@ -39,6 +39,12 @@ pub trait SystemSet: DynHash + Debug + Send + Sync + 'static { /// Creates a boxed clone of the label corresponding to this system set. fn dyn_clone(&self) -> Box; + /// Casts this value to a form where it can be compared with other type-erased values. + fn as_dyn_eq(&self) -> &dyn DynEq; + + /// Feeds this value into the given [`Hasher`]. + fn dyn_hash(&self, state: &mut dyn Hasher); + /// Returns a static reference to a value equal to `self`, if possible. /// This method is used to optimize [interning](bevy_utils::intern). /// @@ -97,6 +103,14 @@ impl SystemSet for Interned { (**self).dyn_clone() } + fn as_dyn_eq(&self) -> &dyn DynEq { + (**self).as_dyn_eq() + } + + fn dyn_hash(&self, state: &mut dyn Hasher) { + (**self).dyn_hash(state) + } + fn dyn_static_ref(&self) -> Option<&'static dyn SystemSet> { Some(self.0) } @@ -111,7 +125,7 @@ impl SystemSet for Interned { impl PartialEq for dyn SystemSet { fn eq(&self, other: &Self) -> bool { - self.dyn_eq(other.as_dyn_eq()) + self.as_dyn_eq().dyn_eq(other.as_dyn_eq()) } } @@ -187,6 +201,15 @@ impl SystemSet for SystemTypeSet { Box::new(*self) } + fn as_dyn_eq(&self) -> &dyn DynEq { + self + } + + fn dyn_hash(&self, mut state: &mut dyn Hasher) { + std::any::TypeId::of::().hash(&mut state); + self.hash(&mut state); + } + fn dyn_static_ref(&self) -> Option<&'static dyn SystemSet> { Some(&Self(PhantomData)) } @@ -208,6 +231,15 @@ impl SystemSet for AnonymousSet { true } + fn as_dyn_eq(&self) -> &dyn DynEq { + self + } + + fn dyn_hash(&self, mut state: &mut dyn Hasher) { + std::any::TypeId::of::().hash(&mut state); + self.hash(&mut state); + } + fn dyn_clone(&self) -> Box { Box::new(*self) } diff --git a/crates/bevy_transform/Cargo.toml b/crates/bevy_transform/Cargo.toml index 9f250745232c5..2de8b9a194e9a 100644 --- a/crates/bevy_transform/Cargo.toml +++ b/crates/bevy_transform/Cargo.toml @@ -15,6 +15,7 @@ bevy_ecs = { path = "../bevy_ecs", version = "0.12.0-dev", features = ["bevy_ref bevy_hierarchy = { path = "../bevy_hierarchy", version = "0.12.0-dev" } bevy_math = { path = "../bevy_math", version = "0.12.0-dev" } bevy_reflect = { path = "../bevy_reflect", version = "0.12.0-dev", features = ["bevy"] } +bevy_utils = { path = "../bevy_utils", version = "0.12.0-dev" } serde = { version = "1", features = ["derive"], optional = true } [dev-dependencies] From ab0e722cb5c8812a779caa7691ce0d0ee807fa9f Mon Sep 17 00:00:00 2001 From: Joseph <21144246+JoJoJet@users.noreply.github.com> Date: Thu, 27 Jul 2023 17:58:41 -0700 Subject: [PATCH 15/53] remove an unnecessary special-case --- crates/bevy_macro_utils/src/lib.rs | 2 +- crates/bevy_utils/src/label.rs | 14 -------------- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index 76df2bb1783f6..2c6e1c8ce5fd3 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -236,7 +236,7 @@ pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStr } fn dyn_hash(&self, mut state: &mut dyn ::std::hash::Hasher) { - let ty_id = #trait_path::inner_type_id(self); + let ty_id = ::std::any::TypeId::of::(); ::std::hash::Hash::hash(&ty_id, &mut state); ::std::hash::Hash::hash(self, &mut state); } diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index cf80103c0201f..c259b52b313a8 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -82,16 +82,6 @@ macro_rules! define_label { $(#[$label_attr])* pub trait $label_trait_name: 'static + Send + Sync + ::std::fmt::Debug { - /// Return's the [TypeId] of this label, or the the ID of the - /// wrappped label type for `Box` - /// - /// [TypeId]: std::any::TypeId - fn inner_type_id(&self) -> ::std::any::TypeId { - std::any::TypeId::of::() - } - /// Clones this ` #[doc = stringify!($label_trait_name)] /// ` @@ -149,10 +139,6 @@ macro_rules! define_label { } impl $label_trait_name for ::bevy_utils::intern::Interned { - fn inner_type_id(&self) -> ::std::any::TypeId { - (**self).inner_type_id() - } - fn dyn_clone(&self) -> Box { (**self).dyn_clone() } From 681b4707bd94ed132388ba913a8b140ca0f81088 Mon Sep 17 00:00:00 2001 From: Joseph <21144246+JoJoJet@users.noreply.github.com> Date: Thu, 27 Jul 2023 22:16:35 -0700 Subject: [PATCH 16/53] Overide `.intern()` for `Interned` --- crates/bevy_ecs/src/schedule/set.rs | 7 ++----- crates/bevy_utils/src/label.rs | 4 ++++ 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index 9028aacf73439..ce0cbd6422c6d 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -90,7 +90,7 @@ pub trait SystemSet: Debug + Send + Sync + 'static { } } -impl SystemSet for Interned { +impl SystemSet for InternedSystemSet { fn system_type(&self) -> Option { (**self).system_type() } @@ -115,10 +115,7 @@ impl SystemSet for Interned { Some(self.0) } - fn intern(&self) -> InternedSystemSet - where - Self: Sized, - { + fn intern(&self) -> Self { *self } } diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index c259b52b313a8..3387997532d54 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -155,6 +155,10 @@ macro_rules! define_label { fn dyn_static_ref(&self) -> Option<&'static dyn $label_trait_name> { Some(self.0) } + + fn intern(&self) -> Self { + *self + } } impl PartialEq for dyn $label_trait_name { From 6c61187cdf62503cddc9b4da64bc4e31a12f6776 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Fri, 28 Jul 2023 13:30:04 +0200 Subject: [PATCH 17/53] Remove use of `std::borrow::Borrow` --- crates/bevy_utils/src/intern.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index 040cc1caa7491..b4d00408db943 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -5,7 +5,6 @@ //! and make comparisons for any type as fast as integers. use std::{ - borrow::Borrow, fmt::Debug, hash::Hash, ops::Deref, @@ -155,13 +154,13 @@ impl Interner { let lock = self.0.get_or_init(Default::default); { let set = lock.read().unwrap_or_else(PoisonError::into_inner); - if let Some(value) = set.get(value.borrow()) { + if let Some(value) = set.get(value) { return Interned(*value); } } { let mut set = lock.write().unwrap_or_else(PoisonError::into_inner); - if let Some(value) = set.get(value.borrow()) { + if let Some(value) = set.get(value) { Interned(*value) } else { let leaked = value.leak(); From 73c05c91e6185d662942bdee402465d273fe41d0 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Fri, 28 Jul 2023 13:41:46 +0200 Subject: [PATCH 18/53] Fix clippy warnings --- crates/bevy_ecs/src/schedule/set.rs | 2 +- crates/bevy_utils/src/label.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index ce0cbd6422c6d..08d1bfc723f96 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -108,7 +108,7 @@ impl SystemSet for InternedSystemSet { } fn dyn_hash(&self, state: &mut dyn Hasher) { - (**self).dyn_hash(state) + (**self).dyn_hash(state); } fn dyn_static_ref(&self) -> Option<&'static dyn SystemSet> { diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index 3387997532d54..445fada4a220e 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -84,7 +84,7 @@ macro_rules! define_label { pub trait $label_trait_name: 'static + Send + Sync + ::std::fmt::Debug { /// Clones this ` #[doc = stringify!($label_trait_name)] - /// ` + ///`. fn dyn_clone(&self) -> Box; /// Casts this value to a form where it can be compared with other type-erased values. @@ -131,7 +131,7 @@ macro_rules! define_label { None } - /// Returns an `interned`(bevy_utils::intern::Interned) value corresponding to `self`. + /// Returns an [`Interned`](bevy_utils::intern::Interned) value corresponding to `self`. fn intern(&self) -> ::bevy_utils::intern::Interned where Self: Sized { $interner_name.intern(self) @@ -149,7 +149,7 @@ macro_rules! define_label { } fn dyn_hash(&self, state: &mut dyn ::std::hash::Hasher) { - (**self).dyn_hash(state) + (**self).dyn_hash(state); } fn dyn_static_ref(&self) -> Option<&'static dyn $label_trait_name> { From 531bb9b772db557dd7314d989d4f1757f8950af1 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Fri, 28 Jul 2023 17:34:51 +0200 Subject: [PATCH 19/53] Improve documentation Co-authored-by: Joseph <21144246+JoJoJet@users.noreply.github.com> --- crates/bevy_utils/src/intern.rs | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index b4d00408db943..9f96ce52cd260 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -18,13 +18,28 @@ use crate::HashSet; /// /// For details on interning, see [the module level docs](self). /// -/// # Implementation details +/// # Comparisons +/// +/// Interned values use reference equality, meaning they implement [`Eq`] +/// and [`Hash`] regardless of whether `T` implements these traits. +/// Two interned values will only compare equal if they were interned using +/// the same [`Interner`] instance. +// NOTE: This type must NEVER implement Borrow since it does not obey that trait's invariants. /// -/// This is just a reference to a value with an `'static` lifetime. -// -// This implements [`Copy`], [`Clone`], [`PartialEq`], [`Eq`] and [`Hash`] regardles of what `T` -// implements, because it only uses the pointer to the value and not the value itself. -// Therefore it MUST NEVER implement [`Borrow`](`std::borrow::Borrow`). +/// ``` +/// # use bevy_utils::intern::*; +/// #[derive(PartialEq, Eq, Hash)] +/// struct Value(i32); +/// impl Leak for Value { +/// // ... +/// # fn leak(&self) -> &'static Self { Box::leak(Box::new(Value(self.0))) } +/// } +/// let interner_1 = Interner::new(); +/// let interner_2 = Interner::new(); +/// // Even though both values are identical, their interned forms do not +/// // compare equal as they use different interner instances. +/// assert_ne!(interner_1.intern(&Value(42)), interner_2.intern(&Value(42))); +/// ``` pub struct Interned(pub &'static T); impl Deref for Interned { From 53e6e2cac43fcd0bbe4fdb330f446ac8b46ff32f Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Fri, 28 Jul 2023 18:16:00 +0200 Subject: [PATCH 20/53] Fix whitespace --- crates/bevy_utils/src/intern.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index 9f96ce52cd260..a086270ca1fe4 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -19,7 +19,7 @@ use crate::HashSet; /// For details on interning, see [the module level docs](self). /// /// # Comparisons -/// +/// /// Interned values use reference equality, meaning they implement [`Eq`] /// and [`Hash`] regardless of whether `T` implements these traits. /// Two interned values will only compare equal if they were interned using From c664fe7fa4baef138028e28672df7c02be1cfd5e Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Fri, 28 Jul 2023 18:26:33 +0200 Subject: [PATCH 21/53] Fix docs --- crates/bevy_utils/src/intern.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index a086270ca1fe4..ad57950dd9d52 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -28,7 +28,7 @@ use crate::HashSet; /// /// ``` /// # use bevy_utils::intern::*; -/// #[derive(PartialEq, Eq, Hash)] +/// #[derive(PartialEq, Eq, Hash, Debug)] /// struct Value(i32); /// impl Leak for Value { /// // ... @@ -144,7 +144,7 @@ impl Leak for str { /// /// The implementation ensures that two equal values return two equal [`Interned`] values. /// -/// To use an [`Interner`], `T` must implement [`StaticRef`], [`Hash`] and [`Eq`]. +/// To use an [`Interner`], `T` must implement [`Leak`], [`Hash`] and [`Eq`]. pub struct Interner(OnceLock>>); impl Interner { @@ -161,7 +161,7 @@ impl Interner { /// [`Interned`] using the obtained static reference. Subsequent calls for the same `value` /// will return [`Interned`] using the same static reference. /// - /// This uses [`StaticRef::static_ref`] to short-circuit the interning process. + /// This uses [`Leak::static_ref`] to short-circuit the interning process. pub fn intern(&self, value: &T) -> Interned { if let Some(value) = value.static_ref() { return Interned(value); From a4d7c81fefcb2238e00c55b5ece6ed80a2f66430 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 5 Aug 2023 08:45:28 +0200 Subject: [PATCH 22/53] Update crates/bevy_utils/src/intern.rs Co-authored-by: Joseph <21144246+JoJoJet@users.noreply.github.com> --- crates/bevy_utils/src/intern.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index ad57950dd9d52..ecf731c19cff7 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -22,7 +22,7 @@ use crate::HashSet; /// /// Interned values use reference equality, meaning they implement [`Eq`] /// and [`Hash`] regardless of whether `T` implements these traits. -/// Two interned values will only compare equal if they were interned using +/// Two interned values are only guaranteed to compare equal if they were interned using /// the same [`Interner`] instance. // NOTE: This type must NEVER implement Borrow since it does not obey that trait's invariants. /// From 11fcbd0baba0a51811a376b33f51901e46c6a048 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 5 Aug 2023 08:45:10 +0200 Subject: [PATCH 23/53] Fix and add tests for label derive macros --- crates/bevy_app/src/app.rs | 85 +++++++++++++++ crates/bevy_ecs/macros/src/set.rs | 2 +- crates/bevy_ecs/src/schedule/set.rs | 161 +++++++++++++++++++++++++++- crates/bevy_macro_utils/src/lib.rs | 2 +- 4 files changed, 247 insertions(+), 3 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index f47a538058a37..47df95f7aa1d3 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -978,4 +978,89 @@ mod tests { app.world.run_schedule(OnEnter(AppState::MainMenu)); assert_eq!(app.world.entities().len(), 2); } + + #[test] + fn test_derive_app_label() { + use super::AppLabel; + use crate::{self as bevy_app}; + + #[derive(AppLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + struct UnitLabel; + + #[derive(AppLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + struct TupleLabel(u32, u32); + + #[derive(AppLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + struct StructLabel { + a: u32, + b: u32, + } + + #[derive(AppLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + enum EnumLabel { + #[default] + Unit, + Tuple(u32, u32), + Struct { + a: u32, + b: u32, + }, + } + + assert_eq!(UnitLabel.intern(), UnitLabel.intern()); + assert_eq!(EnumLabel::Unit.intern(), EnumLabel::Unit.intern()); + assert_ne!(UnitLabel.intern(), EnumLabel::Unit.intern()); + assert_ne!(UnitLabel.intern(), TupleLabel(0, 0).intern()); + assert_ne!(EnumLabel::Unit.intern(), EnumLabel::Tuple(0, 0).intern()); + + assert_eq!(TupleLabel(0, 0).intern(), TupleLabel(0, 0).intern()); + assert_eq!( + EnumLabel::Tuple(0, 0).intern(), + EnumLabel::Tuple(0, 0).intern() + ); + assert_ne!(TupleLabel(0, 0).intern(), TupleLabel(0, 1).intern()); + assert_ne!( + EnumLabel::Tuple(0, 0).intern(), + EnumLabel::Tuple(0, 1).intern() + ); + assert_ne!(TupleLabel(0, 0).intern(), EnumLabel::Tuple(0, 0).intern()); + assert_ne!( + TupleLabel(0, 0).intern(), + StructLabel { a: 0, b: 0 }.intern() + ); + assert_ne!( + EnumLabel::Tuple(0, 0).intern(), + EnumLabel::Struct { a: 0, b: 0 }.intern() + ); + + assert_eq!( + StructLabel { a: 0, b: 0 }.intern(), + StructLabel { a: 0, b: 0 }.intern() + ); + assert_eq!( + EnumLabel::Struct { a: 0, b: 0 }.intern(), + EnumLabel::Struct { a: 0, b: 0 }.intern() + ); + assert_ne!( + StructLabel { a: 0, b: 0 }.intern(), + StructLabel { a: 0, b: 1 }.intern() + ); + assert_ne!( + EnumLabel::Struct { a: 0, b: 0 }.intern(), + EnumLabel::Struct { a: 0, b: 1 }.intern() + ); + assert_ne!( + StructLabel { a: 0, b: 0 }.intern(), + EnumLabel::Struct { a: 0, b: 0 }.intern() + ); + assert_ne!( + StructLabel { a: 0, b: 0 }.intern(), + EnumLabel::Struct { a: 0, b: 0 }.intern() + ); + assert_ne!(StructLabel { a: 0, b: 0 }.intern(), UnitLabel.intern(),); + assert_ne!( + EnumLabel::Struct { a: 0, b: 0 }.intern(), + EnumLabel::Unit.intern() + ); + } } diff --git a/crates/bevy_ecs/macros/src/set.rs b/crates/bevy_ecs/macros/src/set.rs index 0b3fcbc782bd5..6c327da20d3cb 100644 --- a/crates/bevy_ecs/macros/src/set.rs +++ b/crates/bevy_ecs/macros/src/set.rs @@ -53,7 +53,7 @@ pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStrea quote! { match self { #(#variants)* - _ => None + _ => std::option::Option::None } } } else { diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index 08d1bfc723f96..7bf0171bdc720 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -297,7 +297,7 @@ mod tests { use super::*; #[test] - fn test_label() { + fn test_schedule_label() { use crate::{self as bevy_ecs, world::World}; #[derive(Resource)] @@ -322,4 +322,163 @@ mod tests { world.run_schedule(interned); assert!(world.resource::().0); } + + #[test] + fn test_derive_schedule_label() { + use crate::{self as bevy_ecs}; + + #[derive(ScheduleLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + struct UnitLabel; + + #[derive(ScheduleLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + struct TupleLabel(u32, u32); + + #[derive(ScheduleLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + struct StructLabel { + a: u32, + b: u32, + } + + #[derive(ScheduleLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + enum EnumLabel { + #[default] + Unit, + Tuple(u32, u32), + Struct { + a: u32, + b: u32, + }, + } + + assert_eq!(UnitLabel.intern(), UnitLabel.intern()); + assert_eq!(EnumLabel::Unit.intern(), EnumLabel::Unit.intern()); + assert_ne!(UnitLabel.intern(), EnumLabel::Unit.intern()); + assert_ne!(UnitLabel.intern(), TupleLabel(0, 0).intern()); + assert_ne!(EnumLabel::Unit.intern(), EnumLabel::Tuple(0, 0).intern()); + + assert_eq!(TupleLabel(0, 0).intern(), TupleLabel(0, 0).intern()); + assert_eq!( + EnumLabel::Tuple(0, 0).intern(), + EnumLabel::Tuple(0, 0).intern() + ); + assert_ne!(TupleLabel(0, 0).intern(), TupleLabel(0, 1).intern()); + assert_ne!( + EnumLabel::Tuple(0, 0).intern(), + EnumLabel::Tuple(0, 1).intern() + ); + assert_ne!(TupleLabel(0, 0).intern(), EnumLabel::Tuple(0, 0).intern()); + assert_ne!( + TupleLabel(0, 0).intern(), + StructLabel { a: 0, b: 0 }.intern() + ); + assert_ne!( + EnumLabel::Tuple(0, 0).intern(), + EnumLabel::Struct { a: 0, b: 0 }.intern() + ); + + assert_eq!( + StructLabel { a: 0, b: 0 }.intern(), + StructLabel { a: 0, b: 0 }.intern() + ); + assert_eq!( + EnumLabel::Struct { a: 0, b: 0 }.intern(), + EnumLabel::Struct { a: 0, b: 0 }.intern() + ); + assert_ne!( + StructLabel { a: 0, b: 0 }.intern(), + StructLabel { a: 0, b: 1 }.intern() + ); + assert_ne!( + EnumLabel::Struct { a: 0, b: 0 }.intern(), + EnumLabel::Struct { a: 0, b: 1 }.intern() + ); + assert_ne!( + StructLabel { a: 0, b: 0 }.intern(), + EnumLabel::Struct { a: 0, b: 0 }.intern() + ); + assert_ne!( + StructLabel { a: 0, b: 0 }.intern(), + EnumLabel::Struct { a: 0, b: 0 }.intern() + ); + assert_ne!(StructLabel { a: 0, b: 0 }.intern(), UnitLabel.intern(),); + assert_ne!( + EnumLabel::Struct { a: 0, b: 0 }.intern(), + EnumLabel::Unit.intern() + ); + } + + #[test] + fn test_derive_system_set() { + use crate::{self as bevy_ecs}; + + #[derive(SystemSet, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + struct UnitSet; + + #[derive(SystemSet, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + struct TupleSet(u32, u32); + + #[derive(SystemSet, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + struct StructSet { + a: u32, + b: u32, + } + + #[derive(SystemSet, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + enum EnumSet { + #[default] + Unit, + Tuple(u32, u32), + Struct { + a: u32, + b: u32, + }, + } + + assert_eq!(UnitSet.intern(), UnitSet.intern()); + assert_eq!(EnumSet::Unit.intern(), EnumSet::Unit.intern()); + assert_ne!(UnitSet.intern(), EnumSet::Unit.intern()); + assert_ne!(UnitSet.intern(), TupleSet(0, 0).intern()); + assert_ne!(EnumSet::Unit.intern(), EnumSet::Tuple(0, 0).intern()); + + assert_eq!(TupleSet(0, 0).intern(), TupleSet(0, 0).intern()); + assert_eq!(EnumSet::Tuple(0, 0).intern(), EnumSet::Tuple(0, 0).intern()); + assert_ne!(TupleSet(0, 0).intern(), TupleSet(0, 1).intern()); + assert_ne!(EnumSet::Tuple(0, 0).intern(), EnumSet::Tuple(0, 1).intern()); + assert_ne!(TupleSet(0, 0).intern(), EnumSet::Tuple(0, 0).intern()); + assert_ne!(TupleSet(0, 0).intern(), StructSet { a: 0, b: 0 }.intern()); + assert_ne!( + EnumSet::Tuple(0, 0).intern(), + EnumSet::Struct { a: 0, b: 0 }.intern() + ); + + assert_eq!( + StructSet { a: 0, b: 0 }.intern(), + StructSet { a: 0, b: 0 }.intern() + ); + assert_eq!( + EnumSet::Struct { a: 0, b: 0 }.intern(), + EnumSet::Struct { a: 0, b: 0 }.intern() + ); + assert_ne!( + StructSet { a: 0, b: 0 }.intern(), + StructSet { a: 0, b: 1 }.intern() + ); + assert_ne!( + EnumSet::Struct { a: 0, b: 0 }.intern(), + EnumSet::Struct { a: 0, b: 1 }.intern() + ); + assert_ne!( + StructSet { a: 0, b: 0 }.intern(), + EnumSet::Struct { a: 0, b: 0 }.intern() + ); + assert_ne!( + StructSet { a: 0, b: 0 }.intern(), + EnumSet::Struct { a: 0, b: 0 }.intern() + ); + assert_ne!(StructSet { a: 0, b: 0 }.intern(), UnitSet.intern(),); + assert_ne!( + EnumSet::Struct { a: 0, b: 0 }.intern(), + EnumSet::Unit.intern() + ); + } } diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index 2c6e1c8ce5fd3..57f567e52546a 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -212,7 +212,7 @@ pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStr quote! { match self { #(#variants)* - _ => quote! { std::option::Option::None } + _ => std::option::Option::None, } } } else { From b5130c79d1a059a4edd43a844e0f5ef041be4448 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 5 Aug 2023 14:22:18 +0200 Subject: [PATCH 24/53] Replace `::bevy_utils` with `$crate` --- crates/bevy_utils/src/label.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index 445fada4a220e..09cdaba91639e 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -88,7 +88,7 @@ macro_rules! define_label { fn dyn_clone(&self) -> Box; /// Casts this value to a form where it can be compared with other type-erased values. - fn as_dyn_eq(&self) -> &dyn ::bevy_utils::label::DynEq; + fn as_dyn_eq(&self) -> &dyn $crate::label::DynEq; /// Feeds this value into the given [`Hasher`]. /// @@ -132,19 +132,19 @@ macro_rules! define_label { } /// Returns an [`Interned`](bevy_utils::intern::Interned) value corresponding to `self`. - fn intern(&self) -> ::bevy_utils::intern::Interned + fn intern(&self) -> $crate::intern::Interned where Self: Sized { $interner_name.intern(self) } } - impl $label_trait_name for ::bevy_utils::intern::Interned { + impl $label_trait_name for $crate::intern::Interned { fn dyn_clone(&self) -> Box { (**self).dyn_clone() } /// Casts this value to a form where it can be compared with other type-erased values. - fn as_dyn_eq(&self) -> &dyn ::bevy_utils::label::DynEq { + fn as_dyn_eq(&self) -> &dyn $crate::label::DynEq { (**self).as_dyn_eq() } @@ -175,7 +175,7 @@ macro_rules! define_label { } } - impl ::bevy_utils::intern::Leak for dyn $label_trait_name { + impl $crate::intern::Leak for dyn $label_trait_name { fn leak(&self) -> &'static Self { Box::leak(self.dyn_clone()) } @@ -185,7 +185,7 @@ macro_rules! define_label { } } - static $interner_name: ::bevy_utils::intern::Interner = - ::bevy_utils::intern::Interner::new(); + static $interner_name: $crate::intern::Interner = + $crate::intern::Interner::new(); }; } From c1904144e5815e1447d8710f63372124e42eae4a Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 5 Aug 2023 14:28:09 +0200 Subject: [PATCH 25/53] Fix macro hygiene --- crates/bevy_ecs/macros/src/set.rs | 20 ++++++++++---------- crates/bevy_macro_utils/src/lib.rs | 16 ++++++++-------- crates/bevy_utils/src/label.rs | 10 +++++----- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/crates/bevy_ecs/macros/src/set.rs b/crates/bevy_ecs/macros/src/set.rs index 6c327da20d3cb..caae824919e97 100644 --- a/crates/bevy_ecs/macros/src/set.rs +++ b/crates/bevy_ecs/macros/src/set.rs @@ -28,9 +28,9 @@ pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStrea let dyn_static_ref_impl = match input.data { syn::Data::Struct(data) => { if data.fields.is_empty() { - quote! { std::option::Option::Some(&Self) } + quote! { ::std::option::Option::Some(&Self) } } else { - quote! { std::option::Option::None } + quote! { ::std::option::Option::None } } } syn::Data::Enum(data) => { @@ -42,7 +42,7 @@ pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStrea if variant.fields.is_empty() { let span = variant.span(); let variant_ident = variant.ident; - Some(quote_spanned! { span => Self::#variant_ident => std::option::Option::Some(&Self::#variant_ident), }) + Some(quote_spanned! { span => Self::#variant_ident => ::std::option::Option::Some(&Self::#variant_ident), }) } else { use_fallback_variant = true; None @@ -53,7 +53,7 @@ pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStrea quote! { match self { #(#variants)* - _ => std::option::Option::None + _ => ::std::option::Option::None } } } else { @@ -64,12 +64,12 @@ pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStrea } } } - syn::Data::Union(_) => quote! { std::option::Option::None }, + syn::Data::Union(_) => quote! { ::std::option::Option::None }, }; (quote! { impl #impl_generics #trait_path for #ident #ty_generics #where_clause { - fn dyn_clone(&self) -> std::boxed::Box { - std::boxed::Box::new(std::clone::Clone::clone(self)) + fn dyn_clone(&self) -> ::std::boxed::Box { + ::std::boxed::Box::new(::std::clone::Clone::clone(self)) } fn as_dyn_eq(&self) -> &dyn #bevy_utils_path::label::DynEq { @@ -77,11 +77,11 @@ pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStrea } fn dyn_hash(&self, mut state: &mut dyn ::std::hash::Hasher) { - std::hash::Hash::hash(&std::any::TypeId::of::(), &mut state); - std::hash::Hash::hash(self, &mut state); + ::std::hash::Hash::hash(&::std::any::TypeId::of::(), &mut state); + ::std::hash::Hash::hash(self, &mut state); } - fn dyn_static_ref(&self) -> std::option::Option<&'static dyn #trait_path> { + fn dyn_static_ref(&self) -> ::std::option::Option<&'static dyn #trait_path> { #dyn_static_ref_impl } } diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index 57f567e52546a..1eb288909d1ec 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -187,9 +187,9 @@ pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStr let dyn_static_ref_impl = match input.data { syn::Data::Struct(data) => { if data.fields.is_empty() { - quote! { std::option::Option::Some(&Self) } + quote! { ::std::option::Option::Some(&Self) } } else { - quote! { std::option::Option::None } + quote! { ::std::option::Option::None } } } syn::Data::Enum(data) => { @@ -201,7 +201,7 @@ pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStr if variant.fields.is_empty() { let span = variant.span(); let variant_ident = variant.ident; - Some(quote_spanned! { span => Self::#variant_ident => std::option::Option::Some(&Self::#variant_ident), }) + Some(quote_spanned! { span => Self::#variant_ident => ::std::option::Option::Some(&Self::#variant_ident), }) } else { use_fallback_variant = true; None @@ -212,7 +212,7 @@ pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStr quote! { match self { #(#variants)* - _ => std::option::Option::None, + _ => ::std::option::Option::None, } } } else { @@ -223,12 +223,12 @@ pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStr } } } - syn::Data::Union(_) => quote! { std::option::Option::None }, + syn::Data::Union(_) => quote! { ::std::option::Option::None }, }; (quote! { impl #impl_generics #trait_path for #ident #ty_generics #where_clause { - fn dyn_clone(&self) -> std::boxed::Box { - std::boxed::Box::new(std::clone::Clone::clone(self)) + fn dyn_clone(&self) -> ::std::boxed::Box { + ::std::boxed::Box::new(::std::clone::Clone::clone(self)) } fn as_dyn_eq(&self) -> &dyn #bevy_utils_path::label::DynEq { @@ -241,7 +241,7 @@ pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStr ::std::hash::Hash::hash(self, &mut state); } - fn dyn_static_ref(&self) -> std::option::Option<&'static dyn #trait_path> { + fn dyn_static_ref(&self) -> ::std::option::Option<&'static dyn #trait_path> { #dyn_static_ref_impl } } diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index 09cdaba91639e..c33146c07230b 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -85,7 +85,7 @@ macro_rules! define_label { /// Clones this ` #[doc = stringify!($label_trait_name)] ///`. - fn dyn_clone(&self) -> Box; + fn dyn_clone(&self) -> ::std::boxed::Box; /// Casts this value to a form where it can be compared with other type-erased values. fn as_dyn_eq(&self) -> &dyn $crate::label::DynEq; @@ -127,7 +127,7 @@ macro_rules! define_label { /// # Provided implementation /// /// The provided implementation always returns `None`. - fn dyn_static_ref(&self) -> Option<&'static dyn $label_trait_name> { + fn dyn_static_ref(&self) -> ::std::option::Option<&'static dyn $label_trait_name> { None } @@ -139,7 +139,7 @@ macro_rules! define_label { } impl $label_trait_name for $crate::intern::Interned { - fn dyn_clone(&self) -> Box { + fn dyn_clone(&self) -> ::std::boxed::Box { (**self).dyn_clone() } @@ -152,7 +152,7 @@ macro_rules! define_label { (**self).dyn_hash(state); } - fn dyn_static_ref(&self) -> Option<&'static dyn $label_trait_name> { + fn dyn_static_ref(&self) -> ::std::option::Option<&'static dyn $label_trait_name> { Some(self.0) } @@ -180,7 +180,7 @@ macro_rules! define_label { Box::leak(self.dyn_clone()) } - fn static_ref(&self) -> std::option::Option<&'static dyn $label_trait_name> { + fn static_ref(&self) -> ::std::option::Option<&'static dyn $label_trait_name> { self.dyn_static_ref() } } From db71468c74d1d09eecd1d3067352c6499d9c1dec Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 5 Aug 2023 14:31:40 +0200 Subject: [PATCH 26/53] Fix typos --- crates/bevy_ecs/src/schedule/set.rs | 2 +- crates/bevy_utils/src/intern.rs | 2 +- crates/bevy_utils/src/label.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index 7bf0171bdc720..eda133b52adee 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -50,7 +50,7 @@ pub trait SystemSet: Debug + Send + Sync + 'static { /// /// # Invariant /// - /// The following invariants must be hold: + /// The following invariants most hold: /// /// `ptr_eq(a.dyn_static_ref(), b.dyn_static_ref())` if `a.dyn_eq(b)` /// `ptr_neq(a.dyn_static_ref(), b.dyn_static_ref())` if `!a.dyn_eq(b)` diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index ecf731c19cff7..b8be76acbe4a6 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -99,7 +99,7 @@ pub trait Leak { /// /// # Invariant /// - /// The following invariants must be hold: + /// The following invariants most hold: /// /// `ptr_eq(a.static_ref(), b.static_ref())` if `a == b` /// `ptr_neq(a.static_ref(), b.static_ref())` if `a != b` diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index c33146c07230b..2f5fef2ffec20 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -100,7 +100,7 @@ macro_rules! define_label { /// /// # Invariant /// - /// The following invariants must be hold: + /// The following invariants most hold: /// /// `ptr_eq(a.dyn_static_ref(), b.dyn_static_ref())` if `a.dyn_eq(b)` /// `ptr_neq(a.dyn_static_ref(), b.dyn_static_ref())` if `!a.dyn_eq(b)` From 75b2a46d070f43803b3f4a154f4b1629ebf5f98f Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 5 Aug 2023 14:39:46 +0200 Subject: [PATCH 27/53] Disallow deriving label and set traits on unions --- crates/bevy_ecs/macros/src/set.rs | 9 +++++++-- crates/bevy_macro_utils/src/lib.rs | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/macros/src/set.rs b/crates/bevy_ecs/macros/src/set.rs index caae824919e97..07df12d9a79e5 100644 --- a/crates/bevy_ecs/macros/src/set.rs +++ b/crates/bevy_ecs/macros/src/set.rs @@ -12,7 +12,7 @@ use syn::spanned::Spanned; pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStream { let bevy_utils_path = BevyManifest::default().get_path("bevy_utils"); - let ident = input.ident; + let ident = input.ident.clone(); let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); let mut where_clause = where_clause.cloned().unwrap_or_else(|| syn::WhereClause { @@ -64,7 +64,12 @@ pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStrea } } } - syn::Data::Union(_) => quote! { ::std::option::Option::None }, + syn::Data::Union(_) => { + return quote_spanned! { + input.span() => compile_error!("Unions cannot be used as sets."); + } + .into() + } }; (quote! { impl #impl_generics #trait_path for #ident #ty_generics #where_clause { diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index 1eb288909d1ec..ebfe23f05014c 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -172,7 +172,7 @@ pub fn ensure_no_collision(value: Ident, haystack: TokenStream) -> Ident { pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStream { let bevy_utils_path = BevyManifest::default().get_path("bevy_utils"); - let ident = input.ident; + let ident = input.ident.clone(); let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); let mut where_clause = where_clause.cloned().unwrap_or_else(|| syn::WhereClause { where_token: Default::default(), @@ -223,7 +223,12 @@ pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStr } } } - syn::Data::Union(_) => quote! { ::std::option::Option::None }, + syn::Data::Union(_) => { + return quote_spanned! { + input.span() => compile_error!("Unions cannot be used as labels."); + } + .into() + } }; (quote! { impl #impl_generics #trait_path for #ident #ty_generics #where_clause { From f596145b533933b6f220aa8d36c7eb84348f3d80 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 5 Aug 2023 15:01:09 +0200 Subject: [PATCH 28/53] Remove code duplication --- crates/bevy_ecs/macros/src/set.rs | 53 +++------------------- crates/bevy_macro_utils/Cargo.toml | 1 + crates/bevy_macro_utils/src/lib.rs | 54 +++-------------------- crates/bevy_macro_utils/src/static_ref.rs | 51 +++++++++++++++++++++ 4 files changed, 64 insertions(+), 95 deletions(-) create mode 100644 crates/bevy_macro_utils/src/static_ref.rs diff --git a/crates/bevy_ecs/macros/src/set.rs b/crates/bevy_ecs/macros/src/set.rs index 07df12d9a79e5..f8c49b8700f74 100644 --- a/crates/bevy_ecs/macros/src/set.rs +++ b/crates/bevy_ecs/macros/src/set.rs @@ -1,7 +1,6 @@ -use bevy_macro_utils::BevyManifest; +use bevy_macro_utils::{static_ref_impl, BevyManifest}; use proc_macro::TokenStream; -use quote::{quote, quote_spanned}; -use syn::spanned::Spanned; +use quote::quote; /// Derive a set trait /// @@ -25,51 +24,9 @@ pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStrea }) .unwrap(), ); - let dyn_static_ref_impl = match input.data { - syn::Data::Struct(data) => { - if data.fields.is_empty() { - quote! { ::std::option::Option::Some(&Self) } - } else { - quote! { ::std::option::Option::None } - } - } - syn::Data::Enum(data) => { - let mut use_fallback_variant = false; - let variants: Vec<_> = data - .variants - .into_iter() - .filter_map(|variant| { - if variant.fields.is_empty() { - let span = variant.span(); - let variant_ident = variant.ident; - Some(quote_spanned! { span => Self::#variant_ident => ::std::option::Option::Some(&Self::#variant_ident), }) - } else { - use_fallback_variant = true; - None - } - }) - .collect(); - if use_fallback_variant { - quote! { - match self { - #(#variants)* - _ => ::std::option::Option::None - } - } - } else { - quote! { - match self { - #(#variants)* - } - } - } - } - syn::Data::Union(_) => { - return quote_spanned! { - input.span() => compile_error!("Unions cannot be used as sets."); - } - .into() - } + let dyn_static_ref_impl = match static_ref_impl(&input) { + Ok(stream) => stream, + Err(stream) => return stream.into(), }; (quote! { impl #impl_generics #trait_path for #ident #ty_generics #where_clause { diff --git a/crates/bevy_macro_utils/Cargo.toml b/crates/bevy_macro_utils/Cargo.toml index 261e877880652..30a9a2519a360 100644 --- a/crates/bevy_macro_utils/Cargo.toml +++ b/crates/bevy_macro_utils/Cargo.toml @@ -12,4 +12,5 @@ keywords = ["bevy"] toml_edit = "0.19" syn = "2.0" quote = "1.0" +proc-macro2 = "1.0" rustc-hash = "1.0" diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index ebfe23f05014c..f63f878462cfe 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -4,17 +4,19 @@ extern crate proc_macro; mod attrs; mod shape; +mod static_ref; mod symbol; pub use attrs::*; pub use shape::*; +pub use static_ref::*; pub use symbol::*; use proc_macro::{TokenStream, TokenTree}; -use quote::{quote, quote_spanned}; +use quote::quote; use rustc_hash::FxHashSet; use std::{env, path::PathBuf}; -use syn::{spanned::Spanned, Ident}; +use syn::Ident; use toml_edit::{Document, Item}; pub struct BevyManifest { @@ -184,51 +186,9 @@ pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStr }) .unwrap(), ); - let dyn_static_ref_impl = match input.data { - syn::Data::Struct(data) => { - if data.fields.is_empty() { - quote! { ::std::option::Option::Some(&Self) } - } else { - quote! { ::std::option::Option::None } - } - } - syn::Data::Enum(data) => { - let mut use_fallback_variant = false; - let variants: Vec<_> = data - .variants - .into_iter() - .filter_map(|variant| { - if variant.fields.is_empty() { - let span = variant.span(); - let variant_ident = variant.ident; - Some(quote_spanned! { span => Self::#variant_ident => ::std::option::Option::Some(&Self::#variant_ident), }) - } else { - use_fallback_variant = true; - None - } - }) - .collect(); - if use_fallback_variant { - quote! { - match self { - #(#variants)* - _ => ::std::option::Option::None, - } - } - } else { - quote! { - match self { - #(#variants)* - } - } - } - } - syn::Data::Union(_) => { - return quote_spanned! { - input.span() => compile_error!("Unions cannot be used as labels."); - } - .into() - } + let dyn_static_ref_impl = match static_ref_impl(&input) { + Ok(stream) => stream, + Err(stream) => return stream.into(), }; (quote! { impl #impl_generics #trait_path for #ident #ty_generics #where_clause { diff --git a/crates/bevy_macro_utils/src/static_ref.rs b/crates/bevy_macro_utils/src/static_ref.rs new file mode 100644 index 0000000000000..ca5213d4fea03 --- /dev/null +++ b/crates/bevy_macro_utils/src/static_ref.rs @@ -0,0 +1,51 @@ +use proc_macro2::TokenStream; +use quote::{quote, quote_spanned}; +use syn::spanned::Spanned; + +pub fn static_ref_impl(input: &syn::DeriveInput) -> Result { + Ok(match &input.data { + syn::Data::Struct(data) => { + if data.fields.is_empty() { + quote! { ::std::option::Option::Some(&Self) } + } else { + quote! { ::std::option::Option::None } + } + } + syn::Data::Enum(data) => { + let mut use_fallback_variant = false; + let variants: Vec<_> = data + .variants + .iter() + .filter_map(|variant| { + if variant.fields.is_empty() { + let span = variant.span(); + let variant_ident = variant.ident.clone(); + Some(quote_spanned! { span => Self::#variant_ident => ::std::option::Option::Some(&Self::#variant_ident), }) + } else { + use_fallback_variant = true; + None + } + }) + .collect(); + if use_fallback_variant { + quote! { + match self { + #(#variants)* + _ => ::std::option::Option::None, + } + } + } else { + quote! { + match self { + #(#variants)* + } + } + } + } + syn::Data::Union(_) => { + return Err(quote_spanned! { + input.span() => compile_error!("Unions cannot be used as labels."); + }); + } + }) +} From 04dbbeb68bf05c3dfefaca472bafb27bcb6f93d5 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 5 Aug 2023 15:08:10 +0200 Subject: [PATCH 29/53] Simplify fallback variant construction --- crates/bevy_macro_utils/src/static_ref.rs | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/crates/bevy_macro_utils/src/static_ref.rs b/crates/bevy_macro_utils/src/static_ref.rs index ca5213d4fea03..bdb0174fbe6fd 100644 --- a/crates/bevy_macro_utils/src/static_ref.rs +++ b/crates/bevy_macro_utils/src/static_ref.rs @@ -12,7 +12,6 @@ pub fn static_ref_impl(input: &syn::DeriveInput) -> Result { - let mut use_fallback_variant = false; let variants: Vec<_> = data .variants .iter() @@ -22,23 +21,19 @@ pub fn static_ref_impl(input: &syn::DeriveInput) -> Result Self::#variant_ident => ::std::option::Option::Some(&Self::#variant_ident), }) } else { - use_fallback_variant = true; None } }) .collect(); - if use_fallback_variant { - quote! { - match self { - #(#variants)* - _ => ::std::option::Option::None, - } - } + let fallback_variant = if variants.len() < data.variants.len() { + quote!(_ => ::std::option::Option::None,) } else { - quote! { - match self { - #(#variants)* - } + quote!() + }; + quote! { + match self { + #(#variants)* + #fallback_variant } } } From 806b0a59f3b4594dcb179a2a69e5b44d890e5bb4 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 5 Aug 2023 15:11:20 +0200 Subject: [PATCH 30/53] Remove headings --- crates/bevy_ecs/src/schedule/set.rs | 2 -- crates/bevy_utils/src/intern.rs | 2 -- crates/bevy_utils/src/label.rs | 2 -- 3 files changed, 6 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index eda133b52adee..8542386dfd9e1 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -74,8 +74,6 @@ pub trait SystemSet: Debug + Send + Sync + 'static { /// } /// ``` /// - /// # Provided implementation - /// /// The provided implementation always returns `None`. fn dyn_static_ref(&self) -> Option<&'static dyn SystemSet> { None diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index b8be76acbe4a6..c06f26ed69b25 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -123,8 +123,6 @@ pub trait Leak { /// } /// ``` /// - /// # Provided implementation - /// /// The provided implementation always returns `None`. fn static_ref(&self) -> Option<&'static Self> { None diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index 2f5fef2ffec20..0ee7dc034d549 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -124,8 +124,6 @@ macro_rules! define_label { /// } /// ``` /// - /// # Provided implementation - /// /// The provided implementation always returns `None`. fn dyn_static_ref(&self) -> ::std::option::Option<&'static dyn $label_trait_name> { None From afc0a2d7e2d044fe15e4f31dfb8ee000dde5e699 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 5 Aug 2023 15:13:41 +0200 Subject: [PATCH 31/53] Fix derive macro docs --- crates/bevy_derive/src/lib.rs | 3 +-- crates/bevy_ecs/macros/src/lib.rs | 4 ++++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/bevy_derive/src/lib.rs b/crates/bevy_derive/src/lib.rs index 293289b81d0a9..0dbbff57971f3 100644 --- a/crates/bevy_derive/src/lib.rs +++ b/crates/bevy_derive/src/lib.rs @@ -201,8 +201,7 @@ pub fn derive_enum_variant_meta(input: TokenStream) -> TokenStream { /// Generates an impl of the `AppLabel` trait. /// -/// This works only for unit structs, or enums with only unit variants. -/// You may force a struct or variant to behave as if it were fieldless with `#[app_label(ignore_fields)]`. +/// This does not work for unions. #[proc_macro_derive(AppLabel)] pub fn derive_app_label(input: TokenStream) -> TokenStream { let input = syn::parse_macro_input!(input as syn::DeriveInput); diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index bb399579cf963..32580a1cd778d 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -432,6 +432,8 @@ pub fn derive_world_query(input: TokenStream) -> TokenStream { } /// Derive macro generating an impl of the trait `ScheduleLabel`. +/// +/// This does not work for unions. #[proc_macro_derive(ScheduleLabel)] pub fn derive_schedule_label(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); @@ -444,6 +446,8 @@ pub fn derive_schedule_label(input: TokenStream) -> TokenStream { } /// Derive macro generating an impl of the trait `SystemSet`. +/// +/// This does not work for unions. #[proc_macro_derive(SystemSet)] pub fn derive_system_set(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); From 629e02ca4f7f7d73745886c605533c33f1016128 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 5 Aug 2023 15:37:18 +0200 Subject: [PATCH 32/53] Refactor `static_ref_impl` --- crates/bevy_ecs/macros/src/set.rs | 15 ++++++++++----- crates/bevy_macro_utils/src/lib.rs | 16 ++++++++++------ crates/bevy_macro_utils/src/static_ref.rs | 10 ++++------ 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/crates/bevy_ecs/macros/src/set.rs b/crates/bevy_ecs/macros/src/set.rs index f8c49b8700f74..336154f590e93 100644 --- a/crates/bevy_ecs/macros/src/set.rs +++ b/crates/bevy_ecs/macros/src/set.rs @@ -1,6 +1,7 @@ use bevy_macro_utils::{static_ref_impl, BevyManifest}; use proc_macro::TokenStream; -use quote::quote; +use quote::{quote, quote_spanned}; +use syn::spanned::Spanned; /// Derive a set trait /// @@ -9,6 +10,13 @@ use quote::quote; /// - `input`: The [`syn::DeriveInput`] for the struct that we want to derive the set trait for /// - `trait_path`: The [`syn::Path`] to the set trait pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStream { + if let syn::Data::Union(_) = &input.data { + return quote_spanned! { + input.span() => compile_error!("Unions cannot be used as sets."); + } + .into(); + } + let bevy_utils_path = BevyManifest::default().get_path("bevy_utils"); let ident = input.ident.clone(); @@ -24,10 +32,7 @@ pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStrea }) .unwrap(), ); - let dyn_static_ref_impl = match static_ref_impl(&input) { - Ok(stream) => stream, - Err(stream) => return stream.into(), - }; + let dyn_static_ref_impl = static_ref_impl(&input); (quote! { impl #impl_generics #trait_path for #ident #ty_generics #where_clause { fn dyn_clone(&self) -> ::std::boxed::Box { diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index f63f878462cfe..1b2fbd2c65e5f 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -13,10 +13,10 @@ pub use static_ref::*; pub use symbol::*; use proc_macro::{TokenStream, TokenTree}; -use quote::quote; +use quote::{quote, quote_spanned}; use rustc_hash::FxHashSet; use std::{env, path::PathBuf}; -use syn::Ident; +use syn::{spanned::Spanned, Ident}; use toml_edit::{Document, Item}; pub struct BevyManifest { @@ -172,6 +172,13 @@ pub fn ensure_no_collision(value: Ident, haystack: TokenStream) -> Ident { /// - `input`: The [`syn::DeriveInput`] for struct that is deriving the label trait /// - `trait_path`: The path [`syn::Path`] to the label trait pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStream { + if let syn::Data::Union(_) = &input.data { + return quote_spanned! { + input.span() => compile_error!("Unions cannot be used as labels."); + } + .into(); + } + let bevy_utils_path = BevyManifest::default().get_path("bevy_utils"); let ident = input.ident.clone(); @@ -186,10 +193,7 @@ pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStr }) .unwrap(), ); - let dyn_static_ref_impl = match static_ref_impl(&input) { - Ok(stream) => stream, - Err(stream) => return stream.into(), - }; + let dyn_static_ref_impl = static_ref_impl(&input); (quote! { impl #impl_generics #trait_path for #ident #ty_generics #where_clause { fn dyn_clone(&self) -> ::std::boxed::Box { diff --git a/crates/bevy_macro_utils/src/static_ref.rs b/crates/bevy_macro_utils/src/static_ref.rs index bdb0174fbe6fd..46cd9d2e98aed 100644 --- a/crates/bevy_macro_utils/src/static_ref.rs +++ b/crates/bevy_macro_utils/src/static_ref.rs @@ -2,8 +2,8 @@ use proc_macro2::TokenStream; use quote::{quote, quote_spanned}; use syn::spanned::Spanned; -pub fn static_ref_impl(input: &syn::DeriveInput) -> Result { - Ok(match &input.data { +pub fn static_ref_impl(input: &syn::DeriveInput) -> TokenStream { + match &input.data { syn::Data::Struct(data) => { if data.fields.is_empty() { quote! { ::std::option::Option::Some(&Self) } @@ -38,9 +38,7 @@ pub fn static_ref_impl(input: &syn::DeriveInput) -> Result { - return Err(quote_spanned! { - input.span() => compile_error!("Unions cannot be used as labels."); - }); + quote! { ::std::option::Option::None } } - }) + } } From 3f769e52b6be18978bdf1f9556c937bc974fecd7 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 5 Aug 2023 15:50:38 +0200 Subject: [PATCH 33/53] Test derive for generic labels and sets --- crates/bevy_app/src/app.rs | 14 ++++++++++++++ crates/bevy_ecs/src/schedule/set.rs | 24 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 47df95f7aa1d3..a660d01f70043 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -882,6 +882,8 @@ pub struct AppExit; #[cfg(test)] mod tests { + use std::marker::PhantomData; + use bevy_ecs::{ schedule::{OnEnter, States}, system::Commands, @@ -1007,6 +1009,9 @@ mod tests { }, } + #[derive(AppLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + struct GenericLabel(PhantomData); + assert_eq!(UnitLabel.intern(), UnitLabel.intern()); assert_eq!(EnumLabel::Unit.intern(), EnumLabel::Unit.intern()); assert_ne!(UnitLabel.intern(), EnumLabel::Unit.intern()); @@ -1062,5 +1067,14 @@ mod tests { EnumLabel::Struct { a: 0, b: 0 }.intern(), EnumLabel::Unit.intern() ); + + assert_eq!( + GenericLabel::(PhantomData).intern(), + GenericLabel::(PhantomData).intern() + ); + assert_ne!( + GenericLabel::(PhantomData).intern(), + GenericLabel::(PhantomData).intern() + ); } } diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index 8542386dfd9e1..c4f43fe59444c 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -348,6 +348,9 @@ mod tests { }, } + #[derive(ScheduleLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + struct GenericLabel(PhantomData); + assert_eq!(UnitLabel.intern(), UnitLabel.intern()); assert_eq!(EnumLabel::Unit.intern(), EnumLabel::Unit.intern()); assert_ne!(UnitLabel.intern(), EnumLabel::Unit.intern()); @@ -403,6 +406,15 @@ mod tests { EnumLabel::Struct { a: 0, b: 0 }.intern(), EnumLabel::Unit.intern() ); + + assert_eq!( + GenericLabel::(PhantomData).intern(), + GenericLabel::(PhantomData).intern() + ); + assert_ne!( + GenericLabel::(PhantomData).intern(), + GenericLabel::(PhantomData).intern() + ); } #[test] @@ -432,6 +444,9 @@ mod tests { }, } + #[derive(SystemSet, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + struct GenericSet(PhantomData); + assert_eq!(UnitSet.intern(), UnitSet.intern()); assert_eq!(EnumSet::Unit.intern(), EnumSet::Unit.intern()); assert_ne!(UnitSet.intern(), EnumSet::Unit.intern()); @@ -478,5 +493,14 @@ mod tests { EnumSet::Struct { a: 0, b: 0 }.intern(), EnumSet::Unit.intern() ); + + assert_eq!( + GenericSet::(PhantomData).intern(), + GenericSet::(PhantomData).intern() + ); + assert_ne!( + GenericSet::(PhantomData).intern(), + GenericSet::(PhantomData).intern() + ); } } From 8ea3a7f107984ceffabca2d0ab2eb991c23cce06 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 5 Aug 2023 15:58:06 +0200 Subject: [PATCH 34/53] Simplify `static_ref_impl` for enum types --- crates/bevy_macro_utils/src/static_ref.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/bevy_macro_utils/src/static_ref.rs b/crates/bevy_macro_utils/src/static_ref.rs index 46cd9d2e98aed..8e1812725c0eb 100644 --- a/crates/bevy_macro_utils/src/static_ref.rs +++ b/crates/bevy_macro_utils/src/static_ref.rs @@ -25,15 +25,11 @@ pub fn static_ref_impl(input: &syn::DeriveInput) -> TokenStream { } }) .collect(); - let fallback_variant = if variants.len() < data.variants.len() { - quote!(_ => ::std::option::Option::None,) - } else { - quote!() - }; quote! { match self { #(#variants)* - #fallback_variant + #[allow(unreachable_patterns)] + _ => ::std::option::Option::None, } } } From cbf145bf652b33ce40fb718d6d21bd52850b54dd Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 5 Aug 2023 18:56:22 +0200 Subject: [PATCH 35/53] Fix `static_ref_impl` for empty struct and tuple variants --- crates/bevy_app/src/app.rs | 15 +++++++++ crates/bevy_ecs/src/schedule/set.rs | 30 ++++++++++++++++++ crates/bevy_macro_utils/src/static_ref.rs | 38 ++++++++++++++++------- 3 files changed, 72 insertions(+), 11 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index a660d01f70043..d90a22f2e78df 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -998,6 +998,12 @@ mod tests { b: u32, } + #[derive(AppLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + struct EmptyTupleLabel(); + + #[derive(AppLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + struct EmptyStructLabel {} + #[derive(AppLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] enum EnumLabel { #[default] @@ -1007,6 +1013,8 @@ mod tests { a: u32, b: u32, }, + EmptyTuple(), + EmptyStruct {}, } #[derive(AppLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] @@ -1076,5 +1084,12 @@ mod tests { GenericLabel::(PhantomData).intern(), GenericLabel::(PhantomData).intern() ); + + assert!(UnitLabel.dyn_static_ref().is_some()); + assert!(EmptyTupleLabel().dyn_static_ref().is_some()); + assert!(EmptyStructLabel {}.dyn_static_ref().is_some()); + assert!(EnumLabel::Unit.dyn_static_ref().is_some()); + assert!(EnumLabel::EmptyTuple().dyn_static_ref().is_some()); + assert!(EnumLabel::EmptyStruct {}.dyn_static_ref().is_some()); } } diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index c4f43fe59444c..e35d9233675a0 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -337,6 +337,12 @@ mod tests { b: u32, } + #[derive(ScheduleLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + struct EmptyTupleLabel(); + + #[derive(ScheduleLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + struct EmptyStructLabel {} + #[derive(ScheduleLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] enum EnumLabel { #[default] @@ -346,6 +352,8 @@ mod tests { a: u32, b: u32, }, + EmptyTuple(), + EmptyStruct {}, } #[derive(ScheduleLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] @@ -415,6 +423,13 @@ mod tests { GenericLabel::(PhantomData).intern(), GenericLabel::(PhantomData).intern() ); + + assert!(UnitLabel.dyn_static_ref().is_some()); + assert!(EmptyTupleLabel().dyn_static_ref().is_some()); + assert!(EmptyStructLabel {}.dyn_static_ref().is_some()); + assert!(EnumLabel::Unit.dyn_static_ref().is_some()); + assert!(EnumLabel::EmptyTuple().dyn_static_ref().is_some()); + assert!(EnumLabel::EmptyStruct {}.dyn_static_ref().is_some()); } #[test] @@ -433,6 +448,12 @@ mod tests { b: u32, } + #[derive(SystemSet, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + struct EmptyTupleSet(); + + #[derive(SystemSet, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + struct EmptyStructSet {} + #[derive(SystemSet, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] enum EnumSet { #[default] @@ -442,6 +463,8 @@ mod tests { a: u32, b: u32, }, + EmptyTuple(), + EmptyStruct {}, } #[derive(SystemSet, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] @@ -502,5 +525,12 @@ mod tests { GenericSet::(PhantomData).intern(), GenericSet::(PhantomData).intern() ); + + assert!(UnitSet.dyn_static_ref().is_some()); + assert!(EmptyTupleSet().dyn_static_ref().is_some()); + assert!(EmptyStructSet {}.dyn_static_ref().is_some()); + assert!(EnumSet::Unit.dyn_static_ref().is_some()); + assert!(EnumSet::EmptyTuple().dyn_static_ref().is_some()); + assert!(EnumSet::EmptyStruct {}.dyn_static_ref().is_some()); } } diff --git a/crates/bevy_macro_utils/src/static_ref.rs b/crates/bevy_macro_utils/src/static_ref.rs index 8e1812725c0eb..a717801bc6000 100644 --- a/crates/bevy_macro_utils/src/static_ref.rs +++ b/crates/bevy_macro_utils/src/static_ref.rs @@ -4,24 +4,40 @@ use syn::spanned::Spanned; pub fn static_ref_impl(input: &syn::DeriveInput) -> TokenStream { match &input.data { - syn::Data::Struct(data) => { - if data.fields.is_empty() { - quote! { ::std::option::Option::Some(&Self) } - } else { + syn::Data::Struct(data) => match &data.fields { + syn::Fields::Named(fields) if fields.named.is_empty() => { + quote! { ::std::option::Option::Some(&Self{}) } + } + syn::Fields::Unnamed(fields) if fields.unnamed.is_empty() => { + quote! { ::std::option::Option::Some(&Self()) } + } + syn::Fields::Unit => quote! { ::std::option::Option::Some(&Self) }, + _ => { quote! { ::std::option::Option::None } } - } + }, syn::Data::Enum(data) => { let variants: Vec<_> = data .variants .iter() .filter_map(|variant| { - if variant.fields.is_empty() { - let span = variant.span(); - let variant_ident = variant.ident.clone(); - Some(quote_spanned! { span => Self::#variant_ident => ::std::option::Option::Some(&Self::#variant_ident), }) - } else { - None + let span = variant.span(); + let variant_ident = variant.ident.clone(); + match &variant.fields { + syn::Fields::Named(fields) if fields.named.is_empty() => { + Some(quote_spanned! { span => Self::#variant_ident{} => ::std::option::Option::Some(&Self::#variant_ident{}), }) + } + syn::Fields::Unnamed(fields) if fields.unnamed.is_empty() => { + // TODO: Simplify using `Self` after rustc 1.72.0 is released. + let ident = input.ident.clone(); + let (_, ty_generics, _) = input.generics.split_for_impl(); + let turbofish = ty_generics.as_turbofish(); + Some(quote_spanned! { span => Self::#variant_ident() => ::std::option::Option::Some(&#ident #turbofish::#variant_ident()), }) + } + syn::Fields::Unit => { + Some(quote_spanned! { span => Self::#variant_ident => ::std::option::Option::Some(&Self::#variant_ident), }) + } + _ => None } }) .collect(); From 8efc7a17ce8f341b0c75fb86113862c9ab515854 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 5 Aug 2023 19:09:48 +0200 Subject: [PATCH 36/53] Fix formating --- crates/bevy_macro_utils/src/static_ref.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_macro_utils/src/static_ref.rs b/crates/bevy_macro_utils/src/static_ref.rs index a717801bc6000..20fbb18ab4fca 100644 --- a/crates/bevy_macro_utils/src/static_ref.rs +++ b/crates/bevy_macro_utils/src/static_ref.rs @@ -22,7 +22,7 @@ pub fn static_ref_impl(input: &syn::DeriveInput) -> TokenStream { .iter() .filter_map(|variant| { let span = variant.span(); - let variant_ident = variant.ident.clone(); + let variant_ident = variant.ident.clone(); match &variant.fields { syn::Fields::Named(fields) if fields.named.is_empty() => { Some(quote_spanned! { span => Self::#variant_ident{} => ::std::option::Option::Some(&Self::#variant_ident{}), }) From a501af7fc0251b8c5df823fe5e42fee6b2f041fa Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 5 Aug 2023 19:26:59 +0200 Subject: [PATCH 37/53] Simplify `static_ref_impl` again --- crates/bevy_macro_utils/src/static_ref.rs | 32 ++++++----------------- 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/crates/bevy_macro_utils/src/static_ref.rs b/crates/bevy_macro_utils/src/static_ref.rs index 20fbb18ab4fca..09ceabb4e55f4 100644 --- a/crates/bevy_macro_utils/src/static_ref.rs +++ b/crates/bevy_macro_utils/src/static_ref.rs @@ -4,18 +4,13 @@ use syn::spanned::Spanned; pub fn static_ref_impl(input: &syn::DeriveInput) -> TokenStream { match &input.data { - syn::Data::Struct(data) => match &data.fields { - syn::Fields::Named(fields) if fields.named.is_empty() => { + syn::Data::Struct(data) => { + if data.fields.is_empty() { quote! { ::std::option::Option::Some(&Self{}) } - } - syn::Fields::Unnamed(fields) if fields.unnamed.is_empty() => { - quote! { ::std::option::Option::Some(&Self()) } - } - syn::Fields::Unit => quote! { ::std::option::Option::Some(&Self) }, - _ => { + } else { quote! { ::std::option::Option::None } } - }, + } syn::Data::Enum(data) => { let variants: Vec<_> = data .variants @@ -23,21 +18,10 @@ pub fn static_ref_impl(input: &syn::DeriveInput) -> TokenStream { .filter_map(|variant| { let span = variant.span(); let variant_ident = variant.ident.clone(); - match &variant.fields { - syn::Fields::Named(fields) if fields.named.is_empty() => { - Some(quote_spanned! { span => Self::#variant_ident{} => ::std::option::Option::Some(&Self::#variant_ident{}), }) - } - syn::Fields::Unnamed(fields) if fields.unnamed.is_empty() => { - // TODO: Simplify using `Self` after rustc 1.72.0 is released. - let ident = input.ident.clone(); - let (_, ty_generics, _) = input.generics.split_for_impl(); - let turbofish = ty_generics.as_turbofish(); - Some(quote_spanned! { span => Self::#variant_ident() => ::std::option::Option::Some(&#ident #turbofish::#variant_ident()), }) - } - syn::Fields::Unit => { - Some(quote_spanned! { span => Self::#variant_ident => ::std::option::Option::Some(&Self::#variant_ident), }) - } - _ => None + if variant.fields.is_empty() { + Some(quote_spanned! { span => Self::#variant_ident{} => ::std::option::Option::Some(&Self::#variant_ident{}), }) + } else { + None } }) .collect(); From 1d1b8a9d8ae3a5031a7b97c82366f2e58fd21962 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Wed, 9 Aug 2023 12:07:48 +0200 Subject: [PATCH 38/53] Use static items to ensure returning the same static reference --- crates/bevy_macro_utils/src/static_ref.rs | 52 +++++++++++++++++------ 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/crates/bevy_macro_utils/src/static_ref.rs b/crates/bevy_macro_utils/src/static_ref.rs index 09ceabb4e55f4..9864ed5841cb4 100644 --- a/crates/bevy_macro_utils/src/static_ref.rs +++ b/crates/bevy_macro_utils/src/static_ref.rs @@ -1,31 +1,55 @@ use proc_macro2::TokenStream; -use quote::{quote, quote_spanned}; +use quote::{format_ident, quote, quote_spanned}; use syn::spanned::Spanned; pub fn static_ref_impl(input: &syn::DeriveInput) -> TokenStream { + // We cannot support generics correctly since we use static items + if input.generics.const_params().next().is_some() + || input.generics.type_params().next().is_some() + { + return quote! { ::std::option::Option::None }; + } + + let ident = input.ident.clone(); match &input.data { - syn::Data::Struct(data) => { - if data.fields.is_empty() { - quote! { ::std::option::Option::Some(&Self{}) } - } else { - quote! { ::std::option::Option::None } + syn::Data::Struct(data) if data.fields.is_empty() => { + quote! { + static SELF: #ident = #ident{}; + ::std::option::Option::Some(&SELF) } } syn::Data::Enum(data) => { - let variants: Vec<_> = data + let variant_data: Vec<_> = data .variants .iter() - .filter_map(|variant| { + .filter(|variant| variant.fields.is_empty()) + .map(|variant| { let span = variant.span(); let variant_ident = variant.ident.clone(); - if variant.fields.is_empty() { - Some(quote_spanned! { span => Self::#variant_ident{} => ::std::option::Option::Some(&Self::#variant_ident{}), }) - } else { - None - } + let static_ident = format_ident!("SELF_{}", variant_ident); + (span, variant_ident, static_ident) }) .collect(); + + let variant_statics = variant_data + .iter() + .map(|(span, variant_ident, static_ident)| { + quote_spanned! { *span => + #[allow(non_upper_case_globals)] + static #static_ident: #ident = #ident::#variant_ident{}; + } + }); + + let variants = variant_data + .iter() + .map(|(span, variant_ident, static_ident)| { + quote_spanned! { *span => + Self::#variant_ident{} => ::std::option::Option::Some(&#static_ident), + } + }); + quote! { + #(#variant_statics)* match self { #(#variants)* #[allow(unreachable_patterns)] @@ -33,7 +57,7 @@ pub fn static_ref_impl(input: &syn::DeriveInput) -> TokenStream { } } } - syn::Data::Union(_) => { + _ => { quote! { ::std::option::Option::None } } } From 5a43a0ca9182e5b35588e33d9c897848d22db3ff Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sun, 13 Aug 2023 17:00:01 +0200 Subject: [PATCH 39/53] Unify `derive_set` and `derive_label`, reexport and use `DynEq` --- crates/bevy_app/src/app.rs | 2 + crates/bevy_derive/src/lib.rs | 4 +- crates/bevy_ecs/macros/src/lib.rs | 11 ++- crates/bevy_ecs/macros/src/set.rs | 57 ----------- crates/bevy_ecs/src/schedule/set.rs | 147 +++++----------------------- crates/bevy_macro_utils/src/lib.rs | 18 ++-- crates/bevy_utils/src/label.rs | 31 ++++++ 7 files changed, 82 insertions(+), 188 deletions(-) delete mode 100644 crates/bevy_ecs/macros/src/set.rs diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index d90a22f2e78df..f5c7707163e4d 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -23,6 +23,8 @@ bevy_utils::define_label!( APP_LABEL_INTERNER ); +pub use bevy_utils::label::DynEq; + /// A shorthand for `Interned`. pub type InternedAppLabel = Interned; diff --git a/crates/bevy_derive/src/lib.rs b/crates/bevy_derive/src/lib.rs index 0dbbff57971f3..b6766c0dc8959 100644 --- a/crates/bevy_derive/src/lib.rs +++ b/crates/bevy_derive/src/lib.rs @@ -206,6 +206,8 @@ pub fn derive_enum_variant_meta(input: TokenStream) -> TokenStream { pub fn derive_app_label(input: TokenStream) -> TokenStream { let input = syn::parse_macro_input!(input as syn::DeriveInput); let mut trait_path = BevyManifest::default().get_path("bevy_app"); + let mut dyn_eq_path = trait_path.clone(); trait_path.segments.push(format_ident!("AppLabel").into()); - derive_label(input, &trait_path) + dyn_eq_path.segments.push(format_ident!("DynEq").into()); + derive_label(input, "AppLabel", &trait_path, &dyn_eq_path) } diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 32580a1cd778d..3b3fc8e0e22c1 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -2,10 +2,9 @@ extern crate proc_macro; mod component; mod fetch; -mod set; mod states; -use crate::{fetch::derive_world_query_impl, set::derive_set}; +use crate::fetch::derive_world_query_impl; use bevy_macro_utils::{derive_label, ensure_no_collision, get_named_struct_fields, BevyManifest}; use proc_macro::TokenStream; use proc_macro2::Span; @@ -439,10 +438,12 @@ pub fn derive_schedule_label(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); let mut trait_path = bevy_ecs_path(); trait_path.segments.push(format_ident!("schedule").into()); + let mut dyn_eq_path = trait_path.clone(); trait_path .segments .push(format_ident!("ScheduleLabel").into()); - derive_label(input, &trait_path) + dyn_eq_path.segments.push(format_ident!("DynEq").into()); + derive_label(input, "ScheduleName", &trait_path, &dyn_eq_path) } /// Derive macro generating an impl of the trait `SystemSet`. @@ -453,8 +454,10 @@ pub fn derive_system_set(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); let mut trait_path = bevy_ecs_path(); trait_path.segments.push(format_ident!("schedule").into()); + let mut dyn_eq_path = trait_path.clone(); trait_path.segments.push(format_ident!("SystemSet").into()); - derive_set(input, &trait_path) + dyn_eq_path.segments.push(format_ident!("DynEq").into()); + derive_label(input, "SystemSet", &trait_path, &dyn_eq_path) } pub(crate) fn bevy_ecs_path() -> syn::Path { diff --git a/crates/bevy_ecs/macros/src/set.rs b/crates/bevy_ecs/macros/src/set.rs deleted file mode 100644 index 336154f590e93..0000000000000 --- a/crates/bevy_ecs/macros/src/set.rs +++ /dev/null @@ -1,57 +0,0 @@ -use bevy_macro_utils::{static_ref_impl, BevyManifest}; -use proc_macro::TokenStream; -use quote::{quote, quote_spanned}; -use syn::spanned::Spanned; - -/// Derive a set trait -/// -/// # Args -/// -/// - `input`: The [`syn::DeriveInput`] for the struct that we want to derive the set trait for -/// - `trait_path`: The [`syn::Path`] to the set trait -pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStream { - if let syn::Data::Union(_) = &input.data { - return quote_spanned! { - input.span() => compile_error!("Unions cannot be used as sets."); - } - .into(); - } - - let bevy_utils_path = BevyManifest::default().get_path("bevy_utils"); - - let ident = input.ident.clone(); - - let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); - let mut where_clause = where_clause.cloned().unwrap_or_else(|| syn::WhereClause { - where_token: Default::default(), - predicates: Default::default(), - }); - where_clause.predicates.push( - syn::parse2(quote! { - Self: 'static + Send + Sync + Clone + Eq + ::std::fmt::Debug + ::std::hash::Hash - }) - .unwrap(), - ); - let dyn_static_ref_impl = static_ref_impl(&input); - (quote! { - impl #impl_generics #trait_path for #ident #ty_generics #where_clause { - fn dyn_clone(&self) -> ::std::boxed::Box { - ::std::boxed::Box::new(::std::clone::Clone::clone(self)) - } - - fn as_dyn_eq(&self) -> &dyn #bevy_utils_path::label::DynEq { - self - } - - fn dyn_hash(&self, mut state: &mut dyn ::std::hash::Hasher) { - ::std::hash::Hash::hash(&::std::any::TypeId::of::(), &mut state); - ::std::hash::Hash::hash(self, &mut state); - } - - fn dyn_static_ref(&self) -> ::std::option::Option<&'static dyn #trait_path> { - #dyn_static_ref_impl - } - } - }) - .into() -} diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index e35d9233675a0..df3e225f7d916 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -5,8 +5,8 @@ use std::marker::PhantomData; pub use bevy_ecs_macros::{ScheduleLabel, SystemSet}; use bevy_utils::define_label; -use bevy_utils::intern::{Interned, Interner, Leak}; -use bevy_utils::label::DynEq; +use bevy_utils::intern::Interned; +pub use bevy_utils::label::DynEq; use crate::system::{ ExclusiveSystemParamFunction, IsExclusiveFunctionSystem, IsFunctionSystem, SystemParamFunction, @@ -18,129 +18,36 @@ define_label!( SCHEDULE_LABEL_INTERNER ); -static SYSTEM_SET_INTERNER: Interner = Interner::new(); -/// A shorthand for `Interned`. -pub type InternedSystemSet = Interned; -/// A shorthand for `Interned`. -pub type InternedScheduleLabel = Interned; - -/// Types that identify logical groups of systems. -pub trait SystemSet: Debug + Send + Sync + 'static { - /// Returns `Some` if this system set is a [`SystemTypeSet`]. - fn system_type(&self) -> Option { - None - } - - /// Returns `true` if this system set is an [`AnonymousSet`]. - fn is_anonymous(&self) -> bool { - false - } - - /// Creates a boxed clone of the label corresponding to this system set. - fn dyn_clone(&self) -> Box; - - /// Casts this value to a form where it can be compared with other type-erased values. - fn as_dyn_eq(&self) -> &dyn DynEq; - - /// Feeds this value into the given [`Hasher`]. - fn dyn_hash(&self, state: &mut dyn Hasher); - - /// Returns a static reference to a value equal to `self`, if possible. - /// This method is used to optimize [interning](bevy_utils::intern). - /// - /// # Invariant - /// - /// The following invariants most hold: - /// - /// `ptr_eq(a.dyn_static_ref(), b.dyn_static_ref())` if `a.dyn_eq(b)` - /// `ptr_neq(a.dyn_static_ref(), b.dyn_static_ref())` if `!a.dyn_eq(b)` - /// - /// where `ptr_eq` and `ptr_neq` are defined as : - /// ``` - /// fn ptr_eq(x: Option<&'static T>, y: Option<&'static T>) -> bool { - /// match (x, y) { - /// (Some(x), Some(y)) => std::ptr::eq(x, y), - /// (None, None) => true, - /// _ => false, - /// } - /// } - /// - /// fn ptr_neq(x: Option<&'static T>, y: Option<&'static T>) -> bool { - /// match (x, y) { - /// (Some(x), Some(y)) => !std::ptr::eq(x, y), - /// (None, None) => true, - /// _ => false, - /// } - /// } - /// ``` - /// - /// The provided implementation always returns `None`. - fn dyn_static_ref(&self) -> Option<&'static dyn SystemSet> { - None - } - - /// Returns an [`InternedSystemSet`] corresponding to `self`. - fn intern(&self) -> InternedSystemSet - where - Self: Sized, +define_label!( + /// Types that identify logical groups of systems. + SystemSet, + SYSTEM_SET_INTERNER, { - SYSTEM_SET_INTERNER.intern(self) - } -} - -impl SystemSet for InternedSystemSet { - fn system_type(&self) -> Option { - (**self).system_type() - } - - fn is_anonymous(&self) -> bool { - (**self).is_anonymous() - } - - fn dyn_clone(&self) -> Box { - (**self).dyn_clone() - } - - fn as_dyn_eq(&self) -> &dyn DynEq { - (**self).as_dyn_eq() - } - - fn dyn_hash(&self, state: &mut dyn Hasher) { - (**self).dyn_hash(state); - } - - fn dyn_static_ref(&self) -> Option<&'static dyn SystemSet> { - Some(self.0) - } - - fn intern(&self) -> Self { - *self - } -} - -impl PartialEq for dyn SystemSet { - fn eq(&self, other: &Self) -> bool { - self.as_dyn_eq().dyn_eq(other.as_dyn_eq()) - } -} - -impl Eq for dyn SystemSet {} + /// Returns `Some` if this system set is a [`SystemTypeSet`]. + fn system_type(&self) -> Option { + None + } -impl Hash for dyn SystemSet { - fn hash(&self, state: &mut H) { - self.dyn_hash(state); - } -} + /// Returns `true` if this system set is an [`AnonymousSet`]. + fn is_anonymous(&self) -> bool { + false + } + }, + { + fn system_type(&self) -> Option { + (**self).system_type() + } -impl Leak for dyn SystemSet { - fn leak(&self) -> &'static Self { - Box::leak(self.dyn_clone()) + fn is_anonymous(&self) -> bool { + (**self).is_anonymous() + } } +); - fn static_ref(&self) -> Option<&'static dyn SystemSet> { - self.dyn_static_ref() - } -} +/// A shorthand for `Interned`. +pub type InternedSystemSet = Interned; +/// A shorthand for `Interned`. +pub type InternedScheduleLabel = Interned; /// A [`SystemSet`] grouping instances of the same function. /// diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index 1b2fbd2c65e5f..d87a9cf891822 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -170,17 +170,23 @@ pub fn ensure_no_collision(value: Ident, haystack: TokenStream) -> Ident { /// # Args /// /// - `input`: The [`syn::DeriveInput`] for struct that is deriving the label trait -/// - `trait_path`: The path [`syn::Path`] to the label trait -pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStream { +/// - `trait_name`: Name of the label trait +/// - `trait_path`: The [path](`syn::Path`) to the label trait +/// - `dyn_eq_path`: The [path](`syn::Path`) to the `DynEq` trait +pub fn derive_label( + input: syn::DeriveInput, + trait_name: &str, + trait_path: &syn::Path, + dyn_eq_path: &syn::Path, +) -> TokenStream { if let syn::Data::Union(_) = &input.data { + let message = format!("Cannot derive {trait_name} for unions."); return quote_spanned! { - input.span() => compile_error!("Unions cannot be used as labels."); + input.span() => compile_error!(#message); } .into(); } - let bevy_utils_path = BevyManifest::default().get_path("bevy_utils"); - let ident = input.ident.clone(); let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); let mut where_clause = where_clause.cloned().unwrap_or_else(|| syn::WhereClause { @@ -200,7 +206,7 @@ pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStr ::std::boxed::Box::new(::std::clone::Clone::clone(self)) } - fn as_dyn_eq(&self) -> &dyn #bevy_utils_path::label::DynEq { + fn as_dyn_eq(&self) -> &dyn #dyn_eq_path { self } diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index 0ee7dc034d549..d19a28ea1cd43 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -71,6 +71,22 @@ where /// MyNewLabelTrait, /// MY_NEW_LABEL_TRAIT_INTERNER /// ); +/// +/// define_label!( +/// /// Documentation of another label trait +/// MyNewExtendedLabelTrait, +/// MY_NEW_EXTENDED_LABEL_TRAIT_INTERNER, +/// { +/// // Extra methods for the trait can be defined here +/// fn additional_method(&self) -> i32; +/// }, +/// { +/// // Implementation of the extra methods for Interned +/// fn additional_method(&self) -> i32 { +/// 0 +/// } +/// } +/// ); /// ``` #[macro_export] macro_rules! define_label { @@ -79,9 +95,21 @@ macro_rules! define_label { $label_trait_name:ident, $interner_name:ident ) => { + $crate::define_label!($(#[$label_attr])* $label_trait_name, $interner_name, {}, {}); + }; + ( + $(#[$label_attr:meta])* + $label_trait_name:ident, + $interner_name:ident, + { $($trait_extra_methods:tt)* }, + { $($interned_extra_methods_impl:tt)* } + ) => { $(#[$label_attr])* pub trait $label_trait_name: 'static + Send + Sync + ::std::fmt::Debug { + + $($trait_extra_methods)* + /// Clones this ` #[doc = stringify!($label_trait_name)] ///`. @@ -137,6 +165,9 @@ macro_rules! define_label { } impl $label_trait_name for $crate::intern::Interned { + + $($interned_extra_methods_impl)* + fn dyn_clone(&self) -> ::std::boxed::Box { (**self).dyn_clone() } From 65a54aaa0618efb50edf4c2d2d5478487cce010f Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sun, 13 Aug 2023 17:07:56 +0200 Subject: [PATCH 40/53] Remove obsolete import of `bevy_utils` --- crates/bevy_transform/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/bevy_transform/Cargo.toml b/crates/bevy_transform/Cargo.toml index 2de8b9a194e9a..9f250745232c5 100644 --- a/crates/bevy_transform/Cargo.toml +++ b/crates/bevy_transform/Cargo.toml @@ -15,7 +15,6 @@ bevy_ecs = { path = "../bevy_ecs", version = "0.12.0-dev", features = ["bevy_ref bevy_hierarchy = { path = "../bevy_hierarchy", version = "0.12.0-dev" } bevy_math = { path = "../bevy_math", version = "0.12.0-dev" } bevy_reflect = { path = "../bevy_reflect", version = "0.12.0-dev", features = ["bevy"] } -bevy_utils = { path = "../bevy_utils", version = "0.12.0-dev" } serde = { version = "1", features = ["derive"], optional = true } [dev-dependencies] From 99f7cfc6fa77da093e70f913eab703f3a7df647d Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 26 Aug 2023 22:35:18 +0200 Subject: [PATCH 41/53] Add names to blocks for extra methods Co-authored-by: Joseph <21144246+JoJoJet@users.noreply.github.com> --- crates/bevy_ecs/src/schedule/set.rs | 4 ++-- crates/bevy_utils/src/label.rs | 12 +++++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index df3e225f7d916..f41d7d6a170f9 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -22,7 +22,7 @@ define_label!( /// Types that identify logical groups of systems. SystemSet, SYSTEM_SET_INTERNER, - { + extra_methods: { /// Returns `Some` if this system set is a [`SystemTypeSet`]. fn system_type(&self) -> Option { None @@ -33,7 +33,7 @@ define_label!( false } }, - { + extra_methods_impl: { fn system_type(&self) -> Option { (**self).system_type() } diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index d19a28ea1cd43..0161c9a74ba4b 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -95,14 +95,20 @@ macro_rules! define_label { $label_trait_name:ident, $interner_name:ident ) => { - $crate::define_label!($(#[$label_attr])* $label_trait_name, $interner_name, {}, {}); + $crate::define_label!( + $(#[$label_attr])* + $label_trait_name, + $interner_name, + extra_methods: {}, + extra_methods_impl: {} + ); }; ( $(#[$label_attr:meta])* $label_trait_name:ident, $interner_name:ident, - { $($trait_extra_methods:tt)* }, - { $($interned_extra_methods_impl:tt)* } + extra_methods: { $($trait_extra_methods:tt)* }, + extra_methods_impl: { $($interned_extra_methods_impl:tt)* } ) => { $(#[$label_attr])* From 4286b3c7532d764327760ba61f001ab4c31f9819 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 26 Aug 2023 22:45:09 +0200 Subject: [PATCH 42/53] Fix clippy warning --- crates/bevy_utils/src/intern.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index c06f26ed69b25..fe8574b79109a 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -52,7 +52,7 @@ impl Deref for Interned { impl Clone for Interned { fn clone(&self) -> Self { - Self(self.0) + *self } } From ba6d13501c3d450d563117d23577d29fb5f6e7f9 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 26 Aug 2023 23:54:06 +0200 Subject: [PATCH 43/53] Fix docs --- crates/bevy_utils/src/label.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index 0161c9a74ba4b..41c94d43ad110 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -76,11 +76,11 @@ where /// /// Documentation of another label trait /// MyNewExtendedLabelTrait, /// MY_NEW_EXTENDED_LABEL_TRAIT_INTERNER, -/// { +/// extra_methods: { /// // Extra methods for the trait can be defined here /// fn additional_method(&self) -> i32; /// }, -/// { +/// extra_methods_impl: { /// // Implementation of the extra methods for Interned /// fn additional_method(&self) -> i32 { /// 0 From 83c2b8bec261a4f0558f8ac591dee6bf6a94aabe Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sun, 1 Oct 2023 11:01:51 +0200 Subject: [PATCH 44/53] Fix errors after merging --- crates/bevy_ecs/src/schedule/config.rs | 8 +------- crates/bevy_ecs/src/schedule/mod.rs | 4 ++-- crates/bevy_ecs/src/schedule/schedule.rs | 18 +++++++++--------- crates/bevy_ecs/src/system/adapter_system.rs | 4 ++-- crates/bevy_render/src/lib.rs | 2 +- 5 files changed, 15 insertions(+), 21 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/config.rs b/crates/bevy_ecs/src/schedule/config.rs index f0c9acc7fd001..1c60f5b3da700 100644 --- a/crates/bevy_ecs/src/schedule/config.rs +++ b/crates/bevy_ecs/src/schedule/config.rs @@ -530,13 +530,7 @@ impl IntoSystemSetConfigs for SystemSetConfigs { impl IntoSystemSetConfigs for S { fn into_configs(self) -> SystemSetConfigs { - SystemSetConfigs::NodeConfig(SystemSetConfig::new(Box::new(self))) - } -} - -impl IntoSystemSetConfigs for BoxedSystemSet { - fn into_configs(self) -> SystemSetConfigs { - SystemSetConfigs::NodeConfig(SystemSetConfig::new(self)) + SystemSetConfigs::NodeConfig(SystemSetConfig::new(self.intern())) } } diff --git a/crates/bevy_ecs/src/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs index 1d58a7d59fe72..d6ec9657c209f 100644 --- a/crates/bevy_ecs/src/schedule/mod.rs +++ b/crates/bevy_ecs/src/schedule/mod.rs @@ -1010,7 +1010,7 @@ mod tests { schedule.graph_mut().initialize(&mut world); let _ = schedule .graph_mut() - .build_schedule(world.components(), &TestSchedule.dyn_clone()); + .build_schedule(world.components(), TestSchedule.intern()); let ambiguities: Vec<_> = schedule .graph() @@ -1062,7 +1062,7 @@ mod tests { schedule.graph_mut().initialize(&mut world); let _ = schedule .graph_mut() - .build_schedule(world.components(), &TestSchedule.dyn_clone()); + .build_schedule(world.components(), TestSchedule.intern()); let ambiguities: Vec<_> = schedule .graph() diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index 67455db2bce47..567bc97daaaed 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -157,7 +157,7 @@ fn make_executor(kind: ExecutorKind) -> Box { /// } /// ``` pub struct Schedule { - name: BoxedScheduleLabel, + name: InternedScheduleLabel, graph: ScheduleGraph, executable: SystemSchedule, executor: Box, @@ -181,7 +181,7 @@ impl Schedule { /// Constructs an empty `Schedule`. pub fn new(label: impl ScheduleLabel) -> Self { Self { - name: label.dyn_clone(), + name: label.intern(), graph: ScheduleGraph::new(), executable: SystemSchedule::new(), executor: make_executor(ExecutorKind::default()), @@ -262,7 +262,7 @@ impl Schedule { if self.graph.changed { self.graph.initialize(world); self.graph - .update_schedule(&mut self.executable, world.components(), &self.name)?; + .update_schedule(&mut self.executable, world.components(), self.name)?; self.graph.changed = false; self.executor_initialized = false; } @@ -564,7 +564,7 @@ impl ScheduleGraph { for config in &mut configs { config.in_set_inner(set.intern()); } - let mut set_config = SystemSetConfig::new(set.dyn_clone()); + let mut set_config = SystemSetConfig::new(set.intern()); set_config.conditions.extend(collective_conditions); self.configure_set_inner(set_config).unwrap(); } else { @@ -877,7 +877,7 @@ impl ScheduleGraph { pub fn build_schedule( &mut self, components: &Components, - schedule_label: &BoxedScheduleLabel, + schedule_label: InternedScheduleLabel, ) -> Result { // check hierarchy for cycles self.hierarchy.topsort = @@ -1178,7 +1178,7 @@ impl ScheduleGraph { &mut self, schedule: &mut SystemSchedule, components: &Components, - schedule_label: &BoxedScheduleLabel, + schedule_label: InternedScheduleLabel, ) -> Result<(), ScheduleBuildError> { if !self.uninit.is_empty() { return Err(ScheduleBuildError::Uninitialized); @@ -1245,7 +1245,7 @@ impl ProcessNodeConfig for BoxedSystem { } } -impl ProcessNodeConfig for BoxedSystemSet { +impl ProcessNodeConfig for InternedSystemSet { fn process_config(schedule_graph: &mut ScheduleGraph, config: NodeConfig) -> NodeId { schedule_graph.configure_set_inner(config).unwrap() } @@ -1321,7 +1321,7 @@ impl ScheduleGraph { fn optionally_check_hierarchy_conflicts( &self, transitive_edges: &[(NodeId, NodeId)], - schedule_label: &BoxedScheduleLabel, + schedule_label: InternedScheduleLabel, ) -> Result<(), ScheduleBuildError> { if self.settings.hierarchy_detection == LogLevel::Ignore || transitive_edges.is_empty() { return Ok(()); @@ -1533,7 +1533,7 @@ impl ScheduleGraph { &self, conflicts: &[(NodeId, NodeId, Vec)], components: &Components, - schedule_label: &BoxedScheduleLabel, + schedule_label: InternedScheduleLabel, ) -> Result<(), ScheduleBuildError> { if self.settings.ambiguity_detection == LogLevel::Ignore || conflicts.is_empty() { return Ok(()); diff --git a/crates/bevy_ecs/src/system/adapter_system.rs b/crates/bevy_ecs/src/system/adapter_system.rs index 288e91138868b..228b5ddca2700 100644 --- a/crates/bevy_ecs/src/system/adapter_system.rs +++ b/crates/bevy_ecs/src/system/adapter_system.rs @@ -1,7 +1,7 @@ use std::borrow::Cow; use super::{ReadOnlySystem, System}; -use crate::world::unsafe_world_cell::UnsafeWorldCell; +use crate::{schedule::InternedSystemSet, world::unsafe_world_cell::UnsafeWorldCell}; /// Customizes the behavior of an [`AdapterSystem`] /// @@ -143,7 +143,7 @@ where self.system.set_last_run(last_run); } - fn default_system_sets(&self) -> Vec> { + fn default_system_sets(&self) -> Vec { self.system.default_system_sets() } } diff --git a/crates/bevy_render/src/lib.rs b/crates/bevy_render/src/lib.rs index 57c4ecc28e1bb..e99fc69debc40 100644 --- a/crates/bevy_render/src/lib.rs +++ b/crates/bevy_render/src/lib.rs @@ -405,7 +405,7 @@ unsafe fn initialize_render_app(app: &mut App) { app.init_resource::(); let mut render_app = App::empty(); - render_app.main_schedule_label = Box::new(Render); + render_app.main_schedule_label = Render.intern(); let mut extract_schedule = Schedule::new(ExtractSchedule); extract_schedule.set_apply_final_deferred(false); From 563e392617f9c74e57142c3ecb68587611bbc705 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sun, 1 Oct 2023 11:19:16 +0200 Subject: [PATCH 45/53] Remove `static_ref` optimization --- crates/bevy_app/src/app.rs | 9 ---- crates/bevy_ecs/src/schedule/set.rs | 22 -------- crates/bevy_macro_utils/src/lib.rs | 7 --- crates/bevy_macro_utils/src/static_ref.rs | 64 ----------------------- crates/bevy_utils/src/intern.rs | 50 ------------------ crates/bevy_utils/src/label.rs | 42 --------------- 6 files changed, 194 deletions(-) delete mode 100644 crates/bevy_macro_utils/src/static_ref.rs diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 31020145debbe..298c4fb0ff208 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -1025,8 +1025,6 @@ mod tests { a: u32, b: u32, }, - EmptyTuple(), - EmptyStruct {}, } #[derive(AppLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] @@ -1096,12 +1094,5 @@ mod tests { GenericLabel::(PhantomData).intern(), GenericLabel::(PhantomData).intern() ); - - assert!(UnitLabel.dyn_static_ref().is_some()); - assert!(EmptyTupleLabel().dyn_static_ref().is_some()); - assert!(EmptyStructLabel {}.dyn_static_ref().is_some()); - assert!(EnumLabel::Unit.dyn_static_ref().is_some()); - assert!(EnumLabel::EmptyTuple().dyn_static_ref().is_some()); - assert!(EnumLabel::EmptyStruct {}.dyn_static_ref().is_some()); } } diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index c7432ea283469..e1828676a80e1 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -112,10 +112,6 @@ impl SystemSet for SystemTypeSet { std::any::TypeId::of::().hash(&mut state); self.hash(&mut state); } - - fn dyn_static_ref(&self) -> Option<&'static dyn SystemSet> { - Some(&Self(PhantomData)) - } } /// A [`SystemSet`] implicitly created when using @@ -261,8 +257,6 @@ mod tests { a: u32, b: u32, }, - EmptyTuple(), - EmptyStruct {}, } #[derive(ScheduleLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] @@ -332,13 +326,6 @@ mod tests { GenericLabel::(PhantomData).intern(), GenericLabel::(PhantomData).intern() ); - - assert!(UnitLabel.dyn_static_ref().is_some()); - assert!(EmptyTupleLabel().dyn_static_ref().is_some()); - assert!(EmptyStructLabel {}.dyn_static_ref().is_some()); - assert!(EnumLabel::Unit.dyn_static_ref().is_some()); - assert!(EnumLabel::EmptyTuple().dyn_static_ref().is_some()); - assert!(EnumLabel::EmptyStruct {}.dyn_static_ref().is_some()); } #[test] @@ -372,8 +359,6 @@ mod tests { a: u32, b: u32, }, - EmptyTuple(), - EmptyStruct {}, } #[derive(SystemSet, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] @@ -434,12 +419,5 @@ mod tests { GenericSet::(PhantomData).intern(), GenericSet::(PhantomData).intern() ); - - assert!(UnitSet.dyn_static_ref().is_some()); - assert!(EmptyTupleSet().dyn_static_ref().is_some()); - assert!(EmptyStructSet {}.dyn_static_ref().is_some()); - assert!(EnumSet::Unit.dyn_static_ref().is_some()); - assert!(EnumSet::EmptyTuple().dyn_static_ref().is_some()); - assert!(EnumSet::EmptyStruct {}.dyn_static_ref().is_some()); } } diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index d87a9cf891822..5e4b0b14ae7cc 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -4,12 +4,10 @@ extern crate proc_macro; mod attrs; mod shape; -mod static_ref; mod symbol; pub use attrs::*; pub use shape::*; -pub use static_ref::*; pub use symbol::*; use proc_macro::{TokenStream, TokenTree}; @@ -199,7 +197,6 @@ pub fn derive_label( }) .unwrap(), ); - let dyn_static_ref_impl = static_ref_impl(&input); (quote! { impl #impl_generics #trait_path for #ident #ty_generics #where_clause { fn dyn_clone(&self) -> ::std::boxed::Box { @@ -215,10 +212,6 @@ pub fn derive_label( ::std::hash::Hash::hash(&ty_id, &mut state); ::std::hash::Hash::hash(self, &mut state); } - - fn dyn_static_ref(&self) -> ::std::option::Option<&'static dyn #trait_path> { - #dyn_static_ref_impl - } } }) .into() diff --git a/crates/bevy_macro_utils/src/static_ref.rs b/crates/bevy_macro_utils/src/static_ref.rs deleted file mode 100644 index 9864ed5841cb4..0000000000000 --- a/crates/bevy_macro_utils/src/static_ref.rs +++ /dev/null @@ -1,64 +0,0 @@ -use proc_macro2::TokenStream; -use quote::{format_ident, quote, quote_spanned}; -use syn::spanned::Spanned; - -pub fn static_ref_impl(input: &syn::DeriveInput) -> TokenStream { - // We cannot support generics correctly since we use static items - if input.generics.const_params().next().is_some() - || input.generics.type_params().next().is_some() - { - return quote! { ::std::option::Option::None }; - } - - let ident = input.ident.clone(); - match &input.data { - syn::Data::Struct(data) if data.fields.is_empty() => { - quote! { - static SELF: #ident = #ident{}; - ::std::option::Option::Some(&SELF) - } - } - syn::Data::Enum(data) => { - let variant_data: Vec<_> = data - .variants - .iter() - .filter(|variant| variant.fields.is_empty()) - .map(|variant| { - let span = variant.span(); - let variant_ident = variant.ident.clone(); - let static_ident = format_ident!("SELF_{}", variant_ident); - (span, variant_ident, static_ident) - }) - .collect(); - - let variant_statics = variant_data - .iter() - .map(|(span, variant_ident, static_ident)| { - quote_spanned! { *span => - #[allow(non_upper_case_globals)] - static #static_ident: #ident = #ident::#variant_ident{}; - } - }); - - let variants = variant_data - .iter() - .map(|(span, variant_ident, static_ident)| { - quote_spanned! { *span => - Self::#variant_ident{} => ::std::option::Option::Some(&#static_ident), - } - }); - - quote! { - #(#variant_statics)* - match self { - #(#variants)* - #[allow(unreachable_patterns)] - _ => ::std::option::Option::None, - } - } - } - _ => { - quote! { ::std::option::Option::None } - } - } -} diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index fe8574b79109a..3d0d6ef9d85e3 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -93,40 +93,6 @@ impl From<&Interned> for Interned { pub trait Leak { /// Creates a static reference to `self`, possibly leaking memory. fn leak(&self) -> &'static Self; - - /// Returns a static reference to a value equal to `self`, if possible. - /// This method is used by [`Interner::intern`] to optimize the interning process. - /// - /// # Invariant - /// - /// The following invariants most hold: - /// - /// `ptr_eq(a.static_ref(), b.static_ref())` if `a == b` - /// `ptr_neq(a.static_ref(), b.static_ref())` if `a != b` - /// - /// where `ptr_eq` and `ptr_neq` are defined as : - /// ``` - /// fn ptr_eq(x: Option<&'static T>, y: Option<&'static T>) -> bool { - /// match (x, y) { - /// (Some(x), Some(y)) => std::ptr::eq(x, y), - /// (None, None) => true, - /// _ => false, - /// } - /// } - /// - /// fn ptr_neq(x: Option<&'static T>, y: Option<&'static T>) -> bool { - /// match (x, y) { - /// (Some(x), Some(y)) => !std::ptr::eq(x, y), - /// (None, None) => true, - /// _ => false, - /// } - /// } - /// ``` - /// - /// The provided implementation always returns `None`. - fn static_ref(&self) -> Option<&'static Self> { - None - } } impl Leak for str { @@ -158,12 +124,7 @@ impl Interner { /// If it is called the first time for `value`, it will possibly leak the value and return an /// [`Interned`] using the obtained static reference. Subsequent calls for the same `value` /// will return [`Interned`] using the same static reference. - /// - /// This uses [`Leak::static_ref`] to short-circuit the interning process. pub fn intern(&self, value: &T) -> Interned { - if let Some(value) = value.static_ref() { - return Interned(value); - } let lock = self.0.get_or_init(Default::default); { let set = lock.read().unwrap_or_else(PoisonError::into_inner); @@ -208,10 +169,6 @@ mod tests { fn leak(&self) -> &'static Self { &A } - - fn static_ref(&self) -> Option<&'static Self> { - Some(&A) - } } let interner = Interner::default(); @@ -235,13 +192,6 @@ mod tests { A::Y => &A::Y, } } - - fn static_ref(&self) -> Option<&'static Self> { - Some(match self { - A::X => &A::X, - A::Y => &A::Y, - }) - } } let interner = Interner::default(); diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index 41c94d43ad110..034466f07c814 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -129,40 +129,6 @@ macro_rules! define_label { /// [`Hasher`]: std::hash::Hasher fn dyn_hash(&self, state: &mut dyn ::std::hash::Hasher); - /// Returns a static reference to a value equal to `self`, if possible. - /// This method is used to optimize [interning](bevy_utils::intern). - /// - /// # Invariant - /// - /// The following invariants most hold: - /// - /// `ptr_eq(a.dyn_static_ref(), b.dyn_static_ref())` if `a.dyn_eq(b)` - /// `ptr_neq(a.dyn_static_ref(), b.dyn_static_ref())` if `!a.dyn_eq(b)` - /// - /// where `ptr_eq` and `ptr_neq` are defined as : - /// ``` - /// fn ptr_eq(x: Option<&'static T>, y: Option<&'static T>) -> bool { - /// match (x, y) { - /// (Some(x), Some(y)) => std::ptr::eq(x, y), - /// (None, None) => true, - /// _ => false, - /// } - /// } - /// - /// fn ptr_neq(x: Option<&'static T>, y: Option<&'static T>) -> bool { - /// match (x, y) { - /// (Some(x), Some(y)) => !std::ptr::eq(x, y), - /// (None, None) => true, - /// _ => false, - /// } - /// } - /// ``` - /// - /// The provided implementation always returns `None`. - fn dyn_static_ref(&self) -> ::std::option::Option<&'static dyn $label_trait_name> { - None - } - /// Returns an [`Interned`](bevy_utils::intern::Interned) value corresponding to `self`. fn intern(&self) -> $crate::intern::Interned where Self: Sized { @@ -187,10 +153,6 @@ macro_rules! define_label { (**self).dyn_hash(state); } - fn dyn_static_ref(&self) -> ::std::option::Option<&'static dyn $label_trait_name> { - Some(self.0) - } - fn intern(&self) -> Self { *self } @@ -214,10 +176,6 @@ macro_rules! define_label { fn leak(&self) -> &'static Self { Box::leak(self.dyn_clone()) } - - fn static_ref(&self) -> ::std::option::Option<&'static dyn $label_trait_name> { - self.dyn_static_ref() - } } static $interner_name: $crate::intern::Interner = From 6a3760f046f8ba0268a85c88e3ea5e22729d798c Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sun, 1 Oct 2023 11:22:07 +0200 Subject: [PATCH 46/53] Add unit distinction test --- crates/bevy_ecs/src/schedule/set.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index e1828676a80e1..1001f20189dcf 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -209,6 +209,9 @@ mod tests { #[derive(ScheduleLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] struct A; + #[derive(ScheduleLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] + struct B; + let mut world = World::new(); let mut schedule = Schedule::new(A); @@ -224,6 +227,8 @@ mod tests { world.insert_resource(Flag(false)); world.run_schedule(interned); assert!(world.resource::().0); + + assert_ne!(A.intern(), B.intern()); } #[test] From 2af81e9471706c026505e1dafff60091255a6c36 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sun, 1 Oct 2023 12:30:55 +0200 Subject: [PATCH 47/53] Remove proc-macro2 dependency (leftover from static_ref optimization) --- crates/bevy_macro_utils/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/bevy_macro_utils/Cargo.toml b/crates/bevy_macro_utils/Cargo.toml index 30a9a2519a360..261e877880652 100644 --- a/crates/bevy_macro_utils/Cargo.toml +++ b/crates/bevy_macro_utils/Cargo.toml @@ -12,5 +12,4 @@ keywords = ["bevy"] toml_edit = "0.19" syn = "2.0" quote = "1.0" -proc-macro2 = "1.0" rustc-hash = "1.0" From 846734aaa97f142b80f96dc55f68a87436f0b4e0 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sun, 1 Oct 2023 13:53:59 +0200 Subject: [PATCH 48/53] Add and use `ref_eq` instead of relying on `std::ptr::eq` --- crates/bevy_utils/src/intern.rs | 62 ++++++++++++++++++++++++--------- crates/bevy_utils/src/label.rs | 16 ++++++++- 2 files changed, 61 insertions(+), 17 deletions(-) diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index 3d0d6ef9d85e3..e38a45b8575fc 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -8,7 +8,6 @@ use std::{ fmt::Debug, hash::Hash, ops::Deref, - ptr, sync::{OnceLock, PoisonError, RwLock}, }; @@ -60,18 +59,18 @@ impl Copy for Interned {} // Two Interned should only be equal if they are clones from the same instance. // Therefore, we only use the pointer to determine equality. -impl PartialEq for Interned { +impl PartialEq for Interned { fn eq(&self, other: &Self) -> bool { - ptr::eq(self.0, other.0) + self.0.ref_eq(other.0) } } -impl Eq for Interned {} +impl Eq for Interned {} // Important: This must be kept in sync with the PartialEq/Eq implementation -impl Hash for Interned { +impl Hash for Interned { fn hash(&self, state: &mut H) { - ptr::hash(self.0, state); + self.0.ref_hash(state) } } @@ -90,16 +89,31 @@ impl From<&Interned> for Interned { /// A trait for leaking data. /// /// This is used by [`Interner`] to create static references for values that are interned. -pub trait Leak { +pub trait Internable { /// Creates a static reference to `self`, possibly leaking memory. fn leak(&self) -> &'static Self; + + /// Returns `true` if the two references point to the same value. + fn ref_eq(&self, other: &Self) -> bool; + + /// Feeds the reference to the hasher. + fn ref_hash(&self, state: &mut H); } -impl Leak for str { +impl Internable for str { fn leak(&self) -> &'static Self { let str = self.to_owned().into_boxed_str(); Box::leak(str) } + + fn ref_eq(&self, other: &Self) -> bool { + self.len() == other.len() && self.as_ptr() == other.as_ptr() + } + + fn ref_hash(&self, state: &mut H) { + self.len().hash(state); + self.as_ptr().hash(state); + } } /// A thread-safe interner which can be used to create [`Interned`] from `&T` @@ -118,7 +132,7 @@ impl Interner { } } -impl Interner { +impl Interner { /// Return the [`Interned`] corresponding to `value`. /// /// If it is called the first time for `value`, it will possibly leak the value and return an @@ -158,17 +172,25 @@ mod tests { hash::{Hash, Hasher}, }; - use crate::intern::{Interned, Interner, Leak}; + use crate::intern::{Internable, Interned, Interner}; #[test] fn zero_sized_type() { #[derive(PartialEq, Eq, Hash, Debug)] pub struct A; - impl Leak for A { + impl Internable for A { fn leak(&self) -> &'static Self { &A } + + fn ref_eq(&self, other: &Self) -> bool { + std::ptr::eq(self, other) + } + + fn ref_hash(&self, state: &mut H) { + std::ptr::hash(self, state) + } } let interner = Interner::default(); @@ -185,13 +207,21 @@ mod tests { Y, } - impl Leak for A { + impl Internable for A { fn leak(&self) -> &'static Self { match self { A::X => &A::X, A::Y => &A::Y, } } + + fn ref_eq(&self, other: &Self) -> bool { + std::ptr::eq(self, other) + } + + fn ref_hash(&self, state: &mut H) { + std::ptr::hash(self, state) + } } let interner = Interner::default(); @@ -238,16 +268,16 @@ mod tests { #[test] fn same_interned_content() { - let a = Interned(Box::leak(Box::new("A"))); - let b = Interned(Box::leak(Box::new("A"))); + let a = Interned::(Box::leak(Box::new("A"))); + let b = Interned::(Box::leak(Box::new("A"))); assert_ne!(a, b); } #[test] fn different_interned_content() { - let a = Interned(Box::leak(Box::new("A"))); - let b = Interned(Box::leak(Box::new("B"))); + let a = Interned::(Box::leak(Box::new("A"))); + let b = Interned::(Box::leak(Box::new("B"))); assert_ne!(a, b); } diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index 034466f07c814..20b63f8850b72 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -172,10 +172,24 @@ macro_rules! define_label { } } - impl $crate::intern::Leak for dyn $label_trait_name { + impl $crate::intern::Internable for dyn $label_trait_name { fn leak(&self) -> &'static Self { Box::leak(self.dyn_clone()) } + + fn ref_eq(&self, other: &Self) -> bool { + if self.as_dyn_eq().type_id() == other.as_dyn_eq().type_id() { + (self as *const Self as *const ()) == (other as *const Self as *const ()) + } else { + false + } + } + + fn ref_hash(&self, state: &mut H) { + use ::std::hash::Hash; + self.as_dyn_eq().type_id().hash(state); + (self as *const Self as *const ()).hash(state); + } } static $interner_name: $crate::intern::Interner = From 8d10df768658afa14e3835f3415404f0f3fe2740 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sun, 1 Oct 2023 14:01:17 +0200 Subject: [PATCH 49/53] Fix missing `;` --- crates/bevy_utils/src/intern.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index e38a45b8575fc..5099d9ff11325 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -70,7 +70,7 @@ impl Eq for Interned {} // Important: This must be kept in sync with the PartialEq/Eq implementation impl Hash for Interned { fn hash(&self, state: &mut H) { - self.0.ref_hash(state) + self.0.ref_hash(state); } } @@ -189,7 +189,7 @@ mod tests { } fn ref_hash(&self, state: &mut H) { - std::ptr::hash(self, state) + std::ptr::hash(self, state); } } @@ -220,7 +220,7 @@ mod tests { } fn ref_hash(&self, state: &mut H) { - std::ptr::hash(self, state) + std::ptr::hash(self, state); } } From 4b1a9fd4c9d3c1f26543842f361d0f991bd85e33 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sun, 1 Oct 2023 14:08:58 +0200 Subject: [PATCH 50/53] Fix test --- crates/bevy_utils/src/intern.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index 5099d9ff11325..15313ac4d19b9 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -268,8 +268,8 @@ mod tests { #[test] fn same_interned_content() { - let a = Interned::(Box::leak(Box::new("A"))); - let b = Interned::(Box::leak(Box::new("A"))); + let a = Interned::(Box::leak(Box::new("A".to_string()))); + let b = Interned::(Box::leak(Box::new("A".to_string()))); assert_ne!(a, b); } From 813dd42342f55e189d6d4e06c51c6af6b722240e Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sun, 1 Oct 2023 14:21:05 +0200 Subject: [PATCH 51/53] Fix doctest --- crates/bevy_utils/src/intern.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index 15313ac4d19b9..94d4207265975 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -29,9 +29,11 @@ use crate::HashSet; /// # use bevy_utils::intern::*; /// #[derive(PartialEq, Eq, Hash, Debug)] /// struct Value(i32); -/// impl Leak for Value { +/// impl Internable for Value { /// // ... /// # fn leak(&self) -> &'static Self { Box::leak(Box::new(Value(self.0))) } +/// # fn ref_eq(&self, other: &Self) -> bool { std::ptr::eq(self, other ) } +/// # fn ref_hash(&self, state: &mut H) { std::ptr::hash(self, state); } /// } /// let interner_1 = Interner::new(); /// let interner_2 = Interner::new(); @@ -276,8 +278,8 @@ mod tests { #[test] fn different_interned_content() { - let a = Interned::(Box::leak(Box::new("A"))); - let b = Interned::(Box::leak(Box::new("B"))); + let a = Interned::("A"); + let b = Interned::("B"); assert_ne!(a, b); } From 83ee17088cc4d396dbfee8ca66241ac5f2aef717 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sun, 1 Oct 2023 14:40:17 +0200 Subject: [PATCH 52/53] Fix doc link --- crates/bevy_utils/src/intern.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index 94d4207265975..15c0b41db39d0 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -88,10 +88,10 @@ impl From<&Interned> for Interned { } } -/// A trait for leaking data. +/// A trait for internable values. /// /// This is used by [`Interner`] to create static references for values that are interned. -pub trait Internable { +pub trait Internable: Hash + Eq { /// Creates a static reference to `self`, possibly leaking memory. fn leak(&self) -> &'static Self; @@ -124,7 +124,7 @@ impl Internable for str { /// /// The implementation ensures that two equal values return two equal [`Interned`] values. /// -/// To use an [`Interner`], `T` must implement [`Leak`], [`Hash`] and [`Eq`]. +/// To use an [`Interner`], `T` must implement [`Internable`]. pub struct Interner(OnceLock>>); impl Interner { @@ -134,7 +134,7 @@ impl Interner { } } -impl Interner { +impl Interner { /// Return the [`Interned`] corresponding to `value`. /// /// If it is called the first time for `value`, it will possibly leak the value and return an From e2acc8b7eceb434d5a8edb6342457f5718c91712 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Wed, 25 Oct 2023 14:21:09 -0700 Subject: [PATCH 53/53] Flip comparison order --- crates/bevy_utils/src/intern.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index 15c0b41db39d0..27473ce471280 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -109,7 +109,7 @@ impl Internable for str { } fn ref_eq(&self, other: &Self) -> bool { - self.len() == other.len() && self.as_ptr() == other.as_ptr() + self.as_ptr() == other.as_ptr() && self.len() == other.len() } fn ref_hash(&self, state: &mut H) {