Clean up MIR drop generation

[matthewjasper]

• Don't assign twice to the destination of a while loop containing a break expression
• Use as_temp to evaluate statement expression
• Avoid consecutive StorageLives for the condition of a while loop
• Unify return, break and continue handling, and move it to scopes.rs
• Make some of the scopes.rs internals private
• Don't use Places that are always Locals in MIR drop generation

Closes #42371
Closes #61579
Closes #61731
Closes #61834
Closes #61910
Closes #62115

[rust-highfive]

r? @eddyb

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

[Centril]

N.B. As part of implementing while a && let b = c { ... } I'll make a PR to remove hir::ExprKind::While and then hair::ExprKind::Loop will presumably need to drop it's condition field and so the else branch would be floated out and the if branch would be removed... I hope the changes here do not cause problems for this...?

[matthewjasper]

That should be fine, this is only a problem because while loops are special cased. For any sensible HIR lowering this won't be a problem. The temporary will probably end up storage-live for the whole loop body, but that isn't observable. The following two functions generate almost identical optimized MIR after this PR, for example.

fn while_loop(c: bool) {
    while get_bool(c) {
        if get_bool(c) {
            break;
        }
    }
}

// What the above should probably expand to
fn exp_while_loop(c: bool) {
    loop {
        match get_bool(c) {
            true => {
                {if get_bool(c) {
                    break;
                }}
                continue;
            }
            _ => {}
        }
        break;
    }
}

[Centril]

Cool.

For any sensible HIR lowering this won't be a problem.

Specifically, the HIR lowering should likely be:

'label: while $cond $block

==>

'label: loop {
    match DropTemps($cond) {
        true => $block,
        _ => break,
    }
}

(is there a particular reason you are using continue; and an empty block?)

[matthewjasper]

It's slightly closer to what the RFC specified. What you're suggesting is probably better.

[Centril]

This should be unnecessary now with #61988.

[bors]

☔ The latest upstream changes (presumably #60730) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

☔ The latest upstream changes (presumably #61915) made this pull request unmergeable. Please resolve the merge conflicts.

[eddyb]

r? @pnkfelix

[nikomatsakis]

This looks very good. I left a few questions.

[nikomatsakis]

@bors r+

[bors]

📌 Commit 07e5297faf2966a77c466b0b5ad936c0deb828ac has been approved by nikomatsakis

[bors]

☔ The latest upstream changes (presumably #62119) made this pull request unmergeable. Please resolve the merge conflicts.

[cramertj]

@bors r=nikomatsakis

[bors]

📌 Commit 3131427 has been approved by nikomatsakis

[bors]

⌛ Testing commit 3131427 with merge d3e2cec...

[bors]

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

[mati865]

Was this performance regression expected?
https://perf.rust-lang.org/compare.html?start=bdd4bda4d5fe7eb24a7209a6c0c968a54869d3b7&end=d3e2cec29225a46298ec4ebf082f34ebd7cfeecf

[matthewjasper]

The ctfe-stress benchmark is now generating more MIR, so I'm not surprised that it's slower. I can't really work out what's going on with unicode_normalization, since the generated MIR is unchanged for the large functions and constants.