Skip to content

Allow storing format_args!() in variable or const #139135

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.

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

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
203 changes: 66 additions & 137 deletions compiler/rustc_ast_lowering/src/format.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
use core::ops::ControlFlow;
use std::borrow::Cow;

use rustc_ast::visit::Visitor;
use rustc_ast::*;
use rustc_data_structures::fx::FxIndexMap;
use rustc_hir as hir;
Expand Down Expand Up @@ -476,77 +474,32 @@ fn expand_format_args<'hir>(
return hir::ExprKind::Call(new, new_args);
}

// If the args array contains exactly all the original arguments once,
// in order, we can use a simple array instead of a `match` construction.
// However, if there's a yield point in any argument except the first one,
// we don't do this, because an Argument cannot be kept across yield points.
//
// This is an optimization, speeding up compilation about 1-2% in some cases.
// See https://github.com/rust-lang/rust/pull/106770#issuecomment-1380790609
let use_simple_array = argmap.len() == arguments.len()
&& argmap.iter().enumerate().all(|(i, (&(j, _), _))| i == j)
&& arguments.iter().skip(1).all(|arg| !may_contain_yield_point(&arg.expr));

let args = if arguments.is_empty() {
let (let_statements, args) = if arguments.is_empty() {
// Generate:
// &<core::fmt::Argument>::none()
//
// Note:
// `none()` just returns `[]`. We use `none()` rather than `[]` to limit the lifetime.
//
// This makes sure that this still fails to compile, even when the argument is inlined:
//
// ```
// let f = format_args!("{}", "a");
// println!("{f}"); // error E0716
// ```
//
// Cases where keeping the object around is allowed, such as `format_args!("a")`,
// are handled above by the `allow_const` case.
let none_fn = ctx.arena.alloc(ctx.expr_lang_item_type_relative(
macsp,
hir::LangItem::FormatArgument,
sym::none,
));
let none = ctx.expr_call(macsp, none_fn, &[]);
ctx.expr(macsp, hir::ExprKind::AddrOf(hir::BorrowKind::Ref, hir::Mutability::Not, none))
} else if use_simple_array {
// Generate:
// &[
// <core::fmt::Argument>::new_display(&arg0),
// <core::fmt::Argument>::new_lower_hex(&arg1),
// <core::fmt::Argument>::new_debug(&arg2),
// …
// ]
let elements = ctx.arena.alloc_from_iter(arguments.iter().zip(argmap).map(
|(arg, ((_, ty), placeholder_span))| {
let placeholder_span =
placeholder_span.unwrap_or(arg.expr.span).with_ctxt(macsp.ctxt());
let arg_span = match arg.kind {
FormatArgumentKind::Captured(_) => placeholder_span,
_ => arg.expr.span.with_ctxt(macsp.ctxt()),
};
let arg = ctx.lower_expr(&arg.expr);
let ref_arg = ctx.arena.alloc(ctx.expr(
arg_span,
hir::ExprKind::AddrOf(hir::BorrowKind::Ref, hir::Mutability::Not, arg),
));
make_argument(ctx, placeholder_span, ref_arg, ty)
},
));
ctx.expr_array_ref(macsp, elements)
// []
(None, ctx.arena.alloc(ctx.expr(macsp, hir::ExprKind::Array(&[]))))
} else {
// Generate:
// &match (&arg0, &arg1, &…) {
// args => [
// <core::fmt::Argument>::new_display(args.0),
// <core::fmt::Argument>::new_lower_hex(args.1),
// <core::fmt::Argument>::new_debug(args.0),
// …
// ]
// }
// super let args = (&arg0, &arg1, &…);
let args_ident = Ident::new(sym::args, macsp);
let (args_pat, args_hir_id) = ctx.pat_ident(macsp, args_ident);
let elements = ctx.arena.alloc_from_iter(arguments.iter().map(|arg| {
let arg_expr = ctx.lower_expr(&arg.expr);
ctx.expr(
arg.expr.span.with_ctxt(macsp.ctxt()),
hir::ExprKind::AddrOf(hir::BorrowKind::Ref, hir::Mutability::Not, arg_expr),
)
}));
let args_tuple = ctx.arena.alloc(ctx.expr(macsp, hir::ExprKind::Tup(elements)));
let let_statement_1 = ctx.stmt_super_let_pat(macsp, args_pat, Some(args_tuple));

// Generate:
// super let args = [
// <core::fmt::Argument>::new_display(args.0),
// <core::fmt::Argument>::new_lower_hex(args.1),
// <core::fmt::Argument>::new_debug(args.0),
// …
// ];
let args = ctx.arena.alloc_from_iter(argmap.iter().map(
|(&(arg_index, ty), &placeholder_span)| {
let arg = &arguments[arg_index];
Expand All @@ -567,58 +520,48 @@ fn expand_format_args<'hir>(
make_argument(ctx, placeholder_span, arg, ty)
},
));
let elements = ctx.arena.alloc_from_iter(arguments.iter().map(|arg| {
let arg_expr = ctx.lower_expr(&arg.expr);
ctx.expr(
arg.expr.span.with_ctxt(macsp.ctxt()),
hir::ExprKind::AddrOf(hir::BorrowKind::Ref, hir::Mutability::Not, arg_expr),
)
}));
let args_tuple = ctx.arena.alloc(ctx.expr(macsp, hir::ExprKind::Tup(elements)));
let array = ctx.arena.alloc(ctx.expr(macsp, hir::ExprKind::Array(args)));
let match_arms = ctx.arena.alloc_from_iter([ctx.arm(args_pat, array)]);
let match_expr = ctx.arena.alloc(ctx.expr_match(
macsp,
args_tuple,
match_arms,
hir::MatchSource::FormatArgs,
));
ctx.expr(
macsp,
hir::ExprKind::AddrOf(hir::BorrowKind::Ref, hir::Mutability::Not, match_expr),
let args = ctx.arena.alloc(ctx.expr(macsp, hir::ExprKind::Array(args)));
let (args_pat, args_hir_id) = ctx.pat_ident(macsp, args_ident);
let let_statement_2 = ctx.stmt_super_let_pat(macsp, args_pat, Some(args));
(
Some([let_statement_1, let_statement_2]),
ctx.arena.alloc(ctx.expr_ident_mut(macsp, args_ident, args_hir_id)),
)
};

if let Some(format_options) = format_options {
// Generate:
// &args
let args =
ctx.expr(macsp, hir::ExprKind::AddrOf(hir::BorrowKind::Ref, hir::Mutability::Not, args));

let call = if let Some(format_options) = format_options {
// Generate:
// <core::fmt::Arguments>::new_v1_formatted(
// lit_pieces,
// args,
// format_options,
// unsafe { ::core::fmt::UnsafeArg::new() }
// )
// unsafe {
// <core::fmt::Arguments>::new_v1_formatted(
// lit_pieces,
// args,
// format_options,
// )
// }
let new_v1_formatted = ctx.arena.alloc(ctx.expr_lang_item_type_relative(
macsp,
hir::LangItem::FormatArguments,
sym::new_v1_formatted,
));
let unsafe_arg_new = ctx.arena.alloc(ctx.expr_lang_item_type_relative(
macsp,
hir::LangItem::FormatUnsafeArg,
sym::new,
));
let unsafe_arg_new_call = ctx.expr_call(macsp, unsafe_arg_new, &[]);
let args = ctx.arena.alloc_from_iter([lit_pieces, args, format_options]);
let call = ctx.expr_call(macsp, new_v1_formatted, args);
let hir_id = ctx.next_id();
let unsafe_arg = ctx.expr_block(ctx.arena.alloc(hir::Block {
stmts: &[],
expr: Some(unsafe_arg_new_call),
hir_id,
rules: hir::BlockCheckMode::UnsafeBlock(hir::UnsafeSource::CompilerGenerated),
span: macsp,
targeted_by_break: false,
}));
let args = ctx.arena.alloc_from_iter([lit_pieces, args, format_options, unsafe_arg]);
hir::ExprKind::Call(new_v1_formatted, args)
hir::ExprKind::Block(
ctx.arena.alloc(hir::Block {
stmts: &[],
expr: Some(call),
hir_id,
rules: hir::BlockCheckMode::UnsafeBlock(hir::UnsafeSource::CompilerGenerated),
span: macsp,
targeted_by_break: false,
}),
None,
)
} else {
// Generate:
// <core::fmt::Arguments>::new_v1(
Expand All @@ -632,35 +575,21 @@ fn expand_format_args<'hir>(
));
let new_args = ctx.arena.alloc_from_iter([lit_pieces, args]);
hir::ExprKind::Call(new_v1, new_args)
}
}

