Scope format! temporaries

[jonhoo]

This places the temporaries that format! generates to refer to its arguments (through &dyn Trait) in a short-lived scope surrounding just the invocation of format!. This enables format! to be used in generators without the temporaries preventing the generator from being Send (due to dyn Trait not being Sync).

See #64477 for details.

[rust-highfive]

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

[jonhoo]

See in particular @nikomatsakis' description of the problem in #64477 (comment) and @Nemo157's proposed solution in #64477 (comment) (which this PR implements).

[Centril]

This is a breaking change (that may only be noticed in the dynamic semantics rather than the static semantics). By inserting the let you are changing the drop order:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=3231f995544ca8cf48045a108a28ac78

[jonhoo]

That is true — I hadn't considered that. I wonder if there's an easy way for us to just cause the created &dyn Trait items to be dropped, and not the inputs. I can't immediately think of one 🤔

[jonhoo]

@Centril thinking some more about this, I don't think there's a way to get out of changing the drop order except by making format! actually parse its arguments and ensure that it takes a reference to each one somehow. I toyed around with something like

macro_rules! format {
    ($($arg:tt)*) => ((|| {$crate::fmt::format($crate::__export::format_args!($($arg)*))})())
}

But the closure tries to move arguments passed to format! without & (despite not having a move annotation), so that won't work.

In #64477 (comment), @nikomatsakis said:

Unfortunately, the format! temporaries include dyn Trait objects that are not Send. So the error is correct. The shorter-term fix would I think be to alter the desugaring of format! so that its intermediate values are Send, I'm afraid.

I'm not sure how we could go about doing that without adding additional bounds on types passed to format! (which would definitely be a breaking change). If you have suggestions though, I'm all ears.

Ultimately, my guess at the moment is that we will either have to change drop order, have format! fully parse its arguments so it can drop only the references, or change the bounds of format!. I think the last one is out. The second one would probably be a fairly large change (possibly even making format! a proc-macro), and would be error-prone as we'd have to replicate the parsing from format_args! and then appropriately transform all inputs. The original suggestion seems like a neat compromise, but we would then have to decide whether the change in drop order is acceptable.

[Centril]

@craterbot run mode=build-and-test

(Changes dynamic semantics possibly.)

[craterbot]

👌 Experiment pr-64856 created and queued.
🤖 Automatically detected try build 4465d0049bf00b9224f1908770f277c145361aed
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[brunoczim]

Do this mean we can use format! inside multi-threaded futures?

[sfackler]

You can already use format in multi-threaded features. You just can't put it in an expression with an .await without this change.

[brunoczim]

It is just that... this does not seem to work.

/*
[dependencies]

tokio = { version = "0.2.6", features = ["io-util", "io-std", "macros"] }
*/

use std::{fmt, future::Future, pin::Pin};
use tokio::io::{self, AsyncWriteExt};

pub trait AsyncWriteFmt {
    fn write_fmt<'this, 'args, 'async_trait>(
        &'this mut self,
        args: fmt::Arguments<'args>,
    ) -> Pin<Box<dyn Future<Output = io::Result<()>> + Send + 'async_trait>>
    where
        'this: 'async_trait,
        Self: Sync + 'async_trait;
}

impl AsyncWriteFmt for io::Stdout {
    fn write_fmt<'this, 'args, 'async_trait>(
        &'this mut self,
        args: fmt::Arguments<'args>,
    ) -> Pin<Box<dyn Future<Output = io::Result<()>> + Send + 'async_trait>>
    where
        'this: 'async_trait,
        Self: Sync + 'async_trait,
    {
        let string = format!("{}", args);
        drop(args);
        Box::pin(async move { self.write_all(string.as_bytes()).await })
    }
}

async fn write_hello() {
    write!(io::stdout(), "Hello, World!\n").await.unwrap()
}

fn main() {
    tokio::spawn(write_hello());
}

write_hello is not Send + Sync here. Because of this, it will fail with the following error on the last nightly:

error: future cannot be sent between threads safely
   --> src/main.rs:48:5
    |
48  |     tokio::spawn(write_hello());
    |     ^^^^^^^^^^^^ future returned by `write_hello` is not `Send`
    | 
   ::: /home/bruno/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-0.2.6/src/task/spawn.rs:123:21
    |
123 |         T: Future + Send + 'static,
    |                     ---- required by this bound in `tokio::task::spawn::spawn`
    |
    = help: within `core::fmt::Void`, the trait `std::marker::Sync` is not implemented for `*mut (dyn std::ops::Fn() + 'static)`
note: future is not `Send` as this value is used across an await
   --> src/main.rs:44:5
    |
44  |     write!(io::stdout(), "Hello, World!\n").await.unwrap()
    |     ---------------------------------------^^^^^^
    |     |
    |     await occurs here, with `$ crate :: format_args ! ($ ($ arg) *)` maybe used later
    |     has type `[std::fmt::ArgumentV1<'_>; 0]`
45  | }
    | - `$ crate :: format_args ! ($ ($ arg) *)` is later dropped here
    = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

EDIT: this will deliberately panic, but the code is just to show compile time issues.

[sfackler]

That error is coming from write!, not format!. fmt::Arguments are not Send, so any future containing them will not be Send.

[brunoczim]

Well, this seems to work, weirdly.

/*
[dependencies]

tokio = { version = "0.2.6", features = ["io-util", "io-std", "macros"] }
async-trait = "0.1.21"
*/

use async_trait::async_trait;
use std::{fmt, future::Future, pin::Pin};
use tokio::io::{self, AsyncWriteExt};

pub trait AsyncWriteFmt {
    fn write_fmt<'this, 'args, 'async_trait>(
        &'this mut self,
        args: fmt::Arguments<'args>,
    ) -> Pin<Box<dyn Future<Output = io::Result<()>> + Send + 'async_trait>>
    where
        'this: 'async_trait,
        Self: Sync + 'async_trait;
}

impl AsyncWriteFmt for io::Stdout {
    fn write_fmt<'this, 'args, 'async_trait>(
        &'this mut self,
        args: fmt::Arguments<'args>,
    ) -> Pin<Box<dyn Future<Output = io::Result<()>> + Send + 'async_trait>>
    where
        'this: 'async_trait,
        Self: Sync + 'async_trait,
    {
        let string = format!("{}", args);
        drop(args);
        Box::pin(async move { self.write_all(string.as_bytes()).await })
    }
}

async fn write_hello() {
    let mut stdout = io::stdout();
    let future = write!(stdout, "Hello, World!\n");
    future.await.unwrap()
}

fn main() {
    tokio::spawn(write_hello());
}