Skip to content

Commit 645a997

Browse files
DJMcNabexjam
authored andcommitted
Make derived SystemParam readonly if possible (bevyengine#4650)
Required for bevyengine#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`.
1 parent 771f8cd commit 645a997

File tree

4 files changed

+68
-1
lines changed

4 files changed

+68
-1
lines changed

crates/bevy_ecs/macros/src/lib.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
324324
}
325325
Ok(())
326326
})
327-
.expect("Invalid 'render_resources' attribute format.");
327+
.expect("Invalid 'system_param' attribute format.");
328328

329329
attributes
330330
}),
@@ -420,6 +420,9 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
420420
}
421421
}
422422
}
423+
424+
// Safety: The `ParamState` is `ReadOnlySystemParamFetch`, so this can only read from the `World`
425+
unsafe impl<TSystemParamState: #path::system::SystemParamState + #path::system::ReadOnlySystemParamFetch, #punctuated_generics> #path::system::ReadOnlySystemParamFetch for FetchState <TSystemParamState, #punctuated_generic_idents> #where_clause {}
423426
};
424427
})
425428
}

crates/bevy_ecs/src/event.rs

+14
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,8 @@ impl<E: Event> std::iter::Extend<E> for Events<E> {
513513

514514
#[cfg(test)]
515515
mod tests {
516+
use crate::{prelude::World, system::SystemState};
517+
516518
use super::*;
517519

518520
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
@@ -753,4 +755,16 @@ mod tests {
753755
vec![EmptyTestEvent::default()]
754756
);
755757
}
758+
759+
#[test]
760+
fn ensure_reader_readonly() {
761+
fn read_for<E: Event>() {
762+
let mut world = World::new();
763+
world.init_resource::<Events<E>>();
764+
let mut state = SystemState::<EventReader<E>>::new(&mut world);
765+
// This can only work if EventReader only reads the world
766+
let _reader = state.get(&world);
767+
}
768+
read_for::<EmptyTestEvent>();
769+
}
756770
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
use bevy_ecs::prelude::*;
2+
use bevy_ecs::system::{ReadOnlySystemParamFetch, SystemParam, SystemState};
3+
4+
#[derive(Component)]
5+
struct Foo;
6+
7+
#[derive(SystemParam)]
8+
struct Mutable<'w, 's> {
9+
a: Query<'w, 's, &'static mut Foo>,
10+
}
11+
12+
fn main() {
13+
// Ideally we'd use:
14+
// let mut world = World::default();
15+
// let state = SystemState::<Mutable>::new(&mut world);
16+
// state.get(&world);
17+
// But that makes the test show absolute paths
18+
assert_readonly::<Mutable>();
19+
}
20+
21+
fn assert_readonly<P: SystemParam>()
22+
where
23+
<P as SystemParam>::Fetch: ReadOnlySystemParamFetch,
24+
{
25+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
warning: unused import: `SystemState`
2+
--> tests/ui/system_param_derive_readonly.rs:2:63
3+
|
4+
2 | use bevy_ecs::system::{ReadOnlySystemParamFetch, SystemParam, SystemState};
5+
| ^^^^^^^^^^^
6+
|
7+
= note: `#[warn(unused_imports)]` on by default
8+
9+
error[E0277]: the trait bound `for<'x> WriteFetch<'x, Foo>: ReadOnlyFetch` is not satisfied
10+
--> tests/ui/system_param_derive_readonly.rs:18:5
11+
|
12+
18 | assert_readonly::<Mutable>();
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `for<'x> ReadOnlyFetch` is not implemented for `WriteFetch<'x, Foo>`
14+
|
15+
= note: required because of the requirements on the impl of `ReadOnlySystemParamFetch` for `QueryState<&'static mut Foo>`
16+
= note: 2 redundant requirements hidden
17+
= note: required because of the requirements on the impl of `ReadOnlySystemParamFetch` for `_::FetchState<(QueryState<&'static mut Foo>,)>`
18+
note: required by a bound in `assert_readonly`
19+
--> tests/ui/system_param_derive_readonly.rs:23:32
20+
|
21+
21 | fn assert_readonly<P: SystemParam>()
22+
| --------------- required by a bound in this
23+
22 | where
24+
23 | <P as SystemParam>::Fetch: ReadOnlySystemParamFetch,
25+
| ^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `assert_readonly`

0 commit comments

Comments
 (0)