Rollup merge of #100277 - m-ou-se:format-args-1, r=compiler-errors

Simplify format_args builtin macro implementation.

Instead of a FxHashMap<Symbol, (usize, Span)> for the named arguments, this now includes the name and span in the elements of the Vec<FormatArg> directly. The FxHashMap still exists to look up the index, but no longer contains the span. Looking up the name or span of an argument is now trivial and does not need the map anymore.

Author: matthiaskrgr

compiler/rustc_builtin_macros/src/format.rs

@@ -130,64 +130,46 @@ impl PositionalNamedArgsLint {
     /// CountIsParam, which contains an index into the arguments.
     fn maybe_add_positional_named_arg(
         &mut self,
-        current_positional_arg: usize,
-        total_args_length: usize,
-        format_argument_index: usize,
+        arg: Option<&FormatArg>,
         ty: PositionalNamedArgType,
         cur_piece: usize,
         inner_span_to_replace: Option<rustc_parse_format::InnerSpan>,
-        names: &FxHashMap<Symbol, (usize, Span)>,
         has_formatting: bool,
     ) {
-        let start_of_named_args = total_args_length - names.len();
-        if current_positional_arg >= start_of_named_args {
-            self.maybe_push(
-                format_argument_index,
-                ty,
-                cur_piece,
-                inner_span_to_replace,
-                names,
-                has_formatting,
-            )
+        if let Some(arg) = arg {
+            if let Some(name) = arg.name {
+                self.push(name, ty, cur_piece, inner_span_to_replace, has_formatting)
+            }
         }
     }
 
-    /// Try constructing a PositionalNamedArg struct and pushing it into the vec of positional
-    /// named arguments. If a named arg associated with `format_argument_index` cannot be found,
-    /// a new item will not be added as the lint cannot be emitted in this case.
-    fn maybe_push(
+    /// Construct a PositionalNamedArg struct and push it into the vec of positional
+    /// named arguments.
+    fn push(
         &mut self,
-        format_argument_index: usize,
+        arg_name: Ident,
         ty: PositionalNamedArgType,
         cur_piece: usize,
         inner_span_to_replace: Option<rustc_parse_format::InnerSpan>,
-        names: &FxHashMap<Symbol, (usize, Span)>,
         has_formatting: bool,
     ) {
-        let named_arg = names
-            .iter()
-            .find(|&(_, &(index, _))| index == format_argument_index)
-            .map(|found| found.clone());
-
-        if let Some((&replacement, &(_, positional_named_arg_span))) = named_arg {
-            // In FormatSpec, `precision_span` starts at the leading `.`, which we want to keep in
-            // the lint suggestion, so increment `start` by 1 when `PositionalArgumentType` is
-            // `Precision`.
-            let inner_span_to_replace = if ty == PositionalNamedArgType::Precision {
-                inner_span_to_replace
-                    .map(|is| rustc_parse_format::InnerSpan { start: is.start + 1, end: is.end })
-            } else {
-                inner_span_to_replace
-            };
-            self.positional_named_args.push(PositionalNamedArg {
-                ty,
-                cur_piece,
-                inner_span_to_replace,
-                replacement,
-                positional_named_arg_span,
-                has_formatting,
-            });
-        }
+        // In FormatSpec, `precision_span` starts at the leading `.`, which we want to keep in
+        // the lint suggestion, so increment `start` by 1 when `PositionalArgumentType` is
+        // `Precision`.
+        let inner_span_to_replace = if ty == PositionalNamedArgType::Precision {
+            inner_span_to_replace
+                .map(|is| rustc_parse_format::InnerSpan { start: is.start + 1, end: is.end })
+        } else {
+            inner_span_to_replace
+        };
+        self.positional_named_args.push(PositionalNamedArg {
+            ty,
+            cur_piece,
+            inner_span_to_replace,
+            replacement: arg_name.name,
+            positional_named_arg_span: arg_name.span,
+            has_formatting,
+        });
     }
 }
 
@@ -211,15 +193,15 @@ struct Context<'a, 'b> {
     /// * `arg_types` (in JSON): `[[0, 1, 0], [0, 1, 1], [0, 1]]`
     /// * `arg_unique_types` (in simplified JSON): `[["o", "x"], ["o", "x"], ["o", "x"]]`
     /// * `names` (in JSON): `{"foo": 2}`
-    args: Vec<P<ast::Expr>>,
+    args: Vec<FormatArg>,
     /// The number of arguments that were added by implicit capturing.
     num_captured_args: usize,
     /// Placeholder slot numbers indexed by argument.
     arg_types: Vec<Vec<usize>>,
     /// Unique format specs seen for each argument.
     arg_unique_types: Vec<Vec<ArgumentType>>,
     /// Map from named arguments to their resolved indices.
-    names: FxHashMap<Symbol, (usize, Span)>,
+    names: FxHashMap<Symbol, usize>,
 
     /// The latest consecutive literal strings, or empty if there weren't any.
     literal: String,
@@ -282,7 +264,7 @@ struct Context<'a, 'b> {
 
 pub struct FormatArg {
     expr: P<ast::Expr>,
-    named: bool,
+    name: Option<Ident>,
 }
 
 /// Parses the arguments from the given list of tokens, returning the diagnostic
@@ -298,9 +280,9 @@ fn parse_args<'a>(
     ecx: &mut ExtCtxt<'a>,
     sp: Span,
     tts: TokenStream,
-) -> PResult<'a, (P<ast::Expr>, Vec<FormatArg>, FxHashMap<Symbol, (usize, Span)>)> {
+) -> PResult<'a, (P<ast::Expr>, Vec<FormatArg>, FxHashMap<Symbol, usize>)> {
     let mut args = Vec::<FormatArg>::new();
-    let mut names = FxHashMap::<Symbol, (usize, Span)>::default();
+    let mut names = FxHashMap::<Symbol, usize>::default();
 
     let mut p = ecx.new_parser_from_tts(tts);
 
@@ -365,9 +347,9 @@ fn parse_args<'a>(
                 p.bump();
                 p.expect(&token::Eq)?;
                 let e = p.parse_expr()?;
-                if let Some((prev, _)) = names.get(&ident.name) {
+                if let Some(&prev) = names.get(&ident.name) {
                     ecx.struct_span_err(e.span, &format!("duplicate argument named `{}`", ident))
-                        .span_label(args[*prev].expr.span, "previously here")
+                        .span_label(args[prev].expr.span, "previously here")
                         .span_label(e.span, "duplicate argument")
                         .emit();
                     continue;
@@ -378,8 +360,8 @@ fn parse_args<'a>(
                 // if the input is valid, we can simply append to the positional
                 // args. And remember the names.
                 let slot = args.len();
-                names.insert(ident.name, (slot, ident.span));
-                args.push(FormatArg { expr: e, named: true });
+                names.insert(ident.name, slot);
+                args.push(FormatArg { expr: e, name: Some(ident) });
             }
             _ => {
                 let e = p.parse_expr()?;
@@ -389,12 +371,12 @@ fn parse_args<'a>(
                         "positional arguments cannot follow named arguments",
                     );
                     err.span_label(e.span, "positional arguments must be before named arguments");
-                    for pos in names.values() {
-                        err.span_label(args[pos.0].expr.span, "named argument");
+                    for &pos in names.values() {
+                        err.span_label(args[pos].expr.span, "named argument");
                     }
                     err.emit();
                 }
-                args.push(FormatArg { expr: e, named: false });
+                args.push(FormatArg { expr: e, name: None });
             }
         }
     }
