Rollup merge of #157070 - GuillaumeGomez:rm-skip_arg, r=JonathanBrouwer

Remove `skip_arg` attribute from `Diagnostic` and `Subdiagnostic` proc-macros

Instead of having users to manually add `#[skip_arg]` for each field that is not used in fluent messages, I think it's better to instead let the proc-macro only call `diag.arg("name", field)` on the fields actually used.

r? @JonathanBrouwer

Author: JonathanBrouwer

compiler/rustc_const_eval/src/const_eval/machine.rs

@@ -844,14 +844,13 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
                 // current number of evaluated terminators is a power of 2. The latter gives us a cheap
                 // way to implement exponential backoff.
                 let span = ecx.cur_span();
+                let mut warn =
+                    ecx.tcx.dcx().create_warn(LongRunningWarn { span, item_span: ecx.tcx.span });
                 // We store a unique number in `force_duplicate` to evade `-Z deduplicate-diagnostics`.
                 // `new_steps` is guaranteed to be unique because `ecx.machine.num_evaluated_steps` is
                 // always increasing.
-                ecx.tcx.dcx().emit_warn(LongRunningWarn {
-                    span,
-                    item_span: ecx.tcx.span,
-                    force_duplicate: new_steps,
-                });
+                warn.arg("force_duplicate", new_steps);
+                warn.emit();
             }
         }
 

compiler/rustc_const_eval/src/errors.rs

@@ -298,8 +298,6 @@ pub(crate) struct LongRunningWarn {
     pub span: Span,
     #[help("the constant being evaluated")]
     pub item_span: Span,
-    // Used for evading `-Z deduplicate-diagnostics`.
-    pub force_duplicate: usize,
 }
 
 #[derive(Subdiagnostic)]

compiler/rustc_hir_typeck/src/callee.rs

