From c1ffb0b675c5bb7fb5d91c19fcb9171f873511d0 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 13 Feb 2024 08:19:55 +1100 Subject: [PATCH] Remove `force_print_diagnostic`. There are a couple of places where we call `inner.emitter.emit_diagnostic` directly rather than going through `inner.emit_diagnostic`, to guarantee the diagnostic is printed. This feels dubious to me, particularly the bypassing of `TRACK_DIAGNOSTIC`. This commit removes those. - In `print_error_count`, it uses `ForceWarning` instead of `Warning`. - It removes `DiagCtxtInner::failure_note`, because it only has three uses and direct use of `emit_diagnostic` is consistent with other similar locations. - It removes `force_print_diagnostic`, and adds `struct_failure_note`, and updates `print_query_stack` accordingly, which makes it more normal. That location doesn't seem to need forced printing anyway. --- compiler/rustc_errors/src/lib.rs | 43 +++++++++++--------- compiler/rustc_query_system/src/query/job.rs | 20 ++++----- 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 7f78ea7aa56b0..965a62e9a8b8e 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -780,11 +780,12 @@ impl DiagCtxt { match (errors.len(), warnings.len()) { (0, 0) => return, (0, _) => { - // Use `inner.emitter` directly, otherwise the warning might not be emitted, e.g. - // with a configuration like `--cap-lints allow --force-warn bare_trait_objects`. - inner - .emitter - .emit_diagnostic(Diagnostic::new(Warning, DiagnosticMessage::Str(warnings))); + // Use `ForceWarning` rather than `Warning` to guarantee emission, e.g. with a + // configuration like `--cap-lints allow --force-warn bare_trait_objects`. + inner.emit_diagnostic(Diagnostic::new( + ForceWarning(None), + DiagnosticMessage::Str(warnings), + )); } (_, 0) => { inner.emit_diagnostic(Diagnostic::new(Error, errors)); @@ -812,20 +813,23 @@ impl DiagCtxt { error_codes.sort(); if error_codes.len() > 1 { let limit = if error_codes.len() > 9 { 9 } else { error_codes.len() }; - inner.failure_note(format!( + let msg1 = format!( "Some errors have detailed explanations: {}{}", error_codes[..limit].join(", "), if error_codes.len() > 9 { "..." } else { "." } - )); - inner.failure_note(format!( + ); + let msg2 = format!( "For more information about an error, try `rustc --explain {}`.", &error_codes[0] - )); + ); + inner.emit_diagnostic(Diagnostic::new(FailureNote, msg1)); + inner.emit_diagnostic(Diagnostic::new(FailureNote, msg2)); } else { - inner.failure_note(format!( + let msg = format!( "For more information about this error, try `rustc --explain {}`.", &error_codes[0] - )); + ); + inner.emit_diagnostic(Diagnostic::new(FailureNote, msg)); } } } @@ -848,10 +852,6 @@ impl DiagCtxt { self.inner.borrow_mut().taught_diagnostics.insert(code) } - pub fn force_print_diagnostic(&self, db: Diagnostic) { - self.inner.borrow_mut().emitter.emit_diagnostic(db); - } - pub fn emit_diagnostic(&self, diagnostic: Diagnostic) -> Option { self.inner.borrow_mut().emit_diagnostic(diagnostic) } @@ -1207,6 +1207,15 @@ impl DiagCtxt { DiagnosticBuilder::new(self, Help, msg) } + #[rustc_lint_diagnostics] + #[track_caller] + pub fn struct_failure_note( + &self, + msg: impl Into, + ) -> DiagnosticBuilder<'_, ()> { + DiagnosticBuilder::new(self, FailureNote, msg) + } + #[rustc_lint_diagnostics] #[track_caller] pub fn struct_allow(&self, msg: impl Into) -> DiagnosticBuilder<'_, ()> { @@ -1406,10 +1415,6 @@ impl DiagCtxtInner { .or_else(|| self.delayed_bugs.get(0).map(|(_, guar)| guar).copied()) } - fn failure_note(&mut self, msg: impl Into) { - self.emit_diagnostic(Diagnostic::new(FailureNote, msg)); - } - fn flush_delayed(&mut self) { if self.delayed_bugs.is_empty() { return; diff --git a/compiler/rustc_query_system/src/query/job.rs b/compiler/rustc_query_system/src/query/job.rs index 3ef9de7da74b2..8d7c0ca014490 100644 --- a/compiler/rustc_query_system/src/query/job.rs +++ b/compiler/rustc_query_system/src/query/job.rs @@ -4,7 +4,7 @@ use crate::query::plumbing::CycleError; use crate::query::DepKind; use crate::query::{QueryContext, QueryStackFrame}; use rustc_data_structures::fx::FxHashMap; -use rustc_errors::{DiagCtxt, Diagnostic, DiagnosticBuilder, Level}; +use rustc_errors::{DiagCtxt, DiagnosticBuilder}; use rustc_hir::def::DefKind; use rustc_session::Session; use rustc_span::Span; @@ -628,15 +628,15 @@ pub fn print_query_stack( }; if Some(count_printed) < num_frames || num_frames.is_none() { // Only print to stderr as many stack frames as `num_frames` when present. - let mut diag = Diagnostic::new( - Level::FailureNote, - format!( - "#{} [{:?}] {}", - count_printed, query_info.query.dep_kind, query_info.query.description - ), - ); - diag.span = query_info.job.span.into(); - dcx.force_print_diagnostic(diag); + // FIXME: needs translation + #[allow(rustc::diagnostic_outside_of_impl)] + #[allow(rustc::untranslatable_diagnostic)] + dcx.struct_failure_note(format!( + "#{} [{:?}] {}", + count_printed, query_info.query.dep_kind, query_info.query.description + )) + .with_span(query_info.job.span) + .emit(); count_printed += 1; }