Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make format_args!("literal") const. #87005

Closed
wants to merge 1 commit into from
+62 −29
Closed

Make format_args!("literal") const. #87005

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion compiler/rustc_builtin_macros/src/format.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -761,6 +761,7 @@ impl<'a, 'b> Context<'a, 'b> {
/// Actually builds the expression which the format_args! block will be
/// expanded to.
fn into_expr(self) -> P<ast::Expr> {
let is_literal = self.str_pieces.len() == 1 && self.args.is_empty();
let mut locals =
Vec::with_capacity((0..self.args.len()).map(|i| self.arg_unique_types[i].len()).sum());
let mut counts = Vec::with_capacity(self.count_args.len());
Expand Down Expand Up @@ -846,7 +847,9 @@ impl<'a, 'b> Context<'a, 'b> {
let args_slice = self.ecx.expr_addr_of(self.macsp, result);

// Now create the fmt::Arguments struct with all our locals we created.
let (fn_name, fn_args) = if self.all_pieces_simple {
let (fn_name, fn_args) = if is_literal {
("new_literal", vec![pieces])
} else if self.all_pieces_simple {
("new_v1", vec![pieces, args_slice])
} else {
// Build up the static array which will store our precompiled
Expand Down
9 changes: 9 additions & 0 deletions library/core/src/fmt/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,15 @@ enum FlagV1 {
}

impl<'a> Arguments<'a> {
/// When using the format_args!() macro, this function is used to generate the
/// 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")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to make it const-unstable at the same time under a different feature gate?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is const-unstable (implicitly). But the format_args! macro expanded code has some kind of span that still lets stable code call this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the fmt_internals feature is allowed:

rust/library/core/src/macros/mod.rs

Line 826 in a84d1b2

#[allow_internal_unstable(fmt_internals)]

Using a different feature for const checking would require this feature to be enabled by the user of format_args!.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would at least prevent it from being insta-stable, right? Maybe use const_panic as feature gate?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

pub const fn new_literal(pieces: &'a [&'static str; 1]) -> Arguments<'a> {
Arguments { pieces, fmt: None, args: &[] }
}

/// When using the format_args!() macro, this function is used to generate the
/// Arguments structure.
#[doc(hidden)]
Expand Down
7 changes: 1 addition & 6 deletions src/test/pretty/dollar-crate.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,5 @@
// pp-exact:dollar-crate.pp

fn main() {
{
::std::io::_print(::core::fmt::Arguments::new_v1(&["rust\n"],
&match () {
() => [],
}));
};
{ ::std::io::_print(::core::fmt::Arguments::new_literal(&["rust\n"])); };
}
30 changes: 8 additions & 22 deletions src/test/pretty/issue-4264.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,29 +32,15 @@
({
let res =
((::alloc::fmt::format as
for<'r> fn(Arguments<'r>) -> String {format})(((::core::fmt::Arguments::new_v1
for<'r> fn(Arguments<'r>) -> String {format})(((::core::fmt::Arguments::new_literal
as
fn(&[&'static str], &[ArgumentV1]) -> Arguments {Arguments::new_v1})((&([("test"
as
&str)]
as
[&str; 1])
as
&[&str; 1]),
(&(match (()
as
())
{
()
=>
([]
as
[ArgumentV1; 0]),
}
as
[ArgumentV1; 0])
as
&[ArgumentV1; 0]))
fn(&[&'static str; 1]) -> Arguments {Arguments::new_literal})((&([("test"
as
&str)]
as
[&str; 1])
as
&[&str; 1]))
as
Arguments))
as String);
Expand Down
8 changes: 8 additions & 0 deletions src/test/ui/consts/const-eval/format_args.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#![crate_type = "lib"]

const A: std::fmt::Arguments = format_args!("literal");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is insta-stable?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think it's just not gated in this WIP form of the PR

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is insta-stable?

Yes. That's why I added needs-fcp.


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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"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.

Copy link
Member

@RalfJung RalfJung Jul 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah right that would not be promoted.
Better change the test do

const B: () = {
  let _ = format_args!(...);
};

32 changes: 32 additions & 0 deletions src/test/ui/consts/const-eval/format_args.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
error[E0015]: calls in constants are limited to constant functions, tuple structs and tuple variants
--> $DIR/format_args.rs:5:32
|
LL | const B: std::fmt::Arguments = format_args!("{}", 123);
| ^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this error originates in the macro `format_args` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0015]: calls in constants are limited to constant functions, tuple structs and tuple variants
--> $DIR/format_args.rs:5:32
|
LL | const B: std::fmt::Arguments = format_args!("{}", 123);
| ^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this error originates in the macro `format_args` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0716]: temporary value dropped while borrowed
--> $DIR/format_args.rs:5:32
|
LL | const B: std::fmt::Arguments = format_args!("{}", 123);
| ^^^^^^^^^^^^^^^^^^^^^^-
| | |
| | temporary value is freed at the end of this statement
| creates a temporary which is freed while still in use
| using this value as a constant requires that borrow lasts for `'static`
|
= note: this error originates in the macro `format_args` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 3 previous errors

Some errors have detailed explanations: E0015, E0716.
For more information about an error, try `rustc --explain E0015`.