Make QueryFilter an unsafe trait (#14790)

# Objective

It's possible to create UB using an implementation of `QueryFilter` that
performs mutable access, but that does not violate any documented safety
invariants.

This code: 
```rust
#[derive(Component)]
struct Foo(usize);

// This derive is a simple way to get a valid WorldQuery impl.  The QueryData impl isn't used.
#[derive(QueryData)]
#[query_data(mutable)]
struct BadFilter<'w> {
    foo: &'w mut Foo,
}

impl QueryFilter for BadFilter<'_> {
    const IS_ARCHETYPAL: bool = false;

    unsafe fn filter_fetch(
        fetch: &mut Self::Fetch<'_>,
        entity: Entity,
        table_row: TableRow,
    ) -> bool {
        // SAFETY: fetch and filter_fetch have the same safety requirements
        let f: &mut usize = &mut unsafe { Self::fetch(fetch, entity, table_row) }.foo.0;
        println!("Got &mut at     {f:p}");
        true
    }
}

let mut world = World::new();
world.spawn(Foo(0));
world.run_system_once(|query: Query<&Foo, BadFilter>| {
    let f: &usize = &query.iter().next().unwrap().0;
    println!("Got & at        {f:p}");
    query.iter().next().unwrap();
    println!("Still have & at {f:p}");
});
```

prints: 

```
Got &mut at     0x1924b92dfb0
Got & at        0x1924b92dfb0
Got &mut at     0x1924b92dfb0
Still have & at 0x1924b92dfb0
```

Which means it had an `&` and `&mut` alive at the same time.

The only `unsafe` there is around `Self::fetch`, but I believe that call
correctly upholds the safety invariant, and matches what `Added` and
`Changed` do.


## Solution

Make `QueryFilter` an unsafe trait and document the requirement that the
`WorldQuery` implementation be read-only.

## Migration Guide

`QueryFilter` is now an `unsafe trait`. If you were manually
implementing it, you will need to verify that the `WorldQuery`
implementation is read-only and then add the `unsafe` keyword to the
`impl`.

Author: chescock

crates/bevy_ecs/macros/src/query_filter.rs

@@ -120,7 +120,8 @@ pub fn derive_query_filter_impl(input: TokenStream) -> TokenStream {
     );
 
     let filter_impl = quote! {
-        impl #user_impl_generics #path::query::QueryFilter
+        // SAFETY: This only performs access that subqueries perform, and they impl `QueryFilter` and so perform no mutable access.
+        unsafe impl #user_impl_generics #path::query::QueryFilter
         for #struct_name #user_ty_generics #user_where_clauses {
             const IS_ARCHETYPAL: bool = true #(&& <#field_types>::IS_ARCHETYPAL)*;
 

crates/bevy_ecs/src/query/filter.rs

@@ -70,12 +70,17 @@ use std::{cell::UnsafeCell, marker::PhantomData};
 /// [`matches_component_set`]: Self::matches_component_set
 /// [`Query`]: crate::system::Query
 /// [`State`]: Self::State
+///
+/// # Safety
+///
+/// The [`WorldQuery`] implementation must not take any mutable access.
+/// This is the same safety requirement as [`ReadOnlyQueryData`](crate::query::ReadOnlyQueryData).
 #[diagnostic::on_unimplemented(
     message = "`{Self}` is not a valid `Query` filter",
     label = "invalid `Query` filter",
     note = "a `QueryFilter` typically uses a combination of `With<T>` and `Without<T>` statements"
 )]
-pub trait QueryFilter: WorldQuery {
+pub unsafe trait QueryFilter: WorldQuery {
     /// Returns true if (and only if) this Filter relies strictly on archetypes to limit which
     /// components are accessed by the Query.
     ///
@@ -201,7 +206,8 @@ unsafe impl<T: Component> WorldQuery for With<T> {
     }
 }
 
-impl<T: Component> QueryFilter for With<T> {
+// SAFETY: WorldQuery impl performs no access at all
+unsafe impl<T: Component> QueryFilter for With<T> {
     const IS_ARCHETYPAL: bool = true;
 
     #[inline(always)]
@@ -311,7 +317,8 @@ unsafe impl<T: Component> WorldQuery for Without<T> {
     }
 }
 
-impl<T: Component> QueryFilter for Without<T> {
+// SAFETY: WorldQuery impl performs no access at all
+unsafe impl<T: Component> QueryFilter for Without<T> {
     const IS_ARCHETYPAL: bool = true;
 
     #[inline(always)]
@@ -490,7 +497,8 @@ macro_rules! impl_or_query_filter {
             }
         }
 
-        impl<$($filter: QueryFilter),*> QueryFilter for Or<($($filter,)*)> {
+            // SAFETY: This only performs access that subqueries perform, and they impl `QueryFilter` and so perform no mutable access.
+            unsafe impl<$($filter: QueryFilter),*> QueryFilter for Or<($($filter,)*)> {
             const IS_ARCHETYPAL: bool = true $(&& $filter::IS_ARCHETYPAL)*;
 
             #[inline(always)]
@@ -512,7 +520,8 @@ macro_rules! impl_tuple_query_filter {
         #[allow(non_snake_case)]
         #[allow(clippy::unused_unit)]
         $(#[$meta])*
-        impl<$($name: QueryFilter),*> QueryFilter for ($($name,)*) {
+        // SAFETY: This only performs access that subqueries perform, and they impl `QueryFilter` and so perform no mutable access.
+        unsafe impl<$($name: QueryFilter),*> QueryFilter for ($($name,)*) {
             const IS_ARCHETYPAL: bool = true $(&& $name::IS_ARCHETYPAL)*;
 
             #[inline(always)]
@@ -734,7 +743,8 @@ unsafe impl<T: Component> WorldQuery for Added<T> {
     }
 }
 
-impl<T: Component> QueryFilter for Added<T> {
+// SAFETY: WorldQuery impl performs only read access on ticks
+unsafe impl<T: Component> QueryFilter for Added<T> {
     const IS_ARCHETYPAL: bool = false;
     #[inline(always)]
     unsafe fn filter_fetch(
@@ -949,7 +959,8 @@ unsafe impl<T: Component> WorldQuery for Changed<T> {
     }
 }
 
-impl<T: Component> QueryFilter for Changed<T> {
+// SAFETY: WorldQuery impl performs only read access on ticks
+unsafe impl<T: Component> QueryFilter for Changed<T> {
     const IS_ARCHETYPAL: bool = false;
 
     #[inline(always)]