yeet unsound lifetime annotations on Query methods (bevyengine#4243)

# Objective
Continuation of bevyengine#2964 (I really should have checked other methods when I made that PR)

yeet unsound lifetime annotations on `Query` methods.
Example unsoundness:
```rust
use bevy::prelude::*;

fn main() {
    App::new().add_startup_system(bar).add_system(foo).run();
}

pub fn bar(mut cmds: Commands) {
    let e = cmds.spawn().insert(Foo { a: 10 }).id();
    cmds.insert_resource(e);
}

#[derive(Component, Debug, PartialEq, Eq)]
pub struct Foo {
    a: u32,
}
pub fn foo(mut query: Query<&mut Foo>, e: Res<Entity>) {
    dbg!("hi");
    {
        let data: &Foo = query.get(*e).unwrap();
        let data2: Mut<Foo> = query.get_mut(*e).unwrap();
        assert_eq!(data, &*data2); // oops UB
    }

    {
        let data: &Foo = query.single();
        let data2: Mut<Foo> = query.single_mut();
        assert_eq!(data, &*data2); // oops UB
    }

    {
        let data: &Foo = query.get_single().unwrap();
        let data2: Mut<Foo> = query.get_single_mut().unwrap();
        assert_eq!(data, &*data2); // oops UB
    }

    {
        let data: &Foo = query.iter().next().unwrap();
        let data2: Mut<Foo> = query.iter_mut().next().unwrap();
        assert_eq!(data, &*data2); // oops UB
    }

    {
        let mut opt_data: Option<&Foo> = None;
        let mut opt_data_2: Option<Mut<Foo>> = None;
        query.for_each(|data| opt_data = Some(data));
        query.for_each_mut(|data| opt_data_2 = Some(data));
        assert_eq!(opt_data.unwrap(), &*opt_data_2.unwrap()); // oops UB
    }
    dbg!("bye");
}

```

## Solution
yeet unsound lifetime annotations on `Query` methods

Co-authored-by: Carter Anderson <mcanders1@gmail.com>

crates/bevy_ecs/src/system/mod.rs

@@ -804,13 +804,13 @@ mod tests {
  }
  fn hold_component<'w>(&mut self, world: &'w World, entity: Entity) -> Holder<'w> {
  let q = self.state_q.get(world);
- let a = q.get(entity).unwrap();
+ let a = q.get_inner(entity).unwrap();
  Holder { value: a }
  }
  fn hold_components<'w>(&mut self, world: &'w World) -> Vec<Holder<'w>> {
  let mut components = Vec::new();
  let q = self.state_q.get(world);
