Soundness issue with random access API within a system

[nolanderc]

The following snippet demonstrates the issue (only occurs when building with --release):

use legion::prelude::*;

fn main() {
    let mut world = World::new();

    world.insert((), Some((14u32,)));

    let query = <Write<u32>>::query();
    let system = SystemBuilder::new("soundness issue")
        .write_component::<u32>()
        .with_query(query)
        .build(|_, world, _, query| {
            for (entity, mut number) in query.iter_entities(world) {
                dbg!(&number);  // prints 14
                *number += 1;
                dbg!(&number);  // prints 15

                let mut number2 = world.get_component_mut::<u32>(entity).unwrap();
                dbg!(&number);  // prints 15
                dbg!(&number2); // prints 15
                *number2 += 1;

                dbg!(&number);  // prints 16
                dbg!(&number2); // prints 16
            }
        });

    system.run(&world);
}

As far as I can tell this is due to some lifetime issue in Query::iter_entities or the fact that runtime borrow checking is disabled in release builds. While I am not against such an optimization, this clearly violates Rust's soundness and aliasing guarantees.

For users who still need/want such an optimization, would it be possible to make this an opt-in feature instead? This has the benefit of making potential issues more visible to the developer.

[gzp79]

Just thinking outloud. Why is there a world parameter for the function in the build? It is against the rule of rust borrow checker. Like sending a vector and a slice of it mutable to the same function. One may use refcell and alike and the runtime can check it but what if the world parameter is removed? If the parameter is not a simple query, but a query bound to a world, ex a new wrapper struct created at the time of execution. Thus it can be iterated without a world parameter. The other use of the world parameter is to get some component in the system without a query.

Instead of this i'm suggesting to have a (world bound) default query that can be used to perform any query on the whole world. Or the world could implement a query interface. And thus during execution this common archtype based query or the whole world is tranformed into a query that can be iterated, further filtered, split by component etc, but this time with the rust lifetime (I'm saying it without knowing the internals).

[Guvante]

It looks like the SubWorld version of iter_entities was missing some lifetime annotations so the compiler wasn't able to notice that iter_entities immutably borrows from world. SubWorld holds onto pointers for lifetime erasure reasons so the wrong lifetime didn't stop it from compiling internally.

Adding the correct lifetimes causes the expected E0499.