Handle closures correctly in MIR inlining

[sinkuu]

Fixes #45894.

[shepmaster]

Auto-assigning to...

r? @nikomatsakis

[nikomatsakis]

Looks great! I left a few style nits.

[nikomatsakis]

Nit: Could we move this to a method (and then get rid of the tcx argument)? (I just find it harder to read things "nested" funtions, that's all.)

[nikomatsakis]

Also, good opportunity to add a comment (the surrounding code is a bit...comment-poor). Perhaps something like:

/// If `arg` is already a temporary, returns it. Otherwise, introduces a fresh
/// temporary `T` and an instruction `T = arg`, and returns `T`.

(Does that sound accurate?)

[nikomatsakis]

Finally, can we change the return value to Lvalue, to reflect the fact that this always returns an Operand::Consume?

[nikomatsakis]

Nit: let's do if let DefPathData::ClosureExpr = tcx.def_key(callsite.callee).disambiguated_data.data

[nikomatsakis]

better yet, let's add a is_closure(def_id: DefId) helper to the tcx (and call it from closure_base_def_id)

[nikomatsakis]

(if we changed return type, this if let would not be necessary)

[nikomatsakis]

nice drive-by cleanup =)

[nikomatsakis]

@bors r+

[bors]

📌 Commit 17cbd9f has been approved by nikomatsakis

[sinkuu]

Rebased.

[arielb1]

@bors r+

[bors]

📌 Commit ec2ff8f has been approved by arielb1

[bors]

⌛ Testing commit ec2ff8f with merge 1cfe6599667b4f0c978e0d911988a39e76340f58...

[bors]

💔 Test failed - status-travis

[kennytm]

Checked arithmetic strikes again! These are CheckedMul in debug build. Please change to a non-checked operator like q & 3 or maybe just write x(q, q).

[01:06:23] ---- [mir-opt] mir-opt/inline-closure.rs stdout ----
[01:06:23] 	thread '[mir-opt] mir-opt/inline-closure.rs' panicked at 'Did not find expected line, error: Mismatch in lines
[01:06:23] Currnt block: None
[01:06:23] Expected Line: "        _8 = CheckedMul(_7, const 2i32); // scope 1 at /checkout/src/test/mir-opt/inline-closure.rs:21:7: 21:10"
[01:06:23] Actual Line: "    _6 = Mul(_7, const 2i32);"
[01:06:23] Expected:
[01:06:23] ... (elided)
[01:06:23] ... (elided)
[01:06:23] bb0: {
[01:06:23]     StorageLive(_3);
[01:06:23]     _3 = [closure@NodeId(28)];
[01:06:23]     StorageLive(_4);
[01:06:23]     _4 = &_3;
[01:06:23]     StorageLive(_5);
[01:06:23]     StorageLive(_6);
[01:06:23]     StorageLive(_7);
[01:06:23]     _7 = _2;
[01:06:23]     _6 = Mul(_7, const 2i32);
[01:06:23]     StorageDead(_7);
[01:06:23]     StorageLive(_8);
[01:06:23]     StorageLive(_9);
[01:06:23]     _9 = _2;
[01:06:23]     _8 = Mul(_9, const 3i32);
[01:06:23]     StorageDead(_9);
[01:06:23]     _5 = (_6, _8);
[01:06:23]     _0 = (_5.0: i32);
[01:06:23]     StorageDead(_5);
[01:06:23]     StorageDead(_8);
[01:06:23]     StorageDead(_6);
[01:06:23]     StorageDead(_4);
[01:06:23]     StorageDead(_3);
[01:06:23]     return;
[01:06:23] }
[01:06:23] ... (elided)
[01:06:23] Actual:
[01:06:23] fn foo(_1: T, _2: i32) -> i32 {
[01:06:23]     let mut _0: i32;
[01:06:23]     scope 1 {
[01:06:23]         let _3: [closure@NodeId(28)];
[01:06:23]         scope 3 {
[01:06:23]         }
[01:06:23]     }
[01:06:23]     scope 2 {
[01:06:23]     }
[01:06:23]     let mut _4: &[closure@NodeId(28)];
[01:06:23]     let mut _5: (i32, i32);
[01:06:23]     let mut _6: i32;
[01:06:23]     let mut _7: i32;
[01:06:23]     let mut _8: (i32, bool);
[01:06:23]     let mut _9: i32;
[01:06:23]     let mut _10: i32;
[01:06:23]     let mut _11: (i32, bool);
[01:06:23]     bb0: {                              
[01:06:23]         StorageLive(_3);
[01:06:23]         _3 = [closure@NodeId(28)];
[01:06:23]         StorageLive(_4);
[01:06:23]         _4 = &_3;
[01:06:23]         StorageLive(_5);
[01:06:23]         StorageLive(_6);
[01:06:23]         StorageLive(_7);
[01:06:23]         _7 = _2;
[01:06:23]         _8 = CheckedMul(_7, const 2i32);
[01:06:23]         assert(!(_8.1: bool), "attempt to multiply with overflow") -> bb1;
[01:06:23]     }
[01:06:23]     bb1: {                              
[01:06:23]         _6 = (_8.0: i32);
[01:06:23]         StorageDead(_7);
[01:06:23]         StorageLive(_9);
[01:06:23]         StorageLive(_10);
[01:06:23]         _10 = _2;
[01:06:23]         _11 = CheckedMul(_10, const 3i32);
[01:06:23]         assert(!(_11.1: bool), "attempt to multiply with overflow") -> bb2;
[01:06:23]     }
[01:06:23]     bb2: {                              
[01:06:23]         _9 = (_11.0: i32);
[01:06:23]         StorageDead(_10);
[01:06:23]         _5 = (_6, _9);
[01:06:23]         _0 = (_5.0: i32);
[01:06:23]         StorageDead(_5);
[01:06:23]         StorageDead(_9);
[01:06:23]         StorageDead(_6);
[01:06:23]         StorageDead(_4);
[01:06:23]         StorageDead(_3);
[01:06:23]         return;
[01:06:23]     }
[01:06:23] }', /checkout/src/tools/compiletest/src/runtest.rs:2337:12
[01:06:23] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:06:23] 
[01:06:23] 
[01:06:23] failures:
[01:06:23]     [mir-opt] mir-opt/inline-closure.rs
[01:06:23] 
[01:06:23] test result: �[31mFAILED�(B�[m. 44 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out

[arielb1]

@bors r+

[bors]

📌 Commit a1f7bad has been approved by arielb1

[bors]

⌛ Testing commit a1f7bad with merge 8a98531...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: arielb1
Pushing 8a98531 to master...