Rollup merge of #118933 - nnethercote:cleanup-errors-even-more, r=compiler-errors

Cleanup errors handlers even more

A sequel to #118587.

r? `@compiler-errors`

3 files changed, 82 insertions, 69 deletions

diff --git a/compiler/rustc_errors/src/diagnostic_builder.rs b/compiler/rustc_errors/src/diagnostic_builder.rs
index b8bd86a72e4..315e47c0971 100644
--- a/compiler/rustc_errors/src/diagnostic_builder.rs
+++ b/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");
                 }
             }
diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs
index 3f257fdd9cf..379883a0c18 100644
--- a/compiler/rustc_errors/src/emitter.rs
+++ b/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);
         }
     }
 }
diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs
index daa8a7706eb..cf73c638d85 100644
--- a/compiler/rustc_errors/src/lib.rs
+++ b/compiler/rustc_errors/src/lib.rs
@@ -585,11 +585,6 @@ impl Handler {
         self
     }
 
-    pub fn treat_err_as_bug(mut self, treat_err_as_bug: NonZeroUsize) -> Self {
-        self.inner.get_mut().flags.treat_err_as_bug = Some(treat_err_as_bug);
-        self
-    }
-
     pub fn with_flags(mut self, flags: HandlerFlags) -> Self {
         self.inner.get_mut().flags = flags;
         self
@@ -727,7 +722,12 @@ impl Handler {
         self.inner.borrow_mut().emit_stashed_diagnostics()
     }
 
-    /// Construct a builder with the `msg` at the level appropriate for the specific `EmissionGuarantee`.
+    /// Construct a builder with the `msg` at the level appropriate for the
+    /// specific `EmissionGuarantee`.
+    ///
+    /// Note: this is necessary for `derive(Diagnostic)`, but shouldn't be used
+    /// outside of that. Instead use `struct_err`, `struct_warn`, etc., which
+    /// make the diagnostic kind clearer.
     #[rustc_lint_diagnostics]
     #[track_caller]
     pub fn struct_diagnostic<G: EmissionGuarantee>(
@@ -942,13 +942,23 @@ impl Handler {
         result
     }
 
-    /// Construct a builder at the `Error` level with the `msg`.
+    /// Construct a builder at the `Fatal` level with the `msg`.
     #[rustc_lint_diagnostics]
     #[track_caller]
     pub fn struct_fatal(&self, msg: impl Into<DiagnosticMessage>) -> DiagnosticBuilder<'_, !> {
         DiagnosticBuilder::new(self, Level::Fatal, msg)
     }
 
+    /// Construct a builder at the `Fatal` level with the `msg`, that doesn't abort.
+    #[rustc_lint_diagnostics]
+    #[track_caller]
+    pub fn struct_almost_fatal(
+        &self,
+        msg: impl Into<DiagnosticMessage>,
+    ) -> DiagnosticBuilder<'_, FatalError> {
+        DiagnosticBuilder::new(self, Level::Fatal, msg)
+    }
+
     /// Construct a builder at the `Help` level with the `msg`.
     #[rustc_lint_diagnostics]
     pub fn struct_help(&self, msg: impl Into<DiagnosticMessage>) -> DiagnosticBuilder<'_, ()> {
