Make format_args!("literal") const. #87005
Conversation
rust-highfive
added
the
S-waiting-on-review
m-ou-se
added
needs-fcp
This comment has been minimized.
This comment has been minimized.
m-ou-se
force-pushed
the
format-args-const
branch
from
35eb454 to
e1092d4
Compare
| const B: std::fmt::Arguments = format_args!("{}", 123); | ||
| //~^ ERROR calls in constants are limited to | ||
| //~| ERROR calls in constants are limited to | ||
| //~| ERROR temporary value |
There was a problem hiding this comment.
Why are there 3 errors here and not just the one we'd expect? I guess that comes from the extra code for the argument 123? "temporary value dropped while borrowed" is a bit odd though...
There was a problem hiding this comment.
"temporary value dropped while borrowed" is a bit odd though...
It expands to
::core::fmt::Arguments::new_v1(
&[""],
&match (&123,) {
(arg0,) => [::core::fmt::ArgumentV1::new(
arg0,
::core::fmt::Display::fmt,
)],
},
);The [::core::fmt::ArgumentV1::new(arg0, ::core::fmt::Display::fmt)] is here borrowed and then dropped before the end of the borrow.
There was a problem hiding this comment.
ah right that would not be promoted.
Better change the test do
const B: () = {
let _ = format_args!(...);
};
| @@ -0,0 +1,8 @@ | |||
| #![crate_type = "lib"] | |||
|
|
|||
| const A: std::fmt::Arguments = format_args!("literal"); | |||
There was a problem hiding this comment.
i think it's just not gated in this WIP form of the PR
There was a problem hiding this comment.
So this is insta-stable?
Yes. That's why I added needs-fcp.
|
i don't think we should make this pattern special. we could just allow format_args in general, just like we allow range syntax, but there's no use for the value you get, unless you use it to panic. |
| /// Arguments structure in case it's just a single string literal. | ||
| #[doc(hidden)] | ||
| #[inline] | ||
| #[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")] |
There was a problem hiding this comment.
Is it possible to make it const-unstable at the same time under a different feature gate?
There was a problem hiding this comment.
It is const-unstable (implicitly). But the format_args! macro expanded code has some kind of span that still lets stable code call this.
There was a problem hiding this comment.
Only the fmt_internals feature is allowed:
rust/library/core/src/macros/mod.rs
Line 826 in a84d1b2
Using a different feature for const checking would require this feature to be enabled by the user of format_args!.
There was a problem hiding this comment.
The reason i want this is so panic!() can use this for const panic. (See #86998) Putting an allow_internal_unstable(const_fmt_internals) on the panic macro won't work through the nested format_args call.
There was a problem hiding this comment.
That would at least prevent it from being insta-stable, right? Maybe use const_panic as feature gate?
There was a problem hiding this comment.
I think we should just go for the const_format_args!() macro for now (#86998) and at some point stabilize format_args!() in const entirely. (And then remove const_format_args!() again.)
|
☔ The latest upstream changes (presumably #83302) made this pull request unmergeable. Please resolve the merge conflicts. |
|
☔ The latest upstream changes (presumably #90485) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@m-ou-se it seems like we can likely close this now -- I think the broader goal still probably makes sense (making format_args! const in general) and we worked around this for the panic case already, so there's not really that much need for the targeted adjustment here? |
|
Closing this based on comment |
This makes
format_args!("literal")const, while leaving all other invocations of format_args!() non-const.cc @rust-lang/wg-const-eval