Locals aren't introduced properly in if-let guards on or-patterns

[LeSeulArtichaut]

I tried this code:

#![feature(if_let_guard)]

fn main() {
    match () {
        () | () if let x = 0 => {
            x;
        },
        _ => {}
    };
}

I expected to see this happen: code compiles successfully

Instead, this happened: the compiler complains about x being possibly uninitialized

error[E0381]: use of possibly-uninitialized variable: `x`
 --> src/main.rs:5:34
  |
5 |         () | () if let x = () => x,
  |                                  ^ use of possibly-uninitialized `x`

This is because each candidate subpattern introduces a different local for x.

cc tracking issue #51114.

Meta

rustc --version --verbose:

rustc 1.56.0-nightly (ccffcafd5 2021-08-11)
binary: rustc
commit-hash: ccffcafd55e58f769d4b0efc0064bf65e76998e4
commit-date: 2021-08-11
host: x86_64-unknown-linux-gnu
release: 1.56.0-nightly
LLVM version: 12.0.1

[osa1]

@rustbot claim

I started debugging this.

Error is generated by MIR borrow checker.

Repro I'm using:

#![feature(if_let_guard)]

pub enum Test {
    A,
    B,
}

pub fn test() -> bool {
    match Test::A {
        Test::A | Test::B if let x = false =>
            x,
        _ => true
    }
}

MIR after promote-consts: (I think this is the MIR used for borrow checking?)

MIR

// MIR for `test` after SimplifyCfg-promote-consts

fn test() -> bool {
    let mut _0: bool;                    // return place in scope 0 at test.rs:8:18: 8:22
    let mut _1: Test;                    // in scope 0 at test.rs:9:11: 9:18
    let mut _2: isize;                   // in scope 0 at test.rs:10:9: 10:16
    let mut _3: &Test;                   // in scope 0 at test.rs:9:11: 9:18
    let mut _4: bool;                    // in scope 0 at test.rs:10:38: 10:43
    let _5: bool;                        // in scope 0 at test.rs:10:34: 10:35
    let mut _6: bool;                    // in scope 0 at test.rs:10:38: 10:43
    let _7: bool;                        // in scope 0 at test.rs:10:34: 10:35
    scope 1 {
        debug x => _5;                   // in scope 1 at test.rs:10:34: 10:35
    }
    scope 2 {
        debug x => _7;                   // in scope 2 at test.rs:10:34: 10:35
    }

    bb0: {
        StorageLive(_1);                 // scope 0 at test.rs:9:11: 9:18
        _1 = Test::A;                    // scope 0 at test.rs:9:11: 9:18
        FakeRead(ForMatchedPlace(None), _1); // scope 0 at test.rs:9:11: 9:18
        _2 = discriminant(_1);           // scope 0 at test.rs:10:9: 10:16
        switchInt(move _2) -> [0_isize: bb1, 1_isize: bb3, otherwise: bb2]; // scope 0 at test.rs:10:9: 10:16
    }

    bb1: {
        falseEdge -> [real: bb5, imaginary: bb3]; // scope 0 at test.rs:10:9: 10:16
    }

    bb2: {
        _0 = const true;                 // scope 0 at test.rs:12:14: 12:18
        goto -> bb11;                    // scope 0 at test.rs:9:5: 13:6
    }

    bb3: {
        falseEdge -> [real: bb8, imaginary: bb2]; // scope 0 at test.rs:10:19: 10:26
    }

    bb4: {
        _0 = _7;                         // scope 0 at test.rs:11:13: 11:14
        StorageDead(_7);                 // scope 0 at test.rs:11:13: 11:14
        goto -> bb11;                    // scope 0 at test.rs:9:5: 13:6
    }

    bb5: {
        _3 = &shallow _1;                // scope 0 at test.rs:9:11: 9:18
        StorageLive(_4);                 // scope 0 at test.rs:10:38: 10:43
        _4 = const false;                // scope 0 at test.rs:10:38: 10:43
        FakeRead(ForMatchedPlace(None), _4); // scope 0 at test.rs:10:38: 10:43
        falseEdge -> [real: bb7, imaginary: bb6]; // scope 0 at test.rs:10:34: 10:35
    }

    bb6: {
        StorageDead(_5);                 // scope 0 at test.rs:11:13: 11:14
        falseEdge -> [real: bb2, imaginary: bb3]; // scope 0 at test.rs:10:38: 10:43
    }

    bb7: {
        StorageLive(_5);                 // scope 0 at test.rs:10:34: 10:35
        _5 = _4;                         // scope 0 at test.rs:10:34: 10:35
        FakeRead(ForMatchGuard, _3);     // scope 0 at test.rs:10:42: 10:43
        goto -> bb4;                     // scope 0 at test.rs:9:5: 13:6
    }

    bb8: {
        _3 = &shallow _1;                // scope 0 at test.rs:9:11: 9:18
        StorageLive(_6);                 // scope 0 at test.rs:10:38: 10:43
        _6 = const false;                // scope 0 at test.rs:10:38: 10:43
        FakeRead(ForMatchedPlace(None), _6); // scope 0 at test.rs:10:38: 10:43
        falseEdge -> [real: bb10, imaginary: bb9]; // scope 0 at test.rs:10:34: 10:35
    }

    bb9: {
        StorageDead(_7);                 // scope 0 at test.rs:11:13: 11:14
        falseEdge -> [real: bb2, imaginary: bb2]; // scope 0 at test.rs:10:38: 10:43
    }

    bb10: {
        StorageLive(_7);                 // scope 0 at test.rs:10:34: 10:35
        _7 = _6;                         // scope 0 at test.rs:10:34: 10:35
        FakeRead(ForMatchGuard, _3);     // scope 0 at test.rs:10:42: 10:43
        goto -> bb4;                     // scope 0 at test.rs:9:5: 13:6
    }

    bb11: {
        StorageDead(_6);                 // scope 0 at test.rs:14:1: 14:2
        StorageDead(_4);                 // scope 0 at test.rs:14:1: 14:2
        StorageDead(_1);                 // scope 0 at test.rs:14:1: 14:2
        return;                          // scope 0 at test.rs:14:2: 14:2
    }
}

