MIR borrowck: wrong error for move into closure

[vramana]

# ramana at pc in ~/Documents/rust-lang/rust on git:fix-e0504 ✖︎ [18:05:33]
→ ./build/x86_64-unknown-linux-gnu/stage1/bin/rustc src/test/compile-fail/E0504.rs -Z borrowck=mir 
error[E0505]: cannot move out of `fancy_num` because it is borrowed
  --> src/test/compile-fail/E0504.rs:23:13
   |
21 |       let fancy_ref = &fancy_num;
   |                       ---------- borrow of `fancy_num` occurs here
22 | 
23 |       let x = move || {
   |  _____________^
24 | |         println!("child function: {}", fancy_num.num); //~ ERROR E0504
25 | |     };
   | |_____^ move out of `fancy_num` occurs here

error: aborting due to previous error


# ramana at pc in ~/Documents/rust-lang/rust on git:fix-e0504 ✖︎ [18:12:36]
→ ./build/x86_64-unknown-linux-gnu/stage1/bin/rustc src/test/compile-fail/E0504.rs                 
error[E0504]: cannot move `fancy_num` into closure because it is borrowed
  --> src/test/compile-fail/E0504.rs:24:40
   |
21 |     let fancy_ref = &fancy_num;
   |                      --------- borrow of `fancy_num` occurs here
...
24 |         println!("child function: {}", fancy_num.num); //~ ERROR E0504
   |                                        ^^^^^^^^^ move into closure occurs here

error: aborting due to previous error

cc @arielb1

I am working on this.

[vramana]

Hi,

I was little busy work stuff. I am taking a stab at this issue again today. I am using simpler case that generates less MIR code

struct FancyNum {
    num: u8,
}

fn main() {
    let mut fancy_num = FancyNum { num: 5 };
    // let fancy_ref = &fancy_num;

    let mut x = move || {
        fancy_num.num += 1;
    };

    x();
}

Generated MIR here

When I uncomment the line let fancy_ref = &fancy_num;, this borrow conflits with this _3 = move _1 in MIR above

       StorageLive(_2);                 // bb0[2]: scope 3 at src/main.rs:9:9: 9:14
       StorageLive(_3);                 // bb0[3]: scope 3 at src/main.rs:9:17: 11:6
       _3 = move _1;                    // bb0[4]: scope 3 at src/main.rs:9:17: 11:6
+      The line that causes the error.
       _2 = [closure@src/main.rs:9:17: 11:6] { fancy_num: move _3 }; // bb0[5]: scope 3 at src/main.rs:9:17: 11:6
                                         // closure
                                         // └ def_id: DefId(0/1:10 ~ playground[72d2]::main[0]::{{closure}}[0])

Note: _3 will be _4 in the debug log because of the extra variable fancy_ref (_2)

So the error occurs way before we enter the closure.

rust/src/librustc_mir/borrow_check/mod.rs

Lines 349 to 354 in 1699293

	self.consume_rvalue(
	ContextKind::AssignRhs.new(location),
	(rhs, span),
	location,
	flow_state,
	);

rust/src/librustc_mir/borrow_check/mod.rs

Lines 1016 to 1023 in 1699293

	// move of place: check if this is move of already borrowed path
	self.access_place(
	context,
	(place, span),
	(Deep, Write(WriteKind::Move)),
	LocalMutationIsAllowed::Yes,
	flow_state,
	);

rust/src/librustc_mir/borrow_check/mod.rs

Lines 850 to 851 in 1699293

	error_reported = true;
	this.report_move_out_while_borrowed(context, place_span, &borrow)

This call generates the error. The information we have is the place (_1) we access and the span (scope 3 at src/main.rs:9:17: 11:6) where we access it. How do I find the usages of _1 in this span?

[arielb1]

@vramana

You need to find uses of _1 within the closure. For that, you can follow down the MIR basic block until you find the closure statement, look at the closure def-id, and look for uses of the appropriate upvar within the closure's body (use a MIR visitor and visit_lvalue, they will be _0.<field#> or (*_0).<field#>, and you might also want to recurse if you encounter nested closures).

[vramana]

For that, you can follow down the MIR basic block until you find the closure statement

I don't know how to implement this in code probably because I am not too familiar with API. But I will give it a try.

[vramana]

@arielb1 There is no visit_lvalue function. Can you explain your hints in a little more detail?

[arielb1]

@vramana

Sorry for the delay. lvalue is now place, so visit_lvalue is now visit_place.

[nikomatsakis]

I wasn't able to observe this error, at least not with @vramana's minimized example:

https://play.rust-lang.org/?gist=6af331fc0c3dd1f455a2ecda1308991c&version=nightly

(I also tried removing the comment)

[nikomatsakis]

I guess this example gets an error:

#![feature(nll)]

struct FancyNum {
    num: u8,
}

fn main() {
    let mut fancy_num = FancyNum { num: 5 };
    let fancy_ref = &fancy_num;

    let mut x = move || {
        fancy_num.num += 1;
    };

    x();
    
    drop(fancy_ref);
}

prints:

error[E0505]: cannot move out of `fancy_num` because it is borrowed
  --> src/main.rs:11:17
   |
9  |       let fancy_ref = &fancy_num;
   |                       ---------- borrow of `fancy_num` occurs here
10 | 
11 |       let mut x = move || {
   |  _________________^
12 | |         fancy_num.num += 1;
13 | |     };
   | |_____^ move out of `fancy_num` occurs here
...
17 |       drop(fancy_ref);
   |            --------- borrow later used here

[nikomatsakis]

I'm having trouble deciding how to categorize this -- I guess it's ultimately a "minor missing suggestion" and so belongs in the Release Candidate camp. Still, let me leave a few mentoring notes here. I'll leave a few notes on strategy, at least:

First, in the MIR borrowck, we have a move of some value and we need to see whether that value is being moved into a closure. This would be right around here, in the code:

rust/src/librustc_mir/borrow_check/error_reporting.rs

Line 172 in 860d169

err.span_label(span, format!("move out of {} occurs here", value_msg));

We need to examine the MIR statement that performs the move (the location of which is identified by context.loc). If it is an assignment where the right hand side is an Rvalue::Aggregate, and the def_id of the aggregate is a closure (you can test with tcx.is_closure(def_id)), then indeed the move is occuring as part of closure construction.

In that case, you can find the index of the move amongst the arguments of the aggregate. That will be the upvar index. We can check the result of the freevars(def_id) query for the closure's def-id. Each FreeVar describes one variable captured by the closure; the vector has the same ordering as the aggregate fields. The field span of this struct is the span where the value is used within the closure:

rust/src/librustc/hir/mod.rs

Lines 2220 to 2221 in 860d169

	// First span where it is accessed (there can be multiple).
	pub span: Span

So we should extract that value and add the label on that span instead.