Skip to content

Commit

Permalink
Allows conversion of mutable queries to immutable queries (bevyengine…
Browse files Browse the repository at this point in the history
…#5376)

# Objective

- Allows conversion of mutable queries to immutable queries.
- Fixes bevyengine#4606

## Solution

- Add `to_readonly` method on `Query`, which uses `QueryState::as_readonly`
- `AsRef` is not feasible because creation of new queries is needed.

---

## Changelog

### Added

- Allows conversion of mutable queries to immutable queries using `Query::to_readonly`.
  • Loading branch information
harudagondi authored and ItsDoot committed Feb 1, 2023
1 parent e4c6e77 commit 617d10f
Show file tree
Hide file tree
Showing 4 changed files with 261 additions and 0 deletions.
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

0 comments on commit 617d10f

Please sign in to comment.