Make derived SystemParam readonly if possible (#4650)

Required for https://github.com/bevyengine/bevy/pull/4402.

# Objective

- derived `SystemParam` implementations were never `ReadOnlySystemParamFetch`
- We want them to be, e.g. for `EventReader`

## Solution

- If possible, 'forward' the impl of `ReadOnlySystemParamFetch`.
This commit is contained in:
Daniel McNab 2022-05-09 16:09:33 +00:00
parent 4c878ef790
commit 38a940dbbe
4 changed files with 68 additions and 1 deletions

View file

@ -324,7 +324,7 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
}
Ok(())
})
.expect("Invalid 'render_resources' attribute format.");
.expect("Invalid 'system_param' attribute format.");
attributes
}),
@ -420,6 +420,9 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
}
}
}
// Safety: The `ParamState` is `ReadOnlySystemParamFetch`, so this can only read from the `World`
unsafe impl<TSystemParamState: #path::system::SystemParamState + #path::system::ReadOnlySystemParamFetch, #punctuated_generics> #path::system::ReadOnlySystemParamFetch for FetchState <TSystemParamState, #punctuated_generic_idents> #where_clause {}
};
})
}

View file

@ -513,6 +513,8 @@ impl<E: Event> std::iter::Extend<E> for Events<E> {
#[cfg(test)]
mod tests {
use crate::{prelude::World, system::SystemState};
use super::*;
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
@ -753,4 +755,16 @@ mod tests {
vec![EmptyTestEvent::default()]
);
}
#[test]
fn ensure_reader_readonly() {
fn read_for<E: Event>() {
let mut world = World::new();
world.init_resource::<Events<E>>();
let mut state = SystemState::<EventReader<E>>::new(&mut world);
// This can only work if EventReader only reads the world
let _reader = state.get(&world);
}
read_for::<EmptyTestEvent>();
}
}

View file

@ -0,0 +1,25 @@
use bevy_ecs::prelude::*;
use bevy_ecs::system::{ReadOnlySystemParamFetch, SystemParam, SystemState};
#[derive(Component)]
struct Foo;
#[derive(SystemParam)]
struct Mutable<'w, 's> {
a: Query<'w, 's, &'static mut Foo>,
}
fn main() {
// Ideally we'd use:
// let mut world = World::default();
// let state = SystemState::<Mutable>::new(&mut world);
// state.get(&world);
// But that makes the test show absolute paths
assert_readonly::<Mutable>();
}
fn assert_readonly<P: SystemParam>()
where
<P as SystemParam>::Fetch: ReadOnlySystemParamFetch,
{
}

View file

@ -0,0 +1,25 @@
warning: unused import: `SystemState`
--> tests/ui/system_param_derive_readonly.rs:2:63
|
2 | use bevy_ecs::system::{ReadOnlySystemParamFetch, SystemParam, SystemState};
| ^^^^^^^^^^^
|
= note: `#[warn(unused_imports)]` on by default
error[E0277]: the trait bound `for<'x> WriteFetch<'x, Foo>: ReadOnlyFetch` is not satisfied
--> tests/ui/system_param_derive_readonly.rs:18:5
|
18 | assert_readonly::<Mutable>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `for<'x> ReadOnlyFetch` is not implemented for `WriteFetch<'x, Foo>`
|
= note: required because of the requirements on the impl of `ReadOnlySystemParamFetch` for `QueryState<&'static mut Foo>`
= note: 2 redundant requirements hidden
= note: required because of the requirements on the impl of `ReadOnlySystemParamFetch` for `_::FetchState<(QueryState<&'static mut Foo>,)>`
note: required by a bound in `assert_readonly`
--> tests/ui/system_param_derive_readonly.rs:23:32
|
21 | fn assert_readonly<P: SystemParam>()
| --------------- required by a bound in this
22 | where
23 | <P as SystemParam>::Fetch: ReadOnlySystemParamFetch,
| ^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `assert_readonly`