[MIR Borrowck] Handle borrows of array elements

[sinkuu]

Fixes #45537.

[nikomatsakis]

r? @arielb1

[pnkfelix]

Don't we need to recur on walk down other variants of ProjectionElem as well here? E.g. what if we there is a Field-access or a Downcast mixed into the projection?

[pnkfelix]

Thinking more about it, the real bug here may be the use of Lvalue within the struct BorrowData. It probably needs to be an abstraction of Lvalue, in order to unify cases like a[0] with a[1] while still allowing us to differentiate cases like a[0].foo and a[0].bar...

[pnkfelix]

Here is a concrete example that this PR does not currently handle:

    struct S<T> { x: T, y: T, }
    let mut v = [S { x: 1, y: 2 },
                 S { x: 3, y: 4 },
                 S { x: 5, y: 6 }];
    let p = &v[0].x;
    if true {
        use_x(*p);
    } else {
        use_x(22);
    }
    v[0].x += 1; //[ast]~ ERROR cannot assign to `v[..]`

[pnkfelix]

Note that when I said "It probably needs to be an abstraction of Lvalue", I was basically talking about what the move paths code does; see here: https://github.com/rust-lang/rust/blob/master/src/librustc_mir/dataflow/move_paths/abs_domain.rs (and here)

[bors]

☔ The latest upstream changes (presumably #46032) made this pull request unmergeable. Please resolve the merge conflicts.

[pnkfelix]

So, to start off: I have not read this version of the PR in depth yet. To be honest I was actually expecting a change to the struct BorrowData like replacing the lvalue it holds with a MovePathIndex. (I know I didn't suggest such a thing; it was just something that's been sitting in the back of my head.)

Skimming over this version of the change, I'm a little surprised by the need to revise how the various kinds of prefixes are handled.

Well, let me put it another way: I thought the previous version of the code was pretty readable, in terms of being able to tell immediately, at each iterator, which kind of iteration was being performed. This new version is doing calculations involving ps.len() - bps.len(), and I would feel pretty scared trying to modify those for loops as they are written here.

• (If we want to stick with the new representation (with these extra fields in BorrowData), then I would want to at least encapsulate this representational choice? E.g. keep the existing enum for selecting what kind of prefix-set we want to iterate over, and have the iterator itself do these machinations with the various lengths. BUT: first I want to figure out if this representation is something we want to adopt.)

[pnkfelix]

Hi @sinkuu by the way, thank you for working on this Pull Request, and sorry it took so long for me to look at your revised version

I just spoke with @arielb1 and it sounds like they have a more general fix in the works for this bug, so I am going to close this Pull Request.

If you would like to help out with other issues related to the NLL work, I invite you to join the WG-compiler-nll gitter instance ( https://gitter.im/rust-impl-period/WG-compiler-nll ) so that we can all coordinate work accordingly.