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.

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

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Allow storing format_args!() in variable #140748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8ff2c1a
Allow storing `format_args!()` in `let`.
m-ou-se May 7, 2025
5b763fb
Add test for new format_args!() temporary lifetimes.
m-ou-se May 7, 2025
ddd9ee9
Remove outdated test.
m-ou-se May 7, 2025
4ca0f01
Update mir-opt tests.
m-ou-se May 7, 2025
f608da2
Remove 'this error originates in format_args_nl' from test output.
m-ou-se May 7, 2025
58a5ab5
Update unpretty tests.
m-ou-se May 7, 2025
42fc5ae
Fix span of AddrOf in format_args!() expansion.
m-ou-se Jun 4, 2025
a693c52
Bless/update tests.
m-ou-se May 7, 2025
2c175e1
Update/bless clippy tests.
m-ou-se Jun 4, 2025
2da3a66
Use expr_ref.
m-ou-se Jun 19, 2025
b4f2cac
Remove old format_args diagnostic.
m-ou-se Jun 19, 2025
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
View file
Open in desktop
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
206 changes: 77 additions & 129 deletions compiler/rustc_ast_lowering/src/format.rs
View file
Open in desktop
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.with_ctxt(macsp.ctxt()), 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,47 @@ 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_ref(macsp, 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 +594,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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2301,6 +2301,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
38 changes: 11 additions & 27 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3201,14 +3201,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
let expr_ty: Option<Ty<'_>> =
visitor.prop_expr.map(|expr| typeck_results.expr_ty(expr).peel_refs());

let is_format_arguments_item = if let Some(expr_ty) = expr_ty
&& let ty::Adt(adt, _) = expr_ty.kind()
{
self.infcx.tcx.is_lang_item(adt.did(), LangItem::FormatArguments)
} else {
false
};

if visitor.found == 0
&& stmt.span.contains(proper_span)
&& let Some(p) = sm.span_to_margin(stmt.span)
Expand Down Expand Up @@ -3236,25 +3228,17 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
""
};

if !is_format_arguments_item {
let addition = format!(
"let {}binding = {};\n{}",
mutability,
s,
" ".repeat(p)
);
err.multipart_suggestion_verbose(
msg,
vec![
(stmt.span.shrink_to_lo(), addition),
(proper_span, "binding".to_string()),
],
Applicability::MaybeIncorrect,
);
} else {
err.note("the result of `format_args!` can only be assigned directly if no placeholders in its arguments are used");
err.note("to learn more, visit <https://doc.rust-lang.org/std/macro.format_args.html>");
}
let addition =
format!("let {}binding = {};\n{}", mutability, s, " ".repeat(p));
err.multipart_suggestion_verbose(
msg,
vec![
(stmt.span.shrink_to_lo(), addition),
(proper_span, "binding".to_string()),
],
Applicability::MaybeIncorrect,
);

suggested = true;
break;
}
Expand Down
Loading
Loading