From bdc6d82f9ac21a9c0b8a5e3f5728a5cbb50a09e2 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 13 Feb 2024 09:13:52 +1100 Subject: [PATCH 1/3] Make `struct_span_note` call `struct_note`. So it follows the same pattern as all the other `struct_span_*` methods. --- compiler/rustc_errors/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index e033d66fccf41..7f78ea7aa56b0 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1179,7 +1179,7 @@ impl DiagCtxt { span: impl Into, msg: impl Into, ) -> DiagnosticBuilder<'_, ()> { - DiagnosticBuilder::new(self, Note, msg).with_span(span) + self.struct_note(msg).with_span(span) } #[rustc_lint_diagnostics] From c1ffb0b675c5bb7fb5d91c19fcb9171f873511d0 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 13 Feb 2024 08:19:55 +1100 Subject: [PATCH 2/3] 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; } From 56b451a67ad058b8d28a0d9f566ec63a36dce9d5 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 13 Feb 2024 16:19:40 +1100 Subject: [PATCH 3/3] Fix `DiagCtxtInner::reset_err_count`. Several fields were not being reset. Using destructuring makes it much harder to miss a field. --- compiler/rustc_errors/src/lib.rs | 60 ++++++++++++++++++++++++-------- 1 file changed, 45 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 965a62e9a8b8e..fbd812609eee2 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -78,6 +78,7 @@ use std::fmt; use std::hash::Hash; use std::io::Write; use std::num::NonZeroUsize; +use std::ops::DerefMut; use std::panic; use std::path::{Path, PathBuf}; @@ -666,22 +667,51 @@ impl DiagCtxt { /// tools that want to reuse a `Parser` cleaning the previously emitted diagnostics as well as /// the overall count of emitted error diagnostics. pub fn reset_err_count(&self) { + // Use destructuring so that if a field gets added to `DiagCtxtInner`, it's impossible to + // fail to update this method as well. let mut inner = self.inner.borrow_mut(); - inner.stashed_err_count = 0; - inner.deduplicated_err_count = 0; - inner.deduplicated_warn_count = 0; - inner.must_produce_diag = false; - inner.has_printed = false; - inner.suppressed_expected_diag = false; - - // actually free the underlying memory (which `clear` would not do) - inner.err_guars = Default::default(); - inner.lint_err_guars = Default::default(); - inner.delayed_bugs = Default::default(); - inner.taught_diagnostics = Default::default(); - inner.emitted_diagnostic_codes = Default::default(); - inner.emitted_diagnostics = Default::default(); - inner.stashed_diagnostics = Default::default(); + let DiagCtxtInner { + flags: _, + err_guars, + lint_err_guars, + delayed_bugs, + stashed_err_count, + deduplicated_err_count, + deduplicated_warn_count, + emitter: _, + must_produce_diag, + has_printed, + suppressed_expected_diag, + taught_diagnostics, + emitted_diagnostic_codes, + emitted_diagnostics, + stashed_diagnostics, + future_breakage_diagnostics, + check_unstable_expect_diagnostics, + unstable_expect_diagnostics, + fulfilled_expectations, + ice_file: _, + } = inner.deref_mut(); + + // For the `Vec`s and `HashMap`s, we overwrite with an empty container to free the + // underlying memory (which `clear` would not do). + *err_guars = Default::default(); + *lint_err_guars = Default::default(); + *delayed_bugs = Default::default(); + *stashed_err_count = 0; + *deduplicated_err_count = 0; + *deduplicated_warn_count = 0; + *must_produce_diag = false; + *has_printed = false; + *suppressed_expected_diag = false; + *taught_diagnostics = Default::default(); + *emitted_diagnostic_codes = Default::default(); + *emitted_diagnostics = Default::default(); + *stashed_diagnostics = Default::default(); + *future_breakage_diagnostics = Default::default(); + *check_unstable_expect_diagnostics = false; + *unstable_expect_diagnostics = Default::default(); + *fulfilled_expectations = Default::default(); } /// Stash a given diagnostic with the given `Span` and [`StashKey`] as the key.