Does the MIR generated for match still build in order-deps visible to MIR-borrowck?

[pnkfelix]

Spawned off of PR #57609

The above PR has an interesting unit test that checks that we aren't assuming too much order dependence; the test (from file named "match-cfg-fake-edges.rs") looks like this (play):

fn all_previous_tests_may_be_done(y: &mut (bool, bool)) {
    let r = &mut y.1;
    // We don't actually test y.1 to select the second arm, but we don't want
    // borrowck results to be based on the order we match patterns.
    match y {
        (false, true) => 1, //~ ERROR cannot use `y.1` because it was mutably borrowed
        (true, _) => { r; 2 } // (use of "above" borrow)
        (false, _) => 3,
    };
}

while reviewing that test case, I wondered whether order-dependence, as implicitly defined by the above test, would also include this variant:

fn any_tests_may_be_done(y: &mut (bool, bool)) {
    let r = &mut y.1;
    // We don't test y.1 to select the first arm. Does this present
    // an instance where borrowck results are based on the order we
    // match patterns?
    match y {
        (true, _) => { r; 2 } // (use of "below" borrow)
        (false, true) => 1, //~ ERROR cannot use `y.1` because it was mutably borrowed
        (false, _) => 3,
    };
}

Today, all_previous_tests_may_be_done is rejected (with a warning under NLL migration mode, and a hard error if you opt into #![feature(nll), but any_tests_may_be_done is accepted.

Is this an internally consistent policy?

---

Maybe my question is most simply put like this:

• since we are adding some "fake edges" from some match-arms to the other arms that follow (to try to avoid prematurely tying ourselves to a particular code-generation strategy for MIR, in terms of what inputs MIR-borrowck will accept), ...
• shouldn't we be adding even more such "fake edges" that go from the bottom arms back up to the ones before them?

That would increase our flexibility for MIR code-generation, in theory, even further, right?

[pnkfelix]

nominating for discussion ... this is in principle, I think, something to discuss at T-compiler, not just NLL. (Although I guess since this choice only affects MIR-borrowck, maybe the WG-nll is the right "expert group" to deliver initial advice here...)

[pnkfelix]

(it might be hard to implement this change via a warning-cycle. Its hard to track how new control-edges affect errors. Though I guess we could do something similar to migration mode: Tag the fake-edges added here as "even more special", then run MIR-borrowck with the full control-flow graph, and if we see an error, re-run dataflow+MIR-borrowck but this time ignore the specially-tagged edges, and then if that does not error, downgrade the original error to a warning...)

[pnkfelix]

triage: P-medium, NLL-complete.

• NLL-complete because the choice here is not about rejecting an instance of unsound code, but rather just restricting the set of accepted programs in order to provide for future flexibility.

[pnkfelix]

Also, I would probably be happy to come to a conclusion that says: "we do build in some order dependencies"; I just want us to try to be clear about the motivation for the original fake-branches in the control-flow graph and that we are not fooling ourselves with regards to how much future-proofing we have in place here.

[pnkfelix]

@matthewjasper points out during the NLL meeting that a cyclic control-flow graph for match would interact badly with guards and that If we really wanted to try to prepare for a change as described here, it would probably be best handled by adding more fake-reads (that would be modelling the speculative execution of patterns).