From 63a5e8465895ab432ad450563831fc5b8ce6710f Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Wed, 5 Feb 2025 14:37:37 -0500 Subject: [PATCH 1/2] Shorten the 'world lifetime returned from `QueryLens::query()`. Add a new `QueryLens::query_inner()` to handle cases that were relying on the longer lifetime. --- crates/bevy_ecs/src/system/query.rs | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index ecca479d3bf9c..aace8277522b3 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -2097,7 +2097,21 @@ pub struct QueryLens<'w, Q: QueryData, F: QueryFilter = ()> { impl<'w, Q: QueryData, F: QueryFilter> QueryLens<'w, Q, F> { /// Create a [`Query`] from the underlying [`QueryState`]. - pub fn query(&mut self) -> Query<'w, '_, Q, F> { + pub fn query(&mut self) -> Query<'_, '_, Q, F> { + Query { + world: self.world, + state: &self.state, + last_run: self.last_run, + this_run: self.this_run, + } + } +} + +impl<'w, Q: ReadOnlyQueryData, F: QueryFilter> QueryLens<'w, Q, F> { + /// Create a [`Query`] from the underlying [`QueryState`]. + /// This returns results with the actual "inner" world lifetime, + /// so it may only be used with read-only queries to prevent mutable aliasing. + pub fn query_inner(&self) -> Query<'w, '_, Q, F> { Query { world: self.world, state: &self.state, @@ -2108,9 +2122,9 @@ impl<'w, Q: QueryData, F: QueryFilter> QueryLens<'w, Q, F> { } impl<'w, 's, Q: QueryData, F: QueryFilter> From<&'s mut QueryLens<'w, Q, F>> - for Query<'w, 's, Q, F> + for Query<'s, 's, Q, F> { - fn from(value: &'s mut QueryLens<'w, Q, F>) -> Query<'w, 's, Q, F> { + fn from(value: &'s mut QueryLens<'w, Q, F>) -> Query<'s, 's, Q, F> { value.query() } } From 80147ffbe0de5f9a00feb9afb5ba2c760e909b75 Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Wed, 5 Feb 2025 19:35:29 -0500 Subject: [PATCH 2/2] Add compile-fail test. --- .../tests/ui/query_lens_lifetime_safety.rs | 24 +++++++++++++++++++ .../ui/query_lens_lifetime_safety.stderr | 5 ++++ 2 files changed, 29 insertions(+) create mode 100644 crates/bevy_ecs/compile_fail/tests/ui/query_lens_lifetime_safety.rs create mode 100644 crates/bevy_ecs/compile_fail/tests/ui/query_lens_lifetime_safety.stderr diff --git a/crates/bevy_ecs/compile_fail/tests/ui/query_lens_lifetime_safety.rs b/crates/bevy_ecs/compile_fail/tests/ui/query_lens_lifetime_safety.rs new file mode 100644 index 0000000000000..462e9dbf1f056 --- /dev/null +++ b/crates/bevy_ecs/compile_fail/tests/ui/query_lens_lifetime_safety.rs @@ -0,0 +1,24 @@ +use bevy_ecs::prelude::*; +use bevy_ecs::system::SystemState; + +#[derive(Component, Eq, PartialEq, Debug)] +struct Foo(u32); + +fn main() { + let mut world = World::default(); + let e = world.spawn(Foo(10_u32)).id(); + + let mut system_state = SystemState::>::new(&mut world); + { + let mut query = system_state.get_mut(&mut world); + let mut lens = query.as_query_lens(); + dbg!("hi"); + { + let mut data: Mut = lens.query().get_inner(e).unwrap(); + let mut data2: Mut = lens.query().get_inner(e).unwrap(); + //~^ E0499 + assert_eq!(&mut *data, &mut *data2); // oops UB + } + dbg!("bye"); + } +} diff --git a/crates/bevy_ecs/compile_fail/tests/ui/query_lens_lifetime_safety.stderr b/crates/bevy_ecs/compile_fail/tests/ui/query_lens_lifetime_safety.stderr new file mode 100644 index 0000000000000..060b32029556f --- /dev/null +++ b/crates/bevy_ecs/compile_fail/tests/ui/query_lens_lifetime_safety.stderr @@ -0,0 +1,5 @@ +error[E0499]: cannot borrow `lens` as mutable more than once at a time + --> tests/ui\query_lens_lifetime_safety.rs:18:39 + | +17 | let mut data: Mut = lens.query().get_inner(e).unwrap(); + | ---- first mutable borrow occurs here