temporary lifetime around await is (maybe) unexpectedly short

[nikomatsakis]

So @sfackler pointed out something to me. The problem @sfackler reported was that temporary lifetimes around await were too short. The effect was that foo(&[123]).await would give an error where foo(&[123]) would not (in sync code). My assumption was that this was caused by our temporary lifetimes being derived from the desugared version of await, which doesn't seem like what one might expect.

Here is an example showing the problem:

async version, playground

sync version, playground

[nikomatsakis]

cc @rust-lang/lang

[eddyb]

I assume this would also apply to foo("123").await, i.e. the input is definitely &'static _, and that lifetime is not propagated to the output?

[matthewjasper]

So we could get the expected temporary lifetime by changing the HIR lowering from

{
    let mut pinned = future;
    loop {
        match ::std::future::poll_with_tls_context(unsafe {
            <::std::pin::Pin>::new_unchecked(&mut pinned)
        }) {
            ::std::task::Poll::Ready(result) => break result,
            ::std::task::Poll::Pending => {}
        }
        yield ();
    }
}

to

match future {
    mut pinned => loop {
        match ::std::future::poll_with_tls_context(unsafe {
            <::std::pin::Pin>::new_unchecked(&mut pinned)
        }) {
            ::std::task::Poll::Ready(result) => break result,
            ::std::task::Poll::Pending => {}
        }
        yield ();
    }
}

There is a trade off for a larger temporary lifetime:

• All temporaries in future will be considered to live across a yield for the purpose of auto-traits.
• Borrowed temporaries in future are likely to be considered to be live across the yield for the purpose of the generator transform.

[Centril]

This does seem quite unexpected; cc also #63778.

[nikomatsakis]

Another example, copied from #63778

struct Test(String);

impl Test {
    async fn borrow_async(&self) {}

    fn borrow(&self) {}

    fn with(&mut self, s: &str) -> &mut Self {
        self.0 = s.into();
        self
    }
}

async fn test() {
    // error[E0716]: temporary value dropped while borrowed
    Test("".to_string()).with("123").borrow_async().await;
}

fn main() {
    // Temporary outlives the borrow() call
    Test("".to_string()).with("123").borrow();
}

https://play.rust-lang.org/?version=nightly&mode=release&edition=2018&gist=2649e4f172c54090d759b2f9484b31e1

[nikomatsakis]

I've nominated for discussion at the @rust-lang/lang meeting -- as @matthewjasper points out, longer temporary lifetimes are in some sense less efficient, but I still think the correct thing is to be consistent with the behavior of sync code, and I would advocate for that (seems analogous to us deciding not to alter the drop order).

[nikomatsakis]

We discussed this in the lang-team meeting last Thursday. We decided that we want to align the semantics of sync/async code (i.e., this is indeed a bug).

There are some downsides: temporaries that live across the await will cause larger generators, and fixing that will be non-trivial in that it affects when destructors run which is visible to end users.

Nonetheless, the "surprise factor" -- and the fact that eronomic APIs are often dependent on the existing temporary lifetime rules -- seems to dictate that the semantics should align.

(It may be possible in the future to enable destructors to sometimes run earlier -- but that would be a design we can pursue independently and uniformly for both sync/async code.)