Skip to content

Commit

Permalink
Auto merge of rust-lang#95149 - cjgillot:once-diag, r=estebank
Browse files Browse the repository at this point in the history
Remove `Session::one_time_diagnostic`

This is untracked mutable state, which modified the behaviour of queries.
It was used for 2 things: some full-blown errors, but mostly for lint declaration notes ("the lint level is defined here" notes).

It is replaced by the diagnostic deduplication infra which already exists in the diagnostic emitter.
A new diagnostic level `OnceNote` is introduced specifically for lint notes, to deduplicate subdiagnostics.

As a drive-by, diagnostic emission takes a `&mut` to allow dropping the `SubDiagnostic`s.
  • Loading branch information
bors committed Mar 26, 2022
2 parents a2ebd5a + f7d5b7a commit c749254
Show file tree
Hide file tree
Showing 46 changed files with 227 additions and 359 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2361,8 +2361,8 @@ mod error {
if !self.errors.buffered.is_empty() {
self.errors.buffered.sort_by_key(|diag| diag.sort_span);

for diag in self.errors.buffered.drain(..) {
self.infcx.tcx.sess.diagnostic().emit_diagnostic(&diag);
for mut diag in self.errors.buffered.drain(..) {
self.infcx.tcx.sess.diagnostic().emit_diagnostic(&mut diag);
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1748,7 +1748,7 @@ impl SharedEmitterMain {
if let Some(code) = diag.code {
d.code(code);
}
handler.emit_diagnostic(&d);
handler.emit_diagnostic(&mut d);
}
Ok(SharedEmitterMessage::InlineAsmError(cookie, msg, level, source)) => {
let msg = msg.strip_prefix("error: ").unwrap_or(&msg);
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/transform/check_consts/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,8 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
// "secondary" errors if they occurred.
let secondary_errors = mem::take(&mut self.secondary_errors);
if self.error_emitted.is_none() {
for error in secondary_errors {
self.tcx.sess.diagnostic().emit_diagnostic(&error);
for mut error in secondary_errors {
self.tcx.sess.diagnostic().emit_diagnostic(&mut error);
}
} else {
assert!(self.tcx.sess.has_errors().is_some());
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1181,8 +1181,8 @@ pub fn report_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) {
// a .span_bug or .bug call has already printed what
// it wants to print.
if !info.payload().is::<rustc_errors::ExplicitBug>() {
let d = rustc_errors::Diagnostic::new(rustc_errors::Level::Bug, "unexpected panic");
handler.emit_diagnostic(&d);
let mut d = rustc_errors::Diagnostic::new(rustc_errors::Level::Bug, "unexpected panic");
handler.emit_diagnostic(&mut d);
}

let mut xs: Vec<Cow<'static, str>> = vec![
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ fn annotation_type_for_level(level: Level) -> AnnotationType {
AnnotationType::Error
}
Level::Warning => AnnotationType::Warning,
Level::Note => AnnotationType::Note,
Level::Note | Level::OnceNote => AnnotationType::Note,
Level::Help => AnnotationType::Help,
// FIXME(#59346): Not sure how to map this level
Level::FailureNote => AnnotationType::Error,
Expand Down
21 changes: 20 additions & 1 deletion compiler/rustc_errors/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,12 @@ impl Diagnostic {
| Level::Error { .. }
| Level::FailureNote => true,

Level::Warning | Level::Note | Level::Help | Level::Allow | Level::Expect(_) => false,
Level::Warning
| Level::Note
| Level::OnceNote
| Level::Help
| Level::Allow
| Level::Expect(_) => false,
}
}

Expand Down Expand Up @@ -333,13 +338,27 @@ impl Diagnostic {
self
}

/// Prints the span with a note above it.
/// This is like [`Diagnostic::note()`], but it gets its own span.
pub fn note_once(&mut self, msg: &str) -> &mut Self {
self.sub(Level::OnceNote, msg, MultiSpan::new(), None);
self
}

/// Prints the span with a note above it.
/// This is like [`Diagnostic::note()`], but it gets its own span.
pub fn span_note<S: Into<MultiSpan>>(&mut self, sp: S, msg: &str) -> &mut Self {
self.sub(Level::Note, msg, sp.into(), None);
self
}

/// Prints the span with a note above it.
/// This is like [`Diagnostic::note()`], but it gets its own span.
pub fn span_note_once<S: Into<MultiSpan>>(&mut self, sp: S, msg: &str) -> &mut Self {
self.sub(Level::OnceNote, msg, sp.into(), None);
self
}

/// Add a warning attached to this diagnostic.
pub fn warn(&mut self, msg: &str) -> &mut Self {
self.sub(Level::Warning, msg, MultiSpan::new(), None);
Expand Down
14 changes: 10 additions & 4 deletions compiler/rustc_errors/src/diagnostic_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ impl EmissionGuarantee for ErrorGuaranteed {
DiagnosticBuilderState::Emittable(handler) => {
db.inner.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation;

let guar = handler.emit_diagnostic(&db.inner.diagnostic);
let guar = handler.emit_diagnostic(&mut db.inner.diagnostic);

// Only allow a guarantee if the `level` wasn't switched to a
// non-error - the field isn't `pub`, but the whole `Diagnostic`
Expand Down Expand Up @@ -190,7 +190,7 @@ impl EmissionGuarantee for () {
DiagnosticBuilderState::Emittable(handler) => {
db.inner.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation;

handler.emit_diagnostic(&db.inner.diagnostic);
handler.emit_diagnostic(&mut db.inner.diagnostic);
}
// `.emit()` was previously called, disallowed from repeating it.
DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => {}
Expand Down Expand Up @@ -396,11 +396,17 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> {
) -> &mut Self);

forward!(pub fn note(&mut self, msg: &str) -> &mut Self);
forward!(pub fn note_once(&mut self, msg: &str) -> &mut Self);
forward!(pub fn span_note(
&mut self,
sp: impl Into<MultiSpan>,
msg: &str,
) -> &mut Self);
forward!(pub fn span_note_once(
&mut self,
sp: impl Into<MultiSpan>,
msg: &str,
) -> &mut Self);
forward!(pub fn warn(&mut self, msg: &str) -> &mut Self);
forward!(pub fn span_warn(&mut self, sp: impl Into<MultiSpan>, msg: &str) -> &mut Self);
forward!(pub fn help(&mut self, msg: &str) -> &mut Self);
Expand Down Expand Up @@ -500,11 +506,11 @@ impl Drop for DiagnosticBuilderInner<'_> {
// No `.emit()` or `.cancel()` calls.
DiagnosticBuilderState::Emittable(handler) => {
if !panicking() {
handler.emit_diagnostic(&Diagnostic::new(
handler.emit_diagnostic(&mut Diagnostic::new(
Level::Bug,
"the following error was constructed but not emitted",
));
handler.emit_diagnostic(&self.diagnostic);
handler.emit_diagnostic(&mut self.diagnostic);
panic!();
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ impl Emitter for SilentEmitter {
if let Some(ref note) = self.fatal_note {
d.note(note);
}
self.fatal_handler.emit_diagnostic(&d);
self.fatal_handler.emit_diagnostic(&mut d);
}
}
}
Expand Down
55 changes: 37 additions & 18 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]
#![feature(crate_visibility_modifier)]
#![feature(drain_filter)]
#![feature(backtrace)]
#![feature(if_let_guard)]
#![feature(let_else)]
Expand Down Expand Up @@ -919,7 +920,7 @@ impl Handler {
self.inner.borrow_mut().force_print_diagnostic(db)
}

pub fn emit_diagnostic(&self, diagnostic: &Diagnostic) -> Option<ErrorGuaranteed> {
pub fn emit_diagnostic(&self, diagnostic: &mut Diagnostic) -> Option<ErrorGuaranteed> {
self.inner.borrow_mut().emit_diagnostic(diagnostic)
}

Expand Down Expand Up @@ -993,25 +994,25 @@ impl HandlerInner {
self.taught_diagnostics.insert(code.clone())
}

fn force_print_diagnostic(&mut self, db: Diagnostic) {
self.emitter.emit_diagnostic(&db);
fn force_print_diagnostic(&mut self, mut db: Diagnostic) {
self.emitter.emit_diagnostic(&mut db);
}

/// Emit all stashed diagnostics.
fn emit_stashed_diagnostics(&mut self) -> Option<ErrorGuaranteed> {
let diags = self.stashed_diagnostics.drain(..).map(|x| x.1).collect::<Vec<_>>();
let mut reported = None;
diags.iter().for_each(|diag| {
for mut diag in diags {
if diag.is_error() {
reported = Some(ErrorGuaranteed(()));
}
self.emit_diagnostic(diag);
});
self.emit_diagnostic(&mut diag);
}
reported
}

// FIXME(eddyb) this should ideally take `diagnostic` by value.
fn emit_diagnostic(&mut self, diagnostic: &Diagnostic) -> Option<ErrorGuaranteed> {
fn emit_diagnostic(&mut self, diagnostic: &mut Diagnostic) -> Option<ErrorGuaranteed> {
if diagnostic.level == Level::DelayedBug {
// FIXME(eddyb) this should check for `has_errors` and stop pushing
// once *any* errors were emitted (and truncate `delayed_span_bugs`
Expand Down Expand Up @@ -1070,7 +1071,23 @@ impl HandlerInner {
// Only emit the diagnostic if we've been asked to deduplicate and
// haven't already emitted an equivalent diagnostic.
if !(self.flags.deduplicate_diagnostics && already_emitted(self)) {
self.emitter.emit_diagnostic(diagnostic);
debug!(?diagnostic);
debug!(?self.emitted_diagnostics);
let already_emitted_sub = |sub: &mut SubDiagnostic| {
debug!(?sub);
if sub.level != Level::OnceNote {
return false;
}
let mut hasher = StableHasher::new();
sub.hash(&mut hasher);
let diagnostic_hash = hasher.finish();
debug!(?diagnostic_hash);
!self.emitted_diagnostics.insert(diagnostic_hash)
};

diagnostic.children.drain_filter(already_emitted_sub).for_each(|_| {});

self.emitter.emit_diagnostic(&diagnostic);
if diagnostic.is_error() {
self.deduplicated_err_count += 1;
} else if diagnostic.level == Warning {
Expand Down Expand Up @@ -1221,22 +1238,22 @@ impl HandlerInner {
let mut diagnostic = Diagnostic::new(Level::DelayedBug, msg);
diagnostic.set_span(sp.into());
diagnostic.note(&format!("delayed at {}", std::panic::Location::caller()));
self.emit_diagnostic(&diagnostic).unwrap()
self.emit_diagnostic(&mut diagnostic).unwrap()
}

// FIXME(eddyb) note the comment inside `impl Drop for HandlerInner`, that's
// where the explanation of what "good path" is (also, it should be renamed).
fn delay_good_path_bug(&mut self, msg: &str) {
let diagnostic = Diagnostic::new(Level::DelayedBug, msg);
let mut diagnostic = Diagnostic::new(Level::DelayedBug, msg);
if self.flags.report_delayed_bugs {
self.emit_diagnostic(&diagnostic);
self.emit_diagnostic(&mut diagnostic);
}
let backtrace = std::backtrace::Backtrace::force_capture();
self.delayed_good_path_bugs.push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace));
}

fn failure(&mut self, msg: &str) {
self.emit_diagnostic(&Diagnostic::new(FailureNote, msg));
self.emit_diagnostic(&mut Diagnostic::new(FailureNote, msg));
}

fn fatal(&mut self, msg: &str) -> FatalError {
Expand All @@ -1253,11 +1270,11 @@ impl HandlerInner {
if self.treat_err_as_bug() {
self.bug(msg);
}
self.emit_diagnostic(&Diagnostic::new(level, msg)).unwrap()
self.emit_diagnostic(&mut Diagnostic::new(level, msg)).unwrap()
}

fn bug(&mut self, msg: &str) -> ! {
self.emit_diagnostic(&Diagnostic::new(Bug, msg));
self.emit_diagnostic(&mut Diagnostic::new(Bug, msg));
panic::panic_any(ExplicitBug);
}

Expand All @@ -1267,7 +1284,7 @@ impl HandlerInner {
if no_bugs {
// Put the overall explanation before the `DelayedBug`s, to
// frame them better (e.g. separate warnings from them).
self.emit_diagnostic(&Diagnostic::new(Bug, explanation));
self.emit_diagnostic(&mut Diagnostic::new(Bug, explanation));
no_bugs = false;
}

Expand All @@ -1283,7 +1300,7 @@ impl HandlerInner {
}
bug.level = Level::Bug;

self.emit_diagnostic(&bug);
self.emit_diagnostic(&mut bug);
}

// Panic with `ExplicitBug` to avoid "unexpected panic" messages.
Expand Down Expand Up @@ -1350,6 +1367,8 @@ pub enum Level {
},
Warning,
Note,
/// A note that is only emitted once.
OnceNote,
Help,
FailureNote,
Allow,
Expand All @@ -1372,7 +1391,7 @@ impl Level {
Warning => {
spec.set_fg(Some(Color::Yellow)).set_intense(cfg!(windows));
}
Note => {
Note | OnceNote => {
spec.set_fg(Some(Color::Green)).set_intense(true);
}
Help => {
Expand All @@ -1389,7 +1408,7 @@ impl Level {
Bug | DelayedBug => "error: internal compiler error",
Fatal | Error { .. } => "error",
Warning => "warning",
Note => "note",
Note | OnceNote => "note",
Help => "help",
FailureNote => "failure-note",
Allow => panic!("Shouldn't call on allowed error"),
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_expand/src/proc_macro_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -771,8 +771,8 @@ impl server::Diagnostic for Rustc<'_, '_> {
) {
diag.sub(level.to_internal(), msg, MultiSpan::from_spans(spans), None);
}
fn emit(&mut self, diag: Self::Diagnostic) {
self.sess().span_diagnostic.emit_diagnostic(&diag);
fn emit(&mut self, mut diag: Self::Diagnostic) {
self.sess().span_diagnostic.emit_diagnostic(&mut diag);
}
}

Expand Down
Loading

0 comments on commit c749254

Please sign in to comment.