Rollup merge of #153508 - JonathanBrouwer:improved_eager_format, r=GuillaumeGomez

Clean up the eager formatting API

For #151366 (comment)

Previously eager formatting worked by throwing the arguments into a diag, formatting, then removing the args again. This is ugly so instead we now just do the formatting completely separately.
This PR has nice commits, so I recommend reviewing commit by commit.

r? @GuillaumeGomez

Author: JonathanBrouwer

compiler/rustc_borrowck/src/diagnostics/mod.rs

@@ -4,6 +4,7 @@ use std::collections::BTreeMap;
 
 use rustc_abi::{FieldIdx, VariantIdx};
 use rustc_data_structures::fx::FxIndexMap;
+use rustc_errors::formatting::DiagMessageAddArg;
 use rustc_errors::{Applicability, Diag, DiagMessage, EmissionGuarantee, MultiSpan, listify, msg};
 use rustc_hir::def::{CtorKind, Namespace};
 use rustc_hir::{
@@ -1309,12 +1310,9 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                         && !spans.is_empty()
                     {
                         let mut span: MultiSpan = spans.clone().into();
-                        err.arg("ty", param_ty.to_string());
-                        let msg = err.dcx.eagerly_format_to_string(
-                            msg!("`{$ty}` is made to be an `FnOnce` closure here"),
-                            err.args.iter(),
-                        );
-                        err.remove_arg("ty");
+                        let msg = msg!("`{$ty}` is made to be an `FnOnce` closure here")
+                            .arg("ty", param_ty.to_string())
+                            .format();
                         for sp in spans {
                             span.push_span_label(sp, msg.clone());
                         }

compiler/rustc_builtin_macros/src/errors.rs

@@ -1,4 +1,5 @@
 use rustc_errors::codes::*;
+use rustc_errors::formatting::DiagMessageAddArg;
 use rustc_errors::{
     Diag, DiagCtxtHandle, Diagnostic, EmissionGuarantee, Level, MultiSpan, SingleLabelManySpans,
     Subdiagnostic, msg,
@@ -763,15 +764,17 @@ pub(crate) struct FormatUnusedArg {
 // form of diagnostic.
 impl Subdiagnostic for FormatUnusedArg {
     fn add_to_diag<G: EmissionGuarantee>(self, diag: &mut Diag<'_, G>) {
-        diag.arg("named", self.named);
-        let msg = diag.eagerly_format(msg!(
-            "{$named ->
-                [true] named argument
-                *[false] argument
-            } never used"
-        ));
-        diag.remove_arg("named");
-        diag.span_label(self.span, msg);
+        diag.span_label(
+            self.span,
+            msg!(
+                "{$named ->
+                    [true] named argument
+                    *[false] argument
+                } never used"
+            )
+            .arg("named", self.named)
+            .format(),
+        );
     }
 }
 
@@ -946,17 +949,14 @@ pub(crate) struct AsmClobberNoReg {
 
 impl<'a, G: EmissionGuarantee> Diagnostic<'a, G> for AsmClobberNoReg {
     fn into_diag(self, dcx: DiagCtxtHandle<'a>, level: Level) -> Diag<'a, G> {
-        // eager translation as `span_labels` takes `AsRef<str>`
-        let lbl1 = dcx.eagerly_format_to_string(msg!("clobber_abi"), [].into_iter());
-        let lbl2 = dcx.eagerly_format_to_string(msg!("generic outputs"), [].into_iter());
         Diag::new(
             dcx,
             level,
             msg!("asm with `clobber_abi` must specify explicit registers for outputs"),
         )
         .with_span(self.spans.clone())
-        .with_span_labels(self.clobbers, &lbl1)
-        .with_span_labels(self.spans, &lbl2)
+        .with_span_labels(self.clobbers, "clobber_abi")
+        .with_span_labels(self.spans, "generic outputs")
     }
 }
 

compiler/rustc_codegen_llvm/src/errors.rs

@@ -2,7 +2,9 @@ use std::ffi::CString;
 use std::path::Path;
 
 use rustc_data_structures::small_c_str::SmallCStr;
-use rustc_errors::{Diag, DiagCtxtHandle, Diagnostic, EmissionGuarantee, Level, msg};
+use rustc_errors::{
+    Diag, DiagCtxtHandle, Diagnostic, EmissionGuarantee, Level, format_diag_message, msg,
+};
 use rustc_macros::Diagnostic;
 use rustc_span::Span;
 
@@ -24,7 +26,7 @@ impl<G: EmissionGuarantee> Diagnostic<'_, G> for ParseTargetMachineConfig<'_> {
     fn into_diag(self, dcx: DiagCtxtHandle<'_>, level: Level) -> Diag<'_, G> {
         let diag: Diag<'_, G> = self.0.into_diag(dcx, level);
         let (message, _) = diag.messages.first().expect("`LlvmError` with no message");
-        let message = dcx.eagerly_format_to_string(message.clone(), diag.args.iter());
+        let message = format_diag_message(message, &diag.args);
         Diag::new(
             dcx,
             level,

compiler/rustc_const_eval/src/check_consts/ops.rs

@@ -265,6 +265,7 @@ fn build_error_for_const_call<'tcx>(
             }
         }
         CallKind::FnCall { fn_trait_id, self_ty } => {
+            let kind = ccx.const_kind();
             let note = match self_ty.kind() {
                 FnDef(def_id, ..) => {
                     let span = tcx.def_span(*def_id);
@@ -274,8 +275,8 @@ fn build_error_for_const_call<'tcx>(
 
                     Some(errors::NonConstClosureNote::FnDef { span })
                 }
-                FnPtr(..) => Some(errors::NonConstClosureNote::FnPtr),
-                Closure(..) => Some(errors::NonConstClosureNote::Closure),
+                FnPtr(..) => Some(errors::NonConstClosureNote::FnPtr { kind }),
+                Closure(..) => Some(errors::NonConstClosureNote::Closure { kind }),
                 _ => None,
             };
 

compiler/rustc_const_eval/src/errors.rs

@@ -4,9 +4,10 @@ use std::fmt::Write;
 use either::Either;
 use rustc_abi::WrappingRange;
 use rustc_errors::codes::*;
+use rustc_errors::formatting::DiagMessageAddArg;
 use rustc_errors::{
-    Diag, DiagArgValue, DiagMessage, Diagnostic, EmissionGuarantee, Level, MultiSpan,
-    Subdiagnostic, msg,
+    Diag, DiagArgMap, DiagArgValue, DiagMessage, Diagnostic, EmissionGuarantee, Level, MultiSpan,
+    Subdiagnostic, format_diag_message, msg,
 };
 use rustc_hir::ConstContext;
 use rustc_macros::{Diagnostic, Subdiagnostic};
@@ -359,14 +360,11 @@ pub struct FrameNote {
 
 impl Subdiagnostic for FrameNote {
     fn add_to_diag<G: EmissionGuarantee>(self, diag: &mut Diag<'_, G>) {
-        diag.arg("times", self.times);
-        diag.arg("where_", self.where_);
-        diag.arg("instance", self.instance);
         let mut span: MultiSpan = self.span.into();
         if self.has_label && !self.span.is_dummy() {
             span.push_span_label(self.span, msg!("the failure occurred here"));
         }
-        let msg = diag.eagerly_format(msg!(
+        let msg = msg!(
             r#"{$times ->
                 [0] inside {$where_ ->
                     [closure] closure
@@ -379,10 +377,11 @@ impl Subdiagnostic for FrameNote {
                     *[other] {""}
                 } ...]
             }"#
-        ));
-        diag.remove_arg("times");
-        diag.remove_arg("where_");
-        diag.remove_arg("instance");
+        )
+        .arg("times", self.times)
+        .arg("where_", self.where_)
+        .arg("instance", self.instance)
+        .format();
         diag.span_note(span, msg);
     }
 }
@@ -534,7 +533,7 @@ pub enum NonConstClosureNote {
             *[other] {""}
         }s"#
     )]
-    FnPtr,
+    FnPtr { kind: ConstContext },
     #[note(
         r#"closures need an RFC before allowed to be called in {$kind ->
             [const] constant
@@ -543,7 +542,7 @@ pub enum NonConstClosureNote {
             *[other] {""}
         }s"#
     )]
-    Closure,
+    Closure { kind: ConstContext },
 }
 
 #[derive(Subdiagnostic)]
@@ -624,7 +623,7 @@ pub trait ReportErrorExt {
             let mut diag = dcx.struct_allow(DiagMessage::Str(String::new().into()));
             let message = self.diagnostic_message();
             self.add_args(&mut diag);
-            let s = dcx.eagerly_format_to_string(message, diag.args.iter());
+            let s = format_diag_message(&message, &diag.args).into_owned();
             diag.cancel();
             s
         })
@@ -1086,12 +1085,12 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> {
         }
 
         let message = if let Some(path) = self.path {
-            err.dcx.eagerly_format_to_string(
-                msg!("constructing invalid value at {$path}"),
-                [("path".into(), DiagArgValue::Str(path.into()))].iter().map(|(a, b)| (a, b)),
+            format_diag_message(
+                &msg!("constructing invalid value at {$path}"),
+                &DiagArgMap::from_iter([("path".into(), DiagArgValue::Str(path.into()))]),
             )
         } else {
-            err.dcx.eagerly_format_to_string(msg!("constructing invalid value"), [].into_iter())
+            Cow::Borrowed("constructing invalid value")
         };
 
         err.arg("front_matter", message);
@@ -1117,12 +1116,13 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> {
                 msg!("in the range {$lo}..={$hi}")
             };
 
-            let args = [
-                ("lo".into(), DiagArgValue::Str(lo.to_string().into())),
-                ("hi".into(), DiagArgValue::Str(hi.to_string().into())),
-            ];
-            let args = args.iter().map(|(a, b)| (a, b));
-            let message = err.dcx.eagerly_format_to_string(msg, args);
+            let message = format_diag_message(
+                &msg,
+                &DiagArgMap::from_iter([
+                    ("lo".into(), DiagArgValue::Str(lo.to_string().into())),
+                    ("hi".into(), DiagArgValue::Str(hi.to_string().into())),
+                ]),
+            );
             err.arg("in_range", message);
         }
 
@@ -1132,19 +1132,18 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> {
             }
             PointerAsInt { expected } | Uninit { expected } => {
                 let msg = match expected {
-                    ExpectedKind::Reference => msg!("expected a reference"),
-                    ExpectedKind::Box => msg!("expected a box"),
-                    ExpectedKind::RawPtr => msg!("expected a raw pointer"),
-                    ExpectedKind::InitScalar => msg!("expected initialized scalar value"),
-                    ExpectedKind::Bool => msg!("expected a boolean"),
-                    ExpectedKind::Char => msg!("expected a unicode scalar value"),
-                    ExpectedKind::Float => msg!("expected a floating point number"),
-                    ExpectedKind::Int => msg!("expected an integer"),
-                    ExpectedKind::FnPtr => msg!("expected a function pointer"),
-                    ExpectedKind::EnumTag => msg!("expected a valid enum tag"),
-                    ExpectedKind::Str => msg!("expected a string"),
+                    ExpectedKind::Reference => "expected a reference",
+                    ExpectedKind::Box => "expected a box",
+                    ExpectedKind::RawPtr => "expected a raw pointer",
+                    ExpectedKind::InitScalar => "expected initialized scalar value",
+                    ExpectedKind::Bool => "expected a boolean",
+                    ExpectedKind::Char => "expected a unicode scalar value",
+                    ExpectedKind::Float => "expected a floating point number",
+                    ExpectedKind::Int => "expected an integer",
+                    ExpectedKind::FnPtr => "expected a function pointer",
+                    ExpectedKind::EnumTag => "expected a valid enum tag",
+                    ExpectedKind::Str => "expected a string",
                 };
-                let msg = err.dcx.eagerly_format_to_string(msg, [].into_iter());
                 err.arg("expected", msg);
             }
             InvalidEnumTag { value }

compiler/rustc_const_eval/src/interpret/eval_context.rs

@@ -1,7 +1,7 @@
 use either::{Left, Right};
 use rustc_abi::{Align, HasDataLayout, Size, TargetDataLayout};
 use rustc_data_structures::debug_assert_matches;
-use rustc_errors::{DiagCtxtHandle, msg};
+use rustc_errors::{DiagCtxtHandle, format_diag_message, msg};
 use rustc_hir::def_id::DefId;
 use rustc_hir::limit::Limit;
 use rustc_middle::mir::interpret::{ErrorHandled, InvalidMetaKind, ReportedErrorInfo};
@@ -235,9 +235,9 @@ pub fn format_interp_error<'tcx>(dcx: DiagCtxtHandle<'_>, e: InterpErrorInfo<'tc
     let mut diag = dcx.struct_allow("");
     let msg = e.diagnostic_message();
     e.add_args(&mut diag);
-    let s = dcx.eagerly_format_to_string(msg, diag.args.iter());
+    let msg = format_diag_message(&msg, &diag.args).into_owned();
     diag.cancel();
-    s
+    msg
 }
 
 impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {

compiler/rustc_errors/src/diagnostic.rs

@@ -236,9 +236,6 @@ pub struct DiagInner {
     pub suggestions: Suggestions,
     pub args: DiagArgMap,
 
-    // This is used to store args and restore them after a subdiagnostic is rendered.
-    pub reserved_args: DiagArgMap,
-
     /// This is not used for highlighting or rendering any error message. Rather, it can be used
     /// as a sort key to sort a buffer of diagnostics. By default, it is the primary span of
     /// `span` if there is one. Otherwise, it is `DUMMY_SP`.
@@ -269,7 +266,6 @@ impl DiagInner {
             children: vec![],
             suggestions: Suggestions::Enabled(vec![]),
             args: Default::default(),
-            reserved_args: Default::default(),
             sort_span: DUMMY_SP,
             is_lint: None,
             long_ty_path: None,
@@ -334,14 +330,6 @@ impl DiagInner {
         self.args.swap_remove(name);
     }
 
-    pub fn store_args(&mut self) {
-        self.reserved_args = self.args.clone();
-    }
-
-    pub fn restore_args(&mut self) {
-        self.args = std::mem::take(&mut self.reserved_args);
-    }
-
     pub fn emitted_at_sub_diag(&self) -> Subdiag {
         let track = format!("-Ztrack-diagnostics: created at {}", self.emitted_at);
         Subdiag {
@@ -1144,16 +1132,6 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> {
         self
     }
 
-    /// Fluent variables are not namespaced from each other, so when
-    /// `Diagnostic`s and `Subdiagnostic`s use the same variable name,
-    /// one value will clobber the other. Eagerly formatting the
-    /// diagnostic uses the variables defined right then, before the
-    /// clobbering occurs.
-    pub fn eagerly_format(&self, msg: impl Into<DiagMessage>) -> DiagMessage {
-        let args = self.args.iter();
-        self.dcx.eagerly_format(msg.into(), args)
-    }
-
     with_fn! { with_span,
     /// Add a span.
     pub fn span(&mut self, sp: impl Into<MultiSpan>) -> &mut Self {
@@ -1341,12 +1319,6 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> {
         self.downgrade_to_delayed_bug();
         self.emit()
     }
-
-    pub fn remove_arg(&mut self, name: &str) {
-        if let Some(diag) = self.diag.as_mut() {
-            diag.remove_arg(name);
-        }
-    }
 }
 
 /// Destructor bomb: every `Diag` must be consumed (emitted, cancelled, etc.)