Mark block exits as reachable if the block can break.

[goffrie]

This only happens when desugaring catch expressions for now, but regular blocks (in HIR) can be broken from - respect that when doing reachability analysis.

Fixes #45124.

[rust-highfive]

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

[eddyb]

r? @nikomatsakis

[nikomatsakis]

Hmm, this doesn't seem quite right because of the possibility of dead code. Consider something like this:

let x = 'a: {
    panic!(); break 'a 22;
};

or perhaps

let x = do catch {
    panic!("wtf");
    Err(22)?;
    Ok(())
};

That said, I think that ExprLoop is comparably imprecise. I'm trying to remember all the implications of this and make sure there's not some kind of soundness issue here.

[nikomatsakis]

(Thoughts?)

[goffrie]

Is it unsound to mark code as potentially reachable (erm, Diverges::Maybe) when it in fact is never reachable? Seems like it shouldn't be - from my understanding, it should only be unsound to make the wrong call in the opposite direction.

Also maybe we could skip marking .may_break if the break itself is unreachable, though I wouldn't personally advocate for it.

[arielb1]

This is currently the case for loop (is there a test for it? there ought to be):

fn main() {
    let x: u32 = { //~ ERROR mismatched types
        loop {
            panic!();
            break;
        };
    };
}

From a soundness perspective, IIRC if an arm is never reachable, then it is potentially reachable, so regarding something that always diverges as something that potentially reaches its exit should always be sound.

[nikomatsakis]

Sorry for delay. Busy couple of days. Anyway, I agree that ExprLoop is treated the same way, and I also agreed it ought not to be unsound to approximate in this way. (In particular, I think we only care when things definitely diverge -- this makes sense since clearly we can't tell that things definitely don't diverge (damn halting problem).)

I was thinking more about what behavior we actually want. I actually think it's consistent with our general strategy around typing dead code to allow break -- even in dead code -- to make a loop or block be considered non-diverging. For example, the example that @arielb1 gave here] seems consistent with the rules we adopted for dead-code, which mean that e.g. the following code doesn't build:

#![allow(unreachable_code)]

fn main() {
    let x: u32 = { //~ ERROR mismatched types
        panic!();
        'a'
    };
}

In particular, I would consider break 'a' to be equivalent to using 'a' as a tail expression.

Essentially -- to the extent possible -- we are aiming to type-check dead-code as if it could become live. This includes not only the types of values produced by dead-code but also the control-flow that the dead-code would have introduced.

We might want to tweak that last bit but, regardless, this PR is consistent with what we do now.

[nikomatsakis]

@bors r+

[bors]

📌 Commit 57f03ea has been approved by nikomatsakis

[bors]

⌛ Testing commit 57f03ea with merge c0e0a38...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing c0e0a38 to master...