diff --git a/compiler/rustc_macros/src/diagnostics/diagnostic.rs b/compiler/rustc_macros/src/diagnostics/diagnostic.rs index 738aa802c9832..015419358c451 100644 --- a/compiler/rustc_macros/src/diagnostics/diagnostic.rs +++ b/compiler/rustc_macros/src/diagnostics/diagnostic.rs @@ -52,8 +52,10 @@ impl<'a> DiagnosticDerive<'a> { } }; + let formatting_init = &builder.formatting_init; quote! { #init + #formatting_init #preamble #body #diag @@ -101,9 +103,10 @@ impl<'a> LintDiagnosticDerive<'a> { let body = builder.body(&variant); let diag = &builder.parent.diag; - + let formatting_init = &builder.formatting_init; quote! { #preamble + #formatting_init #body #diag } diff --git a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs index 017bf2365d62b..dcbe89251cb36 100644 --- a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs +++ b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs @@ -5,9 +5,9 @@ use crate::diagnostics::error::{ DiagnosticDeriveError, }; use crate::diagnostics::utils::{ - bind_style_of_field, build_field_mapping, report_error_if_not_applied_to_span, - report_type_error, should_generate_set_arg, type_is_unit, type_matches_path, FieldInfo, - FieldInnerTy, FieldMap, HasFieldMap, SetOnce, SpannedOption, SubdiagnosticKind, + build_field_mapping, report_error_if_not_applied_to_span, report_type_error, + should_generate_set_arg, type_is_unit, type_matches_path, FieldInfo, FieldInnerTy, FieldMap, + HasFieldMap, SetOnce, SpannedOption, SubdiagnosticKind, }; use proc_macro2::{Ident, Span, TokenStream}; use quote::{format_ident, quote}; @@ -40,6 +40,9 @@ pub(crate) struct DiagnosticDeriveVariantBuilder<'parent> { /// The parent builder for the entire type. pub parent: &'parent DiagnosticDeriveBuilder, + /// Initialization of format strings for code suggestions. + pub formatting_init: TokenStream, + /// Span of the struct or the enum variant. pub span: proc_macro::Span, @@ -88,19 +91,7 @@ impl DiagnosticDeriveBuilder { } } - for variant in structure.variants_mut() { - // First, change the binding style of each field based on the code that will be - // generated for the field - e.g. `set_arg` calls needs by-move bindings, whereas - // `set_primary_span` only needs by-ref. - variant.bind_with(|bi| bind_style_of_field(bi.ast()).0); - - // Then, perform a stable sort on bindings which generates code for by-ref bindings - // before code generated for by-move bindings. Any code generated for the by-ref - // bindings which creates a reference to the by-move fields will happen before the - // by-move bindings move those fields and make them inaccessible. - variant.bindings_mut().sort_by_cached_key(|bi| bind_style_of_field(bi.ast())); - } - + structure.bind_with(|_| synstructure::BindStyle::Move); let variants = structure.each_variant(|variant| { let span = match structure.ast().data { syn::Data::Struct(..) => span, @@ -112,6 +103,7 @@ impl DiagnosticDeriveBuilder { parent: &self, span, field_map: build_field_mapping(variant), + formatting_init: TokenStream::new(), slug: None, code: None, }; @@ -143,16 +135,14 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { /// Generates calls to `span_label` and similar functions based on the attributes on fields or /// calls to `set_arg` when no attributes are present. - /// - /// Expects use of `Self::each_variant` which will have sorted bindings so that by-ref bindings - /// (which may create references to by-move bindings) have their code generated first - - /// necessary as code for suggestions uses formatting machinery and the value of other fields - /// (any given field can be referenced multiple times, so must be accessed through a borrow); - /// and when passing fields to `add_subdiagnostic` or `set_arg` for Fluent, fields must be - /// accessed by-move. pub fn body<'s>(&mut self, variant: &VariantInfo<'s>) -> TokenStream { let mut body = quote! {}; - for binding in variant.bindings() { + // Generate `set_arg` calls first.. + for binding in variant.bindings().iter().filter(|bi| should_generate_set_arg(bi.ast())) { + body.extend(self.generate_field_code(binding)); + } + // ..and then subdiagnostic additions. + for binding in variant.bindings().iter().filter(|bi| !should_generate_set_arg(bi.ast())) { body.extend(self.generate_field_attrs_code(binding)); } body @@ -274,24 +264,27 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { } } - fn generate_field_attrs_code(&mut self, binding_info: &BindingInfo<'_>) -> TokenStream { + fn generate_field_code(&mut self, binding_info: &BindingInfo<'_>) -> TokenStream { + let diag = &self.parent.diag; + let field = binding_info.ast(); let field_binding = &binding_info.binding; - if should_generate_set_arg(&field) { - let diag = &self.parent.diag; - let ident = field.ident.as_ref().unwrap(); - // strip `r#` prefix, if present - let ident = format_ident!("{}", ident); - return quote! { - #diag.set_arg( - stringify!(#ident), - #field_binding - ); - }; + let ident = field.ident.as_ref().unwrap(); + let ident = format_ident!("{}", ident); // strip `r#` prefix, if present + + quote! { + #diag.set_arg( + stringify!(#ident), + #field_binding + ); } + } + + fn generate_field_attrs_code(&mut self, binding_info: &BindingInfo<'_>) -> TokenStream { + let field = binding_info.ast(); + let field_binding = &binding_info.binding; - let needs_move = bind_style_of_field(&field).is_move(); let inner_ty = FieldInnerTy::from_type(&field.ty); field @@ -304,10 +297,8 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { let (binding, needs_destructure) = if needs_clone { // `primary_span` can accept a `Vec` so don't destructure that. (quote! { #field_binding.clone() }, false) - } else if needs_move { - (quote! { #field_binding }, true) } else { - (quote! { *#field_binding }, true) + (quote! { #field_binding }, true) }; let generated_code = self @@ -440,8 +431,8 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { .unwrap_or_else(|| quote! { rustc_errors::Applicability::Unspecified }); let style = suggestion_kind.to_suggestion_style(); + self.formatting_init.extend(code_init); Ok(quote! { - #code_init #diag.span_suggestion_with_style( #span_field, rustc_errors::fluent::#slug, @@ -490,7 +481,7 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { // If `ty` is `Span` w/out applicability, then use `Applicability::Unspecified`. ty @ Type::Path(..) if type_matches_path(ty, &["rustc_span", "Span"]) => { let binding = &info.binding.binding; - Ok((quote!(*#binding), None)) + Ok((quote!(#binding), None)) } // If `ty` is `(Span, Applicability)` then return tokens accessing those. Type::Tuple(tup) => { diff --git a/compiler/rustc_macros/src/diagnostics/utils.rs b/compiler/rustc_macros/src/diagnostics/utils.rs index 5e1cd842b9bbb..4fd4adc511267 100644 --- a/compiler/rustc_macros/src/diagnostics/utils.rs +++ b/compiler/rustc_macros/src/diagnostics/utils.rs @@ -5,13 +5,12 @@ use proc_macro::Span; use proc_macro2::TokenStream; use quote::{format_ident, quote, ToTokens}; use std::cell::RefCell; -use std::cmp::Ordering; use std::collections::{BTreeSet, HashMap}; use std::fmt; use std::str::FromStr; use syn::{spanned::Spanned, Attribute, Field, Meta, Type, TypeTuple}; use syn::{MetaList, MetaNameValue, NestedMeta, Path}; -use synstructure::{BindStyle, BindingInfo, VariantInfo}; +use synstructure::{BindingInfo, VariantInfo}; use super::error::invalid_nested_attr; @@ -650,65 +649,8 @@ impl quote::IdentFragment for SubdiagnosticKind { } } -/// Wrapper around `synstructure::BindStyle` which implements `Ord`. -#[derive(PartialEq, Eq)] -pub(super) struct OrderedBindStyle(pub(super) BindStyle); - -impl OrderedBindStyle { - /// Is `BindStyle::Move` or `BindStyle::MoveMut`? - pub(super) fn is_move(&self) -> bool { - matches!(self.0, BindStyle::Move | BindStyle::MoveMut) - } -} - -impl Ord for OrderedBindStyle { - fn cmp(&self, other: &Self) -> Ordering { - match (self.is_move(), other.is_move()) { - // If both `self` and `other` are the same, then ordering is equal. - (true, true) | (false, false) => Ordering::Equal, - // If `self` is not a move then it should be considered less than `other` (so that - // references are sorted first). - (false, _) => Ordering::Less, - // If `self` is a move then it must be greater than `other` (again, so that references - // are sorted first). - (true, _) => Ordering::Greater, - } - } -} - -impl PartialOrd for OrderedBindStyle { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - /// Returns `true` if `field` should generate a `set_arg` call rather than any other diagnostic /// call (like `span_label`). pub(super) fn should_generate_set_arg(field: &Field) -> bool { field.attrs.is_empty() } - -/// Returns `true` if `field` needs to have code generated in the by-move branch of the -/// generated derive rather than the by-ref branch. -pub(super) fn bind_style_of_field(field: &Field) -> OrderedBindStyle { - let generates_set_arg = should_generate_set_arg(field); - let is_multispan = type_matches_path(&field.ty, &["rustc_errors", "MultiSpan"]); - // FIXME(davidtwco): better support for one field needing to be in the by-move and - // by-ref branches. - let is_subdiagnostic = field - .attrs - .iter() - .map(|attr| attr.path.segments.last().unwrap().ident.to_string()) - .any(|attr| attr == "subdiagnostic"); - - // `set_arg` calls take their argument by-move.. - let needs_move = generates_set_arg - // If this is a `MultiSpan` field then it needs to be moved to be used by any - // attribute.. - || is_multispan - // If this a `#[subdiagnostic]` then it needs to be moved as the other diagnostic is - // unlikely to be `Copy`.. - || is_subdiagnostic; - - OrderedBindStyle(if needs_move { BindStyle::Move } else { BindStyle::Ref }) -}