emit fake reads for indexing after every bounds-check

[dianne]

To fix #161852, we need fake borrows on indexed-into slice pointers to live through subsequent bounds-checks. This PR does that.

The following is now rejected:

fn main() {
    let mut x: &[&[&[u32]]] = &[&[&[0]]];
    let y: &[&[&[u32]]] = &[];
    x[0][{ x = y; 0 }][{ return; 0 }];
    //~^ ERROR: cannot assign `x` in indexing expression
}

This does not affect locations from which bounds-checks are unreachable. The following was already and is still accepted:

fn main() {
    let mut x: &[&[&[u32]]] = &[&[&[0]]];
    let y: &[&[&[u32]]] = &[];
    x[0][{ x = y; return; 0 }];
}

Similar to #161581, I imagine if we want to be stricter about that, we'll either need fake reads at any point we diverge or some other mechanism for keeping fake borrows alive that isn't as sensitive to CFG structure.

[dianne]

I think this makes the total number of fake reads quadratic in how much nested slice indexing we do, but hopefully that'd only matter in stress tests? Surprisingly, no MIR building tests needed blessing, so I don't have an example to point to.

View changes since the review

[matthewjasper]

This needs 3 or more nested array/slice indexes, so it's not that surprising that no tests are affected. It definitely doesn't seem likely that there's any real code that's going to have large enough numbers of nested indexes for this to have a significant impact.

[dianne]

r? @matthewjasper

Like #161581, I'm not fully sure where this falls between lang and types in terms of design responsibility, but I'll start by lang-nominating it alongside that. The ask is equivalent: from a user's perspective, what's the language property we're expressing with fake borrows in indexing expressions? Does this approach fit that, or should we be stricter?

[matthewjasper]

The code here looks good. I'll wait for lang to discuss.

The original motivation for the fake borrows here was to make a minimal change to (try to) fix the soundness issue. I considered mimicking the borrows done by the Index trait, but I think that was too restrictive and broke too much code.

View changes since this review