@@ -965,8 +975,7 @@ impl Handler {
     #[rustc_lint_diagnostics]
     #[track_caller]
     pub fn span_fatal(&self, span: impl Into<MultiSpan>, msg: impl Into<DiagnosticMessage>) -> ! {
-        self.emit_diag_at_span(Diagnostic::new(Fatal, msg), span);
-        FatalError.raise()
+        self.struct_span_fatal(span, msg).emit()
     }
 
     #[rustc_lint_diagnostics]
@@ -977,8 +986,7 @@ impl Handler {
         msg: impl Into<DiagnosticMessage>,
         code: DiagnosticId,
     ) -> ! {
-        self.emit_diag_at_span(Diagnostic::new_with_code(Fatal, Some(code), msg), span);
-        FatalError.raise()
+        self.struct_span_fatal_with_code(span, msg, code).emit()
     }
 
     #[rustc_lint_diagnostics]
@@ -988,7 +996,7 @@ impl Handler {
         span: impl Into<MultiSpan>,
         msg: impl Into<DiagnosticMessage>,
     ) -> ErrorGuaranteed {
-        self.emit_diag_at_span(Diagnostic::new(Error { lint: false }, msg), span).unwrap()
+        self.struct_span_err(span, msg).emit()
     }
 
     #[rustc_lint_diagnostics]
@@ -999,17 +1007,13 @@ impl Handler {
         msg: impl Into<DiagnosticMessage>,
         code: DiagnosticId,
     ) -> ErrorGuaranteed {
-        self.emit_diag_at_span(
-            Diagnostic::new_with_code(Error { lint: false }, Some(code), msg),
-            span,
-        )
-        .unwrap()
+        self.struct_span_err_with_code(span, msg, code).emit()
     }
 
     #[rustc_lint_diagnostics]
     #[track_caller]
     pub fn span_warn(&self, span: impl Into<MultiSpan>, msg: impl Into<DiagnosticMessage>) {
-        self.emit_diag_at_span(Diagnostic::new(Warning(None), msg), span);
+        self.struct_span_warn(span, msg).emit()
     }
 
     #[rustc_lint_diagnostics]
@@ -1020,10 +1024,10 @@ impl Handler {
         msg: impl Into<DiagnosticMessage>,
         code: DiagnosticId,
     ) {
-        self.emit_diag_at_span(Diagnostic::new_with_code(Warning(None), Some(code), msg), span);
+        self.struct_span_warn_with_code(span, msg, code).emit()
     }
 
-    pub fn span_bug(&self, span: impl Into<MultiSpan>, msg: impl Into<String>) -> ! {
+    pub fn span_bug(&self, span: impl Into<MultiSpan>, msg: impl Into<DiagnosticMessage>) -> ! {
         self.inner.borrow_mut().span_bug(span, msg)
     }
 
@@ -1035,7 +1039,7 @@ impl Handler {
     pub fn span_delayed_bug(
         &self,
         sp: impl Into<MultiSpan>,
-        msg: impl Into<String>,
+        msg: impl Into<DiagnosticMessage>,
     ) -> ErrorGuaranteed {
         let mut inner = self.inner.borrow_mut();
 
@@ -1046,11 +1050,11 @@ impl Handler {
             inner.err_count + inner.lint_err_count + inner.delayed_bug_count() + 1 >= c.get()
         }) {
             // FIXME: don't abort here if report_delayed_bugs is off
-            inner.span_bug(sp, msg.into());
+            inner.span_bug(sp, msg);
         }
-        let mut diagnostic = Diagnostic::new(Level::DelayedBug, msg.into());
-        diagnostic.set_span(sp.into());
-        inner.emit_diagnostic(&mut diagnostic).unwrap()
+        let mut diagnostic = Diagnostic::new(Level::DelayedBug, msg);
+        diagnostic.set_span(sp);
+        inner.emit_diagnostic(diagnostic).unwrap()
     }
 
     // FIXME(eddyb) note the comment inside `impl Drop for HandlerInner`, that's
@@ -1060,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));
@@ -1068,20 +1072,22 @@ impl Handler {
 
     #[track_caller]
     pub fn span_bug_no_panic(&self, span: impl Into<MultiSpan>, msg: impl Into<DiagnosticMessage>) {
-        self.emit_diag_at_span(Diagnostic::new(Bug, msg), span);
+        let mut diag = Diagnostic::new(Bug, msg);
+        diag.set_span(span);
+        self.emit_diagnostic(diag);
     }
 
     #[track_caller]
     #[rustc_lint_diagnostics]
     pub fn span_note(&self, span: impl Into<MultiSpan>, msg: impl Into<DiagnosticMessage>) {
-        self.emit_diag_at_span(Diagnostic::new(Note, msg), span);
+        self.struct_span_note(span, msg).emit()
     }
 
     #[track_caller]
     #[rustc_lint_diagnostics]
     pub fn struct_span_note(
         &self,
-        span: Span,
+        span: impl Into<MultiSpan>,
         msg: impl Into<DiagnosticMessage>,
     ) -> DiagnosticBuilder<'_, ()> {
         let mut db = DiagnosticBuilder::new(self, Note, msg);
@@ -1091,22 +1097,22 @@ impl Handler {
 
     #[rustc_lint_diagnostics]
     pub fn fatal(&self, msg: impl Into<DiagnosticMessage>) -> ! {
-        DiagnosticBuilder::<FatalError>::new(self, Fatal, msg).emit().raise()
+        self.struct_fatal(msg).emit()
     }
 
     #[rustc_lint_diagnostics]
     pub fn err(&self, msg: impl Into<DiagnosticMessage>) -> ErrorGuaranteed {
-        DiagnosticBuilder::<ErrorGuaranteed>::new(self, Error { lint: false }, msg).emit()
+        self.struct_err(msg).emit()
     }
 
     #[rustc_lint_diagnostics]
     pub fn warn(&self, msg: impl Into<DiagnosticMessage>) {
-        DiagnosticBuilder::<()>::new(self, Warning(None), msg).emit();
+        self.struct_warn(msg).emit()
     }
 
     #[rustc_lint_diagnostics]
     pub fn note(&self, msg: impl Into<DiagnosticMessage>) {
-        DiagnosticBuilder::<()>::new(self, Note, msg).emit();
+        self.struct_note(msg).emit()
     }
 
     pub fn bug(&self, msg: impl Into<DiagnosticMessage>) -> ! {
@@ -1181,10 +1187,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}")));
             }
         }
 