fn may_contain_yield_point(e: &ast::Expr) -> bool {
struct MayContainYieldPoint;

impl Visitor<'_> for MayContainYieldPoint {
type Result = ControlFlow<()>;

fn visit_expr(&mut self, e: &ast::Expr) -> ControlFlow<()> {
if let ast::ExprKind::Await(_, _) | ast::ExprKind::Yield(_) = e.kind {
ControlFlow::Break(())
} else {
visit::walk_expr(self, e)
}
}

fn visit_mac_call(&mut self, _: &ast::MacCall) -> ControlFlow<()> {
// Macros should be expanded at this point.
unreachable!("unexpanded macro in ast lowering");
}
};

fn visit_item(&mut self, _: &ast::Item) -> ControlFlow<()> {
// Do not recurse into nested items.
ControlFlow::Continue(())
}
if let Some(let_statements) = let_statements {
// Generate:
// {
// super let …
// super let …
// <core::fmt::Arguments>::new_…(…)
// }
let call = ctx.arena.alloc(ctx.expr(macsp, call));
let block = ctx.block_all(macsp, ctx.arena.alloc_from_iter(let_statements), Some(call));
hir::ExprKind::Block(block, None)
} else {
call
}

MayContainYieldPoint.visit_expr(e).is_break()
}

