From b38ed1afa6cae65e5895f419ab8af98bdf5af840 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 16 Feb 2024 06:07:49 +1100 Subject: [PATCH 1/3] Overhaul `Diagnostic` args. First, introduce a typedef `DiagnosticArgMap`. Second, make the `args` field public, and remove the `args` getter and `replace_args` setter. These were necessary previously because the getter had a `#[allow(rustc::potential_query_instability)]` attribute, but that was removed in #120931 when the args were changed from `FxHashMap` to `FxIndexMap`. (All the other `Diagnostic` fields are public.) --- compiler/rustc_codegen_llvm/src/errors.rs | 3 +-- compiler/rustc_codegen_ssa/src/back/write.rs | 12 ++++++------ compiler/rustc_const_eval/src/errors.rs | 4 ++-- .../src/interpret/eval_context.rs | 2 +- .../src/annotate_snippet_emitter_writer.rs | 2 +- compiler/rustc_errors/src/diagnostic.rs | 16 +++++----------- compiler/rustc_errors/src/emitter.rs | 2 +- compiler/rustc_errors/src/json.rs | 2 +- compiler/rustc_errors/src/lib.rs | 10 +++++----- .../passes/lint/check_code_block_syntax.rs | 2 +- 10 files changed, 24 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/errors.rs b/compiler/rustc_codegen_llvm/src/errors.rs index 87e3774068b..24b3aa4223a 100644 --- a/compiler/rustc_codegen_llvm/src/errors.rs +++ b/compiler/rustc_codegen_llvm/src/errors.rs @@ -103,8 +103,7 @@ impl IntoDiagnostic<'_, G> for ParseTargetMachineConfig<'_ fn into_diagnostic(self, dcx: &'_ DiagCtxt, level: Level) -> DiagnosticBuilder<'_, G> { let diag: DiagnosticBuilder<'_, G> = self.0.into_diagnostic(dcx, level); let (message, _) = diag.messages.first().expect("`LlvmError` with no message"); - let message = dcx.eagerly_translate_to_string(message.clone(), diag.args()); - + let message = dcx.eagerly_translate_to_string(message.clone(), diag.args.iter()); DiagnosticBuilder::new(dcx, level, fluent::codegen_llvm_parse_target_machine_config) .with_arg("error", message) } diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index 7a981217b52..0258d1d493d 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -16,8 +16,8 @@ use rustc_data_structures::sync::Lrc; use rustc_errors::emitter::Emitter; use rustc_errors::translation::Translate; use rustc_errors::{ - DiagCtxt, DiagnosticArgName, DiagnosticArgValue, DiagnosticBuilder, DiagnosticMessage, ErrCode, - FatalError, FluentBundle, Level, Style, + DiagCtxt, DiagnosticArgMap, DiagnosticBuilder, DiagnosticMessage, ErrCode, FatalError, + FluentBundle, Level, Style, }; use rustc_fs_util::link_or_copy; use rustc_hir::def_id::{CrateNum, LOCAL_CRATE}; @@ -1001,7 +1001,7 @@ pub struct CguMessage; struct Diagnostic { msgs: Vec<(DiagnosticMessage, Style)>, - args: FxIndexMap, + args: DiagnosticArgMap, code: Option, lvl: Level, } @@ -1813,8 +1813,8 @@ impl Translate for SharedEmitter { impl Emitter for SharedEmitter { fn emit_diagnostic(&mut self, diag: rustc_errors::Diagnostic) { - let args: FxIndexMap = - diag.args().map(|(name, arg)| (name.clone(), arg.clone())).collect(); + let args: DiagnosticArgMap = + diag.args.iter().map(|(name, arg)| (name.clone(), arg.clone())).collect(); drop(self.sender.send(SharedEmitterMessage::Diagnostic(Diagnostic { msgs: diag.messages.clone(), args: args.clone(), @@ -1857,7 +1857,7 @@ impl SharedEmitterMain { let dcx = sess.dcx(); let mut d = rustc_errors::Diagnostic::new_with_messages(diag.lvl, diag.msgs); d.code = diag.code; // may be `None`, that's ok - d.replace_args(diag.args); + d.args = diag.args; dcx.emit_diagnostic(d); } Ok(SharedEmitterMessage::InlineAsmError(cookie, msg, level, source)) => { diff --git a/compiler/rustc_const_eval/src/errors.rs b/compiler/rustc_const_eval/src/errors.rs index 2fd34b3c7fc..267f3acaaa5 100644 --- a/compiler/rustc_const_eval/src/errors.rs +++ b/compiler/rustc_const_eval/src/errors.rs @@ -437,7 +437,7 @@ pub trait ReportErrorExt { let mut diag = dcx.struct_allow(DiagnosticMessage::Str(String::new().into())); let message = self.diagnostic_message(); self.add_args(&mut diag); - let s = dcx.eagerly_translate_to_string(message, diag.args()); + let s = dcx.eagerly_translate_to_string(message, diag.args.iter()); diag.cancel(); s }) @@ -864,7 +864,7 @@ impl<'tcx> ReportErrorExt for InvalidProgramInfo<'tcx> { let dummy_level = Level::Bug; let dummy_diag: DiagnosticBuilder<'_, ()> = e.into_diagnostic().into_diagnostic(diag.dcx, dummy_level); - for (name, val) in dummy_diag.args() { + for (name, val) in dummy_diag.args.iter() { diag.arg(name.clone(), val.clone()); } dummy_diag.cancel(); diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index 517994d4741..cb308ab53ec 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -446,7 +446,7 @@ pub fn format_interp_error<'tcx>(dcx: &DiagCtxt, e: InterpErrorInfo<'tcx>) -> St let mut diag = dcx.struct_allow(""); let msg = e.diagnostic_message(); e.add_args(&mut diag); - let s = dcx.eagerly_translate_to_string(msg, diag.args()); + let s = dcx.eagerly_translate_to_string(msg, diag.args.iter()); diag.cancel(); s } diff --git a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs index 1a34a83c1a4..52b5a7eff48 100644 --- a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs +++ b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs @@ -45,7 +45,7 @@ impl Translate for AnnotateSnippetEmitter { impl Emitter for AnnotateSnippetEmitter { /// The entry point for the diagnostics generation fn emit_diagnostic(&mut self, mut diag: Diagnostic) { - let fluent_args = to_fluent_args(diag.args()); + let fluent_args = to_fluent_args(diag.args.iter()); let mut suggestions = diag.suggestions.unwrap_or(vec![]); self.primary_span_formatted(&mut diag.span, &mut suggestions, &fluent_args); diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index f096f015910..65c49a6085c 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -43,6 +43,8 @@ pub enum DiagnosticArgValue { StrListSepByAnd(Vec>), } +pub type DiagnosticArgMap = FxIndexMap; + /// Trait for types that `DiagnosticBuilder::emit` can return as a "guarantee" /// (or "proof") token that the emission happened. pub trait EmissionGuarantee: Sized { @@ -275,7 +277,7 @@ pub struct Diagnostic { pub span: MultiSpan, pub children: Vec, pub suggestions: Result, SuggestionsDisabled>, - args: FxIndexMap, + pub args: DiagnosticArgMap, /// 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 @@ -403,14 +405,6 @@ impl Diagnostic { self.args.insert(name.into(), arg.into_diagnostic_arg()); } - pub fn args(&self) -> impl Iterator> { - self.args.iter() - } - - pub fn replace_args(&mut self, args: FxIndexMap) { - self.args = args; - } - /// Fields used for Hash, and PartialEq trait. fn keys( &self, @@ -431,7 +425,7 @@ impl Diagnostic { &self.span, &self.children, &self.suggestions, - self.args().collect(), + self.args.iter().collect(), // omit self.sort_span &self.is_lint, // omit self.emitted_at @@ -1165,7 +1159,7 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> { subdiagnostic: impl AddToDiagnostic, ) -> &mut Self { subdiagnostic.add_to_diagnostic_with(self, |diag, msg| { - let args = diag.args(); + let args = diag.args.iter(); let msg = diag.subdiagnostic_message_to_diagnostic_message(msg); dcx.eagerly_translate(msg, args) }); diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index df94b69004b..c4b2c28fc23 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -519,7 +519,7 @@ impl Emitter for HumanEmitter { } fn emit_diagnostic(&mut self, mut diag: Diagnostic) { - let fluent_args = to_fluent_args(diag.args()); + let fluent_args = to_fluent_args(diag.args.iter()); let mut suggestions = diag.suggestions.unwrap_or(vec![]); self.primary_span_formatted(&mut diag.span, &mut suggestions, &fluent_args); diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index 470e3d52452..e57b414c52d 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -341,7 +341,7 @@ struct UnusedExterns<'a, 'b, 'c> { impl Diagnostic { fn from_errors_diagnostic(diag: crate::Diagnostic, je: &JsonEmitter) -> Diagnostic { - let args = to_fluent_args(diag.args()); + let args = to_fluent_args(diag.args.iter()); let sugg = diag.suggestions.iter().flatten().map(|sugg| { let translated_message = je.translate_message(&sugg.msg, &args).map_err(Report::new).unwrap(); diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 052d9b3a783..547f8b68c1b 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -37,9 +37,10 @@ extern crate self as rustc_errors; pub use codes::*; pub use diagnostic::{ - AddToDiagnostic, BugAbort, DecorateLint, Diagnostic, DiagnosticArg, DiagnosticArgName, - DiagnosticArgValue, DiagnosticBuilder, DiagnosticStyledString, EmissionGuarantee, FatalAbort, - IntoDiagnostic, IntoDiagnosticArg, StringPart, SubDiagnostic, SubdiagnosticMessageOp, + AddToDiagnostic, BugAbort, DecorateLint, Diagnostic, DiagnosticArg, DiagnosticArgMap, + DiagnosticArgName, DiagnosticArgValue, DiagnosticBuilder, DiagnosticStyledString, + EmissionGuarantee, FatalAbort, IntoDiagnostic, IntoDiagnosticArg, StringPart, SubDiagnostic, + SubdiagnosticMessageOp, }; pub use diagnostic_impls::{ DiagnosticArgFromDisplay, DiagnosticSymbolList, ExpectedLifetimeParameter, @@ -1482,9 +1483,8 @@ impl DiagCtxtInner { diag: &Diagnostic, msg: impl Into, ) -> SubdiagnosticMessage { - let args = diag.args(); let msg = diag.subdiagnostic_message_to_diagnostic_message(msg); - self.eagerly_translate(msg, args) + self.eagerly_translate(msg, diag.args.iter()) } fn flush_delayed(&mut self) { diff --git a/src/librustdoc/passes/lint/check_code_block_syntax.rs b/src/librustdoc/passes/lint/check_code_block_syntax.rs index 6649894f9c2..f3537873dc2 100644 --- a/src/librustdoc/passes/lint/check_code_block_syntax.rs +++ b/src/librustdoc/passes/lint/check_code_block_syntax.rs @@ -159,7 +159,7 @@ impl Emitter for BufferEmitter { fn emit_diagnostic(&mut self, diag: Diagnostic) { let mut buffer = self.buffer.borrow_mut(); - let fluent_args = to_fluent_args(diag.args()); + let fluent_args = to_fluent_args(diag.args.iter()); let translated_main_message = self .translate_message(&diag.messages[0].0, &fluent_args) .unwrap_or_else(|e| panic!("{e}")); From ad5d7f43c95e09f1e0efed8673d8a515d0f2054a Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 13 Feb 2024 17:27:24 +1100 Subject: [PATCH 2/3] Overhaul `rustc_codegen_ssa::back::write::Diagnostic`. - Make it more closely match `rustc_errors::Diagnostic`, by making the field names match, and adding `children`, which requires adding `rustc_codegen_ssa::back::write::Subdiagnostic`. - Check that we aren't missing important info when converting diagnostics. - Add better comments. - Tweak `rustc_errors::Diagnostic::replace_args` so that we don't need to do any cloning when converting diagnostics. --- compiler/rustc_codegen_ssa/src/back/write.rs | 80 ++++++++++++++------ tests/ui/lto/issue-11154.stderr | 4 +- 2 files changed, 60 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index 0258d1d493d..54831265691 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -17,7 +17,7 @@ use rustc_errors::emitter::Emitter; use rustc_errors::translation::Translate; use rustc_errors::{ DiagCtxt, DiagnosticArgMap, DiagnosticBuilder, DiagnosticMessage, ErrCode, FatalError, - FluentBundle, Level, Style, + FluentBundle, Level, MultiSpan, Style, }; use rustc_fs_util::link_or_copy; use rustc_hir::def_id::{CrateNum, LOCAL_CRATE}; @@ -999,11 +999,29 @@ pub(crate) enum Message { /// process another codegen unit. pub struct CguMessage; +// A cut-down version of `rustc_errors::Diagnostic` that impls `Send`, which +// can be used to send diagnostics from codegen threads to the main thread. +// It's missing the following fields from `rustc_errors::Diagnostic`. +// - `span`: it doesn't impl `Send`. +// - `suggestions`: it doesn't impl `Send`, and isn't used for codegen +// diagnostics. +// - `sort_span`: it doesn't impl `Send`. +// - `is_lint`: lints aren't relevant during codegen. +// - `emitted_at`: not used for codegen diagnostics. struct Diagnostic { - msgs: Vec<(DiagnosticMessage, Style)>, - args: DiagnosticArgMap, + level: Level, + messages: Vec<(DiagnosticMessage, Style)>, code: Option, - lvl: Level, + children: Vec, + args: DiagnosticArgMap, +} + +// A cut-down version of `rustc_errors::SubDiagnostic` that impls `Send`. It's +// missing the following fields from `rustc_errors::SubDiagnostic`. +// - `span`: it doesn't impl `Send`. +pub struct Subdiagnostic { + level: Level, + messages: Vec<(DiagnosticMessage, Style)>, } #[derive(PartialEq, Clone, Copy, Debug)] @@ -1812,23 +1830,29 @@ impl Translate for SharedEmitter { } impl Emitter for SharedEmitter { - fn emit_diagnostic(&mut self, diag: rustc_errors::Diagnostic) { - let args: DiagnosticArgMap = - diag.args.iter().map(|(name, arg)| (name.clone(), arg.clone())).collect(); - drop(self.sender.send(SharedEmitterMessage::Diagnostic(Diagnostic { - msgs: diag.messages.clone(), - args: args.clone(), - code: diag.code, - lvl: diag.level(), - }))); - for child in &diag.children { - drop(self.sender.send(SharedEmitterMessage::Diagnostic(Diagnostic { - msgs: child.messages.clone(), - args: args.clone(), - code: None, - lvl: child.level, - }))); - } + fn emit_diagnostic(&mut self, mut diag: rustc_errors::Diagnostic) { + // Check that we aren't missing anything interesting when converting to + // the cut-down local `Diagnostic`. + assert_eq!(diag.span, MultiSpan::new()); + assert_eq!(diag.suggestions, Ok(vec![])); + assert_eq!(diag.sort_span, rustc_span::DUMMY_SP); + assert_eq!(diag.is_lint, None); + // No sensible check for `diag.emitted_at`. + + let args = mem::replace(&mut diag.args, DiagnosticArgMap::default()); + drop( + self.sender.send(SharedEmitterMessage::Diagnostic(Diagnostic { + level: diag.level(), + messages: diag.messages, + code: diag.code, + children: diag + .children + .into_iter() + .map(|child| Subdiagnostic { level: child.level, messages: child.messages }) + .collect(), + args, + })), + ); drop(self.sender.send(SharedEmitterMessage::AbortIfErrors)); } @@ -1854,9 +1878,21 @@ impl SharedEmitterMain { match message { Ok(SharedEmitterMessage::Diagnostic(diag)) => { + // The diagnostic has been received on the main thread. + // Convert it back to a full `Diagnostic` and emit. let dcx = sess.dcx(); - let mut d = rustc_errors::Diagnostic::new_with_messages(diag.lvl, diag.msgs); + let mut d = + rustc_errors::Diagnostic::new_with_messages(diag.level, diag.messages); d.code = diag.code; // may be `None`, that's ok + d.children = diag + .children + .into_iter() + .map(|sub| rustc_errors::SubDiagnostic { + level: sub.level, + messages: sub.messages, + span: MultiSpan::new(), + }) + .collect(); d.args = diag.args; dcx.emit_diagnostic(d); } diff --git a/tests/ui/lto/issue-11154.stderr b/tests/ui/lto/issue-11154.stderr index 4d52f6c0f3d..9f5d7518982 100644 --- a/tests/ui/lto/issue-11154.stderr +++ b/tests/ui/lto/issue-11154.stderr @@ -1,6 +1,6 @@ error: cannot prefer dynamic linking when performing LTO - -note: only 'staticlib', 'bin', and 'cdylib' outputs are supported with LTO + | + = note: only 'staticlib', 'bin', and 'cdylib' outputs are supported with LTO error: aborting due to 1 previous error From 6efffd723bc87f3a577f4f52fcffb6335406e9cb Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 15 Feb 2024 16:22:22 +1100 Subject: [PATCH 3/3] Remove `SharedEmitterMessage::AbortIfErrors`. It's always paired wth `SharedEmitterMessage::Diagnostic`, so the two can be merged. --- compiler/rustc_codegen_ssa/src/back/write.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index 54831265691..c2fc32130ea 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -1784,7 +1784,6 @@ fn spawn_work<'a, B: ExtraBackendMethods>( enum SharedEmitterMessage { Diagnostic(Diagnostic), InlineAsmError(u32, String, Level, Option<(String, Vec)>), - AbortIfErrors, Fatal(String), } @@ -1853,7 +1852,6 @@ impl Emitter for SharedEmitter { args, })), ); - drop(self.sender.send(SharedEmitterMessage::AbortIfErrors)); } fn source_map(&self) -> Option<&Lrc> { @@ -1895,6 +1893,7 @@ impl SharedEmitterMain { .collect(); d.args = diag.args; dcx.emit_diagnostic(d); + sess.dcx().abort_if_errors(); } Ok(SharedEmitterMessage::InlineAsmError(cookie, msg, level, source)) => { assert!(matches!(level, Level::Error | Level::Warning | Level::Note)); @@ -1927,9 +1926,6 @@ impl SharedEmitterMain { err.emit(); } - Ok(SharedEmitterMessage::AbortIfErrors) => { - sess.dcx().abort_if_errors(); - } Ok(SharedEmitterMessage::Fatal(msg)) => { sess.dcx().fatal(msg); }