@@ -1251,8 +1257,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 {
@@ -1327,14 +1342,6 @@ impl Handler {
         note.into_diagnostic(self)
     }
 
-    fn emit_diag_at_span(
-        &self,
-        mut diag: Diagnostic,
-        sp: impl Into<MultiSpan>,
-    ) -> Option<ErrorGuaranteed> {
-        self.emit_diagnostic(diag.set_span(sp))
-    }
-
     pub fn emit_artifact_notification(&self, path: &Path, artifact_type: &str) {
         self.inner.borrow_mut().emitter.emit_artifact_notification(path, artifact_type);
     }
@@ -1374,7 +1381,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);
             }
         }
 
@@ -1416,7 +1423,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 }) {
@@ -1436,14 +1443,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;
@@ -1579,17 +1592,15 @@ impl HandlerInner {
     }
 
     #[track_caller]
-    fn span_bug(&mut self, sp: impl Into<MultiSpan>, msg: impl Into<String>) -> ! {
-        self.emit_diag_at_span(Diagnostic::new(Bug, msg.into()), sp);
+    fn span_bug(&mut self, sp: impl Into<MultiSpan>, msg: impl Into<DiagnosticMessage>) -> ! {
+        let mut diag = Diagnostic::new(Bug, msg);
+        diag.set_span(sp);
+        self.emit_diagnostic(diag);
         panic::panic_any(ExplicitBug);
     }
 
-    fn emit_diag_at_span(&mut self, mut diag: Diagnostic, sp: impl Into<MultiSpan>) {
-        self.emit_diagnostic(diag.set_span(sp));
-    }
-
     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(
@@ -1621,7 +1632,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;
             }
 
@@ -1636,7 +1647,7 @@ impl HandlerInner {
             }
             bug.level = Level::Bug;
 
-            self.emit_diagnostic(&mut bug);
+            self.emit_diagnostic(bug);
         }
 
         // Panic with `DelayedBugPanic` to avoid "unexpected panic" messages.