Make QueryFilter an unsafe trait

[chescock]

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:

#[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.