Auto merge of rust-lang#95149 - cjgillot:once-diag, r=estebank

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.

Author: bors

compiler/rustc_borrowck/src/lib.rs

@@ -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);
                 }
             }
 

compiler/rustc_codegen_ssa/src/back/write.rs

@@ -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);

compiler/rustc_const_eval/src/transform/check_consts/check.rs

@@ -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());

compiler/rustc_driver/src/lib.rs

@@ -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![

compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs

@@ -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,

compiler/rustc_errors/src/diagnostic.rs

@@ -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,
         }
     }
 
@@ -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);

compiler/rustc_errors/src/diagnostic_builder.rs

@@ -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`
@@ -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 => {}
@@ -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);
@@ -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!();
                 }
             }

compiler/rustc_errors/src/emitter.rs

@@ -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);
         }
     }
 }

compiler/rustc_errors/src/lib.rs

@@ -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)]
@@ -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)
     }
 
@@ -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`
@@ -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 {
@@ -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 {
@@ -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);
     }
 
@@ -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;
             }
 
@@ -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.
@@ -1350,6 +1367,8 @@ pub enum Level {
     },
     Warning,
     Note,
+    /// A note that is only emitted once.
+    OnceNote,
     Help,
     FailureNote,
     Allow,
@@ -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 => {
@@ -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"),

compiler/rustc_expand/src/proc_macro_server.rs

@@ -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);
     }
 }