Problematic variable is _7. It's only read by bb4, which has two incoming edges: from bb10 and bb7. bb10 initializes _7 so that's fine. There's one path that goes through bb7 which is bb0 -> bb1 -> bb5 (real) -> bb7 (real) -> bb4, and _7 is not initialized in this path.

The problematic path has _4 = false (bb5) which must be for the guard as we don't have any other uses of false in this program. Then does _5 = _4 in the next block (bb7). Debug info says both of these variables are for x. The problem is the path for Test::A initializes _5, and the path for Test::B initializes _7.

[osa1]

IfLet guards and or patterns seem to be not handled correctly in the pattern match compiler (rustc_mir_build::build::matches). I'm not sure if this is the problem or a symptom, but currently when we split candidates with or patterns into candidates without or patterns we duplicate the IfLet. This means the if let is lowered twice, as we also see in the MIR in my comment above. I don't understand rustc's pattern match compiler in all the details but this doesn't seem right to me. I'd expect if let part to be compiled once, and expect to see one block for it in the MIR.

[Frago9876543210]

another strange behaviour happen when mixing if_let_guard and let_chains

#![feature(if_let_guard, let_chains)]

fn main() {
    let xs = vec![0i32];
    if let Some(val) = xs.iter().next() && let bind_me = 5 {
        dbg!(val, bind_me);
    }
}

error[E0381]: use of possibly-uninitialized variable: `val`
 --> src/main.rs:6:14
  |
6 |         dbg!(val, bind_me);
  |              ^^^ use of possibly-uninitialized `val`

error[E0381]: use of possibly-uninitialized variable: `bind_me`
 --> src/main.rs:6:19
  |
6 |         dbg!(val, bind_me);
  |                   ^^^^^^^ use of possibly-uninitialized `bind_me`

[AlexTMjugador]

Are there any news on this? I've just got bitten by this bug.

[joshtriplett]

Following up on this: I'd love to see if-let guards stabilized, and this looks like the main blocker for that. If anyone else would like to see if-let guards stabilized as well, fixing this would be a necessary step that would get us much closer.

[ouz-a]

@rustbot claim

[ouz-a]

Came to the same conclusion as @osa1 solving this requires more than a simple fix since this will involve changing how we build match in mir.

[LeSeulArtichaut]

another strange behaviour happen when mixing if_let_guard and let_chains

#![feature(if_let_guard, let_chains)]

fn main() {
    let xs = vec![0i32];
    if let Some(val) = xs.iter().next() && let bind_me = 5 {
        dbg!(val, bind_me);
    }
}

This one seems to be fixed now. I bisected the fix to 9ad5d82...5e57faa, I'm guessing #88642

[LeSeulArtichaut]

@rustbot claim

#104669 should fix this, though I need to do some more testing and I'm expecting bad diagnostics in some cases