Overly conservative async capture analysis when values are borrowed

[nikomatsakis]

There's probably a bug filed already but this code does not compile, though it really ought to:

use std::sync::Mutex;

async fn foo(m: &Mutex<u32>) {
    let lock = m.lock().unwrap();
    if condition(&lock) {
        drop(lock);
        return bar().await;
    }
    
    drop(lock);
    return bar().await;
}

async fn bar() { }

fn is_send<T: Send>(t: T) { }

fn condition(x: &u32) -> bool {
    false
}

fn main() {
    let m = Mutex::new(22);
    is_send(foo(&m));
}

I believe the problem is specific to the &lock, which causes the capture analysis to get nervous -- even though lock is dropped. This is distilled from real-world code within Amazon.

Error you get today:

error: future cannot be sent between threads safely
  --> src/main.rs:26:13
   |
26 |     is_send(foo(&m));
   |             ^^^^^^^ future returned by `foo` is not `Send`
   |
   = help: within `impl Future<Output = ()>`, the trait `Send` is not implemented for `MutexGuard<'_, u32>`, which is required by `impl Future<Output = ()>: Send`
note: future is not `Send` as this value is used across an await
  --> src/main.rs:9:22
   |
6  |     let lock = m.lock().unwrap();
   |         ---- has type `MutexGuard<'_, u32>` which is not `Send`
...
9  |         return bar().await;
   |                      ^^^^^ await occurs here, with `lock` maybe used later
note: required by a bound in `is_send`
  --> src/main.rs:18:15
   |
18 | fn is_send<T: Send>(t: T) { }
   |               ^^^^ required by this bound in `is_send`

I'm nominated for async just to get some eyes on this. I'd be interested to discuss fixes, I have a few thoughts, though I'd have to look at the code too.

[nikomatsakis]

This is a variant of sorts of #57017

[compiler-errors]

Possibly fixed by #128846, which I would like to get landed eventually.

[compiler-errors]

Yes, it is fixed by that PR.

[nikomatsakis]

Ooh, nice

[Enselic]

Triage: Then someone just need to ensure there is a regression test. Either existing or new. Labelling as E-needs-test.

[lsunsi]

@Enselic Is this issue really needs-test? From what I understand the regression test comes after it's fixed, right? The PR that would supposedly have fixed the issue was closed and rust playground won't compile the original code in stable, beta or nightly.