Async closures are not allowed to reference all captured lifetimes if one of them is invariant

[tmandry]

Full example

async fn go<'a>(value: &'a i32) {
    let closure = async |scope: ScopeRef<'_, 'a>| {
        let _future1 = scope.spawn(async {
            let _v = *value;
        });
    };
}

yields

error: lifetime may not live long enough
  --> examples/repro.rs:52:63
   |
52 |       let closure = async |scope: ScopeRef<'_, 'a, i32>| -> i32 {
   |  ___________________-------------------------------------------_^
   | |                   |                      |
   | |                   |                      let's call the lifetime of this reference `'2`
   | |                   lifetime `'1` represents this closure's body
53 | |         let _future1 = scope.spawn(async { value });
54 | |         22
55 | |     };
   | |_____^ closure was supposed to return data with lifetime `'2` but it is returning data with lifetime `'1`
   |
   = note: closure implements `Fn`, so references to captured variables can't escape the closure
   = note: requirement occurs because of the type `Scope<'_, '_, i32>`, which makes the generic argument `'_` invariant
   = note: the struct `Scope<'scope, 'env, R>` is invariant over the parameter `'scope`

closure was supposed to return data with lifetime '2 ('scope) but it is returning data with lifetime '1 (closure body)

It's reasonable that we would create a lifetime representing the closure body, but naturally, the future should be allowed to reference it. The invariant lifetime must be mucking things up somehow.

(spawn has no lifetime params so I would not expect there to be a new lifetime created when it is called.)

note: closure implements Fn, so references to captured variables can't escape the closure

The closure definitely shouldn't try to implement Fn in this case, I don't know why it does.

note: requirement occurs because of the type Scope<'_, '_, i32>, which makes the generic argument '_ invariant
note: the struct Scope<'scope, 'env, R> is invariant over the parameter 'scope

It's correct that this lifetime is invariant. We must write to a vec of futures that outlive 'scope, so any variance would be unsound.

cc @compiler-errors

[compiler-errors]

@tmandry: Several things here:

Firstly, regarding this message:

The closure definitely shouldn't try to implement Fn in this case, I don't know why it does.

It's just that the borrowck diagnostics code isn't yet modified to say async Fn for async closure borrow errors; just a diagnostics issue. Don't worry too much about it, since it should be fixed separately.

Secondly, I think this all might just be a red herring with the lifetime invariance. The "which makes the generic argument '_ invariant" message is just a side-effect of some other error reporting and doesn't actually have to do with the issue here, probably?

So the real problem here is that the future that you pass into scope.spawn() actually borrows the captured value reference from the async closure, explaining the "but it is returning data with lifetime '1 (closure body)" part of the error message.

This can be fixed by passing an async move instead.

The fact that Rust chooses to borrow rather than copy the captured value (ref) into the inner async block is a consequence of some closure capture rules that I've been interested in investigating further, but I haven't really spent the time wrapping my head around them totally.

My vibe is that the preference to borrow rather than copy/move args can allow more code to compile in some cases, but obviously this may cause other problems (e.g. this issue).

This also probably has something to do with how the first example, scope_with_box, ends up implicitly copying the value ref rather than reborrowing it from the outer closure. Specifically, I'm actually surprised that the scope_with_box example doesn't end up giving a similar error about returning data referencing the captures from the outer closure.

[tmandry]

Thanks for your support @compiler-errors! Five stars would minimize again.

I'm also surprised by the difference from scope_with_box, and am interested if it points to something clever we could do for the closure case. From a lang perspective I'd really like it if we could make the original code compile. Using move to mean "copy my reference instead of borrowing it" is really quite subtle and not the meaning most people would associate with that keyword.

That said, in this particular API I think you always want move.

[compiler-errors]

@tmandry: I think my next step will be to understand the box case. I'm not certain if it's worth thinking about what to change until we can completely understand what's going on here.

I do agree that the code going from fail->pass with the addition of a move kw is really unnecessarily subtle, tho.

Regarding diagnostics, I don't know if there's a way to get borrowck to understand how to suggest this change, though, given that it's pretty far along past closure capture analysis.

And the same question for making closure capture analysis more sophisticated too. Again--will probably need to understand exactly what is happening better.

[tmandry]

I think I found an important difference: In the box case the future is bound to outlive 'scope (by the definition of BoxFuture), but not in the closure case. This is an important bound because we need to be able to poll the future for as long as 'scope is live.

Adding such a bound (see line 31) fails compilation with a couple of 'static obligations, for some reason.

[compiler-errors]

Well partly this is because we don't need to prove that outlives bound for all 'scope, but just for all 'scope shorter than 'env. We don't have conditional higher ranked lifetime bounds, yet.

The type erasure of dyn probably side-steps this problem for the box case, so we're not accidentally requiring for<'scope> 'env: 'scope to hold by accident.

[traviscross]

@rustbot labels +WG-async +AsyncAwait-Triaged

We discussed this in the async triage call and, based on the discussion here in this thread, this is definitely triaged, and it seems that more investigation is needed.