Component iteration without modification tracking

[parasyte]

Since 0e2d67a, IntoIter now always yields Mut<T> for mutable components. In the latest published release, this was only true for components with tracking enabled, e.g. using the derive macro with #[track(Modification)].

There is one big problem with this new approach because field accesses to mutable components go through DerefMut which is not supported by two-phase borrows with operator traits like AddAssign. See:

• Borrow compilation error with vec.swap(0, vec.len() - 1), while vec.push(vec.len()) works. rust-lang/rust#74319
• Getter functions and indexing don't seem to follow the same lifetime rules rust-lang/rust#58419
• ... and others linked in these issues.

Any component that has a struct field with a bitwise assignment operator implementation will have borrow check errors for code that normally wouldn't (without the DerefMut). An example using glam::Vec2 for its AddAssign impl:

use glam::f32::Vec2;
use shipyard::{Component, IntoIter as _, ViewMut, World};

#[derive(Component, Debug)]
struct Calc {
    vec: Vec2,
    scale: f32,
}

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

    world.run(|mut calc: ViewMut<Calc>| {
        for mut calc in (&mut calc).iter() {
            calc.vec += calc.scale;
        }
    });
}

This works on the released version (0.6.2, with a warning about an unnecessary mutable variable), but fails on git HEAD:

error[E0502]: cannot borrow `calc` as immutable because it is also borrowed as mutable
  --> src\main.rs:15:25
   |
15 |             calc.vec += calc.scale;
   |             ------------^^^^------
   |             |           |
   |             |           immutable borrow occurs here
   |             mutable borrow occurs here
   |             mutable borrow later used here
   |
help: try adding a local storing this...
  --> src\main.rs:15:25
   |
15 |             calc.vec += calc.scale;
   |                         ^^^^^^^^^^
help: ...and then using that local here
  --> src\main.rs:15:13
   |
15 |             calc.vec += calc.scale;
   |             ^^^^^^^^^^^^^^^^^^^^^^

The workaround is trivial, just bind calc.scale to a temporary variable and add-assign with that on the right-hand side. But this is a pretty clear breaking change, and also a gnarly UX.

It would be nice to be able to get &mut Calc out of the iterator instead of Mut<Calc>. Both because it causes this issue, and because I don't need mutation tracking. That sounds like unnecessary overhead for default iteration behavior, doesn't it?

I am not sure what motivated the change, maybe #157 (comment) had something to do with it?

[leudz]

The main motivations are listed here: https://shipyard.zulipchat.com/#narrow/stream/269448-update/topic/On.20demand.20tracking/near/325747110.
I agree it's not perfect, Mut is annoying and a cargo feature can't be added to solve the problem.

I think this is the third or forth iteration on the modification tracking design. So far they all come with some big problem but some of them can't even be worked around.

[parasyte]

Ah I didn't know there was a zulip. 👀 I'll see you there!