- for a in q.iter() {
+ for a in q.iter_inner() {
  components.push(Holder { value: a });
  }
  components

crates/bevy_ecs/src/system/query.rs

@@ -3,7 +3,7 @@ use crate::{
  entity::Entity,
  query::{
  Fetch, FilterFetch, NopFetch, QueryCombinationIter, QueryEntityError, QueryIter,
- QueryState, WorldQuery,
+ QueryState, ReadOnlyFetch, WorldQuery,
  },
  world::{Mut, World},
 };
@@ -299,7 +299,7 @@ where
  /// # bevy_ecs::system::assert_is_system(report_names_system);
  /// ```
  #[inline]
- pub fn iter(&'s self) -> QueryIter<'w, 's, Q, Q::ReadOnlyFetch, F> {
+ pub fn iter(&self) -> QueryIter<'_, 's, Q, Q::ReadOnlyFetch, F> {
  // SAFE: system runs without conflicts with other systems.
  // same-system queries have runtime borrow checks when they conflict
  unsafe {
@@ -454,17 +454,19 @@ where
  /// # bevy_ecs::system::assert_is_system(report_names_system);
  /// ```
  #[inline]
- pub fn for_each<FN: FnMut(<Q::ReadOnlyFetch as Fetch<'w, 's>>::Item)>(&'s self, f: FN) {
+ pub fn for_each<'this>(
+ &'this self,
+ f: impl FnMut(<Q::ReadOnlyFetch as Fetch<'this, 's>>::Item),
+ ) {
  // SAFE: system runs without conflicts with other systems.
  // same-system queries have runtime borrow checks when they conflict
  unsafe {
- self.state
- .for_each_unchecked_manual::<Q::ReadOnlyFetch, FN>(
- self.world,
- f,
- self.last_change_tick,
- self.change_tick,
- );
+ self.state.for_each_unchecked_manual::<Q::ReadOnlyFetch, _>(
+ self.world,
+ f,
+ self.last_change_tick,
+ self.change_tick,
+ );
  };
  }
 
@@ -524,17 +526,17 @@ where
  ///* `batch_size` - The number of batches to spawn
  ///* `f` - The function to run on each item in the query
  #[inline]
- pub fn par_for_each<FN: Fn(<Q::ReadOnlyFetch as Fetch<'w, 's>>::Item) + Send + Sync + Clone>(
- &'s self,
+ pub fn par_for_each<'this>(
+ &'this self,
  task_pool: &TaskPool,
  batch_size: usize,
- f: FN,
+ f: impl Fn(<Q::ReadOnlyFetch as Fetch<'this, 's>>::Item) + Send + Sync + Clone,
  ) {
  // SAFE: system runs without conflicts with other systems. same-system queries have runtime
  // borrow checks when they conflict
  unsafe {
  self.state
- .par_for_each_unchecked_manual::<Q::ReadOnlyFetch, FN>(
+ .par_for_each_unchecked_manual::<Q::ReadOnlyFetch, _>(
  self.world,
  task_pool,
  batch_size,
@@ -601,9 +603,9 @@ where
  /// ```
  #[inline]
  pub fn get(
- &'s self,
+ &self,
  entity: Entity,
- ) -> Result<<Q::ReadOnlyFetch as Fetch<'w, 's>>::Item, QueryEntityError> {
+ ) -> Result<<Q::ReadOnlyFetch as Fetch<'_, 's>>::Item, QueryEntityError> {
  // SAFE: system runs without conflicts with other systems.
  // same-system queries have runtime borrow checks when they conflict
  unsafe {
@@ -834,7 +836,7 @@ where
  /// Panics if the number of query results is not exactly one. Use
  /// [`get_single`](Self::get_single) to return a `Result` instead of panicking.
  #[track_caller]
- pub fn single(&'s self) -> <Q::ReadOnlyFetch as Fetch<'w, 's>>::Item {
+ pub fn single(&self) -> <Q::ReadOnlyFetch as Fetch<'_, 's>>::Item {
  self.get_single().unwrap()
  }
 
@@ -870,8 +872,8 @@ where
  /// # bevy_ecs::system::assert_is_system(player_scoring_system);
  /// ```
  pub fn get_single(
- &'s self,
- ) -> Result<<Q::ReadOnlyFetch as Fetch<'w, 's>>::Item, QuerySingleError> {
+ &self,
+ ) -> Result<<Q::ReadOnlyFetch as Fetch<'_, 's>>::Item, QuerySingleError> {
  let mut query = self.iter();
  let first = query.next();
  let extra = query.next().is_some();
@@ -1038,3 +1040,90 @@ pub enum QuerySingleError {
  #[error("Multiple entities fit the query {0}!")]
  MultipleEntities(&'static str),
 }
+
+impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F>
+where
+ F::Fetch: FilterFetch,
+ Q::Fetch: ReadOnlyFetch,
+{
+ /// Returns the query result for the given [`Entity`], with the actual "inner" world lifetime.
+ ///
+ /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is
+ /// returned instead.
+ ///
+ /// This can only return immutable data (mutable data will be cast to an immutable form).
+ /// See [`get_mut`](Self::get_mut) for queries that contain at least one mutable component.
+ ///
+ /// # Example
+ ///
+ /// Here, `get` is used to retrieve the exact query result of the entity specified by the
+ /// `SelectedCharacter` resource.
+ ///
+ /// ```
+ /// # use bevy_ecs::prelude::*;
+ /// #
+ /// # struct SelectedCharacter { entity: Entity }
+ /// # #[derive(Component)]
+ /// # struct Character { name: String }
+ /// #
+ /// fn print_selected_character_name_system(
+ /// query: Query<&Character>,
+ /// selection: Res<SelectedCharacter>
+ /// )
+ /// {
+ /// if let Ok(selected_character) = query.get(selection.entity) {
+ /// println!("{}", selected_character.name);
+ /// }
+ /// }
+ /// # bevy_ecs::system::assert_is_system(print_selected_character_name_system);
+ /// ```
+ #[inline]
+ pub fn get_inner(
+ &'s self,
+ entity: Entity,
+ ) -> Result<<Q::ReadOnlyFetch as Fetch<'w, 's>>::Item, QueryEntityError> {
+ // SAFE: system runs without conflicts with other systems.
+ // same-system queries have runtime borrow checks when they conflict
+ unsafe {
+ self.state.get_unchecked_manual::<Q::ReadOnlyFetch>(
+ self.world,
+ entity,
+ self.last_change_tick,
+ self.change_tick,
+ )
+ }
+ }
+
+ /// Returns an [`Iterator`] over the query results, with the actual "inner" world lifetime.
+ ///
+ /// This can only return immutable data (mutable data will be cast to an immutable form).
+ /// See [`Self::iter_mut`] for queries that contain at least one mutable component.
+ ///
+ /// # Example
+ ///
+ /// Here, the `report_names_system` iterates over the `Player` component of every entity
+ /// that contains it:
+ ///
+ /// ```
+ /// # use bevy_ecs::prelude::*;
+ /// #
+ /// # #[derive(Component)]
+ /// # struct Player { name: String }
+ /// #
+ /// fn report_names_system(query: Query<&Player>) {
+ /// for player in query.iter() {
+ /// println!("Say hello to {}!", player.name);
+ /// }
+ /// }
+ /// # bevy_ecs::system::assert_is_system(report_names_system);
+ /// ```
+ #[inline]
+ pub fn iter_inner(&'s self) -> QueryIter<'w, 's, Q, Q::ReadOnlyFetch, F> {
+ // SAFE: system runs without conflicts with other systems.
+ // same-system queries have runtime borrow checks when they conflict
+ unsafe {
+ self.state
+ .iter_unchecked_manual(self.world, self.last_change_tick, self.change_tick)
+ }
+ }
+}

crates/bevy_ecs_compile_fail_tests/tests/ui/query_lifetime_safety.rs

@@ -0,0 +1,92 @@
+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().insert(Foo(10_u32)).id();
+
+ let mut system_state = SystemState::<Query<&mut Foo>>::new(&mut world);
+ {
+ let mut query = system_state.get_mut(&mut world);
+ dbg!("hi");
+ {
+ let data: &Foo = query.get(e).unwrap();
+ let mut data2: Mut<Foo> = query.get_mut(e).unwrap();
+ assert_eq!(data, &mut *data2); // oops UB
+ }
+
+ {
+ let mut data2: Mut<Foo> = query.get_mut(e).unwrap();
+ let data: &Foo = query.get(e).unwrap();
+ assert_eq!(data, &mut *data2); // oops UB
+ }
+
+ {
+ let data: &Foo = query.get_component::<Foo>(e).unwrap();
+ let mut data2: Mut<Foo> = query.get_component_mut(e).unwrap();
+ assert_eq!(data, &mut *data2); // oops UB
+ }
+
+ {
+ let mut data2: Mut<Foo> = query.get_component_mut(e).unwrap();
+ let data: &Foo = query.get_component::<Foo>(e).unwrap();
+ assert_eq!(data, &mut *data2); // oops UB
+ }
+
+ {
+ let data: &Foo = query.single();
+ let mut data2: Mut<Foo> = query.single_mut();
+ assert_eq!(data, &mut *data2); // oops UB
+ }
+
+ {
+ let mut data2: Mut<Foo> = query.single_mut();
+ let data: &Foo = query.single();
+ assert_eq!(data, &mut *data2); // oops UB
+ }
+
+ {
+ let data: &Foo = query.get_single().unwrap();
+ let mut data2: Mut<Foo> = query.get_single_mut().unwrap();
+ assert_eq!(data, &mut *data2); // oops UB
+ }
+
+ {
+ let mut data2: Mut<Foo> = query.get_single_mut().unwrap();
+ let data: &Foo = query.get_single().unwrap();
+ assert_eq!(data, &mut *data2); // oops UB
+ }
+
+ {
+ let data: &Foo = query.iter().next().unwrap();
+ let mut data2: Mut<Foo> = query.iter_mut().next().unwrap();
+ assert_eq!(data, &mut *data2); // oops UB
+ }
+
+ {
+ let mut data2: Mut<Foo> = query.iter_mut().next().unwrap();
+ let data: &Foo = query.iter().next().unwrap();
+ assert_eq!(data, &mut *data2); // oops UB
+ }
+
+ {
+ let mut opt_data: Option<&Foo> = None;
+ let mut opt_data_2: Option<Mut<Foo>> = None;
+ query.for_each(|data| opt_data = Some(data));
+ query.for_each_mut(|data| opt_data_2 = Some(data));
+ assert_eq!(opt_data.unwrap(), &mut *opt_data_2.unwrap()); // oops UB
+ }
+
+ {
+ let mut opt_data_2: Option<Mut<Foo>> = None;
+ let mut opt_data: Option<&Foo> = None;
+ query.for_each_mut(|data| opt_data_2 = Some(data));
+ query.for_each(|data| opt_data = Some(data));
+ assert_eq!(opt_data.unwrap(), &mut *opt_data_2.unwrap()); // oops UB
+ }
+ dbg!("bye");
+ }
+}