rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.57k stars 12.74k forks source link

Allow storing `format_args!` in a let binding #92698

Open piegamesde opened 2 years ago

piegamesde commented 2 years ago

At the moment, we cannot store the result of a format_args! in a value:

// Okay, we can somehow store it in a variable
let foo = format_args!("{} foo {:?}", 1, 2);

// But once we try to use it, we get
// "error[E0716]: temporary value dropped while borrowed"
// "note: consider using a `let` binding to create a longer lived value"
// "note: this error originates in the macro `format_args`"
println!("{}", foo);

// Using it directly is okay though
println!("{}", format_args!("{} foo {:?}", 1, 2));

// We can get pretty creative within the "direct usage" constraints :D
let foo = match format_args!("{} foo {:?}", 1, 2) {
    foo => {println!("{}", foo);},
};

// But even if we manage to store the value in a struct, …
struct Foo<'a> {foo: std::fmt::Arguments<'a> }
let foo = Foo { foo: format_args!("{} foo {:?}", 1, 2) };

// … we still cannot make use of it outside of the scope it was created in
println!("{}", foo.foo); // <- error[E0716]: temporary value dropped while borrowed

The list of confused or annoyed users by this is rather long:

I understand if the format_args! macro cannot be changed, but then please provide an alternative way of building fmt::Arguments without that restriction. Even a small performance overhead (for example cloning the values) would be an improvement compared to the common workarounds that have to be used otherwise.

dtolnay commented 2 years ago

This would be fixed by this accepted RFC: #15023.

HindrikStegenga commented 2 years ago

So I ran into this issue, but at the same time I managed to find a rather nifty workaround. Basically it works if you use an anonymous closure and directly invoke it. (In my case I needed the length of the outputted string)

        (|buf : &mut fmt::Formatter, args : std::fmt::Arguments<'_>| {
            let i = args.to_string().len();
            buf.write_fmt(args)?;

            for _ in 0..(128 - i) {
                write!(buf, " ")?;
            }
            writeln!(buf, "{}:{}", file, line)
        })(buf, format_args!("[{}][{}] - {}", level, target, args))

For me this works and compiles on stable 1.61.

piegamesde commented 2 years ago

Yes, this looks pretty much like the same hack fern is using with its FormatCallback.

albel727 commented 2 years ago

@HindrikStegenga

(In my case I needed the length of the outputted string)

An exercise in futility, since you're calling args.to_string(), i.e. allocating a temporary string with the output anyway. You could've just called format!() right away and reused that same string for the rest of your work with buf.write_str(), instead of bothering with format_args!() and then redoing the formatting work a second time with buf.write_fmt().

If you really want to get the length of the outputted string without allocating, you could try calculating it manually, e.g. level.len() + target.len() + args.len() + 7. Or, more generally, you could make your implementor of std::fmt::Write, that would sum the lengths of the received strings in its write_str(). You could then feed your args to it with std::fmt::write().

Example in Playground

tmccombs commented 1 year ago

At the very least this surprising behavior should be better documented. And maybe, the error message should give a better recommendation of what to do when this happens for format_args!.

The suggestion of changing let args = format_args!("{}", thing); to

let binding = format_args!("{}", thing);
let args = binding;

isn't really helpful.

piegamesde commented 1 year ago

With the changes made to format_args! in https://github.com/rust-lang/rust/pull/106745/, would it allow us to fix this issue?

m-ou-se commented 1 year ago

With the changes made to format_args! in #106745, would it allow us to fix this issue?

Nope, that's unrelated. format_args's expansion includes temporaries whose lifetimes need to be extended. We currently don't have a flexible way to do that. Temporary lifetime extension only applies to very few types of expressions, which don't include function calls. See also some related thoughts here on zulip: https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/design.20meeting.202023-03-15.3A.20temporary.20lifetimes/near/351415051

Stargateur commented 1 year ago

At the very least this surprising behavior should be better documented. And maybe, the error message should give a better recommendation of what to do when this happens for format_args!.

The suggestion of changing let args = format_args!("{}", thing); to

let binding = format_args!("{}", thing);
let args = binding;

isn't really helpful.

Yes the error message is very misleading and doesn't make sense. We can add this stackoverflow question to the list of confused people.

We could at least link this issue in the documentation of format_args

daniel-pfeiffer commented 1 year ago

To make a deeply nested format_args more readable, I want to pull it apart

let y = format_args!("<{x}>");
println!("{y}");

As noted above, this gives

6 | let y = format_args!("<{x}>");
  |         ^^^^^^^^^^^^^^^^^^^^^- temporary value is freed at the end of this statement
  |         |
  |         creates a temporary value which is freed while still in use
7 | println!("{y}");
  |           --- borrow later used here
  |
  = note: this error originates in the macro `format_args` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider using a `let` binding to create a longer lived value
  |
6 + let binding = format_args!("<{x}>");
7 ~ let y = binding;

Weird suggestion, but if I follow it

let binding = format_args!("<{x}>");
let y = binding;
println!("{y}");

It chokes on its own previous suggestion, giving an even weirder one, as it doesn't see binding is already used

6 | let binding = format_args!("<{x}>");
  |               ^^^^^^^^^^^^^^^^^^^^^- temporary value is freed at the end of this statement
  |               |
  |               creates a temporary value which is freed while still in use
7 | let y = binding;
  |         ------- borrow later used here
  |
  = note: this error originates in the macro `format_args` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider using a `let` binding to create a longer lived value
  |
