This is a tracking issue for the RFC 2795 (rust-lang/rfcs#2795).
Steps:
Checklist for the tracking issue:
Steps:
format_args!
behind format_implicit_args
feature (name to be bikeshed?)format_args!(concat!(...))
case that doesn't risk spurious macro hygienepanic!
. Behind feature gate and / or edition switch (see unresolved question)Unresolved Questions:
format_args!(concat!(...))
- perhaps try out a new unstable macro as per this commentpanic!
solution - perhaps based off this commentProgress update from me: I've been busy but have started work on the implementation of this feature.
A bikeshed on the feature name: I'm leaning towards to the name format_args_capture
over the original proposed name format_args_implicits
from the RFC.
I think that format_args_capture
reflects the mechanics of the feature better - it enables capturing of variables from the surrounding scope. It also avoids the word "implicit" which I find can raise peoples' blood temperature (sometimes for good reason).
The feature gate error message (on nightly) would then look like this:
error: there is no argument named `foo`
--> C:\Users\david\dev\rust\src\test\ui\fmt\feature-gate-format-args-capture.rs:2:14
|
LL | format!("{foo}"); //~ ERROR: there is no argument named `foo`
| ^^^^^
|
= note: did you intend to capture a variable `foo` from the surrounding scope?
= help: add `#![feature(format_args_capture)]` to the crate attributes to enable
error: aborting due to previous error
Is it too late to use a different feature name?
maybe format_args_capture_shorthand
? "shorthand" is used in the docs for struct
field init shorthand.
@davidhewitt format_args_capture
seems reasonable, given the error message, and I agree with avoiding the word implicit
.
FWIW at least in my mind capturing implies something closer to what e.g. closures do where the variable is stored within the returned Arguments struct. But that's not something that can be added to Arguments given our stability guarantees, I think, even though I've often wanted it -- so maybe that's not a major concern.
I will note though that the naming of the feature gate and diagnostics feel like something that we can adjust over time and in the implementation PR, definitely no need to get them right immediately :)
- [ ] Final design of the
panic!
solution - perhaps based off this comment
Some notes about this problem:
There's several macros that currently that handle their no-formatting-arguments case specially, mostly for efficiency:
std::panic!()
core::panic!()
assert!()
, which forwards to either std::panic!()
or core::panic!()
In all cases, this results in these two problems:
x!("{}")
doesn't give an error, but just prints/uses literal "{}"
.x!("{var}")
will not work with this RFC.In the case of std::panic!()
, this happens because it accepts objects of any type. E.g. panic!(5)
will 'throw' a Box<Any>
containing 5
. So a panic!("hello")
will trigger that same case.
core::panic!()
doesn't throw boxes around, but still has a special case for efficiency. core::panic!("hello")
will call core::panicking::panic
(instead of panic_fmt
), which takes a &str
and constructs an optimal fmt::Arguments
using that &str
to avoid pulling in any formatting code.
Most of the crates from the community using a special no-arguments case are doing this for efficiency as well. Some will use args.to_string()
to format the message into a buffer first, to reduce the number of write/log/etc. calls. This would be an unnecessary copy of a &str
in the trivial case, which they try to avoid.
In the case of core::panic
and the macros found in various crates, this could've been solved if Arguments::as_str() -> Option<&str>
had existed, which would've meant that none of these macros needed a special case for efficiency. The trivial case could've been handled by the printing code instead, and the compiler would've easily eliminated the allocating case completely from the resulting binary. Unfortunately, changing that now will be a breaking change.
Not only will x!("{}")
stop working, but most of these macros (including core::panic!()
) also accept full expressions that evaluate to a &str
right now, because they simply forward their one argument to a function that takes a &str
. So:
core::panic!(&String::from("hello"))
is accepted today.Unless breaking these examples is acceptable, this would need to be changed over a Rust edition. For both std
's and core
's panic!()
(and assert!()
). What exact behaviour makes most sense (also in terms for warnings or clippy lints) in edition 2018 at that point, is still unclear. What behaviour makes sense in edition 2021 is also not clear:
std::panic!(&String::from("hello {}"))
? If it accepts arbitrary objects, this would still compile, but act differently than std::panic!("hello {}")
. Also, special casing on a string literal is currently not possible with macro_rules!()
. $x:literal
is possible, but would break panic!(5)
. Perhaps a separate panic function or macro for throwing boxes is necessary. (In that case, using panic!()
for arbitraty objects could already get deprecated in edition 2018.)core::panic!()
. The most trivial solution would mean that core::panic!({ let x = "abc"; x })
no longer compiles. Special casing on $x:literal
would work fine here, but it might be better to keep things simple.assert!()
follows the behaviour of either core::panic!()
or std::panic!()
depending on whether it is used in no_std
.)For the macros in the community crates, there are two options w.r.t. an edition change:
format_args_or_literal!()
to allow them to have the same behaviour over editions as the core panic macro. This macro would either return an enum, or expect users to use .as_str()
as described above to handle the trivial case. All these crates will need to be updated, but their behaviour will not change for users staying on edition 2018..as_str()
to handle the trivial case, and not tie their behaviour to the Rust edition. These crates will have to release a breaking change and bump their version. (Resulting in a bit more breakage but less complexity.)My proposal:
format_args!()
, without the special case, but the core/std macros only in edition 2021. This means dropping support for panic!("{}")
, panic!({ let x = "abc"; x})
, panic!(5)
, panic!(bla)
etc.Arguments::as_str()
for optimizing the trivial case, instead. Expect community crates to start using that in all editions.panic()
function in std
to throw Box<Any>
s.Update: The core/std part of this is now an RFC: https://github.com/rust-lang/rfcs/pull/3007
Thanks @m-ou-se for the detailed write-up. I'm nominating this for @rust-lang/lang discussion since it has Edition ramifications, although I guess that the design of these macros is in many ways a @rust-lang/libs decision.
I agree with you that an edition is the appropriate way to manage this, and I think we should shoot for panic!("{var}")
working consistently. We'd have to look carefully at how to implement this, I don't think it would work using the mechanisms we have, but I think it could be made to work. I'd probably say that "non-string-constants" should just be deprecated, but I don't have a strong opinion about it.
The automated migration seems easy enough, in any case, since panic!(...)
just becomes panic!("{}", ...)
, right?
The automated migration seems easy enough, in any case, since
panic!(...)
just becomespanic!("{}", ...)
, right?
For core::panic!()
, yes. But for std's, no. std::panic!("hello")
throws a &str
, not a fmt::Arguments
or String
. The standard panic handler would print the same, but custom panic hooks can tell the difference. Also, std::panic!()
can throw things that don't implement Display/Debug right now.
I can see the panic!
change working by exporting a different macro in the new 2021 prelude.
I'm generally in favor of removing Box<Any>
support from the panic!
macro: it's very rarely used and as @m-ou-se said, this functionality can still be provided via a normal function in std.
Something missing from my notes/proposal above is making the behaviour consistent or at least not too unexpected on 2018/2015, after format_implicit_args
becomes stable.
If the changes to core/std's panic!() are gated behind edition 2021, then in 2018/2015 we get:
let a = 5;
println!("{a}"); // prints "5"
panic!("{a}"); // panics with "{a}"
println!("{a} {}", 5); // prints "5 5"
panic!("{a} {}", 5); // panics with "5 5"
Possible ways to make this better:
format_implicit_args
behind edition 2021. No println!("{a}")
in 2018. :(format_implicit_args
for panics behind edition 2021. No panic!("{a} {}", 5)
in 2018.panic!("{a}")
warns, but still prints "{a}"
. (But what should the warning suggest? panic!("{}", a)
? v2::panic!("{a}")
? "Switch to 2021"? ..?)panic!("{")
? panic!("{}", "{")
? panic_str("{")
? ..?)None of these seem great. :(
- Break things and don't gate anything behind an edition, and hope that literal braces (and non string literals) in single argument panics are uncommon right now. Possibly by starting giving deprecation warnings first for a while. (Though what should those warnings suggest for
panic!("{")
?panic!("{}", "{")
?panic_str("{")
? ..?)
This could also be panic!({ "{" })
/ panic!(("{"))
, right? Still not great of course.
This could also be
panic!({ "{" })
/panic!(("{"))
, right? Still not great of course.
Maybe, but if panic!("{")
would be deprecated, I suppose using expressions like that should be deprecated as well.
We discussed this in today's @rust-lang/lang meeting. The sense of the meeting was:
println!
, panic!
, etc all behave in a uniform fashion.panic!({"{"})
ought to be deprecated, however.We did not discuss in detail the comments I see here, such as whether
panic!({"{"})
ought to be deprecated, however.
My personal opinion is that panic!({expression})
would be quite reasonable as a way to 'panic with some explicit value', as opposed to panic!("...", ...)
as the "with arguments" form.
One interesting question that I don't know the answer to is whether panic!("foo")
ought to panic with a &'static str
or a String
-- it introduces an obvious "non-uniformity" in the type, depending on whether it contains "format notation". I do think that panic!("foo{{")
ought to be required in order to name a {
(i.e., we should interpret a string literal as a format string). I think I would tend towards panic!("foo")
raising a String
, and requiring panic!({"foo"})
to panic specifically with a string literal, but that's a lightly held opinion.
My preference would be to deprecate panic!
with arbitrary expressions and only support format strings. I have never seen a use of non-string panics in the wild, so I think this functionality is better provided by a function in std::panicking
that takes a Box<Any>
.
Supporting Anicdote: I used a non-string panic once on accident and it took me well over 30 seconds to even understand what had gone wrong (because the compiler accepted it, so "clearly" i must have typed the right thing).
I too have only only ever experienced non-string panics in error. Especially the first time I did this it took quite a while to figure out where the Box<dyn Any>
was originating from.
@nikomatsakis when you say the lang team intends to keep all existing functionality, do you mean that all of the functionality remains on the macros? Or is moving the "panic with expression" case to a new function potentially on the table?
My reservation with the proposed panic!({foo})
syntax for "panic with box any containing foo" is that it's dangerously close to panic!("{foo}")
. I think especially beginner Rust users won't easily spot the difference and will also be the ones most confused when they find their panic message is broken.
As for a migration strategy, I prefer @m-ou-se 's option 3. I would like to propose that we do the following things at the same time without waiting for an editon change:
panic!
, as this RFC is non-breaking for these cases.panic!
macropanic!("{foo}")
- this would be panic!("{}", format_args!("{foo}"))
if formatting was intended, and maybe panic!({"{foo}"})
if a literal was intended. (Though this nested set of sigils is quite heavy, which again makes me think migrating the "panic with a value" case to a function would be desirable).After the edition change the single-argument panic case changes to exclusively expect a format string, and we're done!
I think this can be implemented by changing the single-argument panic arm to forward to a new compiler-builtin macro. This macro would be the only code that needs to change behaviour with an edition change.
I need to think further about the exact design of this new built-in macro. I think some downstream libraries which behave similarly to panic!
may also want to use this macro to support cross-edition behaviour. I'll try to come back with a design hopefully at the weekend if y'all haven't already figured it out by then! 馃槃
I agree that panic!({"foo"})
is perhaps too close to panic!("{foo}")
. Another option is to have a panic_with!
macro or something like that (though this name is perhaps non-ideal, since it suggests a closure).
I would like to propose that we do the following things at the same time without waiting for an editon change:
In general, I think the way to "phase in" new syntax should fit this mold (and your suggestion does):
panic!("{foo}")
, in your proposal).I guess my one concern with the plan as described is that it seems like it's a bit surprising that panic!("{x} {}", "foo")
will print out x
but panic!("{x}")
will not. It might be cleaner to have folks "opt in" to the new semantics completely, perhaps by an explicit use std::prelude::v2021::panic
or something like that? Have to think about that a bit.
Another option is to have a panic_with! macro or something like that (though this name is perhaps non-ideal, since it suggests a closure).
panic_with!
is actually something I was wondering about yesterday too, though I hadn't considered the point you make about the closure.
One idea that crossed my mind was simply making this panic-with-a-value case a function called start_unwind
. It fits well with the existing catch_unwind
/ resume_unwind
. OTOH if it needs to stay a macro, I might avoid start_unwind!
because the other two are functions.
I kind of like throw!
or panic_throwing!
(because it pairs with "catch" in catch_unwind
). Maybe of the two, panic_throwing!
is better because it keeps the panic
terminology rather than suggesting this is some alternate error handling mechanism.
I guess my one concern with the plan as described is that it seems like it's a bit surprising that panic!("{x} {}", "foo") will print out x but panic!("{x}") will not. It might be cleaner to have folks "opt in" to the new semantics completely, perhaps by an explicit use std::prelude::v2021::panic or something like that? Have to think about that a bit.
馃憤 I don't have a strong opinion on whether it should be opt-in so happy to defer to core team members on this point.
One thing to consider is that perhaps it's already surprising for a lot of people that panic!("{x}")
is not a compile error - so by starting to emit a warning for this case we might rescue a few people from hidden bugs!
panic!
is a macro because it does formatting.
Presumably if it's taking a single value the start_unwind
operation could just be a generic function like fn start_unwind<T>(t: T) -> !
Panicking doesn't necessarily unwind, with the panic=abort
setting, or with no-std
and a custom panic_handler
.
Panicking doesn't necessarily unwind, with the panic=abort setting, or with no-std and a custom panic_handler.
Great point - in which case I think start_unwind
is not a good name for this any more!
Opened #74622 specifically for this arbitrary-payload-throwing panic function, to have a place to bikeshed over the name etc.
My preference would be to deprecate panic! with arbitrary expressions and only support format strings. I have never seen a use of non-string panics in the wild, so I think this functionality is better provided by a function in std::panicking that takes a Box
.
When the ability to use arbitrary expressions was originally proposed in 2012, the only rationale given was "in the future it will probably need to accept any sendable type argument, not just a string", so in retrospect it was probably just a mistaken prediction of how the idioms governing its use would evolve over time. I second the idea that panic should just support format strings (the only possible use case I can imagine is as a minor aid for printline debugging, but we have dbg!
these days for that).
Regarding the panic!
changes, even the already performed changes to core::panic!(foo)
(as part of https://github.com/rust-lang/rust/pull/74056) are potentially problematic:
no_std
(https://github.com/rust-lang/rust/issues/51999#issuecomment-687590082, a nightly-only regression)So whatever we end up doing should be done uniformly for both of these macros -- and maybe there should be some discussion if the above mentioned regressions and increased inconsistency between #[no_std]
and regular code are acceptable given that there is not even a clear transition plan yet.
I forgot to say, both issues could be avoided if Arguments::as_str
returned &str
instead of &'static str
-- but that might cause other problems elsewhere. This was mentioned at https://github.com/rust-lang/rust/pull/74056#issuecomment-653896585 but I do not understand how the static lifetime makes such a big difference here -- Cow<'_, str>
also works very well. On the one hand I can see the argument that format strings are static anyway, on the other hand it does seem potentially useful for Arguments
to be able to support allocation-free non-static strings as well.
I do not understand how the static lifetime makes such a big difference here
as_str()
was made to avoid the need for special-casing macros (like bail!()
here, or like anyhow::anyhow!()
, etc.). They can only use it if it is 'static
, as they keep the string around in an object, instead of printing it directly.
That makes sense. And this then implies that core::panic!(foo)
cannot use core::panicking::panic
any more as foo
might not be static, which directly leads to the issues I mentioned above.
I don't see a good way out of this, but I am satisfied with having understood the tradeoff here.
Relevant discussion regarding panic has recently been proposed in https://github.com/rust-lang/rfcs/pull/3007
Most helpful comment
Progress update from me: I've been busy but have started work on the implementation of this feature.
A bikeshed on the feature name: I'm leaning towards to the name
format_args_capture
over the original proposed nameformat_args_implicits
from the RFC.I think that
format_args_capture
reflects the mechanics of the feature better - it enables capturing of variables from the surrounding scope. It also avoids the word "implicit" which I find can raise peoples' blood temperature (sometimes for good reason).The feature gate error message (on nightly) would then look like this:
Is it too late to use a different feature name?