From 0ed0a8b460e5f973593a4ee95adde3e68f4178d0 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 3 Oct 2022 15:34:00 +0100 Subject: [PATCH 01/13] macros: tidy up lint changes Small tweaks to changes made in a previous PR, unrelated to eager translation. Signed-off-by: David Wood --- compiler/rustc_macros/src/diagnostics/diagnostic.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_macros/src/diagnostics/diagnostic.rs b/compiler/rustc_macros/src/diagnostics/diagnostic.rs index 83040a652b18a..406a56cd4ae3e 100644 --- a/compiler/rustc_macros/src/diagnostics/diagnostic.rs +++ b/compiler/rustc_macros/src/diagnostics/diagnostic.rs @@ -105,8 +105,8 @@ impl<'a> LintDiagnosticDerive<'a> { }); let msg = builder.each_variant(&mut structure, |mut builder, variant| { - // HACK(wafflelapkin): initialize slug (???) - let _preamble = builder.preamble(&variant); + // Collect the slug by generating the preamble. + let _ = builder.preamble(&variant); match builder.slug.value_ref() { None => { @@ -125,7 +125,10 @@ impl<'a> LintDiagnosticDerive<'a> { let diag = &builder.diag; structure.gen_impl(quote! { gen impl<'__a> rustc_errors::DecorateLint<'__a, ()> for @Self { - fn decorate_lint<'__b>(self, #diag: &'__b mut rustc_errors::DiagnosticBuilder<'__a, ()>) -> &'__b mut rustc_errors::DiagnosticBuilder<'__a, ()> { + fn decorate_lint<'__b>( + self, + #diag: &'__b mut rustc_errors::DiagnosticBuilder<'__a, ()> + ) -> &'__b mut rustc_errors::DiagnosticBuilder<'__a, ()> { use rustc_errors::IntoDiagnosticArg; #implementation } From 58ccef2d1fec9575910fbf9f334e19fb9692df38 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 3 Oct 2022 14:02:49 +0100 Subject: [PATCH 02/13] errors: use `HashMap` to store diagnostic args Eager translation will enable subdiagnostics to be translated multiple times with different arguments - this requires the ability to replace the value of one argument with a new value, which is better suited to a `HashMap` than the previous storage, a `Vec`. Signed-off-by: David Wood --- compiler/rustc_codegen_ssa/src/back/write.rs | 7 +++-- .../src/annotate_snippet_emitter_writer.rs | 4 +-- compiler/rustc_errors/src/diagnostic.rs | 19 ++++++++---- compiler/rustc_errors/src/emitter.rs | 4 +-- compiler/rustc_errors/src/json.rs | 4 +-- compiler/rustc_errors/src/translation.rs | 30 +++++++++++++------ 6 files changed, 45 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index 680b9b642d9b2..ea61fddfb2d36 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -15,7 +15,10 @@ use rustc_data_structures::profiling::TimingGuard; use rustc_data_structures::profiling::VerboseTimingGuard; use rustc_data_structures::sync::Lrc; use rustc_errors::emitter::Emitter; -use rustc_errors::{translation::Translate, DiagnosticId, FatalError, Handler, Level}; +use rustc_errors::{ + translation::{to_fluent_args, Translate}, + DiagnosticId, FatalError, Handler, Level, +}; use rustc_fs_util::link_or_copy; use rustc_hir::def_id::{CrateNum, LOCAL_CRATE}; use rustc_incremental::{ @@ -1750,7 +1753,7 @@ impl Translate for SharedEmitter { impl Emitter for SharedEmitter { fn emit_diagnostic(&mut self, diag: &rustc_errors::Diagnostic) { - let fluent_args = self.to_fluent_args(diag.args()); + let fluent_args = to_fluent_args(diag.args()); drop(self.sender.send(SharedEmitterMessage::Diagnostic(Diagnostic { msg: self.translate_messages(&diag.message, &fluent_args).to_string(), code: diag.code.clone(), diff --git a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs index b32fc3c719bbd..f14b8ee3254f3 100644 --- a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs +++ b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs @@ -7,7 +7,7 @@ use crate::emitter::FileWithAnnotatedLines; use crate::snippet::Line; -use crate::translation::Translate; +use crate::translation::{to_fluent_args, Translate}; use crate::{ CodeSuggestion, Diagnostic, DiagnosticId, DiagnosticMessage, Emitter, FluentBundle, LazyFallbackBundle, Level, MultiSpan, Style, SubDiagnostic, @@ -46,7 +46,7 @@ impl Translate for AnnotateSnippetEmitterWriter { impl Emitter for AnnotateSnippetEmitterWriter { /// The entry point for the diagnostics generation fn emit_diagnostic(&mut self, diag: &Diagnostic) { - let fluent_args = self.to_fluent_args(diag.args()); + let fluent_args = to_fluent_args(diag.args()); let mut children = diag.children.clone(); let (mut primary_span, suggestions) = self.primary_span_formatted(&diag, &fluent_args); diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 31e410aaaf082..a267bf6c0b4d1 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -27,7 +27,11 @@ pub struct SuggestionsDisabled; /// Simplified version of `FluentArg` that can implement `Encodable` and `Decodable`. Collection of /// `DiagnosticArg` are converted to `FluentArgs` (consuming the collection) at the start of /// diagnostic emission. -pub type DiagnosticArg<'source> = (Cow<'source, str>, DiagnosticArgValue<'source>); +pub type DiagnosticArg<'iter, 'source> = + (&'iter DiagnosticArgName<'source>, &'iter DiagnosticArgValue<'source>); + +/// Name of a diagnostic argument. +pub type DiagnosticArgName<'source> = Cow<'source, str>; /// Simplified version of `FluentValue` that can implement `Encodable` and `Decodable`. Converted /// to a `FluentValue` by the emitter to be used in diagnostic translation. @@ -229,7 +233,7 @@ pub struct Diagnostic { pub span: MultiSpan, pub children: Vec, pub suggestions: Result, SuggestionsDisabled>, - args: Vec>, + args: FxHashMap, DiagnosticArgValue<'static>>, /// 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 @@ -321,7 +325,7 @@ impl Diagnostic { span: MultiSpan::new(), children: vec![], suggestions: Ok(vec![]), - args: vec![], + args: Default::default(), sort_span: DUMMY_SP, is_lint: false, } @@ -956,8 +960,11 @@ impl Diagnostic { self } - pub fn args(&self) -> &[DiagnosticArg<'static>] { - &self.args + // Exact iteration order of diagnostic arguments shouldn't make a difference to output because + // they're only used in interpolation. + #[allow(rustc::potential_query_instability)] + pub fn args<'a>(&'a self) -> impl Iterator> { + self.args.iter() } pub fn set_arg( @@ -965,7 +972,7 @@ impl Diagnostic { name: impl Into>, arg: impl IntoDiagnosticArg, ) -> &mut Self { - self.args.push((name.into(), arg.into_diagnostic_arg())); + self.args.insert(name.into(), arg.into_diagnostic_arg()); self } diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 66fbb8f1213e0..cd6413bc3ec62 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -14,7 +14,7 @@ use rustc_span::{FileLines, SourceFile, Span}; use crate::snippet::{Annotation, AnnotationType, Line, MultilineAnnotation, Style, StyledString}; use crate::styled_buffer::StyledBuffer; -use crate::translation::Translate; +use crate::translation::{to_fluent_args, Translate}; use crate::{ CodeSuggestion, Diagnostic, DiagnosticId, DiagnosticMessage, FluentBundle, Handler, LazyFallbackBundle, Level, MultiSpan, SubDiagnostic, SubstitutionHighlight, SuggestionStyle, @@ -535,7 +535,7 @@ impl Emitter for EmitterWriter { } fn emit_diagnostic(&mut self, diag: &Diagnostic) { - let fluent_args = self.to_fluent_args(diag.args()); + let fluent_args = to_fluent_args(diag.args()); let mut children = diag.children.clone(); let (mut primary_span, suggestions) = self.primary_span_formatted(&diag, &fluent_args); diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index 1680c6accd78c..4cc7be47fc2c6 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -13,7 +13,7 @@ use rustc_span::source_map::{FilePathMapping, SourceMap}; use crate::emitter::{Emitter, HumanReadableErrorType}; use crate::registry::Registry; -use crate::translation::Translate; +use crate::translation::{to_fluent_args, Translate}; use crate::DiagnosticId; use crate::{ CodeSuggestion, FluentBundle, LazyFallbackBundle, MultiSpan, SpanLabel, SubDiagnostic, @@ -312,7 +312,7 @@ struct UnusedExterns<'a, 'b, 'c> { impl Diagnostic { fn from_errors_diagnostic(diag: &crate::Diagnostic, je: &JsonEmitter) -> Diagnostic { - let args = je.to_fluent_args(diag.args()); + let args = to_fluent_args(diag.args()); let sugg = diag.suggestions.iter().flatten().map(|sugg| { let translated_message = je.translate_message(&sugg.msg, &args); Diagnostic { diff --git a/compiler/rustc_errors/src/translation.rs b/compiler/rustc_errors/src/translation.rs index 4f407badb3f9e..c2ec2526a4aea 100644 --- a/compiler/rustc_errors/src/translation.rs +++ b/compiler/rustc_errors/src/translation.rs @@ -4,6 +4,27 @@ use rustc_data_structures::sync::Lrc; use rustc_error_messages::FluentArgs; use std::borrow::Cow; +/// Convert diagnostic arguments (a rustc internal type that exists to implement +/// `Encodable`/`Decodable`) into `FluentArgs` which is necessary to perform translation. +/// +/// Typically performed once for each diagnostic at the start of `emit_diagnostic` and then +/// passed around as a reference thereafter. +pub fn to_fluent_args<'iter, 'arg: 'iter>( + iter: impl Iterator>, +) -> FluentArgs<'arg> { + let mut args = if let Some(size) = iter.size_hint().1 { + FluentArgs::with_capacity(size) + } else { + FluentArgs::new() + }; + + for (k, v) in iter { + args.set(k.clone(), v.clone()); + } + + args +} + pub trait Translate { /// Return `FluentBundle` with localized diagnostics for the locale requested by the user. If no /// language was requested by the user then this will be `None` and `fallback_fluent_bundle` @@ -15,15 +36,6 @@ pub trait Translate { /// unavailable for the requested locale. fn fallback_fluent_bundle(&self) -> &FluentBundle; - /// Convert diagnostic arguments (a rustc internal type that exists to implement - /// `Encodable`/`Decodable`) into `FluentArgs` which is necessary to perform translation. - /// - /// Typically performed once for each diagnostic at the start of `emit_diagnostic` and then - /// passed around as a reference thereafter. - fn to_fluent_args<'arg>(&self, args: &[DiagnosticArg<'arg>]) -> FluentArgs<'arg> { - FromIterator::from_iter(args.iter().cloned()) - } - /// Convert `DiagnosticMessage`s to a string, performing translation if necessary. fn translate_messages( &self, From 1a74a826884c22a99d9d261369d40be9efc86922 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 3 Oct 2022 14:09:05 +0100 Subject: [PATCH 03/13] errors: `AddToDiagnostic::add_to_diagnostic_with` `AddToDiagnostic::add_to_diagnostic_with` is similar to the previous `AddToDiagnostic::add_to_diagnostic` but takes a function that can be used by the caller to modify diagnostic messages originating from the subdiagnostic (such as performing translation eagerly). `add_to_diagnostic` now just calls `add_to_diagnostic_with` with an empty closure. Signed-off-by: David Wood --- compiler/rustc_ast_lowering/src/errors.rs | 15 +++++-- compiler/rustc_ast_passes/src/errors.rs | 12 +++-- compiler/rustc_errors/src/diagnostic.rs | 19 ++++++-- compiler/rustc_infer/src/errors/mod.rs | 33 +++++++++++--- .../src/errors/note_and_explain.rs | 9 +++- compiler/rustc_lint/src/errors.rs | 15 +++++-- compiler/rustc_macros/src/diagnostics/mod.rs | 4 +- .../src/diagnostics/subdiagnostic.rs | 45 ++++++++++++------- compiler/rustc_query_system/src/error.rs | 7 ++- .../ui-fulldeps/internal-lints/diagnostics.rs | 12 +++-- .../internal-lints/diagnostics.stderr | 8 ++-- 11 files changed, 129 insertions(+), 50 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/errors.rs b/compiler/rustc_ast_lowering/src/errors.rs index 63ff64b00bed6..c6c85ffa84dd7 100644 --- a/compiler/rustc_ast_lowering/src/errors.rs +++ b/compiler/rustc_ast_lowering/src/errors.rs @@ -1,4 +1,7 @@ -use rustc_errors::{fluent, AddToDiagnostic, Applicability, Diagnostic, DiagnosticArgFromDisplay}; +use rustc_errors::{ + fluent, AddToDiagnostic, Applicability, Diagnostic, DiagnosticArgFromDisplay, + SubdiagnosticMessage, +}; use rustc_macros::{Diagnostic, Subdiagnostic}; use rustc_span::{symbol::Ident, Span, Symbol}; @@ -19,7 +22,10 @@ pub struct UseAngleBrackets { } impl AddToDiagnostic for UseAngleBrackets { - fn add_to_diagnostic(self, diag: &mut Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { diag.multipart_suggestion( fluent::ast_lowering::use_angle_brackets, vec![(self.open_param, String::from("<")), (self.close_param, String::from(">"))], @@ -69,7 +75,10 @@ pub enum AssocTyParenthesesSub { } impl AddToDiagnostic for AssocTyParenthesesSub { - fn add_to_diagnostic(self, diag: &mut Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { match self { Self::Empty { parentheses_span } => diag.multipart_suggestion( fluent::ast_lowering::remove_parentheses, diff --git a/compiler/rustc_ast_passes/src/errors.rs b/compiler/rustc_ast_passes/src/errors.rs index 035f0ce1cbc42..ba2ed24fc08fc 100644 --- a/compiler/rustc_ast_passes/src/errors.rs +++ b/compiler/rustc_ast_passes/src/errors.rs @@ -1,6 +1,6 @@ //! Errors emitted by ast_passes. -use rustc_errors::{fluent, AddToDiagnostic, Applicability, Diagnostic}; +use rustc_errors::{fluent, AddToDiagnostic, Applicability, Diagnostic, SubdiagnosticMessage}; use rustc_macros::{Diagnostic, Subdiagnostic}; use rustc_span::{Span, Symbol}; @@ -17,7 +17,10 @@ pub struct ForbiddenLet { } impl AddToDiagnostic for ForbiddenLetReason { - fn add_to_diagnostic(self, diag: &mut Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { match self { Self::GenericForbidden => {} Self::NotSupportedOr(span) => { @@ -228,7 +231,10 @@ pub struct ExternBlockSuggestion { } impl AddToDiagnostic for ExternBlockSuggestion { - fn add_to_diagnostic(self, diag: &mut Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { let start_suggestion = if let Some(abi) = self.abi { format!("extern \"{}\" {{", abi) } else { diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index a267bf6c0b4d1..2a6dd6da415cf 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -203,9 +203,20 @@ impl IntoDiagnosticArg for ast::token::TokenKind { /// `#[derive(Subdiagnostic)]` -- see [rustc_macros::Subdiagnostic]. #[cfg_attr(bootstrap, rustc_diagnostic_item = "AddSubdiagnostic")] #[cfg_attr(not(bootstrap), rustc_diagnostic_item = "AddToDiagnostic")] -pub trait AddToDiagnostic { +pub trait AddToDiagnostic +where + Self: Sized, +{ /// Add a subdiagnostic to an existing diagnostic. - fn add_to_diagnostic(self, diag: &mut Diagnostic); + fn add_to_diagnostic(self, diag: &mut Diagnostic) { + self.add_to_diagnostic_with(diag, |_, m| m); + } + + /// Add a subdiagnostic to an existing diagnostic where `f` is invoked on every message used + /// (to optionally perform eager translation). + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, f: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage; } /// Trait implemented by lint types. This should not be implemented manually. Instead, use @@ -921,8 +932,8 @@ impl Diagnostic { self } - /// Add a subdiagnostic from a type that implements `Subdiagnostic` - see - /// [rustc_macros::Subdiagnostic]. + /// Add a subdiagnostic from a type that implements `Subdiagnostic` (see + /// [rustc_macros::Subdiagnostic]). pub fn subdiagnostic(&mut self, subdiagnostic: impl AddToDiagnostic) -> &mut Self { subdiagnostic.add_to_diagnostic(self); self diff --git a/compiler/rustc_infer/src/errors/mod.rs b/compiler/rustc_infer/src/errors/mod.rs index 85b877652c6aa..500900d3d4a74 100644 --- a/compiler/rustc_infer/src/errors/mod.rs +++ b/compiler/rustc_infer/src/errors/mod.rs @@ -1,6 +1,7 @@ use hir::GenericParamKind; use rustc_errors::{ - fluent, AddToDiagnostic, Applicability, DiagnosticMessage, DiagnosticStyledString, MultiSpan, + fluent, AddToDiagnostic, Applicability, Diagnostic, DiagnosticMessage, DiagnosticStyledString, + MultiSpan, SubdiagnosticMessage, }; use rustc_hir as hir; use rustc_hir::{FnRetTy, Ty}; @@ -229,7 +230,10 @@ pub enum RegionOriginNote<'a> { } impl AddToDiagnostic for RegionOriginNote<'_> { - fn add_to_diagnostic(self, diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { let mut label_or_note = |span, msg: DiagnosticMessage| { let sub_count = diag.children.iter().filter(|d| d.span.is_dummy()).count(); let expanded_sub_count = diag.children.iter().filter(|d| !d.span.is_dummy()).count(); @@ -290,7 +294,10 @@ pub enum LifetimeMismatchLabels { } impl AddToDiagnostic for LifetimeMismatchLabels { - fn add_to_diagnostic(self, diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { match self { LifetimeMismatchLabels::InRet { param_span, ret_span, span, label_var1 } => { diag.span_label(param_span, fluent::infer::declared_different); @@ -340,7 +347,10 @@ pub struct AddLifetimeParamsSuggestion<'a> { } impl AddToDiagnostic for AddLifetimeParamsSuggestion<'_> { - fn add_to_diagnostic(self, diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { let mut mk_suggestion = || { let ( hir::Ty { kind: hir::TyKind::Rptr(lifetime_sub, _), .. }, @@ -439,7 +449,10 @@ pub struct IntroducesStaticBecauseUnmetLifetimeReq { } impl AddToDiagnostic for IntroducesStaticBecauseUnmetLifetimeReq { - fn add_to_diagnostic(mut self, diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with(mut self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { self.unmet_requirements .push_span_label(self.binding_span, fluent::infer::msl_introduces_static); diag.span_note(self.unmet_requirements, fluent::infer::msl_unmet_req); @@ -451,7 +464,10 @@ pub struct ImplNote { } impl AddToDiagnostic for ImplNote { - fn add_to_diagnostic(self, diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { match self.impl_span { Some(span) => diag.span_note(span, fluent::infer::msl_impl_note), None => diag.note(fluent::infer::msl_impl_note), @@ -466,7 +482,10 @@ pub enum TraitSubdiag { // FIXME(#100717) used in `Vec` so requires eager translation/list support impl AddToDiagnostic for TraitSubdiag { - fn add_to_diagnostic(self, diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { match self { TraitSubdiag::Note { span } => { diag.span_note(span, "this has an implicit `'static` lifetime requirement"); diff --git a/compiler/rustc_infer/src/errors/note_and_explain.rs b/compiler/rustc_infer/src/errors/note_and_explain.rs index 7f54918f73614..201a3c7100cc8 100644 --- a/compiler/rustc_infer/src/errors/note_and_explain.rs +++ b/compiler/rustc_infer/src/errors/note_and_explain.rs @@ -1,5 +1,7 @@ use crate::infer::error_reporting::nice_region_error::find_anon_type; -use rustc_errors::{self, fluent, AddToDiagnostic, IntoDiagnosticArg}; +use rustc_errors::{ + self, fluent, AddToDiagnostic, Diagnostic, IntoDiagnosticArg, SubdiagnosticMessage, +}; use rustc_middle::ty::{self, TyCtxt}; use rustc_span::{symbol::kw, Span}; @@ -159,7 +161,10 @@ impl RegionExplanation<'_> { } impl AddToDiagnostic for RegionExplanation<'_> { - fn add_to_diagnostic(self, diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { if let Some(span) = self.desc.span { diag.span_note(span, fluent::infer::region_explanation); } else { diff --git a/compiler/rustc_lint/src/errors.rs b/compiler/rustc_lint/src/errors.rs index 880f3fbd00e60..97d012fb611d0 100644 --- a/compiler/rustc_lint/src/errors.rs +++ b/compiler/rustc_lint/src/errors.rs @@ -1,4 +1,7 @@ -use rustc_errors::{fluent, AddToDiagnostic, ErrorGuaranteed, Handler, IntoDiagnostic}; +use rustc_errors::{ + fluent, AddToDiagnostic, Diagnostic, ErrorGuaranteed, Handler, IntoDiagnostic, + SubdiagnosticMessage, +}; use rustc_macros::{Diagnostic, Subdiagnostic}; use rustc_session::lint::Level; use rustc_span::{Span, Symbol}; @@ -23,7 +26,10 @@ pub enum OverruledAttributeSub { } impl AddToDiagnostic for OverruledAttributeSub { - fn add_to_diagnostic(self, diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { match self { OverruledAttributeSub::DefaultSource { id } => { diag.note(fluent::lint::default_source); @@ -88,7 +94,10 @@ pub struct RequestedLevel { } impl AddToDiagnostic for RequestedLevel { - fn add_to_diagnostic(self, diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { diag.note(fluent::lint::requested_level); diag.set_arg( "level", diff --git a/compiler/rustc_macros/src/diagnostics/mod.rs b/compiler/rustc_macros/src/diagnostics/mod.rs index 4166816b5e3c7..f98cc66e9e93e 100644 --- a/compiler/rustc_macros/src/diagnostics/mod.rs +++ b/compiler/rustc_macros/src/diagnostics/mod.rs @@ -9,7 +9,7 @@ use diagnostic::{DiagnosticDerive, LintDiagnosticDerive}; pub(crate) use fluent::fluent_messages; use proc_macro2::TokenStream; use quote::format_ident; -use subdiagnostic::SubdiagnosticDerive; +use subdiagnostic::SubdiagnosticDeriveBuilder; use synstructure::Structure; /// Implements `#[derive(Diagnostic)]`, which allows for errors to be specified as a struct, @@ -155,5 +155,5 @@ pub fn lint_diagnostic_derive(s: Structure<'_>) -> TokenStream { /// diag.subdiagnostic(RawIdentifierSuggestion { span, applicability, ident }); /// ``` pub fn session_subdiagnostic_derive(s: Structure<'_>) -> TokenStream { - SubdiagnosticDerive::new(s).into_tokens() + SubdiagnosticDeriveBuilder::new().into_tokens(s) } diff --git a/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs b/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs index 9a2588513f06d..ef17dbd0426fe 100644 --- a/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs +++ b/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs @@ -15,19 +15,19 @@ use syn::{spanned::Spanned, Attribute, Meta, MetaList, MetaNameValue, NestedMeta use synstructure::{BindingInfo, Structure, VariantInfo}; /// The central struct for constructing the `add_to_diagnostic` method from an annotated struct. -pub(crate) struct SubdiagnosticDerive<'a> { - structure: Structure<'a>, +pub(crate) struct SubdiagnosticDeriveBuilder { diag: syn::Ident, + f: syn::Ident, } -impl<'a> SubdiagnosticDerive<'a> { - pub(crate) fn new(structure: Structure<'a>) -> Self { +impl SubdiagnosticDeriveBuilder { + pub(crate) fn new() -> Self { let diag = format_ident!("diag"); - Self { structure, diag } + let f = format_ident!("f"); + Self { diag, f } } - pub(crate) fn into_tokens(self) -> TokenStream { - let SubdiagnosticDerive { mut structure, diag } = self; + pub(crate) fn into_tokens<'a>(self, mut structure: Structure<'a>) -> TokenStream { let implementation = { let ast = structure.ast(); let span = ast.span().unwrap(); @@ -53,8 +53,8 @@ impl<'a> SubdiagnosticDerive<'a> { structure.bind_with(|_| synstructure::BindStyle::Move); let variants_ = structure.each_variant(|variant| { - let mut builder = SubdiagnosticDeriveBuilder { - diag: &diag, + let mut builder = SubdiagnosticDeriveVariantBuilder { + parent: &self, variant, span, fields: build_field_mapping(variant), @@ -72,9 +72,17 @@ impl<'a> SubdiagnosticDerive<'a> { } }; + let diag = &self.diag; + let f = &self.f; let ret = structure.gen_impl(quote! { gen impl rustc_errors::AddToDiagnostic for @Self { - fn add_to_diagnostic(self, #diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with<__F>(self, #diag: &mut rustc_errors::Diagnostic, #f: __F) + where + __F: Fn( + &mut rustc_errors::Diagnostic, + rustc_errors::SubdiagnosticMessage + ) -> rustc_errors::SubdiagnosticMessage, + { use rustc_errors::{Applicability, IntoDiagnosticArg}; #implementation } @@ -88,9 +96,9 @@ impl<'a> SubdiagnosticDerive<'a> { /// for the final generated method. This is a separate struct to `SubdiagnosticDerive` /// only to be able to destructure and split `self.builder` and the `self.structure` up to avoid a /// double mut borrow later on. -struct SubdiagnosticDeriveBuilder<'a> { +struct SubdiagnosticDeriveVariantBuilder<'parent, 'a> { /// The identifier to use for the generated `DiagnosticBuilder` instance. - diag: &'a syn::Ident, + parent: &'parent SubdiagnosticDeriveBuilder, /// Info for the current variant (or the type if not an enum). variant: &'a VariantInfo<'a>, @@ -112,7 +120,7 @@ struct SubdiagnosticDeriveBuilder<'a> { has_suggestion_parts: bool, } -impl<'a> HasFieldMap for SubdiagnosticDeriveBuilder<'a> { +impl<'parent, 'a> HasFieldMap for SubdiagnosticDeriveVariantBuilder<'parent, 'a> { fn get_field_binding(&self, field: &String) -> Option<&TokenStream> { self.fields.get(field) } @@ -156,7 +164,7 @@ impl<'a> FromIterator<&'a SubdiagnosticKind> for KindsStatistics { } } -impl<'a> SubdiagnosticDeriveBuilder<'a> { +impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { fn identify_kind(&mut self) -> Result, DiagnosticDeriveError> { let mut kind_slugs = vec![]; @@ -187,7 +195,7 @@ impl<'a> SubdiagnosticDeriveBuilder<'a> { let ast = binding.ast(); assert_eq!(ast.attrs.len(), 0, "field with attribute used as diagnostic arg"); - let diag = &self.diag; + let diag = &self.parent.diag; let ident = ast.ident.as_ref().unwrap(); // strip `r#` prefix, if present let ident = format_ident!("{}", ident); @@ -442,11 +450,14 @@ impl<'a> SubdiagnosticDeriveBuilder<'a> { let span_field = self.span_field.value_ref(); - let diag = &self.diag; + let diag = &self.parent.diag; + let f = &self.parent.f; let mut calls = TokenStream::new(); for (kind, slug) in kind_slugs { + let message = format_ident!("__message"); + calls.extend(quote! { let #message = #f(#diag, rustc_errors::fluent::#slug.into()); }); + let name = format_ident!("{}{}", if span_field.is_some() { "span_" } else { "" }, kind); - let message = quote! { rustc_errors::fluent::#slug }; let call = match kind { SubdiagnosticKind::Suggestion { suggestion_kind, applicability, code } => { let applicability = applicability diff --git a/compiler/rustc_query_system/src/error.rs b/compiler/rustc_query_system/src/error.rs index 8602a4cf5aef2..debdf9dbf4403 100644 --- a/compiler/rustc_query_system/src/error.rs +++ b/compiler/rustc_query_system/src/error.rs @@ -1,4 +1,4 @@ -use rustc_errors::AddToDiagnostic; +use rustc_errors::{AddToDiagnostic, Diagnostic, SubdiagnosticMessage}; use rustc_macros::{Diagnostic, Subdiagnostic}; use rustc_session::Limit; use rustc_span::{Span, Symbol}; @@ -9,7 +9,10 @@ pub struct CycleStack { } impl AddToDiagnostic for CycleStack { - fn add_to_diagnostic(self, diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { diag.span_note(self.span, &format!("...which requires {}...", self.desc)); } } diff --git a/src/test/ui-fulldeps/internal-lints/diagnostics.rs b/src/test/ui-fulldeps/internal-lints/diagnostics.rs index 9a5100ce17f53..462f5e7849849 100644 --- a/src/test/ui-fulldeps/internal-lints/diagnostics.rs +++ b/src/test/ui-fulldeps/internal-lints/diagnostics.rs @@ -13,7 +13,7 @@ extern crate rustc_span; use rustc_errors::{ AddToDiagnostic, IntoDiagnostic, Diagnostic, DiagnosticBuilder, - ErrorGuaranteed, Handler, fluent + ErrorGuaranteed, Handler, fluent, SubdiagnosticMessage, }; use rustc_macros::{Diagnostic, Subdiagnostic}; use rustc_span::Span; @@ -52,7 +52,10 @@ impl<'a> IntoDiagnostic<'a, ErrorGuaranteed> for TranslatableInIntoDiagnostic { pub struct UntranslatableInAddToDiagnostic; impl AddToDiagnostic for UntranslatableInAddToDiagnostic { - fn add_to_diagnostic(self, diag: &mut Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { diag.note("untranslatable diagnostic"); //~^ ERROR diagnostics should be created using translatable messages } @@ -61,7 +64,10 @@ impl AddToDiagnostic for UntranslatableInAddToDiagnostic { pub struct TranslatableInAddToDiagnostic; impl AddToDiagnostic for TranslatableInAddToDiagnostic { - fn add_to_diagnostic(self, diag: &mut Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { diag.note(fluent::compiletest::note); } } diff --git a/src/test/ui-fulldeps/internal-lints/diagnostics.stderr b/src/test/ui-fulldeps/internal-lints/diagnostics.stderr index f5f92ac1e7fbe..ac820a79db274 100644 --- a/src/test/ui-fulldeps/internal-lints/diagnostics.stderr +++ b/src/test/ui-fulldeps/internal-lints/diagnostics.stderr @@ -11,13 +11,13 @@ LL | #![deny(rustc::untranslatable_diagnostic)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: diagnostics should be created using translatable messages - --> $DIR/diagnostics.rs:56:14 + --> $DIR/diagnostics.rs:59:14 | LL | diag.note("untranslatable diagnostic"); | ^^^^ error: diagnostics should only be created in `IntoDiagnostic`/`AddToDiagnostic` impls - --> $DIR/diagnostics.rs:70:25 + --> $DIR/diagnostics.rs:76:25 | LL | let _diag = handler.struct_err(fluent::compiletest::example); | ^^^^^^^^^^ @@ -29,13 +29,13 @@ LL | #![deny(rustc::diagnostic_outside_of_impl)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: diagnostics should only be created in `IntoDiagnostic`/`AddToDiagnostic` impls - --> $DIR/diagnostics.rs:73:25 + --> $DIR/diagnostics.rs:79:25 | LL | let _diag = handler.struct_err("untranslatable diagnostic"); | ^^^^^^^^^^ error: diagnostics should be created using translatable messages - --> $DIR/diagnostics.rs:73:25 + --> $DIR/diagnostics.rs:79:25 | LL | let _diag = handler.struct_err("untranslatable diagnostic"); | ^^^^^^^^^^ From 24f60a7cdfd10c8ea2a86db7a34e5237a3f2c16d Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 3 Oct 2022 14:14:51 +0100 Subject: [PATCH 04/13] errors: `DiagnosticMessage::Eager` Add variant of `DiagnosticMessage` for eagerly translated messages (messages in the target language which don't need translated by the emitter during emission). Also adds `eager_subdiagnostic` function which is intended to be invoked by the diagnostic derive for subdiagnostic fields which are marked as needing eager translation. Signed-off-by: David Wood --- compiler/rustc_error_messages/src/lib.rs | 29 +++++++++++++++++++++++- compiler/rustc_errors/src/diagnostic.rs | 19 +++++++++++++++- compiler/rustc_errors/src/lib.rs | 11 +++++++++ compiler/rustc_errors/src/translation.rs | 4 +++- 4 files changed, 60 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_error_messages/src/lib.rs b/compiler/rustc_error_messages/src/lib.rs index 18be60975e4eb..560d6c7c69506 100644 --- a/compiler/rustc_error_messages/src/lib.rs +++ b/compiler/rustc_error_messages/src/lib.rs @@ -276,6 +276,18 @@ pub enum SubdiagnosticMessage { /// Non-translatable diagnostic message. // FIXME(davidtwco): can a `Cow<'static, str>` be used here? Str(String), + /// Translatable message which has already been translated eagerly. + /// + /// Some diagnostics have repeated subdiagnostics where the same interpolated variables would + /// be instantiated multiple times with different values. As translation normally happens + /// immediately prior to emission, after the diagnostic and subdiagnostic derive logic has run, + /// the setting of diagnostic arguments in the derived code will overwrite previous variable + /// values and only the final value will be set when translation occurs - resulting in + /// incorrect diagnostics. Eager translation results in translation for a subdiagnostic + /// happening immediately after the subdiagnostic derive's logic has been run. This variant + /// stores messages which have been translated eagerly. + // FIXME(#100717): can a `Cow<'static, str>` be used here? + Eager(String), /// Identifier of a Fluent message. Instances of this variant are generated by the /// `Subdiagnostic` derive. FluentIdentifier(FluentId), @@ -303,8 +315,20 @@ impl> From for SubdiagnosticMessage { #[rustc_diagnostic_item = "DiagnosticMessage"] pub enum DiagnosticMessage { /// Non-translatable diagnostic message. - // FIXME(davidtwco): can a `Cow<'static, str>` be used here? + // FIXME(#100717): can a `Cow<'static, str>` be used here? Str(String), + /// Translatable message which has already been translated eagerly. + /// + /// Some diagnostics have repeated subdiagnostics where the same interpolated variables would + /// be instantiated multiple times with different values. As translation normally happens + /// immediately prior to emission, after the diagnostic and subdiagnostic derive logic has run, + /// the setting of diagnostic arguments in the derived code will overwrite previous variable + /// values and only the final value will be set when translation occurs - resulting in + /// incorrect diagnostics. Eager translation results in translation for a subdiagnostic + /// happening immediately after the subdiagnostic derive's logic has been run. This variant + /// stores messages which have been translated eagerly. + // FIXME(#100717): can a `Cow<'static, str>` be used here? + Eager(String), /// Identifier for a Fluent message (with optional attribute) corresponding to the diagnostic /// message. /// @@ -323,6 +347,7 @@ impl DiagnosticMessage { pub fn with_subdiagnostic_message(&self, sub: SubdiagnosticMessage) -> Self { let attr = match sub { SubdiagnosticMessage::Str(s) => return DiagnosticMessage::Str(s), + SubdiagnosticMessage::Eager(s) => return DiagnosticMessage::Eager(s), SubdiagnosticMessage::FluentIdentifier(id) => { return DiagnosticMessage::FluentIdentifier(id, None); } @@ -331,6 +356,7 @@ impl DiagnosticMessage { match self { DiagnosticMessage::Str(s) => DiagnosticMessage::Str(s.clone()), + DiagnosticMessage::Eager(s) => DiagnosticMessage::Eager(s.clone()), DiagnosticMessage::FluentIdentifier(id, _) => { DiagnosticMessage::FluentIdentifier(id.clone(), Some(attr)) } @@ -379,6 +405,7 @@ impl Into for DiagnosticMessage { fn into(self) -> SubdiagnosticMessage { match self { DiagnosticMessage::Str(s) => SubdiagnosticMessage::Str(s), + DiagnosticMessage::Eager(s) => SubdiagnosticMessage::Eager(s), DiagnosticMessage::FluentIdentifier(id, None) => { SubdiagnosticMessage::FluentIdentifier(id) } diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 2a6dd6da415cf..3e0840caaa693 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -939,6 +939,23 @@ impl Diagnostic { self } + /// Add a subdiagnostic from a type that implements `Subdiagnostic` (see + /// [rustc_macros::Subdiagnostic]). Performs eager translation of any translatable messages + /// used in the subdiagnostic, so suitable for use with repeated messages (i.e. re-use of + /// interpolated variables). + pub fn eager_subdiagnostic( + &mut self, + handler: &crate::Handler, + subdiagnostic: impl AddToDiagnostic, + ) -> &mut Self { + subdiagnostic.add_to_diagnostic_with(self, |diag, msg| { + let args = diag.args(); + let msg = diag.subdiagnostic_message_to_diagnostic_message(msg); + handler.eagerly_translate(msg, args) + }); + self + } + pub fn set_span>(&mut self, sp: S) -> &mut Self { self.span = sp.into(); if let Some(span) = self.span.primary_span() { @@ -994,7 +1011,7 @@ impl Diagnostic { /// Helper function that takes a `SubdiagnosticMessage` and returns a `DiagnosticMessage` by /// combining it with the primary message of the diagnostic (if translatable, otherwise it just /// passes the user's string along). - fn subdiagnostic_message_to_diagnostic_message( + pub(crate) fn subdiagnostic_message_to_diagnostic_message( &self, attr: impl Into, ) -> DiagnosticMessage { diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 7c312da62793f..690de767195ad 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -597,6 +597,17 @@ impl Handler { } } + /// Translate `message` eagerly with `args`. + pub fn eagerly_translate<'a>( + &self, + message: DiagnosticMessage, + args: impl Iterator>, + ) -> SubdiagnosticMessage { + let inner = self.inner.borrow(); + let args = crate::translation::to_fluent_args(args); + SubdiagnosticMessage::Eager(inner.emitter.translate_message(&message, &args).to_string()) + } + // This is here to not allow mutation of flags; // as of this writing it's only used in tests in librustc_middle. pub fn can_emit_warnings(&self) -> bool { diff --git a/compiler/rustc_errors/src/translation.rs b/compiler/rustc_errors/src/translation.rs index c2ec2526a4aea..a7737b467b75b 100644 --- a/compiler/rustc_errors/src/translation.rs +++ b/compiler/rustc_errors/src/translation.rs @@ -55,7 +55,9 @@ pub trait Translate { ) -> Cow<'_, str> { trace!(?message, ?args); let (identifier, attr) = match message { - DiagnosticMessage::Str(msg) => return Cow::Borrowed(&msg), + DiagnosticMessage::Str(msg) | DiagnosticMessage::Eager(msg) => { + return Cow::Borrowed(&msg); + } DiagnosticMessage::FluentIdentifier(identifier, attr) => (identifier, attr), }; From 25cfe1db0d2be165217a7c89c0b75ebd4f771f50 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 3 Oct 2022 14:24:17 +0100 Subject: [PATCH 05/13] macros: `#[subdiagnostic(eager)]` Add support for `eager` argument to the `subdiagnostic` attribute which generates a call to `eager_subdiagnostic`. Signed-off-by: David Wood --- .../src/diagnostics/diagnostic.rs | 13 ++-- .../src/diagnostics/diagnostic_builder.rs | 71 ++++++++++++++----- .../session-diagnostic/diagnostic-derive.rs | 47 ++++++++++++ .../diagnostic-derive.stderr | 42 ++++++++++- 4 files changed, 151 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_macros/src/diagnostics/diagnostic.rs b/compiler/rustc_macros/src/diagnostics/diagnostic.rs index 406a56cd4ae3e..738aa802c9832 100644 --- a/compiler/rustc_macros/src/diagnostics/diagnostic.rs +++ b/compiler/rustc_macros/src/diagnostics/diagnostic.rs @@ -10,27 +10,31 @@ use synstructure::Structure; /// The central struct for constructing the `into_diagnostic` method from an annotated struct. pub(crate) struct DiagnosticDerive<'a> { structure: Structure<'a>, - handler: syn::Ident, builder: DiagnosticDeriveBuilder, } impl<'a> DiagnosticDerive<'a> { pub(crate) fn new(diag: syn::Ident, handler: syn::Ident, structure: Structure<'a>) -> Self { Self { - builder: DiagnosticDeriveBuilder { diag, kind: DiagnosticDeriveKind::Diagnostic }, - handler, + builder: DiagnosticDeriveBuilder { + diag, + kind: DiagnosticDeriveKind::Diagnostic { handler }, + }, structure, } } pub(crate) fn into_tokens(self) -> TokenStream { - let DiagnosticDerive { mut structure, handler, mut builder } = self; + let DiagnosticDerive { mut structure, mut builder } = self; let implementation = builder.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 { handler } = &builder.parent.kind else { + unreachable!() + }; let init = match builder.slug.value_ref() { None => { span_err(builder.span, "diagnostic slug not specified") @@ -56,6 +60,7 @@ impl<'a> DiagnosticDerive<'a> { } }); + let DiagnosticDeriveKind::Diagnostic { handler } = &builder.kind else { unreachable!() }; structure.gen_impl(quote! { gen impl<'__diagnostic_handler_sess, G> rustc_errors::IntoDiagnostic<'__diagnostic_handler_sess, G> diff --git a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs index 9e88dc7a913a2..df4e309086fd2 100644 --- a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs +++ b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs @@ -17,9 +17,9 @@ use syn::{ use synstructure::{BindingInfo, Structure, VariantInfo}; /// What kind of diagnostic is being derived - a fatal/error/warning or a lint? -#[derive(Copy, Clone, PartialEq, Eq)] +#[derive(Clone, PartialEq, Eq)] pub(crate) enum DiagnosticDeriveKind { - Diagnostic, + Diagnostic { handler: syn::Ident }, LintDiagnostic, } @@ -340,18 +340,15 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { let diag = &self.parent.diag; let meta = attr.parse_meta()?; - if let Meta::Path(_) = meta { - let ident = &attr.path.segments.last().unwrap().ident; - let name = ident.to_string(); - let name = name.as_str(); - match name { - "skip_arg" => { - // Don't need to do anything - by virtue of the attribute existing, the - // `set_arg` call will not be generated. - return Ok(quote! {}); - } - "primary_span" => match self.parent.kind { - DiagnosticDeriveKind::Diagnostic => { + let ident = &attr.path.segments.last().unwrap().ident; + let name = ident.to_string(); + match (&meta, name.as_str()) { + // Don't need to do anything - by virtue of the attribute existing, the + // `set_arg` call will not be generated. + (Meta::Path(_), "skip_arg") => return Ok(quote! {}), + (Meta::Path(_), "primary_span") => { + match self.parent.kind { + DiagnosticDeriveKind::Diagnostic { .. } => { report_error_if_not_applied_to_span(attr, &info)?; return Ok(quote! { @@ -363,10 +360,50 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { diag.help("the `primary_span` field attribute is not valid for lint diagnostics") }) } - }, - "subdiagnostic" => return Ok(quote! { #diag.subdiagnostic(#binding); }), - _ => {} + } + } + (Meta::Path(_), "subdiagnostic") => { + return Ok(quote! { #diag.subdiagnostic(#binding); }); + } + (Meta::NameValue(_), "subdiagnostic") => { + throw_invalid_attr!(attr, &meta, |diag| { + diag.help("`eager` is the only supported nested attribute for `subdiagnostic`") + }) + } + (Meta::List(MetaList { ref nested, .. }), "subdiagnostic") => { + if nested.len() != 1 { + throw_invalid_attr!(attr, &meta, |diag| { + diag.help( + "`eager` is the only supported nested attribute for `subdiagnostic`", + ) + }) + } + + let handler = match &self.parent.kind { + DiagnosticDeriveKind::Diagnostic { handler } => handler, + DiagnosticDeriveKind::LintDiagnostic => { + throw_invalid_attr!(attr, &meta, |diag| { + diag.help("eager subdiagnostics are not supported on lints") + }) + } + }; + + let nested_attr = nested.first().expect("pop failed for single element list"); + match nested_attr { + NestedMeta::Meta(meta @ Meta::Path(_)) + if meta.path().segments.last().unwrap().ident.to_string().as_str() + == "eager" => + { + return Ok(quote! { #diag.eager_subdiagnostic(#handler, #binding); }); + } + _ => { + throw_invalid_nested_attr!(attr, nested_attr, |diag| { + diag.help("`eager` is the only supported nested attribute for `subdiagnostic`") + }) + } + } } + _ => (), } let (subdiag, slug) = self.parse_subdiag_attribute(attr)?; diff --git a/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs b/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs index 1dc71abc104f9..460af07a55967 100644 --- a/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs +++ b/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs @@ -678,3 +678,50 @@ enum ExampleEnum { struct RawIdentDiagnosticArg { pub r#type: String, } + +#[derive(Diagnostic)] +#[diag(compiletest::example)] +struct SubdiagnosticBad { + #[subdiagnostic(bad)] +//~^ ERROR `#[subdiagnostic(bad)]` is not a valid attribute + note: Note, +} + +#[derive(Diagnostic)] +#[diag(compiletest::example)] +struct SubdiagnosticBadStr { + #[subdiagnostic = "bad"] +//~^ ERROR `#[subdiagnostic = ...]` is not a valid attribute + note: Note, +} + +#[derive(Diagnostic)] +#[diag(compiletest::example)] +struct SubdiagnosticBadTwice { + #[subdiagnostic(bad, bad)] +//~^ ERROR `#[subdiagnostic(...)]` is not a valid attribute + note: Note, +} + +#[derive(Diagnostic)] +#[diag(compiletest::example)] +struct SubdiagnosticBadLitStr { + #[subdiagnostic("bad")] +//~^ ERROR `#[subdiagnostic("...")]` is not a valid attribute + note: Note, +} + +#[derive(LintDiagnostic)] +#[diag(compiletest::example)] +struct SubdiagnosticEagerLint { + #[subdiagnostic(eager)] +//~^ ERROR `#[subdiagnostic(...)]` is not a valid attribute + note: Note, +} + +#[derive(Diagnostic)] +#[diag(compiletest::example)] +struct SubdiagnosticEagerCorrect { + #[subdiagnostic(eager)] + note: Note, +} diff --git a/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.stderr b/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.stderr index 167198b47f20f..7a42d618707ad 100644 --- a/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.stderr +++ b/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.stderr @@ -533,6 +533,46 @@ LL | #[label] | = help: `#[label]` and `#[suggestion]` can only be applied to fields +error: `#[subdiagnostic(bad)]` is not a valid attribute + --> $DIR/diagnostic-derive.rs:685:21 + | +LL | #[subdiagnostic(bad)] + | ^^^ + | + = help: `eager` is the only supported nested attribute for `subdiagnostic` + +error: `#[subdiagnostic = ...]` is not a valid attribute + --> $DIR/diagnostic-derive.rs:693:5 + | +LL | #[subdiagnostic = "bad"] + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: `eager` is the only supported nested attribute for `subdiagnostic` + +error: `#[subdiagnostic(...)]` is not a valid attribute + --> $DIR/diagnostic-derive.rs:701:5 + | +LL | #[subdiagnostic(bad, bad)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: `eager` is the only supported nested attribute for `subdiagnostic` + +error: `#[subdiagnostic("...")]` is not a valid attribute + --> $DIR/diagnostic-derive.rs:709:21 + | +LL | #[subdiagnostic("bad")] + | ^^^^^ + | + = help: `eager` is the only supported nested attribute for `subdiagnostic` + +error: `#[subdiagnostic(...)]` is not a valid attribute + --> $DIR/diagnostic-derive.rs:717:5 + | +LL | #[subdiagnostic(eager)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: eager subdiagnostics are not supported on lints + error: cannot find attribute `nonsense` in this scope --> $DIR/diagnostic-derive.rs:55:3 | @@ -607,7 +647,7 @@ LL | arg: impl IntoDiagnosticArg, | ^^^^^^^^^^^^^^^^^ required by this bound in `DiagnosticBuilder::<'a, G>::set_arg` = note: this error originates in the derive macro `Diagnostic` (in Nightly builds, run with -Z macro-backtrace for more info) -error: aborting due to 75 previous errors +error: aborting due to 80 previous errors Some errors have detailed explanations: E0277, E0425. For more information about an error, try `rustc --explain E0277`. From ae6c88a026940fc9b2810a57d238559303831f60 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 3 Oct 2022 14:26:29 +0100 Subject: [PATCH 06/13] query_system: finish migration Using eager translation, migrate the remaining repeated cycle stack diagnostic. Signed-off-by: David Wood --- .../locales/en-US/query_system.ftl | 2 ++ compiler/rustc_query_system/src/error.rs | 15 ++++----------- compiler/rustc_query_system/src/lib.rs | 2 +- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_error_messages/locales/en-US/query_system.ftl b/compiler/rustc_error_messages/locales/en-US/query_system.ftl index b914ba52a7353..870e824039cb6 100644 --- a/compiler/rustc_error_messages/locales/en-US/query_system.ftl +++ b/compiler/rustc_error_messages/locales/en-US/query_system.ftl @@ -12,6 +12,8 @@ query_system_cycle_usage = cycle used when {$usage} query_system_cycle_stack_single = ...which immediately requires {$stack_bottom} again +query_system_cycle_stack_middle = ...which requires {$desc}... + query_system_cycle_stack_multiple = ...which again requires {$stack_bottom}, completing the cycle query_system_cycle_recursive_ty_alias = type aliases cannot be recursive diff --git a/compiler/rustc_query_system/src/error.rs b/compiler/rustc_query_system/src/error.rs index debdf9dbf4403..1e74e0e299099 100644 --- a/compiler/rustc_query_system/src/error.rs +++ b/compiler/rustc_query_system/src/error.rs @@ -1,22 +1,15 @@ -use rustc_errors::{AddToDiagnostic, Diagnostic, SubdiagnosticMessage}; use rustc_macros::{Diagnostic, Subdiagnostic}; use rustc_session::Limit; use rustc_span::{Span, Symbol}; +#[derive(Subdiagnostic)] +#[note(query_system::cycle_stack_middle)] pub struct CycleStack { + #[primary_span] pub span: Span, pub desc: String, } -impl AddToDiagnostic for CycleStack { - fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) - where - F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, - { - diag.span_note(self.span, &format!("...which requires {}...", self.desc)); - } -} - #[derive(Copy, Clone)] pub enum HandleCycleError { Error, @@ -56,7 +49,7 @@ pub struct Cycle { #[primary_span] pub span: Span, pub stack_bottom: String, - #[subdiagnostic] + #[subdiagnostic(eager)] pub cycle_stack: Vec, #[subdiagnostic] pub stack_count: StackCount, diff --git a/compiler/rustc_query_system/src/lib.rs b/compiler/rustc_query_system/src/lib.rs index 8f6da73d1f2d2..f47760e9ae6c8 100644 --- a/compiler/rustc_query_system/src/lib.rs +++ b/compiler/rustc_query_system/src/lib.rs @@ -4,7 +4,7 @@ #![feature(min_specialization)] #![feature(extern_types)] #![allow(rustc::potential_query_instability)] -// #![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::untranslatable_diagnostic)] #![deny(rustc::diagnostic_outside_of_impl)] #[macro_use] From f9c6a5d1368b3711a9913a7adb3005bc34f14fd2 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 3 Oct 2022 14:28:02 +0100 Subject: [PATCH 07/13] macros: separate suggestion fmt'ing and emission Diagnostic derives have previously had to take special care when ordering the generated code so that fields were not used after a move. This is unlikely for most fields because a field is either annotated with a subdiagnostic attribute and is thus likely a `Span` and copiable, or is a argument, in which case it is only used once by `set_arg` anyway. However, format strings for code in suggestions can result in fields being used after being moved if not ordered carefully. As a result, the derive currently puts `set_arg` calls last (just before emission), such as: ```rust let diag = { /* create diagnostic */ }; diag.span_suggestion_with_style( span, fluent::crate::slug, format!("{}", __binding_0), Applicability::Unknown, SuggestionStyle::ShowAlways ); /* + other subdiagnostic additions */ diag.set_arg("foo", __binding_0); /* + other `set_arg` calls */ diag.emit(); ``` For eager translation, this doesn't work, as the message being translated eagerly can assume that all arguments are available - so arguments _must_ be set first. Format strings for suggestion code are now separated into two parts - an initialization line that performs the formatting into a variable, and a usage in the subdiagnostic addition. By separating these parts, the initialization can happen before arguments are set, preserving the desired order so that code compiles, while still enabling arguments to be set before subdiagnostics are added. ```rust let diag = { /* create diagnostic */ }; let __code_0 = format!("{}", __binding_0); /* + other formatting */ diag.set_arg("foo", __binding_0); /* + other `set_arg` calls */ diag.span_suggestion_with_style( span, fluent::crate::slug, __code_0, Applicability::Unknown, SuggestionStyle::ShowAlways ); /* + other subdiagnostic additions */ diag.emit(); ``` Signed-off-by: David Wood --- .../src/diagnostics/diagnostic_builder.rs | 6 ++- .../src/diagnostics/subdiagnostic.rs | 50 ++++++++++++++----- .../rustc_macros/src/diagnostics/utils.rs | 48 ++++++++++++++---- .../session-diagnostic/diagnostic-derive.rs | 24 +++++++++ 4 files changed, 105 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs index df4e309086fd2..017bf2365d62b 100644 --- a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs +++ b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs @@ -426,7 +426,8 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { SubdiagnosticKind::Suggestion { suggestion_kind, applicability: static_applicability, - code, + code_field, + code_init, } => { let (span_field, mut applicability) = self.span_and_applicability_of_ty(info)?; @@ -440,10 +441,11 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { let style = suggestion_kind.to_suggestion_style(); Ok(quote! { + #code_init #diag.span_suggestion_with_style( #span_field, rustc_errors::fluent::#slug, - #code, + #code_field, #applicability, #style ); diff --git a/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs b/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs index ef17dbd0426fe..3d4c3ab9fd7c9 100644 --- a/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs +++ b/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs @@ -5,7 +5,7 @@ use crate::diagnostics::error::{ DiagnosticDeriveError, }; use crate::diagnostics::utils::{ - build_field_mapping, report_error_if_not_applied_to_applicability, + build_field_mapping, new_code_ident, report_error_if_not_applied_to_applicability, report_error_if_not_applied_to_span, FieldInfo, FieldInnerTy, FieldMap, HasFieldMap, SetOnce, SpannedOption, SubdiagnosticKind, }; @@ -57,6 +57,7 @@ impl SubdiagnosticDeriveBuilder { parent: &self, variant, span, + formatting_init: TokenStream::new(), fields: build_field_mapping(variant), span_field: None, applicability: None, @@ -105,6 +106,9 @@ struct SubdiagnosticDeriveVariantBuilder<'parent, 'a> { /// Span for the entire type. span: proc_macro::Span, + /// Initialization of format strings for code suggestions. + formatting_init: TokenStream, + /// Store a map of field name to its corresponding field. This is built on construction of the /// derive builder. fields: FieldMap, @@ -230,7 +234,7 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { }; let generated = self - .generate_field_code_inner(kind_stats, attr, info) + .generate_field_code_inner(kind_stats, attr, info, inner_ty.will_iterate()) .unwrap_or_else(|v| v.to_compile_error()); inner_ty.with(binding, generated) @@ -243,13 +247,18 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { kind_stats: KindsStatistics, attr: &Attribute, info: FieldInfo<'_>, + clone_suggestion_code: bool, ) -> Result { let meta = attr.parse_meta()?; match meta { Meta::Path(path) => self.generate_field_code_inner_path(kind_stats, attr, info, path), - Meta::List(list @ MetaList { .. }) => { - self.generate_field_code_inner_list(kind_stats, attr, info, list) - } + Meta::List(list @ MetaList { .. }) => self.generate_field_code_inner_list( + kind_stats, + attr, + info, + list, + clone_suggestion_code, + ), _ => throw_invalid_attr!(attr, &meta), } } @@ -353,6 +362,7 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { attr: &Attribute, info: FieldInfo<'_>, list: MetaList, + clone_suggestion_code: bool, ) -> Result { let span = attr.span().unwrap(); let ident = &list.path.segments.last().unwrap().ident; @@ -390,7 +400,8 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { match nested_name { "code" => { let formatted_str = self.build_format(&value.value(), value.span()); - code.set_once(formatted_str, span); + let code_field = new_code_ident(); + code.set_once((code_field, formatted_str), span); } _ => throw_invalid_nested_attr!(attr, &nested_attr, |diag| { diag.help("`code` is the only valid nested attribute") @@ -398,14 +409,20 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { } } - let Some((code, _)) = code else { + let Some((code_field, formatted_str)) = code.value() else { span_err(span, "`#[suggestion_part(...)]` attribute without `code = \"...\"`") .emit(); return Ok(quote! {}); }; let binding = info.binding; - Ok(quote! { suggestions.push((#binding, #code)); }) + self.formatting_init.extend(quote! { let #code_field = #formatted_str; }); + let code_field = if clone_suggestion_code { + quote! { #code_field.clone() } + } else { + quote! { #code_field } + }; + Ok(quote! { suggestions.push((#binding, #code_field)); }) } _ => throw_invalid_attr!(attr, &Meta::List(list), |diag| { let mut span_attrs = vec![]; @@ -459,7 +476,14 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { let name = format_ident!("{}{}", if span_field.is_some() { "span_" } else { "" }, kind); let call = match kind { - SubdiagnosticKind::Suggestion { suggestion_kind, applicability, code } => { + SubdiagnosticKind::Suggestion { + suggestion_kind, + applicability, + code_init, + code_field, + } => { + self.formatting_init.extend(code_init); + let applicability = applicability .value() .map(|a| quote! { #a }) @@ -468,8 +492,7 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { if let Some(span) = span_field { let style = suggestion_kind.to_suggestion_style(); - - quote! { #diag.#name(#span, #message, #code, #applicability, #style); } + quote! { #diag.#name(#span, #message, #code_field, #applicability, #style); } } else { span_err(self.span, "suggestion without `#[primary_span]` field").emit(); quote! { unreachable!(); } @@ -510,6 +533,7 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { } } }; + calls.extend(call); } @@ -521,11 +545,13 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { .map(|binding| self.generate_field_set_arg(binding)) .collect(); + let formatting_init = &self.formatting_init; Ok(quote! { #init + #formatting_init #attr_args - #calls #plain_args + #calls }) } } diff --git a/compiler/rustc_macros/src/diagnostics/utils.rs b/compiler/rustc_macros/src/diagnostics/utils.rs index 162699c286872..5e1cd842b9bbb 100644 --- a/compiler/rustc_macros/src/diagnostics/utils.rs +++ b/compiler/rustc_macros/src/diagnostics/utils.rs @@ -4,6 +4,7 @@ use crate::diagnostics::error::{ 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; @@ -14,6 +15,19 @@ use synstructure::{BindStyle, BindingInfo, VariantInfo}; use super::error::invalid_nested_attr; +thread_local! { + pub static CODE_IDENT_COUNT: RefCell = RefCell::new(0); +} + +/// Returns an ident of the form `__code_N` where `N` is incremented once with every call. +pub(crate) fn new_code_ident() -> syn::Ident { + CODE_IDENT_COUNT.with(|count| { + let ident = format_ident!("__code_{}", *count.borrow()); + *count.borrow_mut() += 1; + ident + }) +} + /// Checks whether the type name of `ty` matches `name`. /// /// Given some struct at `a::b::c::Foo`, this will return true for `c::Foo`, `b::c::Foo`, or @@ -142,6 +156,15 @@ impl<'ty> FieldInnerTy<'ty> { unreachable!(); } + /// Returns `true` if `FieldInnerTy::with` will result in iteration for this inner type (i.e. + /// that cloning might be required for values moved in the loop body). + pub(crate) fn will_iterate(&self) -> bool { + match self { + FieldInnerTy::Vec(..) => true, + FieldInnerTy::Option(..) | FieldInnerTy::None => false, + } + } + /// Returns `Option` containing inner type if there is one. pub(crate) fn inner_type(&self) -> Option<&'ty Type> { match self { @@ -434,7 +457,12 @@ pub(super) enum SubdiagnosticKind { Suggestion { suggestion_kind: SuggestionKind, applicability: SpannedOption, - code: TokenStream, + /// Identifier for variable used for formatted code, e.g. `___code_0`. Enables separation + /// of formatting and diagnostic emission so that `set_arg` calls can happen in-between.. + code_field: syn::Ident, + /// Initialization logic for `code_field`'s variable, e.g. + /// `let __formatted_code = /* whatever */;` + code_init: TokenStream, }, /// `#[multipart_suggestion{,_short,_hidden,_verbose}]` MultipartSuggestion { @@ -469,7 +497,8 @@ impl SubdiagnosticKind { SubdiagnosticKind::Suggestion { suggestion_kind, applicability: None, - code: TokenStream::new(), + code_field: new_code_ident(), + code_init: TokenStream::new(), } } else if let Some(suggestion_kind) = name.strip_prefix("multipart_suggestion").and_then(|s| s.parse().ok()) @@ -548,9 +577,10 @@ impl SubdiagnosticKind { }; match (nested_name, &mut kind) { - ("code", SubdiagnosticKind::Suggestion { .. }) => { + ("code", SubdiagnosticKind::Suggestion { code_field, .. }) => { let formatted_str = fields.build_format(&value.value(), value.span()); - code.set_once(formatted_str, span); + let code_init = quote! { let #code_field = #formatted_str; }; + code.set_once(code_init, span); } ( "applicability", @@ -582,13 +612,13 @@ impl SubdiagnosticKind { } match kind { - SubdiagnosticKind::Suggestion { code: ref mut code_field, .. } => { - *code_field = if let Some((code, _)) = code { - code + SubdiagnosticKind::Suggestion { ref code_field, ref mut code_init, .. } => { + *code_init = if let Some(init) = code.value() { + init } else { span_err(span, "suggestion without `code = \"...\"`").emit(); - quote! { "" } - } + quote! { let #code_field: String = unreachable!(); } + }; } SubdiagnosticKind::Label | SubdiagnosticKind::Note diff --git a/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs b/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs index 460af07a55967..e873c36e0b39a 100644 --- a/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs +++ b/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs @@ -725,3 +725,27 @@ struct SubdiagnosticEagerCorrect { #[subdiagnostic(eager)] note: Note, } + +// Check that formatting of `correct` in suggestion doesn't move the binding for that field, making +// the `set_arg` call a compile error; and that isn't worked around by moving the `set_arg` call +// after the `span_suggestion` call - which breaks eager translation. + +#[derive(Subdiagnostic)] +#[suggestion_short( + parser::use_instead, + applicability = "machine-applicable", + code = "{correct}" +)] +pub(crate) struct SubdiagnosticWithSuggestion { + #[primary_span] + span: Span, + invalid: String, + correct: String, +} + +#[derive(Diagnostic)] +#[diag(compiletest::example)] +struct SubdiagnosticEagerSuggestion { + #[subdiagnostic(eager)] + sub: SubdiagnosticWithSuggestion, +} From 4d076b8929c34983b4d51acd4f3af18cb3277369 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 3 Oct 2022 16:10:34 +0100 Subject: [PATCH 08/13] macros: simplify field ordering in diag derive Following the approach taken in earlier commits to separate formatting initialization from use in the subdiagnostic derive, simplify the diagnostic derive by removing the field-ordering logic that previously solved this problem. Signed-off-by: David Wood --- .../src/diagnostics/diagnostic.rs | 5 +- .../src/diagnostics/diagnostic_builder.rs | 75 ++++++++----------- .../rustc_macros/src/diagnostics/utils.rs | 60 +-------------- 3 files changed, 38 insertions(+), 102 deletions(-) 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 }) -} From 0571b0af65dcc0dd97cce4b976887ab436e5780d Mon Sep 17 00:00:00 2001 From: SparrowLii Date: Mon, 10 Oct 2022 11:14:32 +0800 Subject: [PATCH 09/13] suggest candidates for unresolved import --- compiler/rustc_resolve/src/diagnostics.rs | 38 ++++++++++++++++++- compiler/rustc_resolve/src/imports.rs | 28 +++++++++++++- src/test/ui/extenv/issue-55897.stderr | 5 +++ src/test/ui/imports/issue-56125.stderr | 12 ++++++ src/test/ui/imports/issue-57015.stderr | 5 +++ .../not-allowed.stderr | 7 ++++ .../portable-intrinsics-arent-exposed.stderr | 5 +++ .../inaccessible-test-modules.stderr | 14 +++++-- .../ui/unresolved/unresolved-candidates.rs | 13 +++++++ .../unresolved/unresolved-candidates.stderr | 26 +++++++++++++ 10 files changed, 145 insertions(+), 8 deletions(-) create mode 100644 src/test/ui/unresolved/unresolved-candidates.rs create mode 100644 src/test/ui/unresolved/unresolved-candidates.stderr diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 98982240af27f..9a3eac2f86622 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -70,6 +70,7 @@ impl TypoSuggestion { } /// A free importable items suggested in case of resolution failure. +#[derive(Debug, Clone)] pub(crate) struct ImportSuggestion { pub did: Option, pub descr: &'static str, @@ -139,6 +140,7 @@ impl<'a> Resolver<'a> { if instead { Instead::Yes } else { Instead::No }, found_use, IsPattern::No, + IsImport::No, path, ); err.emit(); @@ -698,6 +700,7 @@ impl<'a> Resolver<'a> { Instead::No, FoundUse::Yes, IsPattern::Yes, + IsImport::No, vec![], ); } @@ -1481,6 +1484,7 @@ impl<'a> Resolver<'a> { Instead::No, FoundUse::Yes, IsPattern::No, + IsImport::No, vec![], ); @@ -2449,6 +2453,34 @@ enum IsPattern { No, } +/// Whether a binding is part of a use statement. Used for diagnostics. +enum IsImport { + Yes, + No, +} + +pub(crate) fn import_candidates( + session: &Session, + source_span: &IndexVec, + err: &mut Diagnostic, + // This is `None` if all placement locations are inside expansions + use_placement_span: Option, + candidates: &[ImportSuggestion], +) { + show_candidates( + session, + source_span, + err, + use_placement_span, + candidates, + Instead::Yes, + FoundUse::Yes, + IsPattern::No, + IsImport::Yes, + vec![], + ); +} + /// When an entity with a given name is not available in scope, we search for /// entities with that name in all crates. This method allows outputting the /// results of this search in a programmer-friendly way @@ -2462,6 +2494,7 @@ fn show_candidates( instead: Instead, found_use: FoundUse, is_pattern: IsPattern, + is_import: IsImport, path: Vec, ) { if candidates.is_empty() { @@ -2521,7 +2554,8 @@ fn show_candidates( // produce an additional newline to separate the new use statement // from the directly following item. let additional_newline = if let FoundUse::Yes = found_use { "" } else { "\n" }; - candidate.0 = format!("use {};\n{}", &candidate.0, additional_newline); + let add_use = if let IsImport::Yes = is_import { "" } else { "use " }; + candidate.0 = format!("{}{};\n{}", add_use, &candidate.0, additional_newline); } err.span_suggestions( @@ -2551,7 +2585,7 @@ fn show_candidates( err.note(&msg); } - } else { + } else if matches!(is_import, IsImport::No) { assert!(!inaccessible_path_strings.is_empty()); let prefix = diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 5bdb427478199..9e2234ae4a54f 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -1,9 +1,9 @@ //! A bunch of methods and structures more or less related to resolving imports. -use crate::diagnostics::Suggestion; +use crate::diagnostics::{import_candidates, Suggestion}; use crate::Determinacy::{self, *}; use crate::Namespace::{self, *}; -use crate::{module_to_string, names_to_string}; +use crate::{module_to_string, names_to_string, ImportSuggestion}; use crate::{AmbiguityKind, BindingKey, ModuleKind, ResolutionError, Resolver, Segment}; use crate::{Finalize, Module, ModuleOrUniformRoot, ParentScope, PerNS, ScopeSet}; use crate::{NameBinding, NameBindingKind, PathResult}; @@ -406,6 +406,7 @@ struct UnresolvedImportError { label: Option, note: Option, suggestion: Option, + candidate: Option>, } pub struct ImportResolver<'a, 'b> { @@ -497,6 +498,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { label: None, note: None, suggestion: None, + candidate: None, }; if path.contains("::") { errors.push((path, err)) @@ -547,6 +549,16 @@ impl<'a, 'b> ImportResolver<'a, 'b> { } diag.multipart_suggestion(&msg, suggestions, applicability); } + + if let Some(candidate) = &err.candidate { + import_candidates( + self.r.session, + &self.r.source_span, + &mut diag, + Some(err.span), + &candidate, + ) + } } diag.emit(); @@ -664,6 +676,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { Some(finalize), ignore_binding, ); + let no_ambiguity = self.r.ambiguity_errors.len() == prev_ambiguity_errors_len; import.vis.set(orig_vis); let module = match path_res { @@ -706,12 +719,14 @@ impl<'a, 'b> ImportResolver<'a, 'b> { String::from("a similar path exists"), Applicability::MaybeIncorrect, )), + candidate: None, }, None => UnresolvedImportError { span, label: Some(label), note: None, suggestion, + candidate: None, }, }; return Some(err); @@ -754,6 +769,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { label: Some(String::from("cannot glob-import a module into itself")), note: None, suggestion: None, + candidate: None, }); } } @@ -919,11 +935,19 @@ impl<'a, 'b> ImportResolver<'a, 'b> { } }; + let parent_suggestion = + self.r.lookup_import_candidates(ident, TypeNS, &import.parent_scope, |_| true); + Some(UnresolvedImportError { span: import.span, label: Some(label), note, suggestion, + candidate: if !parent_suggestion.is_empty() { + Some(parent_suggestion) + } else { + None + }, }) } else { // `resolve_ident_in_module` reported a privacy error. diff --git a/src/test/ui/extenv/issue-55897.stderr b/src/test/ui/extenv/issue-55897.stderr index e2afe6f34c121..63797d4a71bce 100644 --- a/src/test/ui/extenv/issue-55897.stderr +++ b/src/test/ui/extenv/issue-55897.stderr @@ -26,6 +26,11 @@ error[E0432]: unresolved import `env` | LL | use env; | ^^^ no `env` in the root + | +help: consider importing this module instead + | +LL | use std::env; + | ~~~~~~~~~ error: cannot determine resolution for the macro `env` --> $DIR/issue-55897.rs:6:22 diff --git a/src/test/ui/imports/issue-56125.stderr b/src/test/ui/imports/issue-56125.stderr index 2e4ba86237676..059ca96808d9a 100644 --- a/src/test/ui/imports/issue-56125.stderr +++ b/src/test/ui/imports/issue-56125.stderr @@ -3,6 +3,18 @@ error[E0432]: unresolved import `empty::issue_56125` | LL | use empty::issue_56125; | ^^^^^^^^^^^^^^^^^^ no `issue_56125` in `m3::empty` + | +help: consider importing one of these items instead + | +LL | use crate::m3::last_segment::issue_56125; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +LL | use crate::m3::non_last_segment::non_last_segment::issue_56125; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +LL | use issue_56125::issue_56125; + | ~~~~~~~~~~~~~~~~~~~~~~~~~ +LL | use issue_56125::last_segment::issue_56125; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + and 1 other candidate error[E0659]: `issue_56125` is ambiguous --> $DIR/issue-56125.rs:6:9 diff --git a/src/test/ui/imports/issue-57015.stderr b/src/test/ui/imports/issue-57015.stderr index d200d23ab28ef..3b72d57fee41e 100644 --- a/src/test/ui/imports/issue-57015.stderr +++ b/src/test/ui/imports/issue-57015.stderr @@ -3,6 +3,11 @@ error[E0432]: unresolved import `single_err::something` | LL | use single_err::something; | ^^^^^^^^^^^^^^^^^^^^^ no `something` in `single_err` + | +help: consider importing this module instead + | +LL | use glob_ok::something; + | ~~~~~~~~~~~~~~~~~~~ error: aborting due to previous error diff --git a/src/test/ui/rfc-2126-extern-absolute-paths/not-allowed.stderr b/src/test/ui/rfc-2126-extern-absolute-paths/not-allowed.stderr index a66330ccc46ec..761089cd3871a 100644 --- a/src/test/ui/rfc-2126-extern-absolute-paths/not-allowed.stderr +++ b/src/test/ui/rfc-2126-extern-absolute-paths/not-allowed.stderr @@ -3,6 +3,13 @@ error[E0432]: unresolved import `alloc` | LL | use alloc; | ^^^^^ no external crate `alloc` + | +help: consider importing one of these items instead + | +LL | use core::alloc; + | ~~~~~~~~~~~~ +LL | use std::alloc; + | ~~~~~~~~~~~ error: aborting due to previous error diff --git a/src/test/ui/simd/portable-intrinsics-arent-exposed.stderr b/src/test/ui/simd/portable-intrinsics-arent-exposed.stderr index 870f4064de49b..8881ede0dbca7 100644 --- a/src/test/ui/simd/portable-intrinsics-arent-exposed.stderr +++ b/src/test/ui/simd/portable-intrinsics-arent-exposed.stderr @@ -11,6 +11,11 @@ error[E0432]: unresolved import `std::simd::intrinsics` | LL | use std::simd::intrinsics; | ^^^^^^^^^^^^^^^^^^^^^ no `intrinsics` in `simd` + | +help: consider importing this module instead + | +LL | use std::intrinsics; + | ~~~~~~~~~~~~~~~~ error: aborting due to 2 previous errors diff --git a/src/test/ui/test-attrs/inaccessible-test-modules.stderr b/src/test/ui/test-attrs/inaccessible-test-modules.stderr index a94ea1e79bc51..0c16ecd4c862e 100644 --- a/src/test/ui/test-attrs/inaccessible-test-modules.stderr +++ b/src/test/ui/test-attrs/inaccessible-test-modules.stderr @@ -11,10 +11,16 @@ error[E0432]: unresolved import `test` --> $DIR/inaccessible-test-modules.rs:6:5 | LL | use test as y; - | ----^^^^^ - | | - | no `test` in the root - | help: a similar name exists in the module: `test` + | ^^^^^^^^^ no `test` in the root + | +help: a similar name exists in the module + | +LL | use test as y; + | ~~~~ +help: consider importing this module instead + | +LL | use test::test; + | ~~~~~~~~~~~ error: aborting due to 2 previous errors diff --git a/src/test/ui/unresolved/unresolved-candidates.rs b/src/test/ui/unresolved/unresolved-candidates.rs new file mode 100644 index 0000000000000..38b227f609b26 --- /dev/null +++ b/src/test/ui/unresolved/unresolved-candidates.rs @@ -0,0 +1,13 @@ +mod a { + pub trait Trait {} +} + +mod b { + use Trait; //~ ERROR unresolved import `Trait` +} + +mod c { + impl Trait for () {} //~ ERROR cannot find trait `Trait` in this scope +} + +fn main() {} diff --git a/src/test/ui/unresolved/unresolved-candidates.stderr b/src/test/ui/unresolved/unresolved-candidates.stderr new file mode 100644 index 0000000000000..bbd3eec2a5431 --- /dev/null +++ b/src/test/ui/unresolved/unresolved-candidates.stderr @@ -0,0 +1,26 @@ +error[E0432]: unresolved import `Trait` + --> $DIR/unresolved-candidates.rs:6:9 + | +LL | use Trait; + | ^^^^^ no `Trait` in the root + | +help: consider importing this trait instead + | +LL | use a::Trait; + | ~~~~~~~~~ + +error[E0405]: cannot find trait `Trait` in this scope + --> $DIR/unresolved-candidates.rs:10:10 + | +LL | impl Trait for () {} + | ^^^^^ not found in this scope + | +help: consider importing this trait + | +LL | use a::Trait; + | + +error: aborting due to 2 previous errors + +Some errors have detailed explanations: E0405, E0432. +For more information about an error, try `rustc --explain E0405`. From 693485373b971d1574388b9da785b66c19dc86d2 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 3 Sep 2022 04:57:21 +0000 Subject: [PATCH 10/13] Point out incompatible closure bounds --- .../src/traits/error_reporting/mod.rs | 1 + .../src/traits/error_reporting/suggestions.rs | 67 +++++++++++++++++++ src/test/ui/closures/multiple-fn-bounds.rs | 15 +++++ .../ui/closures/multiple-fn-bounds.stderr | 24 +++++++ 4 files changed, 107 insertions(+) create mode 100644 src/test/ui/closures/multiple-fn-bounds.rs create mode 100644 src/test/ui/closures/multiple-fn-bounds.stderr diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs index 490b0e863225d..4e8baa2dfab6c 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -1255,6 +1255,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { found_span, found_trait_ref, expected_trait_ref, + obligation.cause.code(), ) } else { let (closure_span, found) = found_did diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 980e85b45262d..fda6a2236b195 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -254,8 +254,15 @@ pub trait TypeErrCtxtExt<'tcx> { found_span: Option, found: ty::PolyTraitRef<'tcx>, expected: ty::PolyTraitRef<'tcx>, + cause: &ObligationCauseCode<'tcx>, ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed>; + fn note_conflicting_closure_bounds( + &self, + cause: &ObligationCauseCode<'tcx>, + err: &mut DiagnosticBuilder<'tcx, ErrorGuaranteed>, + ); + fn suggest_fully_qualified_path( &self, err: &mut Diagnostic, @@ -1584,6 +1591,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { found_span: Option, found: ty::PolyTraitRef<'tcx>, expected: ty::PolyTraitRef<'tcx>, + cause: &ObligationCauseCode<'tcx>, ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { pub(crate) fn build_fn_sig_ty<'tcx>( infcx: &InferCtxt<'tcx>, @@ -1645,9 +1653,68 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { let signature_kind = format!("{argument_kind} signature"); err.note_expected_found(&signature_kind, expected_str, &signature_kind, found_str); + self.note_conflicting_closure_bounds(cause, &mut err); + err } + // Add a note if there are two `Fn`-family bounds that have conflicting argument + // requirements, which will always cause a closure to have a type error. + fn note_conflicting_closure_bounds( + &self, + cause: &ObligationCauseCode<'tcx>, + err: &mut DiagnosticBuilder<'tcx, ErrorGuaranteed>, + ) { + // First, look for an `ExprBindingObligation`, which means we can get + // the unsubstituted predicate list of the called function. And check + // that the predicate that we failed to satisfy is a `Fn`-like trait. + if let ObligationCauseCode::ExprBindingObligation(def_id, _, _, idx) = cause + && let predicates = self.tcx.predicates_of(def_id).instantiate_identity(self.tcx) + && let Some(pred) = predicates.predicates.get(*idx) + && let ty::PredicateKind::Trait(trait_pred) = pred.kind().skip_binder() + && ty::ClosureKind::from_def_id(self.tcx, trait_pred.def_id()).is_some() + { + let expected_self = + self.tcx.anonymize_late_bound_regions(pred.kind().rebind(trait_pred.self_ty())); + let expected_substs = self + .tcx + .anonymize_late_bound_regions(pred.kind().rebind(trait_pred.trait_ref.substs)); + + // Find another predicate whose self-type is equal to the expected self type, + // but whose substs don't match. + let other_pred = std::iter::zip(&predicates.predicates, &predicates.spans) + .enumerate() + .find(|(other_idx, (pred, _))| match pred.kind().skip_binder() { + ty::PredicateKind::Trait(trait_pred) + if ty::ClosureKind::from_def_id(self.tcx, trait_pred.def_id()) + .is_some() + && other_idx != idx + // Make sure that the self type matches + // (i.e. constraining this closure) + && expected_self + == self.tcx.anonymize_late_bound_regions( + pred.kind().rebind(trait_pred.self_ty()), + ) + // But the substs don't match (i.e. incompatible args) + && expected_substs + != self.tcx.anonymize_late_bound_regions( + pred.kind().rebind(trait_pred.trait_ref.substs), + ) => + { + true + } + _ => false, + }); + // If we found one, then it's very likely the cause of the error. + if let Some((_, (_, other_pred_span))) = other_pred { + err.span_note( + *other_pred_span, + "closure inferred to have a different signature due to this bound", + ); + } + } + } + fn suggest_fully_qualified_path( &self, err: &mut Diagnostic, diff --git a/src/test/ui/closures/multiple-fn-bounds.rs b/src/test/ui/closures/multiple-fn-bounds.rs new file mode 100644 index 0000000000000..6bb4098e2bbe5 --- /dev/null +++ b/src/test/ui/closures/multiple-fn-bounds.rs @@ -0,0 +1,15 @@ +fn foo bool + Fn(char) -> bool>(f: F) { + //~^ NOTE required by a bound in `foo` + //~| NOTE required by this bound in `foo` + //~| NOTE closure inferred to have a different signature due to this bound + todo!(); +} + +fn main() { + let v = true; + foo(move |x| v); + //~^ ERROR type mismatch in closure arguments + //~| NOTE expected closure signature + //~| NOTE expected due to this + //~| NOTE found signature defined here +} diff --git a/src/test/ui/closures/multiple-fn-bounds.stderr b/src/test/ui/closures/multiple-fn-bounds.stderr new file mode 100644 index 0000000000000..eefc123fed7ac --- /dev/null +++ b/src/test/ui/closures/multiple-fn-bounds.stderr @@ -0,0 +1,24 @@ +error[E0631]: type mismatch in closure arguments + --> $DIR/multiple-fn-bounds.rs:10:5 + | +LL | foo(move |x| v); + | ^^^ -------- found signature defined here + | | + | expected due to this + | + = note: expected closure signature `fn(char) -> _` + found closure signature `for<'a> fn(&'a char) -> _` +note: closure inferred to have a different signature due to this bound + --> $DIR/multiple-fn-bounds.rs:1:11 + | +LL | fn foo bool + Fn(char) -> bool>(f: F) { + | ^^^^^^^^^^^^^^^^^ +note: required by a bound in `foo` + --> $DIR/multiple-fn-bounds.rs:1:31 + | +LL | fn foo bool + Fn(char) -> bool>(f: F) { + | ^^^^^^^^^^^^^^^^ required by this bound in `foo` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0631`. From c1c159fe86317eee433ed77bcc6bb349cc941828 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 9 Oct 2022 06:54:00 +0000 Subject: [PATCH 11/13] Unify tcx.constness and param env constness checks --- .../src/const_eval/fn_queries.rs | 72 ++++++++++++++----- compiler/rustc_metadata/src/rmeta/encoder.rs | 56 +++++++++++---- compiler/rustc_ty_utils/src/ty.rs | 69 +----------------- 3 files changed, 98 insertions(+), 99 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/fn_queries.rs b/compiler/rustc_const_eval/src/const_eval/fn_queries.rs index f1674d04f8d15..cdcebb61c2e8c 100644 --- a/compiler/rustc_const_eval/src/const_eval/fn_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/fn_queries.rs @@ -25,12 +25,10 @@ pub fn is_parent_const_impl_raw(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { /// report whether said intrinsic has a `rustc_const_{un,}stable` attribute. Otherwise, return /// `Constness::NotConst`. fn constness(tcx: TyCtxt<'_>, def_id: DefId) -> hir::Constness { - let def_id = def_id.expect_local(); - let node = tcx.hir().get_by_def_id(def_id); - - match node { + let hir_id = tcx.hir().local_def_id_to_hir_id(def_id.expect_local()); + match tcx.hir().get(hir_id) { hir::Node::Ctor(_) => hir::Constness::Const, - hir::Node::Item(hir::Item { kind: hir::ItemKind::Impl(impl_), .. }) => impl_.constness, + hir::Node::ForeignItem(hir::ForeignItem { kind: hir::ForeignItemKind::Fn(..), .. }) => { // Intrinsics use `rustc_const_{un,}stable` attributes to indicate constness. All other // foreign items cannot be evaluated at compile-time. @@ -41,20 +39,62 @@ fn constness(tcx: TyCtxt<'_>, def_id: DefId) -> hir::Constness { }; if is_const { hir::Constness::Const } else { hir::Constness::NotConst } } - _ => { - if let Some(fn_kind) = node.fn_kind() { - if fn_kind.constness() == hir::Constness::Const { - return hir::Constness::Const; - } - // If the function itself is not annotated with `const`, it may still be a `const fn` - // if it resides in a const trait impl. - let is_const = is_parent_const_impl_raw(tcx, def_id); - if is_const { hir::Constness::Const } else { hir::Constness::NotConst } - } else { - hir::Constness::NotConst + hir::Node::TraitItem(hir::TraitItem { kind: hir::TraitItemKind::Fn(..), .. }) + if tcx.is_const_default_method(def_id) => + { + hir::Constness::Const + } + + hir::Node::Item(hir::Item { kind: hir::ItemKind::Const(..), .. }) + | hir::Node::Item(hir::Item { kind: hir::ItemKind::Static(..), .. }) + | hir::Node::TraitItem(hir::TraitItem { kind: hir::TraitItemKind::Const(..), .. }) + | hir::Node::AnonConst(_) + | hir::Node::ImplItem(hir::ImplItem { kind: hir::ImplItemKind::Const(..), .. }) + | hir::Node::ImplItem(hir::ImplItem { + kind: + hir::ImplItemKind::Fn( + hir::FnSig { + header: hir::FnHeader { constness: hir::Constness::Const, .. }, + .. + }, + .., + ), + .. + }) => hir::Constness::Const, + + hir::Node::ImplItem(hir::ImplItem { + kind: hir::ImplItemKind::Type(..) | hir::ImplItemKind::Fn(..), + .. + }) => { + let parent_hir_id = tcx.hir().get_parent_node(hir_id); + match tcx.hir().get(parent_hir_id) { + hir::Node::Item(hir::Item { + kind: hir::ItemKind::Impl(hir::Impl { constness, .. }), + .. + }) => *constness, + _ => span_bug!( + tcx.def_span(parent_hir_id.owner), + "impl item's parent node is not an impl", + ), } } + + hir::Node::Item(hir::Item { + kind: hir::ItemKind::Fn(hir::FnSig { header: hir::FnHeader { constness, .. }, .. }, ..), + .. + }) + | hir::Node::TraitItem(hir::TraitItem { + kind: + hir::TraitItemKind::Fn(hir::FnSig { header: hir::FnHeader { constness, .. }, .. }, ..), + .. + }) + | hir::Node::Item(hir::Item { + kind: hir::ItemKind::Impl(hir::Impl { constness, .. }), + .. + }) => *constness, + + _ => hir::Constness::NotConst, } } diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index 3fc10197b9129..ab4ecec2af36e 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -1059,6 +1059,43 @@ fn should_encode_const(def_kind: DefKind) -> bool { } } +fn should_encode_constness(def_kind: DefKind) -> bool { + match def_kind { + DefKind::Struct + | DefKind::Union + | DefKind::Enum + | DefKind::Trait + | DefKind::AssocTy + | DefKind::Fn + | DefKind::Const + | DefKind::Static(..) + | DefKind::Ctor(..) + | DefKind::AssocFn + | DefKind::AssocConst + | DefKind::AnonConst + | DefKind::InlineConst + | DefKind::OpaqueTy + | DefKind::ImplTraitPlaceholder + | DefKind::Impl + | DefKind::Closure + | DefKind::Generator => true, + DefKind::Variant + | DefKind::TyAlias + | DefKind::TraitAlias + | DefKind::ForeignTy + | DefKind::Field + | DefKind::TyParam + | DefKind::Mod + | DefKind::ForeignMod + | DefKind::ConstParam + | DefKind::Macro(..) + | DefKind::Use + | DefKind::LifetimeParam + | DefKind::GlobalAsm + | DefKind::ExternCrate => false, + } +} + fn should_encode_trait_impl_trait_tys<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> bool { if tcx.def_kind(def_id) != DefKind::AssocFn { return false; @@ -1165,6 +1202,9 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { { record!(self.tables.trait_impl_trait_tys[def_id] <- table); } + if should_encode_constness(def_kind) { + self.tables.constness.set(def_id.index, tcx.constness(def_id)); + } } let inherent_impls = tcx.crate_inherent_impls(()); for (def_id, implementations) in inherent_impls.inherent_impls.iter() { @@ -1192,7 +1232,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { }; record!(self.tables.variant_data[def_id] <- data); - self.tables.constness.set(def_id.index, hir::Constness::Const); record_array!(self.tables.children[def_id] <- variant.fields.iter().map(|f| { assert!(f.did.is_local()); f.did.index @@ -1220,7 +1259,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { }; record!(self.tables.variant_data[def_id] <- data); - self.tables.constness.set(def_id.index, hir::Constness::Const); if variant.ctor_kind == CtorKind::Fn { record!(self.tables.fn_sig[def_id] <- tcx.fn_sig(def_id)); } @@ -1284,7 +1322,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { record!(self.tables.repr_options[def_id] <- adt_def.repr()); record!(self.tables.variant_data[def_id] <- data); - self.tables.constness.set(def_id.index, hir::Constness::Const); if variant.ctor_kind == CtorKind::Fn { record!(self.tables.fn_sig[def_id] <- tcx.fn_sig(def_id)); } @@ -1320,7 +1357,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { } }; self.tables.asyncness.set(def_id.index, m_sig.header.asyncness); - self.tables.constness.set(def_id.index, hir::Constness::NotConst); } ty::AssocKind::Type => { self.encode_explicit_item_bounds(def_id); @@ -1345,13 +1381,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { let hir::ImplItemKind::Fn(ref sig, body) = ast_item.kind else { bug!() }; self.tables.asyncness.set(def_id.index, sig.header.asyncness); record_array!(self.tables.fn_arg_names[def_id] <- self.tcx.hir().body_param_names(body)); - // Can be inside `impl const Trait`, so using sig.header.constness is not reliable - let constness = if self.tcx.is_const_fn_raw(def_id) { - hir::Constness::Const - } else { - hir::Constness::NotConst - }; - self.tables.constness.set(def_id.index, constness); } ty::AssocKind::Const | ty::AssocKind::Type => {} } @@ -1474,7 +1503,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { hir::ItemKind::Fn(ref sig, .., body) => { self.tables.asyncness.set(def_id.index, sig.header.asyncness); record_array!(self.tables.fn_arg_names[def_id] <- self.tcx.hir().body_param_names(body)); - self.tables.constness.set(def_id.index, sig.header.constness); } hir::ItemKind::Macro(ref macro_def, _) => { if macro_def.macro_rules { @@ -1495,7 +1523,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { hir::ItemKind::Struct(ref struct_def, _) => { let adt_def = self.tcx.adt_def(def_id); record!(self.tables.repr_options[def_id] <- adt_def.repr()); - self.tables.constness.set(def_id.index, hir::Constness::Const); // Encode def_ids for each field and method // for methods, write all the stuff get_trait_method @@ -1524,9 +1551,8 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { is_non_exhaustive: variant.is_field_list_non_exhaustive(), }); } - hir::ItemKind::Impl(hir::Impl { defaultness, constness, .. }) => { + hir::ItemKind::Impl(hir::Impl { defaultness, .. }) => { self.tables.impl_defaultness.set(def_id.index, *defaultness); - self.tables.constness.set(def_id.index, *constness); let trait_ref = self.tcx.impl_trait_ref(def_id); if let Some(trait_ref) = trait_ref { diff --git a/compiler/rustc_ty_utils/src/ty.rs b/compiler/rustc_ty_utils/src/ty.rs index 30efbf6617598..344426a18bca5 100644 --- a/compiler/rustc_ty_utils/src/ty.rs +++ b/compiler/rustc_ty_utils/src/ty.rs @@ -133,77 +133,10 @@ fn param_env(tcx: TyCtxt<'_>, def_id: DefId) -> ty::ParamEnv<'_> { let local_did = def_id.as_local(); let hir_id = local_did.map(|def_id| tcx.hir().local_def_id_to_hir_id(def_id)); - let constness = match hir_id { - Some(hir_id) => match tcx.hir().get(hir_id) { - hir::Node::TraitItem(hir::TraitItem { kind: hir::TraitItemKind::Fn(..), .. }) - if tcx.is_const_default_method(def_id) => - { - hir::Constness::Const - } - - hir::Node::Item(hir::Item { kind: hir::ItemKind::Const(..), .. }) - | hir::Node::Item(hir::Item { kind: hir::ItemKind::Static(..), .. }) - | hir::Node::TraitItem(hir::TraitItem { - kind: hir::TraitItemKind::Const(..), .. - }) - | hir::Node::AnonConst(_) - | hir::Node::ImplItem(hir::ImplItem { kind: hir::ImplItemKind::Const(..), .. }) - | hir::Node::ImplItem(hir::ImplItem { - kind: - hir::ImplItemKind::Fn( - hir::FnSig { - header: hir::FnHeader { constness: hir::Constness::Const, .. }, - .. - }, - .., - ), - .. - }) => hir::Constness::Const, - - hir::Node::ImplItem(hir::ImplItem { - kind: hir::ImplItemKind::Type(..) | hir::ImplItemKind::Fn(..), - .. - }) => { - let parent_hir_id = tcx.hir().get_parent_node(hir_id); - match tcx.hir().get(parent_hir_id) { - hir::Node::Item(hir::Item { - kind: hir::ItemKind::Impl(hir::Impl { constness, .. }), - .. - }) => *constness, - _ => span_bug!( - tcx.def_span(parent_hir_id.owner), - "impl item's parent node is not an impl", - ), - } - } - - hir::Node::Item(hir::Item { - kind: - hir::ItemKind::Fn(hir::FnSig { header: hir::FnHeader { constness, .. }, .. }, ..), - .. - }) - | hir::Node::TraitItem(hir::TraitItem { - kind: - hir::TraitItemKind::Fn( - hir::FnSig { header: hir::FnHeader { constness, .. }, .. }, - .., - ), - .. - }) - | hir::Node::Item(hir::Item { - kind: hir::ItemKind::Impl(hir::Impl { constness, .. }), - .. - }) => *constness, - - _ => hir::Constness::NotConst, - }, - None => hir::Constness::NotConst, - }; - let unnormalized_env = ty::ParamEnv::new( tcx.intern_predicates(&predicates), traits::Reveal::UserFacing, - constness, + tcx.constness(def_id), ); let body_id = From 03fe005f4967439c09b538f1dcf5e38ca5d27019 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Sun, 9 Oct 2022 23:19:50 -0700 Subject: [PATCH 12/13] rustdoc: clean up overly complex `.trait-impl` CSS selectors When added in 45964368f4a2e31c94e9bcf1cef933c087d21544, these multi-class selectors were present in the initial commit, but no reason was given why the shorter selector wouldn't work. --- src/librustdoc/html/static/css/rustdoc.css | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index eb64147d90677..d4228a2ebc638 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -734,9 +734,7 @@ a { display: initial; } -.impl:hover > .anchor, .method.trait-impl:hover > .anchor, -.type.trait-impl:hover > .anchor, .associatedconstant.trait-impl:hover > .anchor, -.associatedtype.trait-impl:hover > .anchor { +.impl:hover > .anchor, .trait-impl:hover > .anchor { display: inline-block; position: absolute; } From 6071b4b8a64571312017d5eaba01276b90ffe0dc Mon Sep 17 00:00:00 2001 From: gimbles Date: Mon, 10 Oct 2022 16:53:13 +0530 Subject: [PATCH 13/13] `let` is not allowed in struct field definitions Co-authored-by: jyn514 Co-authored-by: Esteban Kuber --- compiler/rustc_parse/src/parser/item.rs | 18 +++++++++++++++++- .../ui/parser/removed-syntax-field-let.stderr | 10 ++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index 25425fbb2c6a6..e82044a89c479 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -1788,7 +1788,23 @@ impl<'a> Parser<'a> { } } } else { - self.expected_ident_found() + let mut err = self.expected_ident_found(); + if let Some((ident, _)) = self.token.ident() && ident.as_str() == "let" { + self.bump(); // `let` + let span = self.prev_token.span.until(self.token.span); + err.span_suggestion( + span, + "remove the let, the `let` keyword is not allowed in struct field definitions", + String::new(), + Applicability::MachineApplicable, + ); + err.note("the `let` keyword is not allowed in `struct` fields"); + err.note("see for more information"); + err.emit(); + self.bump(); + return Ok(ident); + } + err }; return Err(err); } diff --git a/src/test/ui/parser/removed-syntax-field-let.stderr b/src/test/ui/parser/removed-syntax-field-let.stderr index ab3d2056581b6..bd1b23f7e3bbb 100644 --- a/src/test/ui/parser/removed-syntax-field-let.stderr +++ b/src/test/ui/parser/removed-syntax-field-let.stderr @@ -1,10 +1,16 @@ error: expected identifier, found keyword `let` --> $DIR/removed-syntax-field-let.rs:2:5 | -LL | struct S { - | - while parsing this struct LL | let foo: (), | ^^^ expected identifier, found keyword + | + = note: the `let` keyword is not allowed in `struct` fields + = note: see for more information +help: remove the let, the `let` keyword is not allowed in struct field definitions + | +LL - let foo: (), +LL + foo: (), + | error: aborting due to previous error