fn for_all_argument_indexes(template: &mut [FormatArgsPiece], mut f: impl FnMut(&mut usize)) {
Expand Down
20 changes: 20 additions & 0 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2230,6 +2230,26 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
self.stmt(span, hir::StmtKind::Let(self.arena.alloc(local)))
}

fn stmt_super_let_pat(
&mut self,
span: Span,
pat: &'hir hir::Pat<'hir>,
init: Option<&'hir hir::Expr<'hir>>,
) -> hir::Stmt<'hir> {
let hir_id = self.next_id();
let local = hir::LetStmt {
super_: Some(span),
hir_id,
init,
pat,
els: None,
source: hir::LocalSource::Normal,
span: self.lower_span(span),
ty: None,
};
self.stmt(span, hir::StmtKind::Let(self.arena.alloc(local)))
}

fn block_expr(&mut self, expr: &'hir hir::Expr<'hir>) -> &'hir hir::Block<'hir> {
self.block_all(expr.span, &[], Some(expr))
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/check_consts/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ fn build_error_for_const_call<'tcx>(
);
err
}
_ if tcx.opt_parent(callee) == tcx.get_diagnostic_item(sym::ArgumentMethods) => {
_ if tcx.opt_parent(callee) == tcx.get_diagnostic_item(sym::FmtArgumentsNew) => {
ccx.dcx().create_err(errors::NonConstFmtMacroCall {
span,
kind: ccx.const_kind(),
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_hir/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,6 @@ language_item_table! {
FormatArguments, sym::format_arguments, format_arguments, Target::Struct, GenericRequirement::None;
FormatCount, sym::format_count, format_count, Target::Enum, GenericRequirement::None;
FormatPlaceholder, sym::format_placeholder, format_placeholder, Target::Struct, GenericRequirement::None;
FormatUnsafeArg, sym::format_unsafe_arg, format_unsafe_arg, Target::Struct, GenericRequirement::None;

ExchangeMalloc, sym::exchange_malloc, exchange_malloc_fn, Target::Fn, GenericRequirement::None;
DropInPlace, sym::drop_in_place, drop_in_place_fn, Target::Fn, GenericRequirement::Minimum(1);
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,6 @@ symbols! {
Arc,
ArcWeak,
Argument,
ArgumentMethods,
ArrayIntoIter,
AsMut,
AsRef,
Expand Down Expand Up @@ -249,6 +248,7 @@ symbols! {
Error,
File,
FileType,
FmtArgumentsNew,
Fn,
FnMut,
FnOnce,
Expand Down Expand Up @@ -1038,7 +1038,6 @@ symbols! {
format_count,
format_macro,
format_placeholder,
format_unsafe_arg,
freeze,
freeze_impls,
freg,
Expand Down
22 changes: 19 additions & 3 deletions library/core/src/fmt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,7 @@ pub struct Arguments<'a> {
/// Used by the format_args!() macro to create a fmt::Arguments object.
#[doc(hidden)]
#[unstable(feature = "fmt_internals", issue = "none")]
#[rustc_diagnostic_item = "FmtArgumentsNew"]
impl<'a> Arguments<'a> {
#[inline]
pub const fn new_const<const N: usize>(pieces: &'a [&'static str; N]) -> Self {
Expand All @@ -635,7 +636,7 @@ impl<'a> Arguments<'a> {
/// When using the format_args!() macro, this function is used to generate the
/// Arguments structure.
#[inline]
pub const fn new_v1<const P: usize, const A: usize>(
pub fn new_v1<const P: usize, const A: usize>(
pieces: &'a [&'static str; P],
args: &'a [rt::Argument<'a>; A],
) -> Arguments<'a> {
Expand All @@ -645,12 +646,23 @@ impl<'a> Arguments<'a> {

/// Specifies nonstandard formatting parameters.
///
/// An `rt::UnsafeArg` is required because the following invariants must be held
/// in order for this function to be safe:
/// Safety:
/// 1. The `pieces` slice must be at least as long as `fmt`.
/// 2. Every `rt::Placeholder::position` value within `fmt` must be a valid index of `args`.
/// 3. Every `rt::Count::Param` within `fmt` must contain a valid index of `args`.
#[inline]
#[cfg(not(bootstrap))]
pub unsafe fn new_v1_formatted(
pieces: &'a [&'static str],
args: &'a [rt::Argument<'a>],
fmt: &'a [rt::Placeholder],
) -> Arguments<'a> {
Arguments { pieces, fmt: Some(fmt), args }
}

/// Bootstrap only.
#[cfg(bootstrap)]
#[inline]
pub const fn new_v1_formatted(
pieces: &'a [&'static str],
args: &'a [rt::Argument<'a>],
Expand All @@ -659,7 +671,11 @@ impl<'a> Arguments<'a> {
) -> Arguments<'a> {
Arguments { pieces, fmt: Some(fmt), args }
}
}

#[doc(hidden)]
#[unstable(feature = "fmt_internals", issue = "none")]
impl<'a> Arguments<'a> {
/// Estimates the length of the formatted text.
///
/// This is intended to be used for setting initial `String` capacity
Expand Down
Loading
Loading