@@ -845,7 +845,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let mut path = None;
         let mut err = self.dcx().create_err(errors::InvalidCallee {
             span: callee_expr.span,
-            ty: callee_ty,
             found: match &unit_variant {
                 Some((_, kind, path)) => format!("{kind} `{path}`"),
                 None => format!("`{}`", self.tcx.short_string(callee_ty, &mut path)),
@@ -949,7 +948,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         }
 
         if let Some(span) = self.tcx.hir_res_span(def) {
-            let callee_ty = callee_ty.to_string();
             let label = match (unit_variant, inner_callee_path) {
                 (Some((_, kind, path)), _) => {
                     err.arg("kind", kind);
@@ -959,6 +957,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 (_, Some(hir::QPath::Resolved(_, path))) => {
                     self.tcx.sess.source_map().span_to_snippet(path.span).ok().map(|p| {
                         err.arg("func", p);
+                        err.arg("ty", callee_ty);
                         msg!("`{$func}` defined here returns `{$ty}`")
                     })
                 }
@@ -968,14 +967,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         // type definitions themselves, but rather variables *of* that type.
                         Res::Local(hir_id) => {
                             err.arg("local_name", self.tcx.hir_name(hir_id));
+                            err.arg("ty", callee_ty);
                             Some(msg!("`{$local_name}` has type `{$ty}`"))
                         }
                         Res::Def(kind, def_id) if kind.ns() == Some(Namespace::ValueNS) => {
                             err.arg("path", self.tcx.def_path_str(def_id));
                             Some(msg!("`{$path}` defined here"))
                         }
                         _ => {
-                            err.arg("path", callee_ty);
+                            err.arg("path", callee_ty.to_string());
                             Some(msg!("`{$path}` defined here"))
                         }
                     }

compiler/rustc_hir_typeck/src/errors.rs

@@ -476,10 +476,9 @@ pub(crate) struct SlicingSuggestion {
 
 #[derive(Diagnostic)]
 #[diag("expected function, found {$found}", code = E0618)]
-pub(crate) struct InvalidCallee<'tcx> {
+pub(crate) struct InvalidCallee {
     #[primary_span]
     pub span: Span,
-    pub ty: Ty<'tcx>,
     pub found: String,
 }
 

compiler/rustc_lint/src/lints.rs

@@ -1562,17 +1562,16 @@ pub(crate) enum NonCamelCaseTypeSub {
 pub(crate) struct NonSnakeCaseDiag<'a> {
     pub sort: &'a str,
     pub name: &'a str,
-    pub sc: String,
     #[subdiagnostic]
     pub sub: NonSnakeCaseDiagSub,
 }
 
 pub(crate) enum NonSnakeCaseDiagSub {
     Label { span: Span },
-    Help,
+    Help { sc: String },
     RenameOrConvertSuggestion { span: Span, suggestion: Ident },
     ConvertSuggestion { span: Span, suggestion: String },
-    SuggestionAndNote { span: Span },
+    SuggestionAndNote { sc: String, span: Span },
 }
 
 impl Subdiagnostic for NonSnakeCaseDiagSub {
@@ -1581,7 +1580,8 @@ impl Subdiagnostic for NonSnakeCaseDiagSub {
             NonSnakeCaseDiagSub::Label { span } => {
                 diag.span_label(span, msg!("should have a snake_case name"));
             }
-            NonSnakeCaseDiagSub::Help => {
+            NonSnakeCaseDiagSub::Help { sc } => {
+                diag.arg("sc", sc);
                 diag.help(msg!("convert the identifier to snake case: `{$sc}`"));
             }
             NonSnakeCaseDiagSub::ConvertSuggestion { span, suggestion } => {
@@ -1600,7 +1600,8 @@ impl Subdiagnostic for NonSnakeCaseDiagSub {
                     Applicability::MaybeIncorrect,
                 );
             }
-            NonSnakeCaseDiagSub::SuggestionAndNote { span } => {
+            NonSnakeCaseDiagSub::SuggestionAndNote { sc, span } => {
+                diag.arg("sc", sc);
                 diag.note(msg!("`{$sc}` cannot be used as a raw identifier"));
                 diag.span_suggestion(
                     span,

compiler/rustc_lint/src/nonstandard_style.rs

@@ -311,18 +311,18 @@ impl NonSnakeCase {
                                 suggestion: sc_ident,
                             }
                         } else {
-                            NonSnakeCaseDiagSub::SuggestionAndNote { span }
+                            NonSnakeCaseDiagSub::SuggestionAndNote { sc, span }
                         }
                     } else {
-                        NonSnakeCaseDiagSub::ConvertSuggestion { span, suggestion: sc.clone() }
+                        NonSnakeCaseDiagSub::ConvertSuggestion { span, suggestion: sc }
                     }
                 } else {
-                    NonSnakeCaseDiagSub::Help
+                    NonSnakeCaseDiagSub::Help { sc }
                 }
             } else {
                 NonSnakeCaseDiagSub::Label { span }
             };
-            cx.emit_span_lint(NON_SNAKE_CASE, span, NonSnakeCaseDiag { sort, name, sc, sub });
+            cx.emit_span_lint(NON_SNAKE_CASE, span, NonSnakeCaseDiag { sort, name, sub });
         }
     }
 }

compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs

@@ -232,6 +232,5 @@ struct OpaqueHiddenInferredBoundLint<'tcx> {
 struct AddBound<'tcx> {
     #[primary_span]
     suggest_span: Span,
-    #[skip_arg]
     trait_ref: TraitPredPrintModifiersAndPath<'tcx>,
 }

compiler/rustc_macros/src/diagnostics/diagnostic.rs

@@ -26,7 +26,7 @@ impl<'a> DiagnosticDerive<'a> {
             let Some(message) = builder.primary_message() else {
                 return DiagnosticDeriveError::ErrorHandled.to_compile_error();
             };
-            let message = message.diag_message(Some(variant));
+            let message = message.diag_message();
 
             let init = quote! {
                 let mut diag = rustc_errors::Diag::new(

compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs

@@ -1,5 +1,7 @@
 #![deny(unused_must_use)]
 
+use std::collections::HashSet;
+
 use proc_macro2::{Ident, Span, TokenStream};
 use quote::{format_ident, quote, quote_spanned};
 use syn::parse::ParseStream;
@@ -52,6 +54,7 @@ where
             formatting_init: TokenStream::new(),
             message: None,
             code: None,
+            used_fields: HashSet::new(),
         };
         f(builder, variant)
     });
@@ -83,6 +86,8 @@ pub(crate) struct DiagnosticDeriveVariantBuilder {
     /// Error codes are a optional part of the struct attribute - this is only set to detect
     /// multiple specifications.
     pub code: SpannedOption<()>,
+
+    pub used_fields: HashSet<proc_macro2::Ident>,
 }
 
 impl DiagnosticDeriveVariantBuilder {
@@ -107,8 +112,7 @@ impl DiagnosticDeriveVariantBuilder {
         let ast = variant.ast();
         let attrs = &ast.attrs;
         let preamble = attrs.iter().map(|attr| {
-            self.generate_structure_code_for_attr(attr, variant)
-                .unwrap_or_else(|v| v.to_compile_error())
+            self.generate_structure_code_for_attr(attr).unwrap_or_else(|v| v.to_compile_error())
         });
 
         quote! {
@@ -120,23 +124,34 @@ impl DiagnosticDeriveVariantBuilder {
     /// calls to `arg` when no attributes are present.
     pub(crate) fn body(&mut self, variant: &VariantInfo<'_>) -> TokenStream {
         let mut body = quote! {};
+        let mut second_part = quote! {};
+
+        // Subdiagnostic additions.
+        for binding in variant.bindings().iter().filter(|bi| !should_generate_arg(bi.ast())) {
+            second_part.extend(self.generate_field_attrs_code(binding));
+        }
         // Generate `arg` calls first..
         for binding in variant.bindings().iter().filter(|bi| should_generate_arg(bi.ast())) {
-            body.extend(self.generate_field_code(binding));
-        }
-        // ..and then subdiagnostic additions.
-        for binding in variant.bindings().iter().filter(|bi| !should_generate_arg(bi.ast())) {
-            body.extend(self.generate_field_attrs_code(binding, variant));
+            if self.is_used_in_message(binding) {
+                body.extend(self.generate_field_code(binding));
+            }
         }
+        body.extend(second_part);
         body
     }
 
+    fn is_used_in_message(&self, binding: &BindingInfo<'_>) -> bool {
+        binding.ast().ident.as_ref().is_some_and(|ident| self.used_fields.contains(ident))
+    }
+
     /// Parse a `SubdiagnosticKind` from an `Attribute`.
     fn parse_subdiag_attribute(
-        &self,
+        &mut self,
         attr: &Attribute,
     ) -> Result<Option<(SubdiagnosticKind, Message, bool)>, DiagnosticDeriveError> {
-        let Some(subdiag) = SubdiagnosticVariant::from_attr(attr, &self.field_map)? else {
+        let Some(subdiag) =
+            SubdiagnosticVariant::from_attr(attr, &self.field_map, &mut self.used_fields)?
+        else {
             // Some attributes aren't errors - like documentation comments - but also aren't
             // subdiagnostics.
             return Ok(None);
@@ -160,7 +175,6 @@ impl DiagnosticDeriveVariantBuilder {
     fn generate_structure_code_for_attr(
         &mut self,
         attr: &Attribute,
-        variant: &VariantInfo<'_>,
     ) -> Result<TokenStream, DiagnosticDeriveError> {
         // Always allow documentation comments.
         if is_doc_comment(attr) {
@@ -183,11 +197,14 @@ impl DiagnosticDeriveVariantBuilder {
                         )
                         .emit();
                     }
-                    self.message = Some(Message {
-                        attr_span: attr.span(),
-                        message_span: message.span(),
-                        value: message.value(),
-                    });
+                    let message = Message::new(
+                        attr.span(),
+                        message.span(),
+                        message.value(),
+                        &self.field_map,
+                        &mut self.used_fields,
+                    );
+                    self.message = Some(message);
                 }
 
                 // Parse arguments
@@ -240,7 +257,7 @@ impl DiagnosticDeriveVariantBuilder {
             | SubdiagnosticKind::NoteOnce
             | SubdiagnosticKind::Help
             | SubdiagnosticKind::HelpOnce
-            | SubdiagnosticKind::Warn => Ok(self.add_subdiagnostic(&fn_ident, message, variant)),
+            | SubdiagnosticKind::Warn => Ok(self.add_subdiagnostic(&fn_ident, message)),
             SubdiagnosticKind::Label | SubdiagnosticKind::Suggestion { .. } => {
                 throw_invalid_attr!(attr, |diag| diag
                     .help("`#[label]` and `#[suggestion]` can only be applied to fields"));
@@ -268,11 +285,7 @@ impl DiagnosticDeriveVariantBuilder {
         }
     }
 
-    fn generate_field_attrs_code(
-        &mut self,
-        binding_info: &BindingInfo<'_>,
-        variant: &VariantInfo<'_>,
-    ) -> TokenStream {
+    fn generate_field_attrs_code(&mut self, binding_info: &BindingInfo<'_>) -> TokenStream {
         let field = binding_info.ast();
         let field_binding = &binding_info.binding;
 
@@ -311,7 +324,6 @@ impl DiagnosticDeriveVariantBuilder {
                         attr,
                         FieldInfo { binding: binding_info, ty: inner_ty, span: &field.span() },
                         binding,
-                        variant
                     )
                     .unwrap_or_else(|v| v.to_compile_error());
 
@@ -329,14 +341,10 @@ impl DiagnosticDeriveVariantBuilder {
         attr: &Attribute,
         info: FieldInfo<'_>,
         binding: TokenStream,
-        variant: &VariantInfo<'_>,
     ) -> Result<TokenStream, DiagnosticDeriveError> {
         let ident = &attr.path().segments.last().unwrap().ident;
         let name = ident.to_string();
         match (&attr.meta, name.as_str()) {
-            // Don't need to do anything - by virtue of the attribute existing, the
-            // `arg` call will not be generated.
-            (Meta::Path(_), "skip_arg") => return Ok(quote! {}),
             (Meta::Path(_), "primary_span") => {
                 report_error_if_not_applied_to_span(attr, &info)?;
 
@@ -359,7 +367,7 @@ impl DiagnosticDeriveVariantBuilder {
         match subdiag {
             SubdiagnosticKind::Label => {
                 report_error_if_not_applied_to_span(attr, &info)?;
-                Ok(self.add_spanned_subdiagnostic(binding, &fn_ident, message, variant))
+                Ok(self.add_spanned_subdiagnostic(binding, &fn_ident, message))
             }
             SubdiagnosticKind::Note
             | SubdiagnosticKind::NoteOnce
@@ -370,11 +378,11 @@ impl DiagnosticDeriveVariantBuilder {
                 if type_matches_path(inner, &["rustc_span", "Span"])
                     || type_matches_path(inner, &["rustc_span", "MultiSpan"])
                 {
-                    Ok(self.add_spanned_subdiagnostic(binding, &fn_ident, message, variant))
+                    Ok(self.add_spanned_subdiagnostic(binding, &fn_ident, message))
                 } else if type_is_unit(inner)
                     || (matches!(info.ty, FieldInnerTy::Plain(_)) && type_is_bool(inner))
                 {
-                    Ok(self.add_subdiagnostic(&fn_ident, message, variant))
+                    Ok(self.add_subdiagnostic(&fn_ident, message))
                 } else {
                     report_type_error(attr, "`Span`, `MultiSpan`, `bool` or `()`")?
                 }
@@ -400,7 +408,7 @@ impl DiagnosticDeriveVariantBuilder {
                     applicability.set_once(quote! { #static_applicability }, span);
                 }
 
-                let message = message.diag_message(Some(variant));
+                let message = message.diag_message();
                 let applicability = applicability
                     .value()
                     .unwrap_or_else(|| quote! { rustc_errors::Applicability::Unspecified });
@@ -428,10 +436,9 @@ impl DiagnosticDeriveVariantBuilder {
         field_binding: TokenStream,
         kind: &Ident,
         message: Message,
-        variant: &VariantInfo<'_>,
     ) -> TokenStream {
         let fn_name = format_ident!("span_{}", kind);
-        let message = message.diag_message(Some(variant));
+        let message = message.diag_message();
         quote! {
             diag.#fn_name(
                 #field_binding,
@@ -442,13 +449,8 @@ impl DiagnosticDeriveVariantBuilder {
 
     /// Adds a subdiagnostic by generating a `diag.span_$kind` call with the current message
     /// and `fluent_attr_identifier`.
-    fn add_subdiagnostic(
-        &self,
-        kind: &Ident,
-        message: Message,
-        variant: &VariantInfo<'_>,
-    ) -> TokenStream {
-        let message = message.diag_message(Some(variant));
+    fn add_subdiagnostic(&self, kind: &Ident, message: Message) -> TokenStream {
+        let message = message.diag_message();
         quote! {
             diag.#kind(#message);
         }