Treat closures as `move` when their type escapes their captures' scope.

[eddyb]

Unlike previously discussed inference of move from lifetimes (which has been deemed unfeasible/undesirable), we can completely ignore lifetime bounds and NLL analysis.

Note: everything discussed here "trivially" doesn't pass lifetime inference/checks currently, so allowing such code to compile should be entirely backwards-compatible.

---

Instead, I think we should focus on expressions that had their types inferred to contain closure types, and compare their (lexical/"drop") scopes with the scopes the closure captures are declared in, e.g.:

let x = String::new();
let f = {
    let y = vec![0];
    Some(|| (x.clone(), y.clone()))
};

typeof f is Option<closure>, where the closure captures x and y, and the only way the closure could be capturing any of them by reference, is if their scopes contain the scope of f.
This is true for x (assuming f is as far as the closure escapes), but not y, and the closure should therefore capture x by value, allowing that code to compile without explicit move.

Another, perhaps more common example is .flat_map(|x| (0..n).map(|i| x + i)) - the inner closure escapes the scope of x (by being returned from the outer closure).

---

If the closure is turned into a trait object (e.g. Box<dyn Fn(...) -> _>) in the scope where it is created in (with all the captures in scope), we can't change anything there, since it'd require lifetime analysis.

But since the stabilization of impl Trait, it's becoming increasingly common to return various types that contain closures (such as iterators), and that case can be readily served by this change, e.g.:

fn compose<A, B, C>(
    f: impl Fn(A) -> B,
    g: impl Fn(B) -> C,
) -> impl Fn(A) -> C {
    // Currently requires `move`, but we can solve that.
    /*move*/ |x| g(f(x))
}

---

There's only one issue I could think of, with this approach: Copy by-value captures can have surprising outcomes, in that the capture leaves the original accessible, without a syntactical indication, e.g.:

let outer_get_x = {
    let mut x = 0;
    let get_x = || x;
    x += 1;
    get_x
};
assert_eq!(outer_get_x(), 0);

If x were to be declared before outer_get_x, then this code would not compile, as the closure would be borrowing it (which would conflict with x += 1 mutating x), instead of holding onto a copy of it.

What can we do? I think that ideally we'd treat the closure as capturing by borrow, but only in terms of preventing mutable access to x for the remainder of its scope, and otherwise capture a copy.
(Then you could only observe a problem if you had a copyable type with interior mutability.)

But I'm not sure what the best implementation approach for that would be.

cc @nikomatsakis @cramertj @withoutboats

[nikomatsakis]

@eddyb

What can we do? I think that ideally we'd treat the closure as capturing by borrow, but only in terms of preventing mutable access to x for the remainder of its scope, and otherwise capture a copy.

Plausible. We had also talked about the idea of generally trying to lint against having a closure that captures a mutable variable where the variable is subsequently modified.

The only problem I see is that you do sometimes want the ability to do this. I could imagine here requesting an explicit move keyword — I think this would presently be required anyway, right?

Put another way: I think that now, in the absence of a move keyword, the closure would always borrow the Copy value, and hence subsequent mutations must be errors. So the new pattern being enabled is only when the move keyword is inferred. Therefore, we can lint against it.

I guess this isn't really different than what you proposed, except that I'm using the word "lint" instead of "special borrowck rule".

Anyway, I'm a fan, this would also address a common problem that I have of wanting something to be "move" but not for all variables, just for those within a certain scope. Example:

let i = vec![1, 2, 3];
let closures = vec![];
for &x in &something {
    closures.push(|| use(i, x)); // ERROR because `x` is out of scope
}

[eddyb]

The only problem I see is that you do sometimes want the ability to do this. I could imagine here requesting an explicit move keyword — I think this would presently be required anyway, right?

Yeah, but I think move is a wart - I'd prefer to make the copy before capturing it, that seems more clear in terms of what's going on, IMO.

[eddyb]

Oh, also, we can lint to remove move if all of your captures are inferred by-value anyway.
That way, we can get people to clean up their code, eventually.