trans tries to clean up values created in a match arm, outside the match.

[eddyb]

Original testcase by @hoeppnertill (reduced from his nock repo):

#![crate_type="lib"]

pub fn test(foo: bool) -> int {
    match foo {
        true => *box 0,
        false => 0
    }
}

Instruction does not dominate all uses!
  %6 = bitcast i8* %5 to i64*
  %8 = bitcast i64* %6 to i8*
LLVM ERROR: Broken function found, compilation aborted!

The debug log reveals that a (shallow free) cleanup of the Box<int> rvalue is scheduled in the scope of AST node 11, while the dereference expression itself has id 16:

DEBUG:rustc::middle::trans::expr: deref_once(expr=expr(16: *box() 0), datum=Datum((i64*:  %7 = bitcast i8* %6 to i64*), Box<int>, RvalueExpr(Rvalue { mode: ByValue })), method_call=MethodCall { expr_id: 16, adjustment: NoAdjustment })
DEBUG:rustc::middle::trans::cleanup: temporary_scope(16) = AstScope(11)
DEBUG:rustc::middle::trans::cleanup: schedule_free_value(AstScope(11), val=(i64*:  %7 = bitcast i8* %6 to i64*), heap=HeapExchange)
DEBUG:rustc::middle::trans::cleanup: schedule_clean_in_ast_scope(cleanup_scope=11)
DEBUG:rustc::middle::trans::expr: deref_once(expr=16, method_call=MethodCall { expr_id: 16, adjustment: NoAdjustment }, result=Datum((i64*:  %7 = bitcast i8* %6 to i64*), int, RvalueExpr(Rvalue { mode: ByRef })))

--pretty expanded,identified reveals that id 11 refers to the entire function's block:

pub fn test(foo /* pat 5 */: bool) -> int {
    (match (foo /* 13 */) {
         (true /* 15 */) /* pat 14 */ =>
         (*(box() (0 /* 18 */) /* 17 */) /* 16 */),
         (false /* 20 */) /* pat 19 */ => (0 /* 21 */),
     } /* 12 */)
} /* block 11 */ /* 3 */

This means the cleanup of a value valid only within a match arm is scheduled outside the arm.
cc @nikomatsakis @pnkfelix

Dupe of #15892. Leaving open for now but we can close either.

[eddyb]

This is caused by a micro-optimization specific to dereferences of Box<T> rvalues. Is this used oftenly enough to justify its existence in the first place? Even if it is, I guess we can disable it in problematic cases like this, where the temporary scope of the match arm's RHS is outside the arm.

[nikomatsakis]

@eddyb I think we can probably remove it, in general removing special treatment of Box<T> seems like a Good Thing

[steveklabnik]

Triage: this still ICEs.

[pnkfelix]

@eddyb can you point me/us at the micro-optimization that you mentioned above, so that I (we) can try removing it ourselves and measure the impact?

[eddyb]

@pnkfelix I only vaguely recall this issue, but I'm pretty sure I was talking about trans::expr::deref_owned_pointer.

[tamird]

triage: still ICEs.

#![feature(box_syntax)]

#![crate_type="lib"]

pub fn test(foo: bool) -> u8 {
    match foo {
        true => *box 0,
        false => 0
    }
}

[pnkfelix]

Indeed, it doesn't even need box_syntax:

pub fn test(foo: bool) -> u8 {
    match foo {
        true => *Box::new(0),
        false => 0
    }
}

fn main() {
    test(true);
}

Does this at least continue to ICE even if LLVM is compiled without assertions? If not, we may want to up the priority on this ticket.