Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - Allows conversion of mutable queries to immutable queries #5376

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 123 additions & 0 deletions crates/bevy_ecs/src/system/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,129 @@ 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<Option<&mut A>>) {
for _ in &mut query {}

immutable_query(query.to_readonly());
}

fn immutable_query(_: Query<Option<&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<C>>) {
for _ in &mut query {}

immutable_query(query.to_readonly());
}

fn immutable_query(_: Query<(&A, &B), With<C>>) {}

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), Without<C>>) {
for _ in &mut query {}

immutable_query(query.to_readonly());
}

fn immutable_query(_: Query<(&A, &B), Without<C>>) {}

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<C>>) {
for _ in &mut query {}

immutable_query(query.to_readonly());
}

fn immutable_query(_: Query<(&A, &B), Added<C>>) {}

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<C>>) {
for _ in &mut query {}

immutable_query(query.to_readonly());
}

fn immutable_query(_: Query<(&A, &B), Changed<C>>) {}

let mut sys = IntoSystem::into_system(mutable_query);
sys.initialize(&mut world);
}
}

#[test]
fn update_archetype_component_access_works() {
use std::collections::HashSet;
Expand Down
18 changes: 18 additions & 0 deletions crates/bevy_ecs/src/system/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,24 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> {
}
}

/// Downgrades all data accessed in this query to a read-only form.
///
/// For example, `Query<(&mut A, &B, &mut C), With<D>>` will become `Query<(&A, &B, &C), With<D>>`.
/// 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.
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).
Expand Down
75 changes: 75 additions & 0 deletions crates/bevy_ecs_compile_fail_tests/tests/ui/query_to_readonly.rs
Original file line number Diff line number Diff line change
@@ -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() {}
Original file line number Diff line number Diff line change
@@ -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