From af8968558db701dbcea1d7d5472f4e1a8a291670 Mon Sep 17 00:00:00 2001 From: Shoyu Vanilla Date: Sat, 13 Dec 2025 03:15:37 +0900 Subject: [PATCH] Do not deduplicate captured args while expanding `format_args!` --- compiler/rustc_ast/src/format.rs | 21 ++++---- compiler/rustc_ast_lowering/src/format.rs | 2 +- compiler/rustc_builtin_macros/src/format.rs | 13 +++-- .../clippy/clippy_lints/src/format_args.rs | 4 +- .../src/unit_types/let_unit_value.rs | 2 +- .../hir-format-args-duplicated-captures.pp | 20 +++++++ .../hir-format-args-duplicated-captures.rs | 9 ++++ ...at-args-duplicated-captured-const-drops.rs | 53 +++++++++++++++++++ 8 files changed, 104 insertions(+), 20 deletions(-) create mode 100644 tests/pretty/hir-format-args-duplicated-captures.pp create mode 100644 tests/pretty/hir-format-args-duplicated-captures.rs create mode 100644 tests/ui/drop/format-args-duplicated-captured-const-drops.rs diff --git a/compiler/rustc_ast/src/format.rs b/compiler/rustc_ast/src/format.rs index cadebb2254d95..f0e20386bc93f 100644 --- a/compiler/rustc_ast/src/format.rs +++ b/compiler/rustc_ast/src/format.rs @@ -77,14 +77,14 @@ pub struct FormatArguments { arguments: Vec, num_unnamed_args: usize, num_explicit_args: usize, - names: FxHashMap, + explicit_names: FxHashMap, } impl FormatArguments { pub fn new() -> Self { Self { arguments: Vec::new(), - names: FxHashMap::default(), + explicit_names: FxHashMap::default(), num_unnamed_args: 0, num_explicit_args: 0, } @@ -93,13 +93,16 @@ impl FormatArguments { pub fn add(&mut self, arg: FormatArgument) -> usize { let index = self.arguments.len(); if let Some(name) = arg.kind.ident() { - self.names.insert(name.name, index); - } else if self.names.is_empty() { + self.explicit_names.insert(name.name, index); + } else if let FormatArgumentKind::Normal = arg.kind + && self.explicit_names.is_empty() + && self.arguments.len() == self.num_unnamed_args + { // Only count the unnamed args before the first named arg. // (Any later ones are errors.) self.num_unnamed_args += 1; } - if !matches!(arg.kind, FormatArgumentKind::Captured(..)) { + if !matches!(arg.kind, FormatArgumentKind::Captured) { // This is an explicit argument. // Make sure that all arguments so far are explicit. assert_eq!( @@ -113,8 +116,8 @@ impl FormatArguments { index } - pub fn by_name(&self, name: Symbol) -> Option<(usize, &FormatArgument)> { - let i = *self.names.get(&name)?; + pub fn by_explicit_name(&self, name: Symbol) -> Option<(usize, &FormatArgument)> { + let i = *self.explicit_names.get(&name)?; Some((i, &self.arguments[i])) } @@ -156,7 +159,7 @@ pub enum FormatArgumentKind { /// `format_args(…, arg = 1)` Named(Ident), /// `format_args("… {arg} …")` - Captured(Ident), + Captured, } impl FormatArgumentKind { @@ -164,7 +167,7 @@ impl FormatArgumentKind { match self { &Self::Normal => None, &Self::Named(id) => Some(id), - &Self::Captured(id) => Some(id), + &Self::Captured => None, } } } diff --git a/compiler/rustc_ast_lowering/src/format.rs b/compiler/rustc_ast_lowering/src/format.rs index 602635af1324e..d6727864c026b 100644 --- a/compiler/rustc_ast_lowering/src/format.rs +++ b/compiler/rustc_ast_lowering/src/format.rs @@ -462,7 +462,7 @@ fn expand_format_args<'hir>( let placeholder_span = placeholder_span.unwrap_or(arg.expr.span).with_ctxt(macsp.ctxt()); let arg_span = match arg.kind { - FormatArgumentKind::Captured(_) => placeholder_span, + FormatArgumentKind::Captured => placeholder_span, _ => arg.expr.span.with_ctxt(macsp.ctxt()), }; let args_ident_expr = ctx.expr_ident(macsp, args_ident, args_hir_id); diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index a0ee7ac19899b..20e6047e2bd6b 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -121,7 +121,7 @@ fn parse_args<'a>(ecx: &ExtCtxt<'a>, sp: Span, tts: TokenStream) -> PResult<'a, p.bump(); p.expect(exp!(Eq))?; let expr = p.parse_expr()?; - if let Some((_, prev)) = args.by_name(ident.name) { + if let Some((_, prev)) = args.by_explicit_name(ident.name) { ecx.dcx().emit_err(errors::FormatDuplicateArg { span: ident.span, prev: prev.kind.ident().unwrap().span, @@ -374,12 +374,11 @@ fn make_format_args( } Name(name, span) => { let name = Symbol::intern(name); - if let Some((index, _)) = args.by_name(name) { + if let Some((index, _)) = args.by_explicit_name(name) { // Name found in `args`, so we resolve it to its index. - if index < args.explicit_args().len() { - // Mark it as used, if it was an explicit argument. - used[index] = true; - } + assert!(index < args.explicit_args().len()); + // Mark it as used, as this is an explicit argument. + used[index] = true; Ok(index) } else { // Name not found in `args`, so we add it as an implicitly captured argument. @@ -394,7 +393,7 @@ fn make_format_args( unnamed_arg_after_named_arg = true; DummyResult::raw_expr(span, Some(guar)) }; - Ok(args.add(FormatArgument { kind: FormatArgumentKind::Captured(ident), expr })) + Ok(args.add(FormatArgument { kind: FormatArgumentKind::Captured, expr })) } } }; diff --git a/src/tools/clippy/clippy_lints/src/format_args.rs b/src/tools/clippy/clippy_lints/src/format_args.rs index 011cbf8c5d41c..3f4c745a61e0a 100644 --- a/src/tools/clippy/clippy_lints/src/format_args.rs +++ b/src/tools/clippy/clippy_lints/src/format_args.rs @@ -447,7 +447,7 @@ impl<'tcx> FormatArgsExpr<'_, 'tcx> { let index = pos.index.unwrap(); let arg = &self.format_args.arguments.all_args()[index]; - if !matches!(arg.kind, FormatArgumentKind::Captured(_)) + if !matches!(arg.kind, FormatArgumentKind::Captured) && let rustc_ast::ExprKind::Path(None, path) = &arg.expr.kind && let [segment] = path.segments.as_slice() && segment.args.is_none() @@ -467,7 +467,7 @@ impl<'tcx> FormatArgsExpr<'_, 'tcx> { // * if we can't inline a numbered argument, e.g. `print!("{0} ...", foo.bar, ...)` // * if allow_mixed_uninlined_format_args is false and this arg hasn't been inlined already pos.kind != FormatArgPositionKind::Number - && (!self.ignore_mixed || matches!(arg.kind, FormatArgumentKind::Captured(_))) + && (!self.ignore_mixed || matches!(arg.kind, FormatArgumentKind::Captured)) } } diff --git a/src/tools/clippy/clippy_lints/src/unit_types/let_unit_value.rs b/src/tools/clippy/clippy_lints/src/unit_types/let_unit_value.rs index 2645e94358e11..38f24533fb3bd 100644 --- a/src/tools/clippy/clippy_lints/src/unit_types/let_unit_value.rs +++ b/src/tools/clippy/clippy_lints/src/unit_types/let_unit_value.rs @@ -202,7 +202,7 @@ impl<'tcx> Visitor<'tcx> for UnitVariableCollector<'_, 'tcx> { { if let Some(macro_call) = self.macro_call && macro_call.arguments.all_args().iter().any(|arg| { - matches!(arg.kind, FormatArgumentKind::Captured(_)) && find_format_arg_expr(ex, arg).is_some() + matches!(arg.kind, FormatArgumentKind::Captured) && find_format_arg_expr(ex, arg).is_some() }) { self.spans.push(VariableUsage::FormatCapture); diff --git a/tests/pretty/hir-format-args-duplicated-captures.pp b/tests/pretty/hir-format-args-duplicated-captures.pp new file mode 100644 index 0000000000000..f9280a59aa115 --- /dev/null +++ b/tests/pretty/hir-format-args-duplicated-captures.pp @@ -0,0 +1,20 @@ +#[attr = MacroUse {arguments: UseAll}] +extern crate std; +#[prelude_import] +use ::std::prelude::rust_2015::*; +//@ pretty-compare-only +//@ pretty-mode:hir +//@ pp-exact:hir-format-args-duplicated-captures.pp + +const X: i32 = 42; + +fn main() { + let _ = + { + super let args = (&X, &X); + super let args = + [format_argument::new_display(args.0), + format_argument::new_display(args.1)]; + unsafe { format_arguments::new(b"\xc0\x01 \xc0\x00", &args) } + }; +} diff --git a/tests/pretty/hir-format-args-duplicated-captures.rs b/tests/pretty/hir-format-args-duplicated-captures.rs new file mode 100644 index 0000000000000..29048995e049c --- /dev/null +++ b/tests/pretty/hir-format-args-duplicated-captures.rs @@ -0,0 +1,9 @@ +//@ pretty-compare-only +//@ pretty-mode:hir +//@ pp-exact:hir-format-args-duplicated-captures.pp + +const X: i32 = 42; + +fn main() { + let _ = format_args!("{X} {X}"); +} diff --git a/tests/ui/drop/format-args-duplicated-captured-const-drops.rs b/tests/ui/drop/format-args-duplicated-captured-const-drops.rs new file mode 100644 index 0000000000000..e3e0109c93cab --- /dev/null +++ b/tests/ui/drop/format-args-duplicated-captured-const-drops.rs @@ -0,0 +1,53 @@ +//! A regression test for https://github.com/rust-lang/rust/issues/145739. +//! `format_args!` should not deduplicate implicitly captured arguments. +//! They must be evaluated once per occurrence, whereas explicitly +//! provided arguments are evaluated only once, regardless of how many +//! times they appear in the format string. +//@ run-pass + +use std::sync::atomic::{AtomicUsize, Ordering}; + +static DROP_COUNTER: AtomicUsize = AtomicUsize::new(0); + +#[derive(Debug)] +struct Foo; + +impl Drop for Foo { + fn drop(&mut self) { + DROP_COUNTER.fetch_add(1, Ordering::Relaxed); + } +} + +fn main() { + assert_eq!(DROP_COUNTER.load(Ordering::Relaxed), 0); + + { + let _x = format_args!("{Foo:?}"); + } + assert_eq!(DROP_COUNTER.load(Ordering::Relaxed), 1); + + { + let _x = format_args!("{Foo:?}{Foo:?}"); + } + // Increased by 2, as `Foo` is constructed for each captured `Foo`. + assert_eq!(DROP_COUNTER.load(Ordering::Relaxed), 3); + + { + let _x = format_args!("{:?}{0:?}", Foo); + } + // Increased by 1, as `foo` is constructed just once as an explicit argument. + assert_eq!(DROP_COUNTER.load(Ordering::Relaxed), 4); + + { + let _x = format_args!("{foo:?}{foo:?}{bar:?}{Foo:?}{Foo:?}", foo = Foo, bar = Foo); + } + // Increased by 4, as `foo` is constructed twice for captured `Foo`, and once for each `foo` and + // `bar`. + assert_eq!(DROP_COUNTER.load(Ordering::Relaxed), 8); + + { + let _x = format_args!("{Foo:?}{Foo:?}", Foo = Foo); + } + // Increased by 1, as `Foo` is shadowed by an explicit argument. + assert_eq!(DROP_COUNTER.load(Ordering::Relaxed), 9); +}