Split Handler::emit_diagnostic in two.

Currently, `emit_diagnostic` takes `&mut self`.

This commit changes it so `emit_diagnostic` takes `self` and the new
`emit_diagnostic_without_consuming` function takes `&mut self`.

I find the distinction useful. The former case is much more common, and
avoids a bunch of `mut` and `&mut` occurrences. We can also restrict the
latter with `pub(crate)` which is nice.

Author: nnethercote

compiler/rustc_borrowck/src/lib.rs

@@ -2502,8 +2502,8 @@ mod error {
             if !self.errors.buffered.is_empty() {
                 self.errors.buffered.sort_by_key(|diag| diag.sort_span);
 
-                for mut diag in self.errors.buffered.drain(..) {
-                    self.infcx.tcx.sess.diagnostic().emit_diagnostic(&mut diag);
+                for diag in self.errors.buffered.drain(..) {
+                    self.infcx.tcx.sess.diagnostic().emit_diagnostic(diag);
                 }
             }
 

compiler/rustc_codegen_ssa/src/back/write.rs

@@ -1848,7 +1848,7 @@ impl SharedEmitterMain {
                         d.code(code);
                     }
                     d.replace_args(diag.args);
-                    handler.emit_diagnostic(&mut d);
+                    handler.emit_diagnostic(d);
                 }
                 Ok(SharedEmitterMessage::InlineAsmError(cookie, msg, level, source)) => {
                     let msg = msg.strip_prefix("error: ").unwrap_or(&msg).to_string();

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

@@ -277,8 +277,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 mut error in secondary_errors {
-                self.tcx.sess.diagnostic().emit_diagnostic(&mut error);
+            for error in secondary_errors {
+                self.tcx.sess.diagnostic().emit_diagnostic(error);
             }
         } else {
             assert!(self.tcx.sess.has_errors().is_some());

compiler/rustc_errors/src/diagnostic_builder.rs

@@ -132,7 +132,7 @@ impl EmissionGuarantee for ErrorGuaranteed {
             DiagnosticBuilderState::Emittable(handler) => {
                 db.inner.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation;
 
-                let guar = handler.emit_diagnostic(&mut db.inner.diagnostic);
+                let guar = handler.emit_diagnostic_without_consuming(&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`
@@ -181,7 +181,7 @@ impl EmissionGuarantee for () {
             DiagnosticBuilderState::Emittable(handler) => {
                 db.inner.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation;
 
-                handler.emit_diagnostic(&mut db.inner.diagnostic);
+                handler.emit_diagnostic_without_consuming(&mut db.inner.diagnostic);
             }
             // `.emit()` was previously called, disallowed from repeating it.
             DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => {}
@@ -207,7 +207,7 @@ impl EmissionGuarantee for Noted {
             // First `.emit()` call, the `&Handler` is still available.
             DiagnosticBuilderState::Emittable(handler) => {
                 db.inner.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation;
-                handler.emit_diagnostic(&mut db.inner.diagnostic);
+                handler.emit_diagnostic_without_consuming(&mut db.inner.diagnostic);
             }
             // `.emit()` was previously called, disallowed from repeating it.
             DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => {}
@@ -236,7 +236,7 @@ impl EmissionGuarantee for Bug {
             DiagnosticBuilderState::Emittable(handler) => {
                 db.inner.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation;
 
-                handler.emit_diagnostic(&mut db.inner.diagnostic);
+                handler.emit_diagnostic_without_consuming(&mut db.inner.diagnostic);
             }
             // `.emit()` was previously called, disallowed from repeating it.
             DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => {}
@@ -260,7 +260,7 @@ impl EmissionGuarantee for ! {
             DiagnosticBuilderState::Emittable(handler) => {
                 db.inner.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation;
 
-                handler.emit_diagnostic(&mut db.inner.diagnostic);
+                handler.emit_diagnostic_without_consuming(&mut db.inner.diagnostic);
             }
             // `.emit()` was previously called, disallowed from repeating it.
             DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => {}
@@ -284,7 +284,7 @@ impl EmissionGuarantee for rustc_span::fatal_error::FatalError {
             DiagnosticBuilderState::Emittable(handler) => {
                 db.inner.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation;
 
-                handler.emit_diagnostic(&mut db.inner.diagnostic);
+                handler.emit_diagnostic_without_consuming(&mut db.inner.diagnostic);
             }
             // `.emit()` was previously called, disallowed from repeating it.
             DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => {}
@@ -365,7 +365,9 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> {
         }
     }
 
-    /// Emit the diagnostic.
+    /// Emit the diagnostic. Does not consume `self`, which may be surprising,
+    /// but there are various places that rely on continuing to use `self`
+    /// after calling `emit`.
     #[track_caller]
     pub fn emit(&mut self) -> G {
         G::diagnostic_builder_emit_producing_guarantee(self)
@@ -640,13 +642,13 @@ impl Drop for DiagnosticBuilderInner<'_> {
             // No `.emit()` or `.cancel()` calls.
             DiagnosticBuilderState::Emittable(handler) => {
                 if !panicking() {
-                    handler.emit_diagnostic(&mut Diagnostic::new(
+                    handler.emit_diagnostic(Diagnostic::new(
                         Level::Bug,
                         DiagnosticMessage::from(
                             "the following error was constructed but not emitted",
                         ),
                     ));
-                    handler.emit_diagnostic(&mut self.diagnostic);
+                    handler.emit_diagnostic_without_consuming(&mut self.diagnostic);
                     panic!("error was constructed but not emitted");
                 }
             }

compiler/rustc_errors/src/emitter.rs

@@ -581,7 +581,7 @@ impl Emitter for SilentEmitter {
             if let Some(ref note) = self.fatal_note {
                 d.note(note.clone());
             }
-            self.fatal_handler.emit_diagnostic(&mut d);
+            self.fatal_handler.emit_diagnostic(d);
         }
     }
 }

compiler/rustc_errors/src/lib.rs

@@ -1054,7 +1054,7 @@ impl Handler {
         }
         let mut diagnostic = Diagnostic::new(Level::DelayedBug, msg);
         diagnostic.set_span(sp);
-        inner.emit_diagnostic(&mut diagnostic).unwrap()
+        inner.emit_diagnostic(diagnostic).unwrap()
     }
 
     // FIXME(eddyb) note the comment inside `impl Drop for HandlerInner`, that's
@@ -1064,7 +1064,7 @@ impl Handler {
 
         let mut diagnostic = Diagnostic::new(Level::DelayedBug, msg);
         if inner.flags.report_delayed_bugs {
-            inner.emit_diagnostic(&mut diagnostic);
+            inner.emit_diagnostic_without_consuming(&mut diagnostic);
         }
         let backtrace = std::backtrace::Backtrace::capture();
         inner.good_path_delayed_bugs.push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace));
@@ -1185,10 +1185,10 @@ impl Handler {
                 DiagnosticMessage::Str(warnings),
             )),
             (_, 0) => {
-                inner.emit_diagnostic(&mut Diagnostic::new(Fatal, errors));
+                inner.emit_diagnostic(Diagnostic::new(Fatal, errors));
             }
             (_, _) => {
-                inner.emit_diagnostic(&mut Diagnostic::new(Fatal, format!("{errors}; {warnings}")));
+                inner.emit_diagnostic(Diagnostic::new(Fatal, format!("{errors}; {warnings}")));
             }
         }
 
@@ -1255,8 +1255,17 @@ impl Handler {
         self.inner.borrow_mut().emitter.emit_diagnostic(&db);
     }
 
-    pub fn emit_diagnostic(&self, diagnostic: &mut Diagnostic) -> Option<ErrorGuaranteed> {
-        self.inner.borrow_mut().emit_diagnostic(diagnostic)
+    pub fn emit_diagnostic(&self, mut diagnostic: Diagnostic) -> Option<ErrorGuaranteed> {
+        self.emit_diagnostic_without_consuming(&mut diagnostic)
+    }
+
+    // It's unfortunate this exists. `emit_diagnostic` is preferred, because it
+    // consumes the diagnostic, thus ensuring it is emitted just once.
+    pub(crate) fn emit_diagnostic_without_consuming(
+        &self,
+        diagnostic: &mut Diagnostic,
+    ) -> Option<ErrorGuaranteed> {
+        self.inner.borrow_mut().emit_diagnostic_without_consuming(diagnostic)
     }
 
     pub fn emit_err<'a>(&'a self, err: impl IntoDiagnostic<'a>) -> ErrorGuaranteed {
@@ -1370,7 +1379,7 @@ impl Handler {
                 // Here the diagnostic is given back to `emit_diagnostic` where it was first
                 // intercepted. Now it should be processed as usual, since the unstable expectation
                 // id is now stable.
-                inner.emit_diagnostic(&mut diag);
+                inner.emit_diagnostic(diag);
             }
         }
 
@@ -1412,7 +1421,7 @@ impl HandlerInner {
         let has_errors = self.has_errors();
         let diags = self.stashed_diagnostics.drain(..).map(|x| x.1).collect::<Vec<_>>();
         let mut reported = None;
-        for mut diag in diags {
+        for diag in diags {
             // Decrement the count tracking the stash; emitting will increment it.
             if diag.is_error() {
                 if matches!(diag.level, Level::Error { lint: true }) {
@@ -1432,14 +1441,20 @@ impl HandlerInner {
                     }
                 }
             }
-            let reported_this = self.emit_diagnostic(&mut diag);
+            let reported_this = self.emit_diagnostic(diag);
             reported = reported.or(reported_this);
         }
         reported
     }
 
-    // FIXME(eddyb) this should ideally take `diagnostic` by value.
-    fn emit_diagnostic(&mut self, diagnostic: &mut Diagnostic) -> Option<ErrorGuaranteed> {
+    fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option<ErrorGuaranteed> {
+        self.emit_diagnostic_without_consuming(&mut diagnostic)
+    }
+
+    fn emit_diagnostic_without_consuming(
+        &mut self,
+        diagnostic: &mut Diagnostic,
+    ) -> Option<ErrorGuaranteed> {
         if matches!(diagnostic.level, Level::Error { .. } | Level::Fatal) && self.treat_err_as_bug()
         {
             diagnostic.level = Level::Bug;
@@ -1576,12 +1591,14 @@ impl HandlerInner {
 
     #[track_caller]
     fn span_bug(&mut self, sp: impl Into<MultiSpan>, msg: impl Into<DiagnosticMessage>) -> ! {
-        self.emit_diagnostic(Diagnostic::new(Bug, msg).set_span(sp));
+        let mut diag = Diagnostic::new(Bug, msg);
+        diag.set_span(sp);
+        self.emit_diagnostic(diag);
         panic::panic_any(ExplicitBug);
     }
 
     fn failure_note(&mut self, msg: impl Into<DiagnosticMessage>) {
-        self.emit_diagnostic(&mut Diagnostic::new(FailureNote, msg));
+        self.emit_diagnostic(Diagnostic::new(FailureNote, msg));
     }
 
     fn flush_delayed(
@@ -1613,7 +1630,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(&mut Diagnostic::new(Bug, explanation));
+                self.emit_diagnostic(Diagnostic::new(Bug, explanation));
                 no_bugs = false;
             }
 
@@ -1628,7 +1645,7 @@ impl HandlerInner {
             }
             bug.level = Level::Bug;
 
-            self.emit_diagnostic(&mut bug);
+            self.emit_diagnostic(bug);
         }
 
         // Panic with `DelayedBugPanic` to avoid "unexpected panic" messages.

compiler/rustc_expand/src/proc_macro_server.rs

@@ -502,7 +502,7 @@ impl server::FreeFunctions for Rustc<'_, '_> {
                 None,
             );
         }
-        self.sess().span_diagnostic.emit_diagnostic(&mut diag);
+        self.sess().span_diagnostic.emit_diagnostic(diag);
     }
 }
 

compiler/rustc_hir_typeck/src/writeback.rs

@@ -504,8 +504,8 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
 
             if !errors_buffer.is_empty() {
                 errors_buffer.sort_by_key(|diag| diag.span.primary_span());
-                for mut diag in errors_buffer {
-                    self.tcx().sess.diagnostic().emit_diagnostic(&mut diag);
+                for diag in errors_buffer {
+                    self.tcx().sess.diagnostic().emit_diagnostic(diag);
                 }
             }
         }

compiler/rustc_parse/src/lib.rs

@@ -51,8 +51,8 @@ macro_rules! panictry_buffer {
         match $e {
             Ok(e) => e,
             Err(errs) => {
-                for mut e in errs {
-                    $handler.emit_diagnostic(&mut e);
+                for e in errs {
+                    $handler.emit_diagnostic(e);
                 }
                 FatalError.raise()
             }
@@ -165,8 +165,8 @@ fn try_file_to_source_file(
 fn file_to_source_file(sess: &ParseSess, path: &Path, spanopt: Option<Span>) -> Lrc<SourceFile> {
     match try_file_to_source_file(sess, path, spanopt) {
         Ok(source_file) => source_file,
-        Err(mut d) => {
-            sess.span_diagnostic.emit_diagnostic(&mut d);
+        Err(d) => {
+            sess.span_diagnostic.emit_diagnostic(d);
             FatalError.raise();
         }
     }

compiler/rustc_query_system/src/dep_graph/graph.rs

@@ -926,8 +926,8 @@ impl<D: Deps> DepGraphData<D> {
 
             let handle = qcx.dep_context().sess().diagnostic();
 
-            for mut diagnostic in side_effects.diagnostics {
-                handle.emit_diagnostic(&mut diagnostic);
+            for diagnostic in side_effects.diagnostics {
+                handle.emit_diagnostic(diagnostic);
             }
         }
     }

src/tools/miri/src/diagnostics.rs

@@ -512,7 +512,7 @@ pub fn report_msg<'tcx>(
         }
     }
 
-    handler.emit_diagnostic(&mut err);
+    handler.emit_diagnostic(err);
 }
 
 impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {

src/tools/rustfmt/src/parse/session.rs

@@ -284,10 +284,8 @@ impl ParseSess {
 // Methods that should be restricted within the parse module.
 impl ParseSess {
     pub(super) fn emit_diagnostics(&self, diagnostics: Vec<Diagnostic>) {
-        for mut diagnostic in diagnostics {
-            self.parse_sess
-                .span_diagnostic
-                .emit_diagnostic(&mut diagnostic);
+        for diagnostic in diagnostics {
+            self.parse_sess.span_diagnostic.emit_diagnostic(diagnostic);
         }
     }