6 + let binding = format_args!("<{x}>");
7 ~ let binding = binding;

So I thought: if some temporary is too short lived, let me turn it all into one statement. Essentially like @HindrikStegenga's passing it to a closure, but in natural order

if let y = format_args!("<{x}>") {
    println!("{y}");
}

Even though this works as desired, it gives

6 | if let y = format_args!("<{x}>") {
  |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: this pattern will always match, so the `if let` is useless
  = help: consider replacing the `if let` with a `let`
  = note: `#[warn(irrefutable_let_patterns)]` on by default
daniel-pfeiffer commented 1 year ago

Until this gets solved, I wrapped @HindrikStegenga's closure in natural syntax:

macro_rules! let_workaround {
    (let $name:ident = $val:expr; $($rest:tt)+) => {
        let f = |$name| { // naming closure avoids #[allow(clippy::redundant_closure_call)]
            let_workaround! { $($rest)+ }
        };
        f($val)
    };
    ($($rest:tt)+) => { $($rest)+ }
}

fn main() {
    let (a, b, c) = (1, 2, 3);
    let f_ab = let_workaround! {
        let f_a = format_args!("a {a}");
        let f_b = format_args!("b {b}");
        format!("{f_a}, {f_b}") // hidden closure can't return format_args 😟
    }; // return value: semicolon on outer statement
    let_workaround! {
        let f_c = format_args!("c {c}");
        let f_abc = format_args!("{f_ab}, {f_c}");
        println!("{f_abc}");
    } // no return value: no semicolon on block
    println!("done");
}

What this can't solve, is the lack of ? :. if else just isn't equivalent, as format_args! also can't be returned from a block. E.g. you can't choose efficient low-level formatting at runtime:

if something { format_args!("a {a}") }
else { format_args!("b {b}") }
daniel-pfeiffer commented 1 year ago

Turns out clippy doesn't like this. What clippy misses here, is that this currently seems to be the only way to extend the temp lifetimes. So clippy is wrong. :-1:

Edit 1: Added an annotation above, to silence this clippy warning. Or try @dtolnay's workaround below. However that's semantically the same as my if let (on June 14th.) So sooner or later it may also warn that the match is useless.

Edit 2: Changing the above again, as I didn't want the annotation to also be on the body of the closure (where it might be valid for some other usage). By splitting it (semantically the same) into a let and a call, clippy shut up on its own for now. Note that even though the braces are macro syntax and not a block, f still does not leak out (because of partial hygiene.)

dtolnay commented 1 year ago

Use match:

macro_rules! let_workaround {
    (let $name:ident = $val:expr; $($rest:tt)+) => {
        match $val {
            $name => {
                let_workaround! { $($rest)+ }
            }
        }
    };
    ($($rest:tt)+) => { $($rest)+ }
}
quaternic commented 1 year ago

@daniel-pfeiffer

What this can't solve, is the lack of ? :. if else just isn't equivalent, as format_args! also can't be returned from a block. E.g. you can't choose efficient low-level formatting at runtime:

if something { format_args!("a {a}") }
else { format_args!("b {b}") }

There is a workaround for that:

macro_rules! select {
($cond:expr, $iftrue:expr, $iffalse:expr) => {
'outer:  {
(
'inner: {
if $cond { break 'inner }
break 'outer $iffalse
},
$iftrue
).1
}
}
}

To clarify the structure, that's essentially like

let tuple = (
if condition { break iftrue } ,
iffalse
);
break tuple.1;

or just

if condition {
break iftrue
}
break iffalse

so select!(cond, A, B) means the same as if cond { A } else { B } but without the separate drop scopes for the branches.

In combination with the above variant of let_workaround from @dtolnay, you can write stuff like

let_workaround! {
    let ab = select!(a < b, format_args!(" < {b}"),
             select!(a > b, format_args!(" > {b}"),
                            format_args!("")));
    let bc = select!(b < c, format_args!(" < {c}"),
             select!(b > c, format_args!(" > {c}"),
                            format_args!("")));
    let ca = select!(c < a, format_args!(" < {a}"),
             select!(c > a, format_args!(" > {a}"),
                            format_args!("")));
    let min = a.min(b).min(c);
    let f_abc = select!(min == a, format_args!("{a}{ab}{bc}{ca}"),
                select!(min == b, format_args!("{b}{bc}{ca}{ab}"),
                select!(min == c, format_args!("{c}{ca}{ab}{bc}"),
                                  unreachable!())));
    write!(f, "{f_abc}")
}

https://godbolt.org/z/EcKqKTKGq

(This came up in related discussion in https://internals.rust-lang.org/t/format-args-with-long-lifetimes/19494)

justinhaubrich commented 1 month ago

So I ran into this issue, but at the same time I managed to find a rather nifty workaround. Basically it works if you use an anonymous closure and directly invoke it. (In my case I needed the length of the outputted string)

        (|buf : &mut fmt::Formatter, args : std::fmt::Arguments<'_>| {
            let i = args.to_string().len();
            buf.write_fmt(args)?;

            for _ in 0..(128 - i) {
                write!(buf, " ")?;
            }
            writeln!(buf, "{}:{}", file, line)
        })(buf, format_args!("[{}][{}] - {}", level, target, args))

For me this works and compiles on stable 1.61.

Thanks for this. Using a closure is the only way I was able to get it to work.

daniel-pfeiffer commented 1 day ago

Since Rust hasn’t yet solved these problems, I’ve extended these solutions and published them.