From 31df50c8973aeb70fcf42b403239f4fc4712988c Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 19 Dec 2023 08:27:37 +1100 Subject: [PATCH] Streamline `Diagnostic` proc macro. First, it is parameterized by the name of the diagnostic and the DiagCtxt. These are given to `session_diagnostic_derive` and `lint_diagnostic_derive`. But the names are hard-wired as "diag" and "handler" (should be "dcx"), and there's no clear reason for the parameterization. So this commit removes the parameterization and hard-wires the names internally. Once that is done `DiagnosticDeriveBuilder` is reduced to a trivial wrapper around `DiagnosticDeriveKind`, and can be removed. Also, `DiagnosticDerive` and `LintDiagnosticDerive` don't need the `builder` field, because it has been reduced to a kind, and they know their own kind. This avoids the need for some `let`/`else`/`unreachable!` kind checks And `DiagnosticDeriveVariantBuilder` no longer needs a lifetime, because the `parent` field is changed to `kind`, which is now a trivial copy type. --- .../src/diagnostics/diagnostic.rs | 53 +++++--------- .../src/diagnostics/diagnostic_builder.rs | 70 +++++++------------ compiler/rustc_macros/src/diagnostics/mod.rs | 5 +- 3 files changed, 45 insertions(+), 83 deletions(-) diff --git a/compiler/rustc_macros/src/diagnostics/diagnostic.rs b/compiler/rustc_macros/src/diagnostics/diagnostic.rs index 76ef2c264a8..be8582d0ba0 100644 --- a/compiler/rustc_macros/src/diagnostics/diagnostic.rs +++ b/compiler/rustc_macros/src/diagnostics/diagnostic.rs @@ -2,7 +2,7 @@ use std::cell::RefCell; -use crate::diagnostics::diagnostic_builder::{DiagnosticDeriveBuilder, DiagnosticDeriveKind}; +use crate::diagnostics::diagnostic_builder::DiagnosticDeriveKind; use crate::diagnostics::error::{span_err, DiagnosticDeriveError}; use crate::diagnostics::utils::SetOnce; use proc_macro2::TokenStream; @@ -13,32 +13,21 @@ use synstructure::Structure; /// The central struct for constructing the `into_diagnostic` method from an annotated struct. pub(crate) struct DiagnosticDerive<'a> { structure: Structure<'a>, - builder: DiagnosticDeriveBuilder, } impl<'a> DiagnosticDerive<'a> { - pub(crate) fn new(diag: syn::Ident, dcx: syn::Ident, structure: Structure<'a>) -> Self { - Self { - builder: DiagnosticDeriveBuilder { - diag, - kind: DiagnosticDeriveKind::Diagnostic { dcx }, - }, - structure, - } + pub(crate) fn new(structure: Structure<'a>) -> Self { + Self { structure } } pub(crate) fn into_tokens(self) -> TokenStream { - let DiagnosticDerive { mut structure, mut builder } = self; - + let DiagnosticDerive { mut structure } = self; + let kind = DiagnosticDeriveKind::Diagnostic; let slugs = RefCell::new(Vec::new()); - let implementation = builder.each_variant(&mut structure, |mut builder, variant| { + let implementation = kind.each_variant(&mut structure, |mut builder, variant| { let preamble = builder.preamble(variant); let body = builder.body(variant); - let diag = &builder.parent.diag; - let DiagnosticDeriveKind::Diagnostic { dcx } = &builder.parent.kind else { - unreachable!() - }; let init = match builder.slug.value_ref() { None => { span_err(builder.span, "diagnostic slug not specified") @@ -62,7 +51,7 @@ impl<'a> DiagnosticDerive<'a> { Some(slug) => { slugs.borrow_mut().push(slug.clone()); quote! { - let mut #diag = #dcx.struct_diagnostic(crate::fluent_generated::#slug); + let mut diag = dcx.struct_diagnostic(crate::fluent_generated::#slug); } } }; @@ -73,12 +62,10 @@ impl<'a> DiagnosticDerive<'a> { #formatting_init #preamble #body - #diag + diag } }); - let DiagnosticDeriveKind::Diagnostic { dcx } = &builder.kind else { unreachable!() }; - // A lifetime of `'a` causes conflicts, but `_sess` is fine. let mut imp = structure.gen_impl(quote! { gen impl<'_sess, G> @@ -90,7 +77,7 @@ impl<'a> DiagnosticDerive<'a> { #[track_caller] fn into_diagnostic( self, - #dcx: &'_sess rustc_errors::DiagCtxt + dcx: &'_sess rustc_errors::DiagCtxt ) -> rustc_errors::DiagnosticBuilder<'_sess, G> { #implementation } @@ -106,36 +93,31 @@ impl<'a> DiagnosticDerive<'a> { /// The central struct for constructing the `decorate_lint` method from an annotated struct. pub(crate) struct LintDiagnosticDerive<'a> { structure: Structure<'a>, - builder: DiagnosticDeriveBuilder, } impl<'a> LintDiagnosticDerive<'a> { - pub(crate) fn new(diag: syn::Ident, structure: Structure<'a>) -> Self { - Self { - builder: DiagnosticDeriveBuilder { diag, kind: DiagnosticDeriveKind::LintDiagnostic }, - structure, - } + pub(crate) fn new(structure: Structure<'a>) -> Self { + Self { structure } } pub(crate) fn into_tokens(self) -> TokenStream { - let LintDiagnosticDerive { mut structure, mut builder } = self; - - let implementation = builder.each_variant(&mut structure, |mut builder, variant| { + let LintDiagnosticDerive { mut structure } = self; + let kind = DiagnosticDeriveKind::LintDiagnostic; + let implementation = kind.each_variant(&mut structure, |mut builder, variant| { let preamble = builder.preamble(variant); let body = builder.body(variant); - let diag = &builder.parent.diag; let formatting_init = &builder.formatting_init; quote! { #preamble #formatting_init #body - #diag + diag } }); let slugs = RefCell::new(Vec::new()); - let msg = builder.each_variant(&mut structure, |mut builder, variant| { + let msg = kind.each_variant(&mut structure, |mut builder, variant| { // Collect the slug by generating the preamble. let _ = builder.preamble(variant); @@ -168,13 +150,12 @@ impl<'a> LintDiagnosticDerive<'a> { } }); - let diag = &builder.diag; let mut imp = structure.gen_impl(quote! { gen impl<'__a> rustc_errors::DecorateLint<'__a, ()> for @Self { #[track_caller] fn decorate_lint<'__b>( self, - #diag: &'__b mut rustc_errors::DiagnosticBuilder<'__a, ()> + diag: &'__b mut rustc_errors::DiagnosticBuilder<'__a, ()> ) { #implementation; } diff --git a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs index 511654d9949..f3e98d68b60 100644 --- a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs +++ b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs @@ -17,28 +17,18 @@ use synstructure::{BindingInfo, Structure, VariantInfo}; use super::utils::SubdiagnosticVariant; /// What kind of diagnostic is being derived - a fatal/error/warning or a lint? -#[derive(Clone, PartialEq, Eq)] +#[derive(Clone, Copy, PartialEq, Eq)] pub(crate) enum DiagnosticDeriveKind { - Diagnostic { dcx: syn::Ident }, + Diagnostic, LintDiagnostic, } -/// Tracks persistent information required for the entire type when building up individual calls to -/// diagnostic methods for generated diagnostic derives - both `Diagnostic` for -/// fatal/errors/warnings and `LintDiagnostic` for lints. -pub(crate) struct DiagnosticDeriveBuilder { - /// The identifier to use for the generated `DiagnosticBuilder` instance. - pub diag: syn::Ident, - /// Kind of diagnostic that should be derived. - pub kind: DiagnosticDeriveKind, -} - /// Tracks persistent information required for a specific variant when building up individual calls /// to diagnostic methods for generated diagnostic derives - both `Diagnostic` for /// fatal/errors/warnings and `LintDiagnostic` for lints. -pub(crate) struct DiagnosticDeriveVariantBuilder<'parent> { - /// The parent builder for the entire type. - pub parent: &'parent DiagnosticDeriveBuilder, +pub(crate) struct DiagnosticDeriveVariantBuilder { + /// The kind for the entire type. + pub kind: DiagnosticDeriveKind, /// Initialization of format strings for code suggestions. pub formatting_init: TokenStream, @@ -59,19 +49,19 @@ pub(crate) struct DiagnosticDeriveVariantBuilder<'parent> { pub code: SpannedOption<()>, } -impl<'a> HasFieldMap for DiagnosticDeriveVariantBuilder<'a> { +impl HasFieldMap for DiagnosticDeriveVariantBuilder { fn get_field_binding(&self, field: &String) -> Option<&TokenStream> { self.field_map.get(field) } } -impl DiagnosticDeriveBuilder { +impl DiagnosticDeriveKind { /// Call `f` for the struct or for each variant of the enum, returning a `TokenStream` with the /// tokens from `f` wrapped in an `match` expression. Emits errors for use of derive on unions /// or attributes on the type itself when input is an enum. - pub(crate) fn each_variant<'s, F>(&mut self, structure: &mut Structure<'s>, f: F) -> TokenStream + pub(crate) fn each_variant<'s, F>(self, structure: &mut Structure<'s>, f: F) -> TokenStream where - F: for<'a, 'v> Fn(DiagnosticDeriveVariantBuilder<'a>, &VariantInfo<'v>) -> TokenStream, + F: for<'v> Fn(DiagnosticDeriveVariantBuilder, &VariantInfo<'v>) -> TokenStream, { let ast = structure.ast(); let span = ast.span().unwrap(); @@ -101,7 +91,7 @@ impl DiagnosticDeriveBuilder { _ => variant.ast().ident.span().unwrap(), }; let builder = DiagnosticDeriveVariantBuilder { - parent: self, + kind: self, span, field_map: build_field_mapping(variant), formatting_init: TokenStream::new(), @@ -119,7 +109,7 @@ impl DiagnosticDeriveBuilder { } } -impl<'a> DiagnosticDeriveVariantBuilder<'a> { +impl DiagnosticDeriveVariantBuilder { /// Generates calls to `code` and similar functions based on the attributes on the type or /// variant. pub(crate) fn preamble(&mut self, variant: &VariantInfo<'_>) -> TokenStream { @@ -184,8 +174,6 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { &mut self, attr: &Attribute, ) -> Result { - let diag = &self.parent.diag; - // Always allow documentation comments. if is_doc_comment(attr) { return Ok(quote! {}); @@ -223,7 +211,7 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { let code = nested.parse::()?; tokens.extend(quote! { - #diag.code(rustc_errors::DiagnosticId::Error(#code.to_string())); + diag.code(rustc_errors::DiagnosticId::Error(#code.to_string())); }); } else { span_err(path.span().unwrap(), "unknown argument") @@ -257,8 +245,6 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { } fn generate_field_code(&mut self, binding_info: &BindingInfo<'_>) -> TokenStream { - let diag = &self.parent.diag; - let field = binding_info.ast(); let mut field_binding = binding_info.binding.clone(); field_binding.set_span(field.ty.span()); @@ -267,7 +253,7 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { let ident = format_ident!("{}", ident); // strip `r#` prefix, if present quote! { - #diag.set_arg( + diag.set_arg( stringify!(#ident), #field_binding ); @@ -322,8 +308,6 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { info: FieldInfo<'_>, binding: TokenStream, ) -> Result { - let diag = &self.parent.diag; - let ident = &attr.path().segments.last().unwrap().ident; let name = ident.to_string(); match (&attr.meta, name.as_str()) { @@ -331,12 +315,12 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { // `set_arg` call will not be generated. (Meta::Path(_), "skip_arg") => return Ok(quote! {}), (Meta::Path(_), "primary_span") => { - match self.parent.kind { - DiagnosticDeriveKind::Diagnostic { .. } => { + match self.kind { + DiagnosticDeriveKind::Diagnostic => { report_error_if_not_applied_to_span(attr, &info)?; return Ok(quote! { - #diag.set_span(#binding); + diag.set_span(#binding); }); } DiagnosticDeriveKind::LintDiagnostic => { @@ -348,13 +332,13 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { } (Meta::Path(_), "subdiagnostic") => { if FieldInnerTy::from_type(&info.binding.ast().ty).will_iterate() { - let DiagnosticDeriveKind::Diagnostic { dcx } = &self.parent.kind else { + let DiagnosticDeriveKind::Diagnostic = self.kind else { // No eager translation for lints. - return Ok(quote! { #diag.subdiagnostic(#binding); }); + return Ok(quote! { diag.subdiagnostic(#binding); }); }; - return Ok(quote! { #diag.eager_subdiagnostic(#dcx, #binding); }); + return Ok(quote! { diag.eager_subdiagnostic(dcx, #binding); }); } else { - return Ok(quote! { #diag.subdiagnostic(#binding); }); + return Ok(quote! { diag.subdiagnostic(#binding); }); } } (Meta::List(meta_list), "subdiagnostic") => { @@ -376,15 +360,15 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { return Ok(quote! {}); } - let dcx = match &self.parent.kind { - DiagnosticDeriveKind::Diagnostic { dcx } => dcx, + match &self.kind { + DiagnosticDeriveKind::Diagnostic => {} DiagnosticDeriveKind::LintDiagnostic => { throw_invalid_attr!(attr, |diag| { diag.help("eager subdiagnostics are not supported on lints") }) } }; - return Ok(quote! { #diag.eager_subdiagnostic(#dcx, #binding); }); + return Ok(quote! { diag.eager_subdiagnostic(dcx, #binding); }); } _ => (), } @@ -442,7 +426,7 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { self.formatting_init.extend(code_init); Ok(quote! { - #diag.span_suggestions_with_style( + diag.span_suggestions_with_style( #span_field, crate::fluent_generated::#slug, #code_field, @@ -463,10 +447,9 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { kind: &Ident, fluent_attr_identifier: Path, ) -> TokenStream { - let diag = &self.parent.diag; let fn_name = format_ident!("span_{}", kind); quote! { - #diag.#fn_name( + diag.#fn_name( #field_binding, crate::fluent_generated::#fluent_attr_identifier ); @@ -476,9 +459,8 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { /// Adds a subdiagnostic by generating a `diag.span_$kind` call with the current slug /// and `fluent_attr_identifier`. fn add_subdiagnostic(&self, kind: &Ident, fluent_attr_identifier: Path) -> TokenStream { - let diag = &self.parent.diag; quote! { - #diag.#kind(crate::fluent_generated::#fluent_attr_identifier); + diag.#kind(crate::fluent_generated::#fluent_attr_identifier); } } diff --git a/compiler/rustc_macros/src/diagnostics/mod.rs b/compiler/rustc_macros/src/diagnostics/mod.rs index a536eb3b04e..d3a4e7ba7d1 100644 --- a/compiler/rustc_macros/src/diagnostics/mod.rs +++ b/compiler/rustc_macros/src/diagnostics/mod.rs @@ -6,7 +6,6 @@ mod utils; use diagnostic::{DiagnosticDerive, LintDiagnosticDerive}; use proc_macro2::TokenStream; -use quote::format_ident; use subdiagnostic::SubdiagnosticDeriveBuilder; use synstructure::Structure; @@ -57,7 +56,7 @@ use synstructure::Structure; /// See rustc dev guide for more examples on using the `#[derive(Diagnostic)]`: /// pub fn session_diagnostic_derive(s: Structure<'_>) -> TokenStream { - DiagnosticDerive::new(format_ident!("diag"), format_ident!("handler"), s).into_tokens() + DiagnosticDerive::new(s).into_tokens() } /// Implements `#[derive(LintDiagnostic)]`, which allows for lints to be specified as a struct, @@ -103,7 +102,7 @@ pub fn session_diagnostic_derive(s: Structure<'_>) -> TokenStream { /// See rustc dev guide for more examples on using the `#[derive(LintDiagnostic)]`: /// pub fn lint_diagnostic_derive(s: Structure<'_>) -> TokenStream { - LintDiagnosticDerive::new(format_ident!("diag"), s).into_tokens() + LintDiagnosticDerive::new(s).into_tokens() } /// Implements `#[derive(Subdiagnostic)]`, which allows for labels, notes, helps and