async/await: awaiting inside a match block captures borrow too eagerly

[seanmonstar]

If you use await!(some_fut) inside an arm of a match X, the generated future eagerly borrows the value of X, if it not needed.

This may not usually be noticeable, but the issue compounds when the type X contains a trait object, and the future you wish to return is impl Future + Send. This causes a misleading error message that "dyn Trait + Send cannot be shared between threads", which is required to for &X: Send.

Example

Here's a simple struct with a trait object:

struct Client(Box<Any + Send>);

Consider a function like this:

impl Client {
    fn status(&self) -> u16 {
        200
    }
}

You could consider using a match to determine what kind of future to await (or what arguments to pass):

async fn get() {
}

pub fn wat() -> impl Future + Send {
    let client = Client(Box::new(true));
    async move {
        match client.status() {
            200 => {
                let _x = await!(get());
            },
            _ => (),
        }
    }
}

If the await is moved out of the match block, all is well:

pub fn ok() -> impl Future + Send {
    let client = Client(Box::new(true));
    async move {
        if client.status() == 200 {
            let _x = await!(get());
        }
    }
}

The wat function causes this compilation error:

error[E0277]: `(dyn std::any::Any + std::marker::Send + 'static)` cannot be shared between threads safely
  --> src/main.rs:21:17
   |
21 | pub fn wat() -> impl Future + Send {
   |                 ^^^^^^^^^^^^^^^^^^ `(dyn std::any::Any + std::marker::Send + 'static)` cannot be shared between threads safely
   |
   = help: the trait `std::marker::Sync` is not implemented for `(dyn std::any::Any + std::marker::Send + 'static)`
   = note: required because of the requirements on the impl of `std::marker::Sync` for `std::ptr::Unique<(dyn std::any::Any + std::marker::Send + 'static)>`
   = note: required because it appears within the type `std::boxed::Box<(dyn std::any::Any + std::marker::Send + 'static)>`
   = note: required because it appears within the type `Client`
   = note: required because of the requirements on the impl of `for<'r> std::marker::Send` for `&Client`
   = note: required because it appears within the type `for<'r> {Client, &'r Client, u16, impl std::future::Future, ()}`
   = note: required because it appears within the type `[static generator@src/main.rs:23:16: 30:6 client:Client for<'r> {Client, &'r Client, u16, impl std::future::Future, ()}]`
   = note: required because it appears within the type `std::future::GenFuture<[static generator@src/main.rs:23:16: 30:6 client:Client for<'r> {Client, &'r Client, u16, impl std::future::Future, ()}]>`
   = note: required because it appears within the type `impl std::future::Future`
   = note: the return type of a function must have a statically known size

Playground: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=2a9dbea32d31457d50d40b99c52ee214 (updated to latest syntax -Niko)

[cramertj]

This is just an artifact of the compiler holding the borrow of client in your match expression for the duration of the match condition-- there's nothing async/await!-specific going on here. There's a potential fix to be made to eliminate the borrow of client sooner, but that's a general MIR representation issue.

[cramertj]

("liveness" is really the tag I want, but we don't have one for that ;) )

[jonas-schievink]

This might be a duplicate of #46525?

[seanmonstar]

I'm not certain it's only that; it does seem related to await!. The compiler seems to be fine with give up the borrow if I call a self or &mut self method inside the match.

[nikomatsakis]

Marking this as deferred -- the problem here (discussed here on Zulip) has to do with how we use an "overapproximation" to decide what data may be inside a generator, at least initially. This is based on the HIR. Only later do we get more precise results when generating MIR.

I think we could resolve this by making that over-appoximation more precise, but @Zoxc expressed a desire to do away with it altogether. This would obviously be better, if we can make it work, though I'm a bit skeptical. (See the Zulip topic for more details.)

[nikomatsakis]

I'm adding the AsyncAwait-Unclear label, because I'd like to revisit this question and try to make some progress here before we stabilize, but I'm not willing to call it blocking yet =)