@@ -410,8 +392,7 @@ impl<'a, 'b> Context<'a, 'b> {
     fn resolve_name_inplace(&mut self, p: &mut parse::Piece<'_>) {
         // NOTE: the `unwrap_or` branch is needed in case of invalid format
         // arguments, e.g., `format_args!("{foo}")`.
-        let lookup =
-            |s: &str| self.names.get(&Symbol::intern(s)).unwrap_or(&(0, Span::default())).0;
+        let lookup = |s: &str| self.names.get(&Symbol::intern(s)).copied().unwrap_or(0);
 
         match *p {
             parse::String(_) => {}
@@ -457,27 +438,21 @@ impl<'a, 'b> Context<'a, 'b> {
                 let pos = match arg.position {
                     parse::ArgumentIs(i) => {
                         self.unused_names_lint.maybe_add_positional_named_arg(
-                            i,
-                            self.args.len(),
-                            i,
+                            self.args.get(i),
                             PositionalNamedArgType::Arg,
                             self.curpiece,
                             Some(arg.position_span),
-                            &self.names,
                             has_precision || has_width,
                         );
 
                         Exact(i)
                     }
                     parse::ArgumentImplicitlyIs(i) => {
                         self.unused_names_lint.maybe_add_positional_named_arg(
-                            i,
-                            self.args.len(),
-                            i,
+                            self.args.get(i),
                             PositionalNamedArgType::Arg,
                             self.curpiece,
                             None,
-                            &self.names,
                             has_precision || has_width,
                         );
                         Exact(i)
@@ -563,13 +538,10 @@ impl<'a, 'b> Context<'a, 'b> {
             parse::CountImplied | parse::CountIs(..) => {}
             parse::CountIsParam(i) => {
                 self.unused_names_lint.maybe_add_positional_named_arg(
-                    i,
-                    self.args.len(),
-                    i,
+                    self.args.get(i),
                     named_arg_type,
                     self.curpiece,
                     *inner_span,
-                    &self.names,
                     true,
                 );
                 self.verify_arg_type(Exact(i), Count);
@@ -622,7 +594,7 @@ impl<'a, 'b> Context<'a, 'b> {
             );
             for arg in &self.args {
                 // Point at the arguments that will be formatted.
-                e.span_label(arg.span, "");
+                e.span_label(arg.expr.span, "");
             }
         } else {
             let (mut refs, spans): (Vec<_>, Vec<_>) = refs.unzip();
@@ -692,7 +664,7 @@ impl<'a, 'b> Context<'a, 'b> {
                         );
                         if let Some(arg) = self.args.get(pos) {
                             e.span_label(
-                                arg.span,
+                                arg.expr.span,
                                 "this parameter corresponds to the precision flag",
                             );
                         }
@@ -771,7 +743,7 @@ impl<'a, 'b> Context<'a, 'b> {
                 match self.names.get(&name) {
                     Some(&idx) => {
                         // Treat as positional arg.
-                        self.verify_arg_type(Capture(idx.0), ty)
+                        self.verify_arg_type(Capture(idx), ty)
                     }
                     None => {
                         // For the moment capturing variables from format strings expanded from macros is
@@ -787,8 +759,11 @@ impl<'a, 'b> Context<'a, 'b> {
                                 self.fmtsp
                             };
                             self.num_captured_args += 1;
-                            self.args.push(self.ecx.expr_ident(span, Ident::new(name, span)));
-                            self.names.insert(name, (idx, span));
+                            self.args.push(FormatArg {
+                                expr: self.ecx.expr_ident(span, Ident::new(name, span)),
+                                name: Some(Ident::new(name, span)),
+                            });
+                            self.names.insert(name, idx);
                             self.verify_arg_type(Capture(idx), ty)
                         } else {
                             let msg = format!("there is no argument named `{}`", name);
@@ -1054,11 +1029,11 @@ impl<'a, 'b> Context<'a, 'b> {
         // evaluated a single time each, in the order written by the programmer,
         // and that the surrounding future/generator (if any) is Send whenever
         // possible.
-        let no_need_for_match =
-            nicely_ordered && !original_args.iter().skip(1).any(|e| may_contain_yield_point(e));
+        let no_need_for_match = nicely_ordered
+            && !original_args.iter().skip(1).any(|arg| may_contain_yield_point(&arg.expr));
 
         for (arg_index, arg_ty) in fmt_arg_index_and_ty {
-            let e = &mut original_args[arg_index];
+            let e = &mut original_args[arg_index].expr;
             let span = e.span;
             let arg = if no_need_for_match {
                 let expansion_span = e.span.with_ctxt(self.macsp.ctxt());
@@ -1087,7 +1062,9 @@ impl<'a, 'b> Context<'a, 'b> {
                 // span is otherwise unavailable in the MIR used by borrowck).
                 let heads = original_args
                     .into_iter()
-                    .map(|e| self.ecx.expr_addr_of(e.span.with_ctxt(self.macsp.ctxt()), e))
+                    .map(|arg| {
+                        self.ecx.expr_addr_of(arg.expr.span.with_ctxt(self.macsp.ctxt()), arg.expr)
+                    })
                     .collect();
 
                 let pat = self.ecx.pat_ident(self.macsp, Ident::new(sym::args, self.macsp));
@@ -1220,7 +1197,7 @@ pub fn expand_preparsed_format_args(
     sp: Span,
     efmt: P<ast::Expr>,
     args: Vec<FormatArg>,
-    names: FxHashMap<Symbol, (usize, Span)>,
+    names: FxHashMap<Symbol, usize>,
     append_newline: bool,
 ) -> P<ast::Expr> {
     // NOTE: this verbose way of initializing `Vec<Vec<ArgumentType>>` is because
@@ -1312,16 +1289,17 @@ pub fn expand_preparsed_format_args(
         if err.should_be_replaced_with_positional_argument {
             let captured_arg_span =
                 fmt_span.from_inner(InnerSpan::new(err.span.start, err.span.end));
-            let positional_args = args.iter().filter(|arg| !arg.named).collect::<Vec<_>>();
+            let n_positional_args =
+                args.iter().rposition(|arg| arg.name.is_none()).map_or(0, |i| i + 1);
             if let Ok(arg) = ecx.source_map().span_to_snippet(captured_arg_span) {
-                let span = match positional_args.last() {
+                let span = match args[..n_positional_args].last() {
                     Some(arg) => arg.expr.span,
                     None => fmt_sp,
                 };
                 e.multipart_suggestion_verbose(
                     "consider using a positional formatting argument instead",
                     vec![
-                        (captured_arg_span, positional_args.len().to_string()),
+                        (captured_arg_span, n_positional_args.to_string()),
                         (span.shrink_to_hi(), format!(", {}", arg)),
                     ],
                     Applicability::MachineApplicable,
@@ -1338,11 +1316,9 @@ pub fn expand_preparsed_format_args(
         .map(|span| fmt_span.from_inner(InnerSpan::new(span.start, span.end)))
         .collect();
 
-    let named_pos: FxHashSet<usize> = names.values().cloned().map(|(i, _)| i).collect();
-
     let mut cx = Context {
         ecx,
-        args: args.into_iter().map(|arg| arg.expr).collect(),
+        args,
         num_captured_args: 0,
         arg_types,
         arg_unique_types,
@@ -1410,14 +1386,12 @@ pub fn expand_preparsed_format_args(
         .enumerate()
         .filter(|(i, ty)| ty.is_empty() && !cx.count_positions.contains_key(&i))
         .map(|(i, _)| {
-            let msg = if named_pos.contains(&i) {
-                // named argument
+            let msg = if cx.args[i].name.is_some() {
                 "named argument never used"
             } else {
-                // positional argument
                 "argument never used"
             };
-            (cx.args[i].span, msg)
+            (cx.args[i].expr.span, msg)
         })
         .collect::<Vec<_>>();