From 14ec60e6e778229e66e2c4c4e4adfc89adebb7d5 Mon Sep 17 00:00:00 2001 From: harudagondi Date: Tue, 19 Jul 2022 12:31:00 +0800 Subject: [PATCH 1/4] Add `to_readonly` --- crates/bevy_ecs/src/system/mod.rs | 93 +++++++++++++++++++++++++++++ crates/bevy_ecs/src/system/query.rs | 14 +++++ 2 files changed, 107 insertions(+) diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs index 17bd3b5a1a377..3cef46e8b1372 100644 --- a/crates/bevy_ecs/src/system/mod.rs +++ b/crates/bevy_ecs/src/system/mod.rs @@ -927,6 +927,99 @@ mod tests { } } + #[test] + fn convert_mut_to_immut() { + { + let mut world = World::new(); + + fn mutable_query(mut query: Query<&mut A>) { + for _ in &mut query {} + + immutable_query(query.to_readonly()); + } + + fn immutable_query(_: Query<&A>) {} + + let mut sys = IntoSystem::into_system(mutable_query); + sys.initialize(&mut world); + } + + { + let mut world = World::new(); + + fn mutable_query(mut query: Query<(&mut A, &B)>) { + for _ in &mut query {} + + immutable_query(query.to_readonly()); + } + + fn immutable_query(_: Query<(&A, &B)>) {} + + let mut sys = IntoSystem::into_system(mutable_query); + sys.initialize(&mut world); + } + + { + let mut world = World::new(); + + fn mutable_query(mut query: Query<(&mut A, &mut B)>) { + for _ in &mut query {} + + immutable_query(query.to_readonly()); + } + + fn immutable_query(_: Query<(&A, &B)>) {} + + let mut sys = IntoSystem::into_system(mutable_query); + sys.initialize(&mut world); + } + + { + let mut world = World::new(); + + fn mutable_query(mut query: Query<(&mut A, &mut B), With>) { + for _ in &mut query {} + + immutable_query(query.to_readonly()); + } + + fn immutable_query(_: Query<(&A, &B), With>) {} + + let mut sys = IntoSystem::into_system(mutable_query); + sys.initialize(&mut world); + } + + { + let mut world = World::new(); + + fn mutable_query(mut query: Query<(&mut A, &mut B), Added>) { + for _ in &mut query {} + + immutable_query(query.to_readonly()); + } + + fn immutable_query(_: Query<(&A, &B), Added>) {} + + let mut sys = IntoSystem::into_system(mutable_query); + sys.initialize(&mut world); + } + + { + let mut world = World::new(); + + fn mutable_query(mut query: Query<(&mut A, &mut B), Changed>) { + for _ in &mut query {} + + immutable_query(query.to_readonly()); + } + + fn immutable_query(_: Query<(&A, &B), Changed>) {} + + let mut sys = IntoSystem::into_system(mutable_query); + sys.initialize(&mut world); + } + } + #[test] fn update_archetype_component_access_works() { use std::collections::HashSet; diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 12a7f9b8b8fa0..149c14a3175c2 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -267,6 +267,20 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { } } + /// Convert the query to readonly. + pub fn to_readonly(&self) -> Query<'w, 's, Q::ReadOnly, F::ReadOnly> { + let new_state = self.state.as_readonly(); + // SAFETY: This is memory safe because it turns the query immutable. + unsafe { + Query::new( + self.world, + new_state, + self.last_change_tick, + self.change_tick, + ) + } + } + /// Returns an [`Iterator`] over the query results. /// /// This can only return immutable data (mutable data will be cast to an immutable form). From 200bfc06fcb16e669fc2292b7f9795bcb5dade65 Mon Sep 17 00:00:00 2001 From: harudagondi Date: Tue, 19 Jul 2022 13:00:04 +0800 Subject: [PATCH 2/4] Added more tests --- crates/bevy_ecs/src/system/mod.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs index 3cef46e8b1372..7b368b62ef471 100644 --- a/crates/bevy_ecs/src/system/mod.rs +++ b/crates/bevy_ecs/src/system/mod.rs @@ -944,6 +944,21 @@ mod tests { sys.initialize(&mut world); } + { + let mut world = World::new(); + + fn mutable_query(mut query: Query>) { + for _ in &mut query {} + + immutable_query(query.to_readonly()); + } + + fn immutable_query(_: Query>) {} + + let mut sys = IntoSystem::into_system(mutable_query); + sys.initialize(&mut world); + } + { let mut world = World::new(); @@ -989,6 +1004,21 @@ mod tests { sys.initialize(&mut world); } + { + let mut world = World::new(); + + fn mutable_query(mut query: Query<(&mut A, &mut B), Without>) { + for _ in &mut query {} + + immutable_query(query.to_readonly()); + } + + fn immutable_query(_: Query<(&A, &B), Without>) {} + + let mut sys = IntoSystem::into_system(mutable_query); + sys.initialize(&mut world); + } + { let mut world = World::new(); From 162a2ad01b8b073070416b0b20ab1821a84cde31 Mon Sep 17 00:00:00 2001 From: harudagondi Date: Tue, 19 Jul 2022 18:56:06 +0800 Subject: [PATCH 3/4] Fix lifetimes and add `fail_to_compile` tests --- crates/bevy_ecs/src/system/query.rs | 2 +- .../tests/ui/query_to_readonly.rs | 75 +++++++++++++++++++ .../tests/ui/query_to_readonly.stderr | 45 +++++++++++ 3 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 crates/bevy_ecs_compile_fail_tests/tests/ui/query_to_readonly.rs create mode 100644 crates/bevy_ecs_compile_fail_tests/tests/ui/query_to_readonly.stderr diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 149c14a3175c2..5fc3ddf3583e1 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -268,7 +268,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { } /// Convert the query to readonly. - pub fn to_readonly(&self) -> Query<'w, 's, Q::ReadOnly, F::ReadOnly> { + pub fn to_readonly(&self) -> Query<'_, '_, Q::ReadOnly, F::ReadOnly> { let new_state = self.state.as_readonly(); // SAFETY: This is memory safe because it turns the query immutable. unsafe { diff --git a/crates/bevy_ecs_compile_fail_tests/tests/ui/query_to_readonly.rs b/crates/bevy_ecs_compile_fail_tests/tests/ui/query_to_readonly.rs new file mode 100644 index 0000000000000..060793023339b --- /dev/null +++ b/crates/bevy_ecs_compile_fail_tests/tests/ui/query_to_readonly.rs @@ -0,0 +1,75 @@ +use bevy_ecs::prelude::*; + +#[derive(Component, Debug)] +struct Foo; + +fn for_loops(mut query: Query<&mut Foo>) { + // this should fail to compile + for _ in query.iter_mut() { + for _ in query.to_readonly().iter() {} + } + + // this should fail to compile + for _ in query.to_readonly().iter() { + for _ in query.iter_mut() {} + } + + // this should *not* fail to compile + for _ in query.to_readonly().iter() { + for _ in query.to_readonly().iter() {} + } + + // this should *not* fail to compile + for _ in query.to_readonly().iter() { + for _ in query.iter() {} + } + + // this should *not* fail to compile + for _ in query.iter() { + for _ in query.to_readonly().iter() {} + } +} + +fn single_mut_query(mut query: Query<&mut Foo>) { + // this should fail to compile + { + let mut mut_foo = query.single_mut(); + + // This solves "temporary value dropped while borrowed" + let readonly_query = query.to_readonly(); + + let ref_foo = readonly_query.single(); + + *mut_foo = Foo; + + println!("{ref_foo:?}"); + } + + // this should fail to compile + { + // This solves "temporary value dropped while borrowed" + let readonly_query = query.to_readonly(); + + let ref_foo = readonly_query.single(); + + let mut mut_foo = query.single_mut(); + + println!("{ref_foo:?}"); + + *mut_foo = Foo; + } + + // this should *not* fail to compile + { + // This solves "temporary value dropped while borrowed" + let readonly_query = query.to_readonly(); + + let readonly_foo = readonly_query.single(); + + let query_foo = query.single(); + + println!("{readonly_foo:?}, {query_foo:?}"); + } +} + +fn main() {} \ No newline at end of file diff --git a/crates/bevy_ecs_compile_fail_tests/tests/ui/query_to_readonly.stderr b/crates/bevy_ecs_compile_fail_tests/tests/ui/query_to_readonly.stderr new file mode 100644 index 0000000000000..b57108182203f --- /dev/null +++ b/crates/bevy_ecs_compile_fail_tests/tests/ui/query_to_readonly.stderr @@ -0,0 +1,45 @@ +error[E0502]: cannot borrow `query` as immutable because it is also borrowed as mutable + --> tests/ui/query_to_readonly.rs:9:18 + | +8 | for _ in query.iter_mut() { + | ---------------- + | | + | mutable borrow occurs here + | mutable borrow later used here +9 | for _ in query.to_readonly().iter() {} + | ^^^^^^^^^^^^^^^^^^^ immutable borrow occurs here + +error[E0502]: cannot borrow `query` as mutable because it is also borrowed as immutable + --> tests/ui/query_to_readonly.rs:14:18 + | +13 | for _ in query.to_readonly().iter() { + | -------------------------- + | | + | immutable borrow occurs here + | immutable borrow later used here +14 | for _ in query.iter_mut() {} + | ^^^^^^^^^^^^^^^^ mutable borrow occurs here + +error[E0502]: cannot borrow `query` as immutable because it is also borrowed as mutable + --> tests/ui/query_to_readonly.rs:39:30 + | +36 | let mut mut_foo = query.single_mut(); + | ------------------ mutable borrow occurs here +... +39 | let readonly_query = query.to_readonly(); + | ^^^^^^^^^^^^^^^^^^^ immutable borrow occurs here +... +43 | *mut_foo = Foo; + | ------- mutable borrow later used here + +error[E0502]: cannot borrow `query` as mutable because it is also borrowed as immutable + --> tests/ui/query_to_readonly.rs:55:27 + | +51 | let readonly_query = query.to_readonly(); + | ------------------- immutable borrow occurs here +... +55 | let mut mut_foo = query.single_mut(); + | ^^^^^^^^^^^^^^^^^^ mutable borrow occurs here +56 | +57 | println!("{ref_foo:?}"); + | ------- immutable borrow later used here From 7479f7c4ecafaedcee099b0414fc40d4a09daa38 Mon Sep 17 00:00:00 2001 From: harudagondi Date: Tue, 19 Jul 2022 23:45:17 +0800 Subject: [PATCH 4/4] Improve docs --- crates/bevy_ecs/src/system/query.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 5fc3ddf3583e1..5a9fe5adc0aa3 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -267,7 +267,11 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { } } - /// Convert the query to readonly. + /// Downgrades all data accessed in this query to a read-only form. + /// + /// For example, `Query<(&mut A, &B, &mut C), With>` will become `Query<(&A, &B, &C), With>`. + /// This can be useful when working around the borrow checker, + /// or reusing functionality between systems via functions that accept query types. pub fn to_readonly(&self) -> Query<'_, '_, Q::ReadOnly, F::ReadOnly> { let new_state = self.state.as_readonly(); // SAFETY: This is memory safe because it turns the query immutable.