From 382917fbb3731ab4a9937a6eb37c2d6b4645da78 Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Mon, 16 Sep 2024 18:56:57 -0400 Subject: [PATCH] Improve type inference in `DynSystemParam::downcast()` by making the type parameter match the return value. (#15103) # Objective Right now, `DynSystemParam::downcast()` always requires the type parameter to be specified with a turbofish. Make it so that it can be inferred from the use of the return value, like: ```rust fn expects_res_a(mut param: DynSystemParam) { let res: Res = param.downcast().unwrap(); } ``` ## Solution The reason this doesn't currently work is that the type parameter is a `'static` version of the `SystemParam` so that it can be used with `Any::downcast_mut()`. Change the method signature so that the type parameter matches the return type, and use `T::Item<'static, 'static>` to get the `'static` version. That means we wind up returning a `T::Item<'static, 'static>::Item<'w, 's>`, so constrain that to be equal to `T`. That works with every `SystemParam` implementation, since they have `T::Item == T` up to lifetimes. --- crates/bevy_ecs/src/system/system_param.rs | 71 ++++++++++++++++------ 1 file changed, 54 insertions(+), 17 deletions(-) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 6b604c54c2..ac038dff1a 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -1894,7 +1894,9 @@ unsafe impl ReadOnlySystemParam for PhantomData {} /// assert!(param.is::>()); /// assert!(!param.is::>()); /// assert!(param.downcast_mut::>().is_none()); -/// let foo: Res = param.downcast::>().unwrap(); +/// let res = param.downcast_mut::>().unwrap(); +/// // The type parameter can be left out if it can be determined from use. +/// let res: Res = param.downcast().unwrap(); /// } /// /// let system = ( @@ -1942,13 +1944,21 @@ impl<'w, 's> DynSystemParam<'w, 's> { } /// Returns `true` if the inner system param is the same as `T`. - pub fn is(&self) -> bool { - self.state.is::>() + pub fn is(&self) -> bool + // See downcast() function for an explanation of the where clause + where + T::Item<'static, 'static>: SystemParam = T> + 'static, + { + self.state.is::>>() } /// Returns the inner system param if it is the correct type. /// This consumes the dyn param, so the returned param can have its original world and state lifetimes. - pub fn downcast(self) -> Option> { + pub fn downcast(self) -> Option + // See downcast() function for an explanation of the where clause + where + T::Item<'static, 'static>: SystemParam = T> + 'static, + { // SAFETY: // - `DynSystemParam::new()` ensures `state` is a `ParamState`, that the world matches, // and that it has access required by the inner system param. @@ -1958,7 +1968,11 @@ impl<'w, 's> DynSystemParam<'w, 's> { /// Returns the inner system parameter if it is the correct type. /// This borrows the dyn param, so the returned param is only valid for the duration of that borrow. - pub fn downcast_mut(&mut self) -> Option> { + pub fn downcast_mut<'a, T: SystemParam>(&'a mut self) -> Option + // See downcast() function for an explanation of the where clause + where + T::Item<'static, 'static>: SystemParam = T> + 'static, + { // SAFETY: // - `DynSystemParam::new()` ensures `state` is a `ParamState`, that the world matches, // and that it has access required by the inner system param. @@ -1971,9 +1985,11 @@ impl<'w, 's> DynSystemParam<'w, 's> { /// but since it only performs read access it can keep the original world lifetime. /// This can be useful with methods like [`Query::iter_inner()`] or [`Res::into_inner()`] /// to obtain references with the original world lifetime. - pub fn downcast_mut_inner( - &mut self, - ) -> Option> { + pub fn downcast_mut_inner<'a, T: ReadOnlySystemParam>(&'a mut self) -> Option + // See downcast() function for an explanation of the where clause + where + T::Item<'static, 'static>: SystemParam = T> + 'static, + { // SAFETY: // - `DynSystemParam::new()` ensures `state` is a `ParamState`, that the world matches, // and that it has access required by the inner system param. @@ -1988,19 +2004,32 @@ impl<'w, 's> DynSystemParam<'w, 's> { /// in [`init_state`](SystemParam::init_state) for the inner system param. /// - `world` must be the same `World` that was used to initialize /// [`state`](SystemParam::init_state) for the inner system param. -unsafe fn downcast<'w, 's, T: SystemParam + 'static>( +unsafe fn downcast<'w, 's, T: SystemParam>( state: &'s mut dyn Any, system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, change_tick: Tick, -) -> Option> { - state.downcast_mut::>().map(|state| { - // SAFETY: - // - The caller ensures the world has access for the underlying system param, - // and since the downcast succeeded, the underlying system param is T. - // - The caller ensures the `world` matches. - unsafe { T::get_param(&mut state.0, system_meta, world, change_tick) } - }) +) -> Option +// We need a 'static version of the SystemParam to use with `Any::downcast_mut()`, +// and we need a <'w, 's> version to actually return. +// The type parameter T must be the one we return in order to get type inference from the return value. +// So we use `T::Item<'static, 'static>` as the 'static version, and require that it be 'static. +// That means the return value will be T::Item<'static, 'static>::Item<'w, 's>, +// so we constrain that to be equal to T. +// Every actual `SystemParam` implementation has `T::Item == T` up to lifetimes, +// so they should all work with this constraint. +where + T::Item<'static, 'static>: SystemParam = T> + 'static, +{ + state + .downcast_mut::>>() + .map(|state| { + // SAFETY: + // - The caller ensures the world has access for the underlying system param, + // and since the downcast succeeded, the underlying system param is T. + // - The caller ensures the `world` matches. + unsafe { T::Item::get_param(&mut state.0, system_meta, world, change_tick) } + }) } /// The [`SystemParam::State`] for a [`DynSystemParam`]. @@ -2323,4 +2352,12 @@ mod tests { schedule.add_systems((non_send_param_set, non_send_param_set, non_send_param_set)); schedule.run(&mut world); } + + fn _dyn_system_param_type_inference(mut p: DynSystemParam) { + // Make sure the downcast() methods are able to infer their type parameters from the use of the return type. + // This is just a compilation test, so there is nothing to run. + let _query: Query<()> = p.downcast_mut().unwrap(); + let _query: Query<()> = p.downcast_mut_inner().unwrap(); + let _query: Query<()> = p.downcast().unwrap(); + } }