match arm bindings have weird lifetimes

[arielb1]

I should have opened this issue a long while ago, but now I have a place in the code I want to link it to.

Match bindings have weird lifetime, especially when guards are involved: instead of the destructors executing when the bindings go out of scope, all of the destructors of all match bindings are run just after the match ends.

e.g. I think this causes unnecessary drop elaboration and etc. load on big functions with many-armed matches.

One very weird case is this:

    fn main() {
         match 0 {
             a | a
             if { println!("a={}", a); false } => {}
             _ => {}
         }
     }

Here, the guard is executed twice, and a is only storage-killed after the match ends, so we have storage-live -> storage-live -> storage-dead, which might annoy some things.

FIXME: remember more weird cases and clean them up.

[arielb1]

I think that it will be hard to fix this in a way AST borrowck and region inference will understand, but afterwards this is fair game.

[nikomatsakis]

I think that this fits into what @pnkfelix is doing in overhauling how we handle match arms?

[nikomatsakis]

I guess this is just a matter of generating better code...?

[pnkfelix]

Re-triaging for #56754. NLL-complete. P-medium. Assigning to self.

[pnkfelix]

In terms of observable semantics with implementations of Drop, its hard today to encounter this as a user, unless you turn on #![feature(bind_by_move_pattern_guards)]. (And even then, I'm not sure a rust developer can tell the difference between the code we generate today versus what @arielb1 desires, at least in terms of potential MIR-borrowck errors.)

So like @nikomatsakis, I think this is just a matter of generating better code.

But I would like to actually be sure of that before we e.g. turn off NLL migrate more (and subsequently stabilize #![feature(bind_by_move_pattern_guards)). Which may mean I want to just do the code-gen changes suggested here.

Increasing priority of this issue to P-high.

[Centril]

Nominating for some discussion on today's lang meeting.

---

Also, here's an example of observable drop order:

#![feature(bind_by_move_pattern_guards)]

#[derive(Debug)]
struct A;

impl A {
    fn foo(&self) -> bool { dbg!(2); false }
}

impl Drop for A {
    fn drop(&mut self) { dbg!(3); }
}

fn main() {
    dbg!(0);
    match dbg!(A) {
        a if a.foo() => { dbg!(4); }
        _ => { dbg!(5); }
    }
    dbg!(6);
}

prints:

[src/main.rs:15] 0 = 0
[src/main.rs:16] A = A
[src/main.rs:7] 2 = 2
[src/main.rs:18] 5 = 5
[src/main.rs:11] 3 = 3
[src/main.rs:20] 6 = 6

showing that 5 happens before 3.
The _ patterns makes no difference.

[matthewjasper]

Fixing this issue, would not change the order there, or any observable runtime behaviour at all.

[pnkfelix]

triage: @matthewjasper do you think this actually warrants P-high? I know we want to do it eventually, but if things are working okay thus far, it seems like we can continue to coast?

[matthewjasper]

It's not really p-high, but I hope to create a PR for this once migrate mode is enabled on all editions.