Skip to content

Allow storing format_args!() in variable #140748

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 9 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
12 changes: 6 additions & 6 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2289,12 +2289,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
span: Span,
elements: &'hir [hir::Expr<'hir>],
) -> hir::Expr<'hir> {
let addrof = hir::ExprKind::AddrOf(
hir::BorrowKind::Ref,
hir::Mutability::Not,
self.arena.alloc(self.expr(span, hir::ExprKind::Array(elements))),
);
self.expr(span, addrof)
let array = self.arena.alloc(self.expr(span, hir::ExprKind::Array(elements)));
self.expr_ref(span, array)
}

pub(super) fn expr_ref(&mut self, span: Span, expr: &'hir hir::Expr<'hir>) -> hir::Expr<'hir> {
self.expr(span, hir::ExprKind::AddrOf(hir::BorrowKind::Ref, hir::Mutability::Not, expr))
}

pub(super) fn expr(&mut self, span: Span, kind: hir::ExprKind<'hir>) -> hir::Expr<'hir> {
Expand Down
207 changes: 78 additions & 129 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,52 @@ 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()
// []
(vec![], ctx.arena.alloc(ctx.expr(macsp, hir::ExprKind::Array(&[]))))
} else if argmap.len() == 1 && arguments.len() == 1 {
// Only one argument, so we don't need to make the `args` tuple.
//
// 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))| {
// super let args = [<core::fmt::Argument>::new_display(&arg)];
let args = ctx.arena.alloc_from_iter(argmap.iter().map(
|(&(arg_index, ty), &placeholder_span)| {
let arg = &arguments[arg_index];
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),
));
let ref_arg = ctx.arena.alloc(ctx.expr_ref(arg.span, arg));
make_argument(ctx, placeholder_span, ref_arg, ty)
},
));
ctx.expr_array_ref(macsp, elements)
let args = ctx.arena.alloc(ctx.expr(macsp, hir::ExprKind::Array(args)));
let args_ident = Ident::new(sym::args, macsp);
let (args_pat, args_hir_id) = ctx.pat_ident(macsp, args_ident);
let let_statement = ctx.stmt_super_let_pat(macsp, args_pat, Some(args));
(vec![let_statement], ctx.arena.alloc(ctx.expr_ident_mut(macsp, args_ident, args_hir_id)))
} 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 +540,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));
(
vec![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 +595,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_statements.is_empty() {
// 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 @@ -2261,6 +2261,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
33 changes: 18 additions & 15 deletions library/core/src/fmt/rt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,30 +192,22 @@ impl Argument<'_> {
}
}

/// Used by `format_args` when all arguments are gone after inlining,
/// when using `&[]` would incorrectly allow for a bigger lifetime.
///
/// This fails without format argument inlining, and that shouldn't be different
/// when the argument is inlined:
///
/// ```compile_fail,E0716
/// let f = format_args!("{}", "a");
/// println!("{f}");
/// ```
/// Bootstrap only.
#[inline]
#[cfg(bootstrap)]
pub const fn none() -> [Self; 0] {
[]
}
}

/// This struct represents the unsafety of constructing an `Arguments`.
/// It exists, rather than an unsafe function, in order to simplify the expansion
/// of `format_args!(..)` and reduce the scope of the `unsafe` block.
/// Bootstrap only.
#[cfg(bootstrap)]
#[lang = "format_unsafe_arg"]
pub struct UnsafeArg {
_private: (),
}

#[cfg(bootstrap)]
impl UnsafeArg {
/// See documentation where `UnsafeArg` is required to know when it is safe to
/// create and use `UnsafeArg`.
Expand Down Expand Up @@ -256,8 +248,7 @@ 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: the following invariants must be held:
/// 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`.
Expand All @@ -269,6 +260,18 @@ impl<'a> Arguments<'a> {
/// const _: () = if false { panic!("a {:1}", "a") };
/// ```
#[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.
#[inline]
#[cfg(bootstrap)]
pub fn new_v1_formatted(
pieces: &'a [&'static str],
args: &'a [rt::Argument<'a>],
Expand Down
16 changes: 16 additions & 0 deletions library/coretests/tests/fmt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,22 @@ mod builders;
mod float;
mod num;

#[test]
#[cfg(not(bootstrap))]
fn test_lifetime() {
// Trigger all different forms of expansion,
// and check that each of them can be stored as a variable.
let a = format_args!("hello");
let a = format_args!("hello {a}");
let a = format_args!("hello {a:1}");
let a = format_args!("hello {a} {a:?}");
assert_eq!(a.to_string(), "hello hello hello hello hello hello hello");

// Without arguments, it should also work in consts.
const A: std::fmt::Arguments<'static> = format_args!("hello");
assert_eq!(A.to_string(), "hello");
}

#[test]
fn test_format_flags() {
// No residual flags left by pointer formatting
Expand Down
Loading
Loading