Implement iter() for mutable Queries (#2305)

A sample implementation of how to have `iter()` work on mutable queries without breaking aliasing rules.

# Objective

- Fixes #753

## Solution

- Added a ReadOnlyFetch to WorldQuery that is the `&T` version of `&mut T` that is used to specify the return type for read only operations like `iter()`.
- ~~As the comment suggests specifying the bound doesn't work due to restrictions on defining recursive implementations (like `Or`). However bounds on the functions are fine~~ Never mind I misread how `Or` was constructed, bounds now exist.
- Note that the only mutable one has a new `Fetch` for readonly as the `State` has to be the same for any of this to work


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

crates/bevy_ecs/src/query/fetch.rs

@@ -43,6 +43,8 @@ use std::{
 pub trait WorldQuery {
     type Fetch: for<'world, 'state> Fetch<'world, 'state, State = Self::State>;
     type State: FetchState;
+    type ReadOnlyFetch: for<'world, 'state> Fetch<'world, 'state, State = Self::State>
+        + ReadOnlyFetch;
 }
 
 pub type QueryItem<'w, 's, Q> = <<Q as WorldQuery>::Fetch as Fetch<'w, 's>>::Item;
@@ -136,6 +138,7 @@ pub unsafe trait ReadOnlyFetch {}
 impl WorldQuery for Entity {
     type Fetch = EntityFetch;
     type State = EntityState;
+    type ReadOnlyFetch = EntityFetch;
 }
 
 /// The [`Fetch`] of [`Entity`].
@@ -222,6 +225,7 @@ impl<'w, 's> Fetch<'w, 's> for EntityFetch {
 impl<T: Component> WorldQuery for &T {
     type Fetch = ReadFetch<T>;
     type State = ReadState<T>;
+    type ReadOnlyFetch = ReadFetch<T>;
 }
 
 /// The [`FetchState`] of `&T`.
@@ -376,6 +380,7 @@ impl<'w, 's, T: Component> Fetch<'w, 's> for ReadFetch<T> {
 impl<T: Component> WorldQuery for &mut T {
     type Fetch = WriteFetch<T>;
     type State = WriteState<T>;
+    type ReadOnlyFetch = ReadOnlyWriteFetch<T>;
 }
 
 /// The [`Fetch`] of `&mut T`.
@@ -403,6 +408,28 @@ impl<T> Clone for WriteFetch<T> {
     }
 }
 
+/// The [`ReadOnlyFetch`] of `&mut T`.
+pub struct ReadOnlyWriteFetch<T> {
+    table_components: NonNull<T>,
+    entities: *const Entity,
+    entity_table_rows: *const usize,
+    sparse_set: *const ComponentSparseSet,
+}
+
+/// SAFETY: access is read only
+unsafe impl<T> ReadOnlyFetch for ReadOnlyWriteFetch<T> {}
+
+impl<T> Clone for ReadOnlyWriteFetch<T> {
+    fn clone(&self) -> Self {
+        Self {
+            table_components: self.table_components,
+            entities: self.entities,
+            entity_table_rows: self.entity_table_rows,
+            sparse_set: self.sparse_set,
+        }
+    }
+}
+
 /// The [`FetchState`] of `&mut T`.
 pub struct WriteState<T> {
     component_id: ComponentId,
@@ -555,9 +582,88 @@ impl<'w, 's, T: Component> Fetch<'w, 's> for WriteFetch<T> {
     }
 }
 
+impl<'w, 's, T: Component> Fetch<'w, 's> for ReadOnlyWriteFetch<T> {
+    type Item = &'w T;
+    type State = WriteState<T>;
+
+    const IS_DENSE: bool = {
+        match T::Storage::STORAGE_TYPE {
+            StorageType::Table => true,
+            StorageType::SparseSet => false,
+        }
+    };
+
+    unsafe fn init(
+        world: &World,
+        state: &Self::State,
+        _last_change_tick: u32,
+        _change_tick: u32,
+    ) -> Self {
+        let mut value = Self {
+            table_components: NonNull::dangling(),
+            entities: ptr::null::<Entity>(),
+            entity_table_rows: ptr::null::<usize>(),
+            sparse_set: ptr::null::<ComponentSparseSet>(),
+        };
+        if T::Storage::STORAGE_TYPE == StorageType::SparseSet {
+            value.sparse_set = world
+                .storages()
+                .sparse_sets
+                .get(state.component_id)
+                .unwrap();
+        }
+        value
+    }
+
+    #[inline]
+    unsafe fn set_archetype(
+        &mut self,
+        state: &Self::State,
+        archetype: &Archetype,
+        tables: &Tables,
+    ) {
+        match T::Storage::STORAGE_TYPE {
+            StorageType::Table => {
+                self.entity_table_rows = archetype.entity_table_rows().as_ptr();
+                let column = tables[archetype.table_id()]
+                    .get_column(state.component_id)
+                    .unwrap();
+                self.table_components = column.get_data_ptr().cast::<T>();
+            }
+            StorageType::SparseSet => self.entities = archetype.entities().as_ptr(),
+        }
+    }
+
+    #[inline]
+    unsafe fn set_table(&mut self, state: &Self::State, table: &Table) {
+        let column = table.get_column(state.component_id).unwrap();
+        self.table_components = column.get_data_ptr().cast::<T>();
+    }
+
+    #[inline]
+    unsafe fn archetype_fetch(&mut self, archetype_index: usize) -> Self::Item {
+        match T::Storage::STORAGE_TYPE {
+            StorageType::Table => {
+                let table_row = *self.entity_table_rows.add(archetype_index);
+                &*self.table_components.as_ptr().add(table_row)
+            }
+            StorageType::SparseSet => {
+                let entity = *self.entities.add(archetype_index);
+                &*(*self.sparse_set).get(entity).unwrap().cast::<T>()
+            }
+        }
+    }
+
+    #[inline]
+    unsafe fn table_fetch(&mut self, table_row: usize) -> Self::Item {
+        &*self.table_components.as_ptr().add(table_row)
+    }
+}
+
 impl<T: WorldQuery> WorldQuery for Option<T> {
     type Fetch = OptionFetch<T::Fetch>;
     type State = OptionState<T::State>;
+    type ReadOnlyFetch = OptionFetch<T::ReadOnlyFetch>;
 }
 
 /// The [`Fetch`] of `Option<T>`.
@@ -733,6 +839,7 @@ impl<T: Component> ChangeTrackers<T> {
 impl<T: Component> WorldQuery for ChangeTrackers<T> {
     type Fetch = ChangeTrackersFetch<T>;
     type State = ChangeTrackersState<T>;
+    type ReadOnlyFetch = ChangeTrackersFetch<T>;
 }
 
 /// The [`FetchState`] of [`ChangeTrackers`].
@@ -983,6 +1090,7 @@ macro_rules! impl_tuple_fetch {
         impl<$($name: WorldQuery),*> WorldQuery for ($($name,)*) {
             type Fetch = ($($name::Fetch,)*);
             type State = ($($name::State,)*);
+            type ReadOnlyFetch = ($($name::ReadOnlyFetch,)*);
         }
 
         /// SAFETY: each item in the tuple is read only
@@ -992,3 +1100,45 @@ macro_rules! impl_tuple_fetch {
 }
 
 all_tuples!(impl_tuple_fetch, 0, 15, F, S);
+
+/// [`Fetch`] that does not actually fetch anything
+///
+/// Mostly useful when something is generic over the Fetch and you don't want to fetch as you will discard the result
+pub struct NopFetch<State> {
+    state: PhantomData<State>,
+}
+
+impl<'w, 's, State: FetchState> Fetch<'w, 's> for NopFetch<State> {
+    type Item = ();
+    type State = State;
+
+    const IS_DENSE: bool = true;
+
+    #[inline(always)]
+    unsafe fn init(
+        _world: &World,
+        _state: &Self::State,
+        _last_change_tick: u32,
+        _change_tick: u32,
+    ) -> Self {
+        Self { state: PhantomData }
+    }
+
+    #[inline(always)]
+    unsafe fn set_archetype(
+        &mut self,
+        _state: &Self::State,
+        _archetype: &Archetype,
+        _tables: &Tables,
+    ) {
+    }
+
+    #[inline(always)]
+    unsafe fn set_table(&mut self, _state: &Self::State, _table: &Table) {}
+
+    #[inline(always)]
+    unsafe fn archetype_fetch(&mut self, _archetype_index: usize) -> Self::Item {}
+
+    #[inline(always)]
+    unsafe fn table_fetch(&mut self, _table_row: usize) -> Self::Item {}
+}

crates/bevy_ecs/src/query/filter.rs

@@ -2,7 +2,7 @@ use crate::{
     archetype::{Archetype, ArchetypeComponentId},
     component::{Component, ComponentId, ComponentStorage, ComponentTicks, StorageType},
     entity::Entity,
-    query::{Access, Fetch, FetchState, FilteredAccess, WorldQuery},
+    query::{Access, Fetch, FetchState, FilteredAccess, ReadOnlyFetch, WorldQuery},
     storage::{ComponentSparseSet, Table, Tables},
     world::World,
 };
@@ -72,6 +72,7 @@ pub struct With<T>(PhantomData<T>);
 impl<T: Component> WorldQuery for With<T> {
     type Fetch = WithFetch<T>;
     type State = WithState<T>;
+    type ReadOnlyFetch = WithFetch<T>;
 }
 
 /// The [`Fetch`] of [`With`].
@@ -162,6 +163,9 @@ impl<'w, 's, T: Component> Fetch<'w, 's> for WithFetch<T> {
     }
 }
 
+// SAFETY: no component access or archetype component access
+unsafe impl<T> ReadOnlyFetch for WithFetch<T> {}
+
 /// Filter that selects entities without a component `T`.
 ///
 /// This is the negation of [`With`].
@@ -191,6 +195,7 @@ pub struct Without<T>(PhantomData<T>);
 impl<T: Component> WorldQuery for Without<T> {
     type Fetch = WithoutFetch<T>;
     type State = WithoutState<T>;
+    type ReadOnlyFetch = WithoutFetch<T>;
 }
 
 /// The [`Fetch`] of [`Without`].
@@ -281,6 +286,9 @@ impl<'w, 's, T: Component> Fetch<'w, 's> for WithoutFetch<T> {
     }
 }
 
+// SAFETY: no component access or archetype component access
+unsafe impl<T> ReadOnlyFetch for WithoutFetch<T> {}
+
 /// A filter that tests if any of the given filters apply.
 ///
 /// This is useful for example if a system with multiple components in a query only wants to run
@@ -338,12 +346,15 @@ macro_rules! impl_query_filter_tuple {
         }
 
         impl<$($filter: WorldQuery),*> WorldQuery for Or<($($filter,)*)>
-            where $($filter::Fetch: FilterFetch),*
+            where $($filter::Fetch: FilterFetch, $filter::ReadOnlyFetch: FilterFetch),*
         {
             type Fetch = Or<($(OrFetch<$filter::Fetch>,)*)>;
             type State = Or<($($filter::State,)*)>;
+            type ReadOnlyFetch = Or<($(OrFetch<$filter::ReadOnlyFetch>,)*)>;
         }
 
+        /// SAFETY: this only works using the filter which doesn't write
+        unsafe impl<$($filter: FilterFetch + ReadOnlyFetch),*> ReadOnlyFetch for Or<($(OrFetch<$filter>,)*)> {}
 
         #[allow(unused_variables)]
         #[allow(non_snake_case)]
@@ -464,9 +475,9 @@ macro_rules! impl_tick_filter {
         impl<T: Component> WorldQuery for $name<T> {
             type Fetch = $fetch_name<T>;
             type State = $state_name<T>;
+            type ReadOnlyFetch = $fetch_name<T>;
         }
 
-
         // SAFETY: this reads the T component. archetype component access and component access are updated to reflect that
         unsafe impl<T: Component> FetchState for $state_name<T> {
             fn init(world: &mut World) -> Self {
@@ -572,6 +583,9 @@ macro_rules! impl_tick_filter {
                 }
             }
         }
+
+        /// SAFETY: read-only access
+        unsafe impl<T: Component> ReadOnlyFetch for $fetch_name<T> {}
     };
 }
 

crates/bevy_ecs/src/query/iter.rs

@@ -4,13 +4,13 @@ use crate::{
     storage::{TableId, Tables},
     world::World,
 };
-use std::mem::MaybeUninit;
+use std::{marker::PhantomData, mem::MaybeUninit};
 
 /// An [`Iterator`] over query results of a [`Query`](crate::system::Query).
 ///
 /// This struct is created by the [`Query::iter`](crate::system::Query::iter) and
 /// [`Query::iter_mut`](crate::system::Query::iter_mut) methods.
-pub struct QueryIter<'w, 's, Q: WorldQuery, F: WorldQuery>
+pub struct QueryIter<'w, 's, Q: WorldQuery, QF: Fetch<'w, 's, State = Q::State>, F: WorldQuery>
 where
     F::Fetch: FilterFetch,
 {
@@ -20,15 +20,16 @@ where
     world: &'w World,
     table_id_iter: std::slice::Iter<'s, TableId>,
     archetype_id_iter: std::slice::Iter<'s, ArchetypeId>,
-    fetch: Q::Fetch,
+    fetch: QF,
     filter: F::Fetch,
     current_len: usize,
     current_index: usize,
 }
 
-impl<'w, 's, Q: WorldQuery, F: WorldQuery> QueryIter<'w, 's, Q, F>
+impl<'w, 's, Q: WorldQuery, QF, F: WorldQuery> QueryIter<'w, 's, Q, QF, F>
 where
     F::Fetch: FilterFetch,
+    QF: Fetch<'w, 's, State = Q::State>,
 {
     /// # Safety
     /// This does not check for mutable query correctness. To be safe, make sure mutable queries
@@ -41,7 +42,7 @@ where
         last_change_tick: u32,
         change_tick: u32,
     ) -> Self {
-        let fetch = <Q::Fetch as Fetch>::init(
+        let fetch = QF::init(
             world,
             &query_state.fetch_state,
             last_change_tick,
@@ -67,69 +68,14 @@ where
             current_index: 0,
         }
     }
-
-    /// Consumes `self` and returns true if there were no elements remaining in this iterator.
-    #[inline(always)]
-    pub(crate) fn none_remaining(mut self) -> bool {
-        // NOTE: this mimics the behavior of `QueryIter::next()`, except that it
-        // never gets a `Self::Item`.
-        unsafe {
-            if Q::Fetch::IS_DENSE && F::Fetch::IS_DENSE {
-                loop {
-                    if self.current_index == self.current_len {
-                        let table_id = match self.table_id_iter.next() {
-                            Some(table_id) => table_id,
-                            None => return true,
-                        };
-                        let table = &self.tables[*table_id];
-                        self.filter.set_table(&self.query_state.filter_state, table);
-                        self.current_len = table.len();
-                        self.current_index = 0;
-                        continue;
-                    }
-
-                    if !self.filter.table_filter_fetch(self.current_index) {
-                        self.current_index += 1;
-                        continue;
-                    }
-
-                    return false;
-                }
-            } else {
-                loop {
-                    if self.current_index == self.current_len {
-                        let archetype_id = match self.archetype_id_iter.next() {
-                            Some(archetype_id) => archetype_id,
-                            None => return true,
-                        };
-                        let archetype = &self.archetypes[*archetype_id];
-                        self.filter.set_archetype(
-                            &self.query_state.filter_state,
-                            archetype,
-                            self.tables,
-                        );
-                        self.current_len = archetype.len();
-                        self.current_index = 0;
-                        continue;
-                    }
-
-                    if !self.filter.archetype_filter_fetch(self.current_index) {
-                        self.current_index += 1;
-                        continue;
-                    }
-
-                    return false;
-                }
-            }
-        }
-    }
 }
 
-impl<'w, 's, Q: WorldQuery, F: WorldQuery> Iterator for QueryIter<'w, 's, Q, F>
+impl<'w, 's, Q: WorldQuery, QF, F: WorldQuery> Iterator for QueryIter<'w, 's, Q, QF, F>
 where
     F::Fetch: FilterFetch,
+    QF: Fetch<'w, 's, State = Q::State>,
 {
-    type Item = <Q::Fetch as Fetch<'w, 's>>::Item;
+    type Item = QF::Item;
 
     // NOTE: If you are changing query iteration code, remember to update the following places, where relevant:
     // QueryIter, QueryIterationCursor, QueryState::for_each_unchecked_manual, QueryState::par_for_each_unchecked_manual
@@ -137,7 +83,7 @@ where
     #[inline(always)]
     fn next(&mut self) -> Option<Self::Item> {
         unsafe {
-            if Q::Fetch::IS_DENSE && F::Fetch::IS_DENSE {
+            if QF::IS_DENSE && F::Fetch::IS_DENSE {
                 loop {
                     if self.current_index == self.current_len {
                         let table_id = self.table_id_iter.next()?;
@@ -207,19 +153,22 @@ where
     }
 }
 
-pub struct QueryCombinationIter<'w, 's, Q: WorldQuery, F: WorldQuery, const K: usize>
+pub struct QueryCombinationIter<'w, 's, Q: WorldQuery, QF, F: WorldQuery, const K: usize>
 where
+    QF: Fetch<'w, 's, State = Q::State>,
     F::Fetch: FilterFetch,
 {
     tables: &'w Tables,
     archetypes: &'w Archetypes,
     query_state: &'s QueryState<Q, F>,
     world: &'w World,
-    cursors: [QueryIterationCursor<'s, Q, F>; K],
+    cursors: [QueryIterationCursor<'w, 's, Q, QF, F>; K],
 }
 
-impl<'w, 's, Q: WorldQuery, F: WorldQuery, const K: usize> QueryCombinationIter<'w, 's, Q, F, K>
+impl<'w, 's, Q: WorldQuery, QF, F: WorldQuery, const K: usize>
+    QueryCombinationIter<'w, 's, Q, QF, F, K>
 where
+    QF: Fetch<'w, 's, State = Q::State>,
     F::Fetch: FilterFetch,
 {
     /// # Safety
@@ -237,7 +186,7 @@ where
         // There is no FromIterator on arrays, so instead initialize it manually with MaybeUninit
 
         // TODO: use MaybeUninit::uninit_array if it stabilizes
-        let mut cursors: [MaybeUninit<QueryIterationCursor<'s, Q, F>>; K] =
+        let mut cursors: [MaybeUninit<QueryIterationCursor<'w, 's, Q, QF, F>>; K] =
             MaybeUninit::uninit().assume_init();
         for (i, cursor) in cursors.iter_mut().enumerate() {
             match i {
@@ -257,8 +206,8 @@ where
         }
 
         // TODO: use MaybeUninit::array_assume_init if it stabilizes
-        let cursors: [QueryIterationCursor<'s, Q, F>; K] =
+        let cursors: [QueryIterationCursor<'w, 's, Q, QF, F>; K] =
-            (&cursors as *const _ as *const [QueryIterationCursor<'s, Q, F>; K]).read();
+            (&cursors as *const _ as *const [QueryIterationCursor<'w, 's, Q, QF, F>; K]).read();
 
         QueryCombinationIter {
             world,
@@ -275,11 +224,9 @@ where
     /// references to the same component, leading to unique reference aliasing.
     ///.
     /// It is always safe for shared access.
-    unsafe fn fetch_next_aliased_unchecked<'a>(
+    unsafe fn fetch_next_aliased_unchecked(&mut self) -> Option<[QF::Item; K]>
-        &mut self,
-    ) -> Option<[<Q::Fetch as Fetch<'a, 's>>::Item; K]>
     where
-        Q::Fetch: Clone,
+        QF: Clone,
         F::Fetch: Clone,
     {
         if K == 0 {
@@ -307,25 +254,23 @@ where
         }
 
         // TODO: use MaybeUninit::uninit_array if it stabilizes
-        let mut values: [MaybeUninit<<Q::Fetch as Fetch<'a, 's>>::Item>; K] =
+        let mut values: [MaybeUninit<QF::Item>; K] = MaybeUninit::uninit().assume_init();
-            MaybeUninit::uninit().assume_init();
 
         for (value, cursor) in values.iter_mut().zip(&mut self.cursors) {
             value.as_mut_ptr().write(cursor.peek_last().unwrap());
         }
 
         // TODO: use MaybeUninit::array_assume_init if it stabilizes
-        let values: [<Q::Fetch as Fetch<'a, 's>>::Item; K] =
+        let values: [QF::Item; K] = (&values as *const _ as *const [QF::Item; K]).read();
-            (&values as *const _ as *const [<Q::Fetch as Fetch<'a, 's>>::Item; K]).read();
 
         Some(values)
     }
 
     /// Get next combination of queried components
     #[inline]
-    pub fn fetch_next(&mut self) -> Option<[<Q::Fetch as Fetch<'_, 's>>::Item; K]>
+    pub fn fetch_next(&mut self) -> Option<[QF::Item; K]>
     where
-        Q::Fetch: Clone,
+        QF: Clone,
         F::Fetch: Clone,
     {
         // safety: we are limiting the returned reference to self,
@@ -338,13 +283,13 @@ where
 // Iterator type is intentionally implemented only for read-only access.
 // Doing so for mutable references would be unsound, because  calling `next`
 // multiple times would allow multiple owned references to the same data to exist.
-impl<'w, 's, Q: WorldQuery, F: WorldQuery, const K: usize> Iterator
+impl<'w, 's, Q: WorldQuery, QF, F: WorldQuery, const K: usize> Iterator
-    for QueryCombinationIter<'w, 's, Q, F, K>
+    for QueryCombinationIter<'w, 's, Q, QF, F, K>
 where
-    Q::Fetch: Clone + ReadOnlyFetch,
+    QF: Fetch<'w, 's, State = Q::State> + Clone + ReadOnlyFetch,
     F::Fetch: Clone + FilterFetch + ReadOnlyFetch,
 {
-    type Item = [<Q::Fetch as Fetch<'w, 's>>::Item; K];
+    type Item = [QF::Item; K];
 
     #[inline]
     fn next(&mut self) -> Option<Self::Item> {
@@ -388,7 +333,10 @@ where
 // (2) each archetype pre-computes length
 // (3) there are no per-entity filters
 // TODO: add an ArchetypeOnlyFilter that enables us to implement this for filters like With<T>
-impl<'w, 's, Q: WorldQuery> ExactSizeIterator for QueryIter<'w, 's, Q, ()> {
+impl<'w, 's, Q: WorldQuery, QF> ExactSizeIterator for QueryIter<'w, 's, Q, QF, ()>
+where
+    QF: Fetch<'w, 's, State = Q::State>,
+{
     fn len(&self) -> usize {
         self.query_state
             .matched_archetypes
@@ -398,18 +346,25 @@ impl<'w, 's, Q: WorldQuery> ExactSizeIterator for QueryIter<'w, 's, Q, ()> {
     }
 }
 
-struct QueryIterationCursor<'s, Q: WorldQuery, F: WorldQuery> {
+struct QueryIterationCursor<
+    'w,
+    's,
+    Q: WorldQuery,
+    QF: Fetch<'w, 's, State = Q::State>,
+    F: WorldQuery,
+> {
     table_id_iter: std::slice::Iter<'s, TableId>,
     archetype_id_iter: std::slice::Iter<'s, ArchetypeId>,
-    fetch: Q::Fetch,
+    fetch: QF,
     filter: F::Fetch,
     current_len: usize,
     current_index: usize,
+    phantom: PhantomData<&'w Q>,
 }
 
-impl<'s, Q: WorldQuery, F: WorldQuery> Clone for QueryIterationCursor<'s, Q, F>
+impl<'w, 's, Q: WorldQuery, QF, F: WorldQuery> Clone for QueryIterationCursor<'w, 's, Q, QF, F>
 where
-    Q::Fetch: Clone,
+    QF: Fetch<'w, 's, State = Q::State> + Clone,
     F::Fetch: Clone,
 {
     fn clone(&self) -> Self {
@@ -420,12 +375,14 @@ where
             filter: self.filter.clone(),
             current_len: self.current_len,
             current_index: self.current_index,
+            phantom: PhantomData,
         }
     }
 }
 
-impl<'s, Q: WorldQuery, F: WorldQuery> QueryIterationCursor<'s, Q, F>
+impl<'w, 's, Q: WorldQuery, QF, F: WorldQuery> QueryIterationCursor<'w, 's, Q, QF, F>
 where
+    QF: Fetch<'w, 's, State = Q::State>,
     F::Fetch: FilterFetch,
 {
     unsafe fn init_empty(
@@ -447,7 +404,7 @@ where
         last_change_tick: u32,
         change_tick: u32,
     ) -> Self {
-        let fetch = <Q::Fetch as Fetch>::init(
+        let fetch = QF::init(
             world,
             &query_state.fetch_state,
             last_change_tick,
@@ -466,14 +423,15 @@ where
             archetype_id_iter: query_state.matched_archetype_ids.iter(),
             current_len: 0,
             current_index: 0,
+            phantom: PhantomData,
         }
     }
 
     /// retrieve item returned from most recent `next` call again.
     #[inline]
-    unsafe fn peek_last<'w>(&mut self) -> Option<<Q::Fetch as Fetch<'w, 's>>::Item> {
+    unsafe fn peek_last(&mut self) -> Option<QF::Item> {
         if self.current_index > 0 {
-            if Q::Fetch::IS_DENSE && F::Fetch::IS_DENSE {
+            if QF::IS_DENSE && F::Fetch::IS_DENSE {
                 Some(self.fetch.table_fetch(self.current_index - 1))
             } else {
                 Some(self.fetch.archetype_fetch(self.current_index - 1))
@@ -487,13 +445,13 @@ where
     // QueryIter, QueryIterationCursor, QueryState::for_each_unchecked_manual, QueryState::par_for_each_unchecked_manual
     // We can't currently reuse QueryIterationCursor in QueryIter for performance reasons. See #1763 for context.
     #[inline(always)]
-    unsafe fn next<'w>(
+    unsafe fn next(
         &mut self,
         tables: &'w Tables,
         archetypes: &'w Archetypes,
         query_state: &'s QueryState<Q, F>,
-    ) -> Option<<Q::Fetch as Fetch<'w, 's>>::Item> {
+    ) -> Option<QF::Item> {
-        if Q::Fetch::IS_DENSE && F::Fetch::IS_DENSE {
+        if QF::IS_DENSE && F::Fetch::IS_DENSE {
             loop {
                 if self.current_index == self.current_len {
                     let table_id = self.table_id_iter.next()?;

crates/bevy_ecs/src/query/state.rs

@@ -3,8 +3,8 @@ use crate::{
     component::ComponentId,
     entity::Entity,
     query::{
-        Access, Fetch, FetchState, FilterFetch, FilteredAccess, QueryCombinationIter, QueryIter,
+        Access, Fetch, FetchState, FilterFetch, FilteredAccess, NopFetch, QueryCombinationIter,
-        ReadOnlyFetch, WorldQuery,
+        QueryIter, WorldQuery,
     },
     storage::TableId,
     world::{World, WorldId},
@@ -73,11 +73,11 @@ where
     /// Checks if the query is empty for the given [`World`], where the last change and current tick are given.
     #[inline]
     pub fn is_empty(&self, world: &World, last_change_tick: u32, change_tick: u32) -> bool {
-        // SAFE: the iterator is instantly consumed via `none_remaining` and the implementation of
+        // SAFE: NopFetch does not access any members while &self ensures no one has exclusive access
-        // `QueryIter::none_remaining` never creates any references to the `<Q::Fetch as Fetch<'w>>::Item`.
         unsafe {
-            self.iter_unchecked_manual(world, last_change_tick, change_tick)
+            self.iter_unchecked_manual::<NopFetch<Q::State>>(world, last_change_tick, change_tick)
-                .none_remaining()
+                .next()
+                .is_none()
         }
     }
 
@@ -139,12 +139,17 @@ where
         &'s mut self,
         world: &'w World,
         entity: Entity,
-    ) -> Result<<Q::Fetch as Fetch<'w, 's>>::Item, QueryEntityError>
+    ) -> Result<<Q::ReadOnlyFetch as Fetch<'w, 's>>::Item, QueryEntityError> {
-    where
+        self.update_archetypes(world);
-        Q::Fetch: ReadOnlyFetch,
-    {
         // SAFETY: query is read only
-        unsafe { self.get_unchecked(world, entity) }
+        unsafe {
+            self.get_unchecked_manual::<Q::ReadOnlyFetch>(
+                world,
+                entity,
+                world.last_change_tick(),
+                world.read_change_tick(),
+            )
+        }
     }
 
     /// Gets the query result for the given [`World`] and [`Entity`].
@@ -154,8 +159,16 @@ where
         world: &'w mut World,
         entity: Entity,
     ) -> Result<<Q::Fetch as Fetch<'w, 's>>::Item, QueryEntityError> {
+        self.update_archetypes(world);
         // SAFETY: query has unique world access
-        unsafe { self.get_unchecked(world, entity) }
+        unsafe {
+            self.get_unchecked_manual::<Q::Fetch>(
+                world,
+                entity,
+                world.last_change_tick(),
+                world.read_change_tick(),
+            )
+        }
     }
 
     #[inline]
@@ -163,14 +176,11 @@ where
         &'s self,
         world: &'w World,
         entity: Entity,
-    ) -> Result<<Q::Fetch as Fetch<'w, 's>>::Item, QueryEntityError>
+    ) -> Result<<Q::ReadOnlyFetch as Fetch<'w, 's>>::Item, QueryEntityError> {
-    where
-        Q::Fetch: ReadOnlyFetch,
-    {
         self.validate_world(world);
         // SAFETY: query is read only and world is validated
         unsafe {
-            self.get_unchecked_manual(
+            self.get_unchecked_manual::<Q::ReadOnlyFetch>(
                 world,
                 entity,
                 world.last_change_tick(),
@@ -192,7 +202,7 @@ where
         entity: Entity,
     ) -> Result<<Q::Fetch as Fetch<'w, 's>>::Item, QueryEntityError> {
         self.update_archetypes(world);
-        self.get_unchecked_manual(
+        self.get_unchecked_manual::<Q::Fetch>(
             world,
             entity,
             world.last_change_tick(),
@@ -207,13 +217,13 @@ where
     ///
     /// This does not check for mutable query correctness. To be safe, make sure mutable queries
     /// have unique access to the components they query.
-    pub unsafe fn get_unchecked_manual<'w, 's>(
+    pub(crate) unsafe fn get_unchecked_manual<'w, 's, QF: Fetch<'w, 's, State = Q::State>>(
         &'s self,
         world: &'w World,
         entity: Entity,
         last_change_tick: u32,
         change_tick: u32,
-    ) -> Result<<Q::Fetch as Fetch<'w, 's>>::Item, QueryEntityError> {
+    ) -> Result<QF::Item, QueryEntityError> {
         let location = world
             .entities
             .get(entity)
@@ -225,8 +235,7 @@ where
             return Err(QueryEntityError::QueryDoesNotMatch);
         }
         let archetype = &world.archetypes[location.archetype_id];
-        let mut fetch =
+        let mut fetch = QF::init(world, &self.fetch_state, last_change_tick, change_tick);
-            <Q::Fetch as Fetch>::init(world, &self.fetch_state, last_change_tick, change_tick);
         let mut filter =
             <F::Fetch as Fetch>::init(world, &self.filter_state, last_change_tick, change_tick);
 
@@ -243,19 +252,28 @@ where
     ///
     /// This can only be called for read-only queries, see [`Self::iter_mut`] for write-queries.
     #[inline]
-    pub fn iter<'w, 's>(&'s mut self, world: &'w World) -> QueryIter<'w, 's, Q, F>
+    pub fn iter<'w, 's>(
-    where
+        &'s mut self,
-        Q::Fetch: ReadOnlyFetch,
+        world: &'w World,
-    {
+    ) -> QueryIter<'w, 's, Q, Q::ReadOnlyFetch, F> {
         // SAFETY: query is read only
-        unsafe { self.iter_unchecked(world) }
+        unsafe {
+            self.update_archetypes(world);
+            self.iter_unchecked_manual(world, world.last_change_tick(), world.read_change_tick())
+        }
     }
 
     /// Returns an [`Iterator`] over the query results for the given [`World`].
     #[inline]
-    pub fn iter_mut<'w, 's>(&'s mut self, world: &'w mut World) -> QueryIter<'w, 's, Q, F> {
+    pub fn iter_mut<'w, 's>(
+        &'s mut self,
+        world: &'w mut World,
+    ) -> QueryIter<'w, 's, Q, Q::Fetch, F> {
         // SAFETY: query has unique world access
-        unsafe { self.iter_unchecked(world) }
+        unsafe {
+            self.update_archetypes(world);
+            self.iter_unchecked_manual(world, world.last_change_tick(), world.read_change_tick())
+        }
     }
 
     /// Returns an [`Iterator`] over all possible combinations of `K` query results without repetition.
@@ -269,10 +287,10 @@ where
     /// This can only be called for read-only queries, see [`Self::iter_combinations_mut`] for
     /// write-queries.
     #[inline]
-    pub fn iter_manual<'w, 's>(&'s self, world: &'w World) -> QueryIter<'w, 's, Q, F>
+    pub fn iter_manual<'w, 's>(
-    where
+        &'s self,
-        Q::Fetch: ReadOnlyFetch,
+        world: &'w World,
-    {
+    ) -> QueryIter<'w, 's, Q, Q::ReadOnlyFetch, F> {
         self.validate_world(world);
         // SAFETY: query is read only and world is validated
         unsafe {
@@ -294,12 +312,16 @@ where
     pub fn iter_combinations<'w, 's, const K: usize>(
         &'s mut self,
         world: &'w World,
-    ) -> QueryCombinationIter<'w, 's, Q, F, K>
+    ) -> QueryCombinationIter<'w, 's, Q, Q::ReadOnlyFetch, F, K> {
-    where
-        Q::Fetch: ReadOnlyFetch,
-    {
         // SAFE: query is read only
-        unsafe { self.iter_combinations_unchecked(world) }
+        unsafe {
+            self.update_archetypes(world);
+            self.iter_combinations_unchecked_manual(
+                world,
+                world.last_change_tick(),
+                world.read_change_tick(),
+            )
+        }
     }
 
     /// Iterates over all possible combinations of `K` query results for the given [`World`]
@@ -313,9 +335,16 @@ where
     pub fn iter_combinations_mut<'w, 's, const K: usize>(
         &'s mut self,
         world: &'w mut World,
-    ) -> QueryCombinationIter<'w, 's, Q, F, K> {
+    ) -> QueryCombinationIter<'w, 's, Q, Q::Fetch, F, K> {
         // SAFE: query has unique world access
-        unsafe { self.iter_combinations_unchecked(world) }
+        unsafe {
+            self.update_archetypes(world);
+            self.iter_combinations_unchecked_manual(
+                world,
+                world.last_change_tick(),
+                world.read_change_tick(),
+            )
+        }
     }
 
     /// Returns an [`Iterator`] over the query results for the given [`World`].
@@ -328,7 +357,7 @@ where
     pub unsafe fn iter_unchecked<'w, 's>(
         &'s mut self,
         world: &'w World,
-    ) -> QueryIter<'w, 's, Q, F> {
+    ) -> QueryIter<'w, 's, Q, Q::Fetch, F> {
         self.update_archetypes(world);
         self.iter_unchecked_manual(world, world.last_change_tick(), world.read_change_tick())
     }
@@ -345,7 +374,7 @@ where
     pub unsafe fn iter_combinations_unchecked<'w, 's, const K: usize>(
         &'s mut self,
         world: &'w World,
-    ) -> QueryCombinationIter<'w, 's, Q, F, K> {
+    ) -> QueryCombinationIter<'w, 's, Q, Q::Fetch, F, K> {
         self.update_archetypes(world);
         self.iter_combinations_unchecked_manual(
             world,
@@ -364,12 +393,12 @@ where
     /// This does not validate that `world.id()` matches `self.world_id`. Calling this on a `world`
     /// with a mismatched WorldId is unsound.
     #[inline]
-    pub(crate) unsafe fn iter_unchecked_manual<'w, 's>(
+    pub(crate) unsafe fn iter_unchecked_manual<'w, 's, QF: Fetch<'w, 's, State = Q::State>>(
         &'s self,
         world: &'w World,
         last_change_tick: u32,
         change_tick: u32,
-    ) -> QueryIter<'w, 's, Q, F> {
+    ) -> QueryIter<'w, 's, Q, QF, F> {
         QueryIter::new(world, self, last_change_tick, change_tick)
     }
 
@@ -384,12 +413,17 @@ where
     /// This does not validate that `world.id()` matches `self.world_id`. Calling this on a `world`
     /// with a mismatched WorldId is unsound.
     #[inline]
-    pub(crate) unsafe fn iter_combinations_unchecked_manual<'w, 's, const K: usize>(
+    pub(crate) unsafe fn iter_combinations_unchecked_manual<
+        'w,
+        's,
+        QF: Fetch<'w, 's, State = Q::State>,
+        const K: usize,
+    >(
         &'s self,
         world: &'w World,
         last_change_tick: u32,
         change_tick: u32,
-    ) -> QueryCombinationIter<'w, 's, Q, F, K> {
+    ) -> QueryCombinationIter<'w, 's, Q, QF, F, K> {
         QueryCombinationIter::new(world, self, last_change_tick, change_tick)
     }
 
@@ -398,30 +432,40 @@ where
     ///
     /// This can only be called for read-only queries, see [`Self::for_each_mut`] for write-queries.
     #[inline]
-    pub fn for_each<'w, 's>(
+    pub fn for_each<'w, 's, FN: FnMut(<Q::ReadOnlyFetch as Fetch<'w, 's>>::Item)>(
         &'s mut self,
         world: &'w World,
-        func: impl FnMut(<Q::Fetch as Fetch<'w, 's>>::Item),
+        func: FN,
-    ) where
+    ) {
-        Q::Fetch: ReadOnlyFetch,
-    {
         // SAFETY: query is read only
         unsafe {
-            self.for_each_unchecked(world, func);
+            self.update_archetypes(world);
+            self.for_each_unchecked_manual::<Q::ReadOnlyFetch, FN>(
+                world,
+                func,
+                world.last_change_tick(),
+                world.read_change_tick(),
+            );
         }
     }
 
     /// Runs `func` on each query result for the given [`World`]. This is faster than the equivalent
     /// iter_mut() method, but cannot be chained like a normal [`Iterator`].
     #[inline]
-    pub fn for_each_mut<'w, 's>(
+    pub fn for_each_mut<'w, 's, FN: FnMut(<Q::Fetch as Fetch<'w, 's>>::Item)>(
         &'s mut self,
         world: &'w mut World,
-        func: impl FnMut(<Q::Fetch as Fetch<'w, 's>>::Item),
+        func: FN,
     ) {
         // SAFETY: query has unique world access
         unsafe {
-            self.for_each_unchecked(world, func);
+            self.update_archetypes(world);
+            self.for_each_unchecked_manual::<Q::Fetch, FN>(
+                world,
+                func,
+                world.last_change_tick(),
+                world.read_change_tick(),
+            );
         }
     }
 
@@ -435,13 +479,13 @@ where
     /// This does not check for mutable query correctness. To be safe, make sure mutable queries
     /// have unique access to the components they query.
     #[inline]
-    pub unsafe fn for_each_unchecked<'w, 's>(
+    pub unsafe fn for_each_unchecked<'w, 's, FN: FnMut(<Q::Fetch as Fetch<'w, 's>>::Item)>(
         &'s mut self,
         world: &'w World,
-        func: impl FnMut(<Q::Fetch as Fetch<'w, 's>>::Item),
+        func: FN,
     ) {
         self.update_archetypes(world);
-        self.for_each_unchecked_manual(
+        self.for_each_unchecked_manual::<Q::Fetch, FN>(
             world,
             func,
             world.last_change_tick(),
@@ -454,33 +498,55 @@ where
     /// This can only be called for read-only queries, see [`Self::par_for_each_mut`] for
     /// write-queries.
     #[inline]
-    pub fn par_for_each<'w, 's>(
+    pub fn par_for_each<
+        'w,
+        's,
+        FN: Fn(<Q::ReadOnlyFetch as Fetch<'w, 's>>::Item) + Send + Sync + Clone,
+    >(
         &'s mut self,
         world: &'w World,
         task_pool: &TaskPool,
         batch_size: usize,
-        func: impl Fn(<Q::Fetch as Fetch<'w, 's>>::Item) + Send + Sync + Clone,
+        func: FN,
-    ) where
+    ) {
-        Q::Fetch: ReadOnlyFetch,
-    {
         // SAFETY: query is read only
         unsafe {
-            self.par_for_each_unchecked(world, task_pool, batch_size, func);
+            self.update_archetypes(world);
+            self.par_for_each_unchecked_manual::<Q::ReadOnlyFetch, FN>(
+                world,
+                task_pool,
+                batch_size,
+                func,
+                world.last_change_tick(),
+                world.read_change_tick(),
+            );
         }
     }
 
     /// Runs `func` on each query result in parallel using the given `task_pool`.
     #[inline]
-    pub fn par_for_each_mut<'w, 's>(
+    pub fn par_for_each_mut<
+        'w,
+        's,
+        FN: Fn(<Q::Fetch as Fetch<'w, 's>>::Item) + Send + Sync + Clone,
+    >(
         &'s mut self,
         world: &'w mut World,
         task_pool: &TaskPool,
         batch_size: usize,
-        func: impl Fn(<Q::Fetch as Fetch<'w, 's>>::Item) + Send + Sync + Clone,
+        func: FN,
     ) {
         // SAFETY: query has unique world access
         unsafe {
-            self.par_for_each_unchecked(world, task_pool, batch_size, func);
+            self.update_archetypes(world);
+            self.par_for_each_unchecked_manual::<Q::Fetch, FN>(
+                world,
+                task_pool,
+                batch_size,
+                func,
+                world.last_change_tick(),
+                world.read_change_tick(),
+            );
         }
     }
 
@@ -493,15 +559,19 @@ where
     /// This does not check for mutable query correctness. To be safe, make sure mutable queries
     /// have unique access to the components they query.
     #[inline]
-    pub unsafe fn par_for_each_unchecked<'w, 's>(
+    pub unsafe fn par_for_each_unchecked<
+        'w,
+        's,
+        FN: Fn(<Q::Fetch as Fetch<'w, 's>>::Item) + Send + Sync + Clone,
+    >(
         &'s mut self,
         world: &'w World,
         task_pool: &TaskPool,
         batch_size: usize,
-        func: impl Fn(<Q::Fetch as Fetch<'w, 's>>::Item) + Send + Sync + Clone,
+        func: FN,
     ) {
         self.update_archetypes(world);
-        self.par_for_each_unchecked_manual(
+        self.par_for_each_unchecked_manual::<Q::Fetch, FN>(
             world,
             task_pool,
             batch_size,
@@ -521,17 +591,21 @@ where
     /// have unique access to the components they query.
     /// This does not validate that `world.id()` matches `self.world_id`. Calling this on a `world`
     /// with a mismatched WorldId is unsound.
-    pub(crate) unsafe fn for_each_unchecked_manual<'w, 's>(
+    pub(crate) unsafe fn for_each_unchecked_manual<
+        'w,
+        's,
+        QF: Fetch<'w, 's, State = Q::State>,
+        FN: FnMut(QF::Item),
+    >(
         &'s self,
         world: &'w World,
-        mut func: impl FnMut(<Q::Fetch as Fetch<'w, 's>>::Item),
+        mut func: FN,
         last_change_tick: u32,
         change_tick: u32,
     ) {
         // NOTE: If you are changing query iteration code, remember to update the following places, where relevant:
         // QueryIter, QueryIterationCursor, QueryState::for_each_unchecked_manual, QueryState::par_for_each_unchecked_manual
-        let mut fetch =
+        let mut fetch = QF::init(world, &self.fetch_state, last_change_tick, change_tick);
-            <Q::Fetch as Fetch>::init(world, &self.fetch_state, last_change_tick, change_tick);
         let mut filter =
             <F::Fetch as Fetch>::init(world, &self.filter_state, last_change_tick, change_tick);
         if Q::Fetch::IS_DENSE && F::Fetch::IS_DENSE {
@@ -577,19 +651,24 @@ where
     /// have unique access to the components they query.
     /// This does not validate that `world.id()` matches `self.world_id`. Calling this on a `world`
     /// with a mismatched WorldId is unsound.
-    pub unsafe fn par_for_each_unchecked_manual<'w, 's>(
+    pub(crate) unsafe fn par_for_each_unchecked_manual<
+        'w,
+        's,
+        QF: Fetch<'w, 's, State = Q::State>,
+        FN: Fn(QF::Item) + Send + Sync + Clone,
+    >(
         &'s self,
         world: &'w World,
         task_pool: &TaskPool,
         batch_size: usize,
-        func: impl Fn(<Q::Fetch as Fetch<'w, 's>>::Item) + Send + Sync + Clone,
+        func: FN,
         last_change_tick: u32,
         change_tick: u32,
     ) {
         // NOTE: If you are changing query iteration code, remember to update the following places, where relevant:
         // QueryIter, QueryIterationCursor, QueryState::for_each_unchecked_manual, QueryState::par_for_each_unchecked_manual
         task_pool.scope(|scope| {
-            if Q::Fetch::IS_DENSE && F::Fetch::IS_DENSE {
+            if QF::IS_DENSE && F::Fetch::IS_DENSE {
                 let tables = &world.storages().tables;
                 for table_id in self.matched_table_ids.iter() {
                     let table = &tables[*table_id];
@@ -597,12 +676,8 @@ where
                     while offset < table.len() {
                         let func = func.clone();
                         scope.spawn(async move {
-                            let mut fetch = <Q::Fetch as Fetch>::init(
+                            let mut fetch =
-                                world,
+                                QF::init(world, &self.fetch_state, last_change_tick, change_tick);
-                                &self.fetch_state,
-                                last_change_tick,
-                                change_tick,
-                            );
                             let mut filter = <F::Fetch as Fetch>::init(
                                 world,
                                 &self.filter_state,
@@ -633,12 +708,8 @@ where
                     while offset < archetype.len() {
                         let func = func.clone();
                         scope.spawn(async move {
-                            let mut fetch = <Q::Fetch as Fetch>::init(
+                            let mut fetch =
-                                world,
+                                QF::init(world, &self.fetch_state, last_change_tick, change_tick);
-                                &self.fetch_state,
-                                last_change_tick,
-                                change_tick,
-                            );
                             let mut filter = <F::Fetch as Fetch>::init(
                                 world,
                                 &self.filter_state,

crates/bevy_ecs/src/system/mod.rs

@@ -785,4 +785,29 @@ mod tests {
             }
         }
     }
+
+    #[test]
+    fn immutable_mut_test() {
+        #[derive(Component, Eq, PartialEq, Debug, Clone, Copy)]
+        struct A(usize);
+
+        let mut world = World::default();
+        world.spawn().insert(A(1));
+        world.spawn().insert(A(2));
+
+        let mut system_state = SystemState::<Query<&mut A>>::new(&mut world);
+        {
+            let mut query = system_state.get_mut(&mut world);
+            assert_eq!(
+                query.iter_mut().map(|m| *m).collect::<Vec<A>>(),
+                vec![A(1), A(2)],
+                "both components returned by iter_mut of &mut"
+            );
+            assert_eq!(
+                query.iter().collect::<Vec<&A>>(),
+                vec![&A(1), &A(2)],
+                "both components returned by iter of &mut"
+            );
+        }
+    }
 }

crates/bevy_ecs/src/system/query.rs

@@ -3,7 +3,7 @@ use crate::{
     entity::Entity,
     query::{
         Fetch, FilterFetch, QueryCombinationIter, QueryEntityError, QueryIter, QueryState,
-        ReadOnlyFetch, WorldQuery,
+        WorldQuery,
     },
     world::{Mut, World},
 };
@@ -277,8 +277,8 @@ where
 
     /// Returns an [`Iterator`] over the query results.
     ///
-    /// This can only be called for read-only queries (due to the [`ReadOnlyFetch`] trait
+    /// This can only return immutable data (mutable data will be cast to an immutable form).
-    /// bound). See [`Self::iter_mut`] for queries that contain at least one mutable component.
+    /// See [`Self::iter_mut`] for queries that contain at least one mutable component.
     ///
     /// # Example
     ///
@@ -299,10 +299,7 @@ where
     /// # report_names_system.system();
     /// ```
     #[inline]
-    pub fn iter(&'s self) -> QueryIter<'w, 's, Q, F>
+    pub fn iter(&'s self) -> QueryIter<'w, 's, Q, Q::ReadOnlyFetch, F> {
-    where
-        Q::Fetch: ReadOnlyFetch,
-    {
         // SAFE: system runs without conflicts with other systems.
         // same-system queries have runtime borrow checks when they conflict
         unsafe {
@@ -332,7 +329,7 @@ where
     /// # gravity_system.system();
     /// ```
     #[inline]
-    pub fn iter_mut(&mut self) -> QueryIter<'_, '_, Q, F> {
+    pub fn iter_mut(&mut self) -> QueryIter<'_, '_, Q, Q::Fetch, F> {
         // SAFE: system runs without conflicts with other systems.
         // same-system queries have runtime borrow checks when they conflict
         unsafe {
@@ -342,17 +339,16 @@ where
     }
 
     /// Returns an [`Iterator`] over all possible combinations of `K` query results without repetition.
-    /// This can only be called for read-only queries
+    /// This can only return immutable data
     ///
     ///  For permutations of size K of query returning N results, you will get:
     /// - if K == N: one permutation of all query results
     /// - if K < N: all possible K-sized combinations of query results, without repetition
     /// - if K > N: empty set (no K-sized combinations exist)
     #[inline]
-    pub fn iter_combinations<const K: usize>(&self) -> QueryCombinationIter<'_, '_, Q, F, K>
+    pub fn iter_combinations<const K: usize>(
-    where
+        &self,
-        Q::Fetch: ReadOnlyFetch,
+    ) -> QueryCombinationIter<'_, '_, Q, Q::ReadOnlyFetch, F, K> {
-    {
         // SAFE: system runs without conflicts with other systems.
         // same-system queries have runtime borrow checks when they conflict
         unsafe {
@@ -389,7 +385,7 @@ where
     #[inline]
     pub fn iter_combinations_mut<const K: usize>(
         &mut self,
-    ) -> QueryCombinationIter<'_, '_, Q, F, K> {
+    ) -> QueryCombinationIter<'_, '_, Q, Q::Fetch, F, K> {
         // SAFE: system runs without conflicts with other systems.
         // same-system queries have runtime borrow checks when they conflict
         unsafe {
@@ -408,7 +404,7 @@ where
     /// This function makes it possible to violate Rust's aliasing guarantees. You must make sure
     /// this call does not result in multiple mutable references to the same component
     #[inline]
-    pub unsafe fn iter_unsafe(&self) -> QueryIter<'_, '_, Q, F> {
+    pub unsafe fn iter_unsafe(&'s self) -> QueryIter<'w, 's, Q, Q::Fetch, F> {
         // SEMI-SAFE: system runs without conflicts with other systems.
         // same-system queries have runtime borrow checks when they conflict
         self.state
@@ -424,7 +420,7 @@ where
     #[inline]
     pub unsafe fn iter_combinations_unsafe<const K: usize>(
         &self,
-    ) -> QueryCombinationIter<'_, '_, Q, F, K> {
+    ) -> QueryCombinationIter<'_, '_, Q, Q::Fetch, F, K> {
         // SEMI-SAFE: system runs without conflicts with other systems.
         // same-system queries have runtime borrow checks when they conflict
         self.state.iter_combinations_unchecked_manual(
@@ -437,7 +433,7 @@ where
     /// Runs `f` on each query result. This is faster than the equivalent iter() method, but cannot
     /// be chained like a normal [`Iterator`].
     ///
-    /// This can only be called for read-only queries, see [`Self::for_each_mut`] for write-queries.
+    /// This can only pass in immutable data, see [`Self::for_each_mut`] for mutable access.
     ///
     /// # Example
     ///
@@ -458,14 +454,12 @@ where
     /// # report_names_system.system();
     /// ```
     #[inline]
-    pub fn for_each(&'s self, f: impl FnMut(<Q::Fetch as Fetch<'w, 's>>::Item))
+    pub fn for_each<FN: FnMut(<Q::ReadOnlyFetch as Fetch<'w, 's>>::Item)>(&'s self, f: FN) {
-    where
-        Q::Fetch: ReadOnlyFetch,
-    {
         // 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(
+            self.state
+                .for_each_unchecked_manual::<Q::ReadOnlyFetch, FN>(
                     self.world,
                     f,
                     self.last_change_tick,
@@ -496,11 +490,11 @@ where
     /// # gravity_system.system();
     /// ```
     #[inline]
-    pub fn for_each_mut<'a>(&'a mut self, f: impl FnMut(<Q::Fetch as Fetch<'a, 'a>>::Item)) {
+    pub fn for_each_mut<'a, FN: FnMut(<Q::Fetch as Fetch<'a, 'a>>::Item)>(&'a mut self, f: FN) {
         // 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(
+            self.state.for_each_unchecked_manual::<Q::Fetch, FN>(
                 self.world,
                 f,
                 self.last_change_tick,
@@ -511,21 +505,20 @@ where
 
     /// Runs `f` on each query result in parallel using the given task pool.
     ///
-    /// This can only be called for read-only queries, see [`Self::par_for_each_mut`] for
+    /// This can only be called for immutable data, see [`Self::par_for_each_mut`] for
-    /// write-queries.
+    /// mutable access.
     #[inline]
-    pub fn par_for_each(
+    pub fn par_for_each<FN: Fn(<Q::ReadOnlyFetch as Fetch<'w, 's>>::Item) + Send + Sync + Clone>(
         &'s self,
         task_pool: &TaskPool,
         batch_size: usize,
-        f: impl Fn(<Q::Fetch as Fetch<'w, 's>>::Item) + Send + Sync + Clone,
+        f: FN,
-    ) where
+    ) {
-        Q::Fetch: ReadOnlyFetch,
-    {
         // 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(
+            self.state
+                .par_for_each_unchecked_manual::<Q::ReadOnlyFetch, FN>(
                     self.world,
                     task_pool,
                     batch_size,
@@ -538,16 +531,16 @@ where
 
     /// Runs `f` on each query result in parallel using the given task pool.
     #[inline]
-    pub fn par_for_each_mut<'a>(
+    pub fn par_for_each_mut<'a, FN: Fn(<Q::Fetch as Fetch<'a, 'a>>::Item) + Send + Sync + Clone>(
         &'a mut self,
         task_pool: &TaskPool,
         batch_size: usize,
-        f: impl Fn(<Q::Fetch as Fetch<'a, 'a>>::Item) + Send + Sync + Clone,
+        f: FN,
     ) {
         // 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(
+            self.state.par_for_each_unchecked_manual::<Q::Fetch, FN>(
                 self.world,
                 task_pool,
                 batch_size,
@@ -563,8 +556,8 @@ where
     /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is
     /// returned instead.
     ///
-    /// This can only be called for read-only queries (due to the [`ReadOnlyFetch`] trait bound).
+    /// 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.
+    /// See [`get_mut`](Self::get_mut) for queries that contain at least one mutable component.
     ///
     /// # Example
     ///
@@ -593,14 +586,11 @@ where
     pub fn get(
         &'s self,
         entity: Entity,
-    ) -> Result<<Q::Fetch as Fetch<'w, 's>>::Item, QueryEntityError>
+    ) -> Result<<Q::ReadOnlyFetch as Fetch<'w, 's>>::Item, QueryEntityError> {
-    where
-        Q::Fetch: ReadOnlyFetch,
-    {
         // SAFE: system runs without conflicts with other systems.
         // same-system queries have runtime borrow checks when they conflict
         unsafe {
-            self.state.get_unchecked_manual(
+            self.state.get_unchecked_manual::<Q::ReadOnlyFetch>(
                 self.world,
                 entity,
                 self.last_change_tick,
@@ -641,7 +631,7 @@ where
         // SAFE: system runs without conflicts with other systems.
         // same-system queries have runtime borrow checks when they conflict
         unsafe {
-            self.state.get_unchecked_manual(
+            self.state.get_unchecked_manual::<Q::Fetch>(
                 self.world,
                 entity,
                 self.last_change_tick,
@@ -661,13 +651,17 @@ where
     /// this call does not result in multiple mutable references to the same component
     #[inline]
     pub unsafe fn get_unchecked(
-        &self,
+        &'s self,
         entity: Entity,
-    ) -> Result<<Q::Fetch as Fetch>::Item, QueryEntityError> {
+    ) -> Result<<Q::Fetch as Fetch<'w, 's>>::Item, QueryEntityError> {
         // SEMI-SAFE: system runs without conflicts with other systems.
         // same-system queries have runtime borrow checks when they conflict
-        self.state
+        self.state.get_unchecked_manual::<Q::Fetch>(
-            .get_unchecked_manual(self.world, entity, self.last_change_tick, self.change_tick)
+            self.world,
+            entity,
+            self.last_change_tick,
+            self.change_tick,
+        )
     }
 
     /// Returns a reference to the [`Entity`]'s [`Component`] of the given type.
@@ -800,9 +794,8 @@ where
     /// Returns a single immutable query result when there is exactly one entity matching
     /// the query.
     ///
-    /// This can only be called for read-only queries (due to the [`ReadOnlyFetch`] trait
+    /// This can only return immutable data. Use [`single_mut`](Self::single_mut) for
-    /// bound). Use [`single_mut`](Self::single_mut) for queries that contain at least one
+    /// queries that contain at least one mutable component.
-    /// mutable component.
     ///
     /// # Example
     ///
@@ -824,19 +817,15 @@ 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::Fetch as Fetch<'w, 's>>::Item
+    pub fn single(&'s self) -> <Q::ReadOnlyFetch as Fetch<'w, 's>>::Item {
-    where
-        Q::Fetch: ReadOnlyFetch,
-    {
         self.get_single().unwrap()
     }
 
     /// Returns a single immutable query result when there is exactly one entity matching
     /// the query.
     ///
-    /// This can only be called for read-only queries (due to the [`ReadOnlyFetch`] trait
+    /// This can only return immutable data. Use [`get_single_mut`](Self::get_single_mut)
-    /// bound). Use [`get_single_mut`](Self::get_single_mut) for queries that contain at least one
+    /// for queries that contain at least one mutable component.
-    /// mutable component.
     ///
     /// If the number of query results is not exactly one, a [`QuerySingleError`] is returned
     /// instead.
@@ -863,10 +852,9 @@ where
     /// }
     /// # player_scoring_system.system();
     /// ```
-    pub fn get_single(&'s self) -> Result<<Q::Fetch as Fetch<'w, 's>>::Item, QuerySingleError>
+    pub fn get_single(
-    where
+        &'s self,
-        Q::Fetch: ReadOnlyFetch,
+    ) -> Result<<Q::ReadOnlyFetch as Fetch<'w, 's>>::Item, QuerySingleError> {
-    {
         let mut query = self.iter();
         let first = query.next();
         let extra = query.next().is_some();
@@ -970,8 +958,6 @@ where
     /// ```
     #[inline]
     pub fn is_empty(&self) -> bool {
-        // TODO: This code can be replaced with `self.iter().next().is_none()` if/when
-        // we sort out how to convert "write" queries to "read" queries.
         self.state
             .is_empty(self.world, self.last_change_tick, self.change_tick)
     }

crates/bevy_ecs_compile_fail_tests/tests/ui/system_state_iter_mut_overlap_safety.rs

@@ -0,0 +1,28 @@
+use bevy_ecs::prelude::*;
+use bevy_ecs::system::SystemState;
+
+#[derive(Component, Eq, PartialEq, Debug, Clone, Copy)]
+struct A(usize);
+
+fn main() {
+    let mut world = World::default();
+    world.spawn().insert(A(1));
+    world.spawn().insert(A(2));
+
+    let mut system_state = SystemState::<Query<&mut A>>::new(&mut world);
+    {
+        let mut query = system_state.get_mut(&mut world);
+        let mut_vec = query.iter_mut().collect::<Vec<bevy_ecs::prelude::Mut<A>>>();
+        assert_eq!(
+            // this should fail to compile due to the later use of mut_vec
+            query.iter().collect::<Vec<&A>>(),
+            vec![&A(1), &A(2)],
+            "both components returned by iter of &mut"
+        );
+        assert_eq!(
+            mut_vec.iter().map(|m| **m).collect::<Vec<A>>(),
+            vec![A(1), A(2)],
+            "both components returned by iter_mut of &mut"
+        );
+    }
+}

crates/bevy_ecs_compile_fail_tests/tests/ui/system_state_iter_mut_overlap_safety.stderr

@@ -0,0 +1,11 @@
+error[E0502]: cannot borrow `query` as immutable because it is also borrowed as mutable
+  --> tests/ui/system_state_iter_mut_overlap_safety.rs:18:13
+   |
+15 |         let mut_vec = query.iter_mut().collect::<Vec<bevy_ecs::prelude::Mut<A>>>();
+   |                       ----- mutable borrow occurs here
+...
+18 |             query.iter().collect::<Vec<&A>>(),
+   |             ^^^^^ immutable borrow occurs here
+...
+23 |             mut_vec.iter().map(|m| **m).collect::<Vec<A>>(),
+   |             ------- mutable borrow later used here

pipelined/bevy_render2/src/render_component.rs

@@ -8,7 +8,7 @@ use bevy_asset::{Asset, Handle};
 use bevy_ecs::{
     component::Component,
     prelude::*,
-    query::{FilterFetch, QueryItem, ReadOnlyFetch, WorldQuery},
+    query::{FilterFetch, QueryItem, WorldQuery},
     system::{
         lifetimeless::{Read, SCommands, SQuery},
         RunSystem, SystemParamItem,
@@ -141,7 +141,6 @@ impl<C, F> Default for ExtractComponentPlugin<C, F> {
 
 impl<C: ExtractComponent> Plugin for ExtractComponentPlugin<C>
 where
-    <C::Query as WorldQuery>::Fetch: ReadOnlyFetch,
     <C::Filter as WorldQuery>::Fetch: FilterFetch,
 {
     fn build(&self, app: &mut App) {
@@ -167,7 +166,6 @@ pub struct ExtractComponentSystem<C: ExtractComponent>(PhantomData<C>);
 impl<C: ExtractComponent> RunSystem for ExtractComponentSystem<C>
 where
     <C::Filter as WorldQuery>::Fetch: FilterFetch,
-    <C::Query as WorldQuery>::Fetch: ReadOnlyFetch,
 {
     type Param = (
         SCommands,
@@ -176,9 +174,9 @@ where
         SQuery<(Entity, C::Query), C::Filter>,
     );
 
-    fn run((mut commands, mut previous_len, query): SystemParamItem<Self::Param>) {
+    fn run((mut commands, mut previous_len, mut query): SystemParamItem<Self::Param>) {
         let mut values = Vec::with_capacity(*previous_len);
-        for (entity, query_item) in query.iter() {
+        for (entity, query_item) in query.iter_mut() {
             values.push((entity, (C::extract_component(query_item),)));
         }
         *previous_len = values.len();