Auto merge of #119606 - nnethercote:consuming-emit, r=oli-obk

Consuming `emit`

This PR makes `DiagnosticBuilder::emit` consuming, i.e. take `self` instead of `&mut self`. This is good because it doesn't make sense to emit a diagnostic twice.

This requires some changes to `DiagnosticBuilder` method changing -- every existing non-consuming chaining method gets a new consuming partner with a `_mv` suffix -- but permits a host of beneficial follow-up changes: more concise code through more chaining, removal of redundant diagnostic construction API methods, and removal of machinery to track the possibility of a diagnostic being emitted multiple times.

r? `@compiler-errors`

1 files changed, 178 insertions, 244 deletions

diff --git a/compiler/rustc_errors/src/diagnostic_builder.rs b/compiler/rustc_errors/src/diagnostic_builder.rs
index e018c14a4a5..e72791d89a2 100644
--- a/compiler/rustc_errors/src/diagnostic_builder.rs
+++ b/compiler/rustc_errors/src/diagnostic_builder.rs
@@ -30,57 +30,44 @@ where
     G: EmissionGuarantee,
 {
     fn into_diagnostic(self, dcx: &'a DiagCtxt, level: Level) -> DiagnosticBuilder<'a, G> {
-        let mut diag = self.node.into_diagnostic(dcx, level);
-        diag.span(self.span);
-        diag
+        self.node.into_diagnostic(dcx, level).span_mv(self.span)
     }
 }
 
 /// Used for emitting structured error messages and other diagnostic information.
+/// Each constructed `DiagnosticBuilder` must be consumed by a function such as
+/// `emit`, `cancel`, `delay_as_bug`, or `into_diagnostic`. A panic occurrs if a
+/// `DiagnosticBuilder` is dropped without being consumed by one of these
+/// functions.
 ///
 /// If there is some state in a downstream crate you would like to
 /// access in the methods of `DiagnosticBuilder` here, consider
 /// extending `DiagCtxtFlags`.
 #[must_use]
-#[derive(Clone)]
 pub struct DiagnosticBuilder<'a, G: EmissionGuarantee = ErrorGuaranteed> {
-    state: DiagnosticBuilderState<'a>,
+    pub dcx: &'a DiagCtxt,
 
-    /// `Diagnostic` is a large type, and `DiagnosticBuilder` is often used as a
-    /// return value, especially within the frequently-used `PResult` type.
-    /// In theory, return value optimization (RVO) should avoid unnecessary
-    /// copying. In practice, it does not (at the time of writing).
-    diagnostic: Box<Diagnostic>,
+    /// Why the `Option`? It is always `Some` until the `DiagnosticBuilder` is
+    /// consumed via `emit`, `cancel`, etc. At that point it is consumed and
+    /// replaced with `None`. Then `drop` checks that it is `None`; if not, it
+    /// panics because a diagnostic was built but not used.
+    ///
+    /// Why the Box? `Diagnostic` is a large type, and `DiagnosticBuilder` is
+    /// often used as a return value, especially within the frequently-used
+    /// `PResult` type. In theory, return value optimization (RVO) should avoid
+    /// unnecessary copying. In practice, it does not (at the time of writing).
+    diag: Option<Box<Diagnostic>>,
 
     _marker: PhantomData<G>,
 }
 
-#[derive(Clone)]
-enum DiagnosticBuilderState<'a> {
-    /// Initial state of a `DiagnosticBuilder`, before `.emit()` or `.cancel()`.
-    ///
-    /// The `Diagnostic` will be emitted through this `DiagCtxt`.
-    Emittable(&'a DiagCtxt),
+// Cloning a `DiagnosticBuilder` is a recipe for a diagnostic being emitted
+// twice, which would be bad.
+impl<G> !Clone for DiagnosticBuilder<'_, G> {}
 
-    /// State of a `DiagnosticBuilder`, after `.emit()` or *during* `.cancel()`.
-    ///
-    /// The `Diagnostic` will be ignored when calling `.emit()`, and it can be
-    /// assumed that `.emit()` was previously called, to end up in this state.
-    ///
-    /// While this is also used by `.cancel()`, this state is only observed by
-    /// the `Drop` `impl` of `DiagnosticBuilder`, because `.cancel()` takes
-    /// `self` by-value specifically to prevent any attempts to `.emit()`.
-    ///
-    // FIXME(eddyb) currently this doesn't prevent extending the `Diagnostic`,
-    // despite that being potentially lossy, if important information is added
-    // *after* the original `.emit()` call.
-    AlreadyEmittedOrDuringCancellation,
-}
-
-// `DiagnosticBuilderState` should be pointer-sized.
 rustc_data_structures::static_assert_size!(
-    DiagnosticBuilderState<'_>,
-    std::mem::size_of::<&DiagCtxt>()
+    DiagnosticBuilder<'_, ()>,
+    2 * std::mem::size_of::<usize>()
 );
 
 /// Trait for types that `DiagnosticBuilder::emit` can return as a "guarantee"
@@ -94,68 +81,50 @@ pub trait EmissionGuarantee: Sized {
     /// `impl` of `EmissionGuarantee`, to make it impossible to create a value
     /// of `Self::EmitResult` without actually performing the emission.
     #[track_caller]
-    fn emit_producing_guarantee(db: &mut DiagnosticBuilder<'_, Self>) -> Self::EmitResult;
+    fn emit_producing_guarantee(db: DiagnosticBuilder<'_, Self>) -> Self::EmitResult;
 }
 
 impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> {
+    /// Takes the diagnostic. For use by methods that consume the
+    /// DiagnosticBuilder: `emit`, `cancel`, etc. Afterwards, `drop` is the
+    /// only code that will be run on `self`.
+    fn take_diag(&mut self) -> Diagnostic {
+        Box::into_inner(self.diag.take().unwrap())
+    }
+
     /// Most `emit_producing_guarantee` functions use this as a starting point.
-    fn emit_producing_nothing(&mut self) {
-        match self.state {
-            // First `.emit()` call, the `&DiagCtxt` is still available.
-            DiagnosticBuilderState::Emittable(dcx) => {
-                self.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation;
-                dcx.emit_diagnostic_without_consuming(&mut self.diagnostic);
-            }
-            // `.emit()` was previously called, disallowed from repeating it.
-            DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => {}
-        }
+    fn emit_producing_nothing(mut self) {
+        let diag = self.take_diag();
+        self.dcx.emit_diagnostic(diag);
+    }
+
+    /// `ErrorGuaranteed::emit_producing_guarantee` uses this.
+    // FIXME(eddyb) make `ErrorGuaranteed` impossible to create outside `.emit()`.
+    fn emit_producing_error_guaranteed(mut self) -> ErrorGuaranteed {
+        let diag = self.take_diag();
+
+        // Only allow a guarantee if the `level` wasn't switched to a
+        // non-error. The field isn't `pub`, but the whole `Diagnostic` can be
+        // overwritten with a new one, thanks to `DerefMut`.
+        assert!(
+            diag.is_error(),
+            "emitted non-error ({:?}) diagnostic from `DiagnosticBuilder<ErrorGuaranteed>`",
+            diag.level,
+        );
+
+        let guar = self.dcx.emit_diagnostic(diag);
+        guar.unwrap()
     }
 }
 
-// FIXME(eddyb) make `ErrorGuaranteed` impossible to create outside `.emit()`.
 impl EmissionGuarantee for ErrorGuaranteed {
-    fn emit_producing_guarantee(db: &mut DiagnosticBuilder<'_, Self>) -> Self::EmitResult {
-        // Contrast this with `emit_producing_nothing`.
-        match db.state {
-            // First `.emit()` call, the `&DiagCtxt` is still available.
-            DiagnosticBuilderState::Emittable(dcx) => {
-                db.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation;
-                let guar = dcx.emit_diagnostic_without_consuming(&mut db.diagnostic);
-
-                // Only allow a guarantee if the `level` wasn't switched to a
-                // non-error - the field isn't `pub`, but the whole `Diagnostic`
-                // can be overwritten with a new one, thanks to `DerefMut`.
-                assert!(
-                    db.diagnostic.is_error(),
-                    "emitted non-error ({:?}) diagnostic \
-                     from `DiagnosticBuilder<ErrorGuaranteed>`",
-                    db.diagnostic.level,
-                );
-                guar.unwrap()
-            }
-            // `.emit()` was previously called, disallowed from repeating it,
-            // but can take advantage of the previous `.emit()`'s guarantee
-            // still being applicable (i.e. as a form of idempotency).
-            DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => {
-                // Only allow a guarantee if the `level` wasn't switched to a
-                // non-error - the field isn't `pub`, but the whole `Diagnostic`
-                // can be overwritten with a new one, thanks to `DerefMut`.
-                assert!(
-                    db.diagnostic.is_error(),
-                    "`DiagnosticBuilder<ErrorGuaranteed>`'s diagnostic \
-                     became non-error ({:?}), after original `.emit()`",
-                    db.diagnostic.level,
-                );
-                #[allow(deprecated)]
-                ErrorGuaranteed::unchecked_claim_error_was_emitted()
-            }
-        }
+    fn emit_producing_guarantee(db: DiagnosticBuilder<'_, Self>) -> Self::EmitResult {
+        db.emit_producing_error_guaranteed()
     }
 }
 
-// FIXME(eddyb) should there be a `Option<ErrorGuaranteed>` impl as well?
 impl EmissionGuarantee for () {
-    fn emit_producing_guarantee(db: &mut DiagnosticBuilder<'_, Self>) -> Self::EmitResult {
+    fn emit_producing_guarantee(db: DiagnosticBuilder<'_, Self>) -> Self::EmitResult {
         db.emit_producing_nothing();
     }
 }
@@ -168,7 +137,7 @@ pub struct BugAbort;
 impl EmissionGuarantee for BugAbort {
     type EmitResult = !;
 
-    fn emit_producing_guarantee(db: &mut DiagnosticBuilder<'_, Self>) -> Self::EmitResult {
+    fn emit_producing_guarantee(db: DiagnosticBuilder<'_, Self>) -> Self::EmitResult {
         db.emit_producing_nothing();
         panic::panic_any(ExplicitBug);
     }
@@ -182,37 +151,48 @@ pub struct FatalAbort;
 impl EmissionGuarantee for FatalAbort {
     type EmitResult = !;
 
-    fn emit_producing_guarantee(db: &mut DiagnosticBuilder<'_, Self>) -> Self::EmitResult {
+    fn emit_producing_guarantee(db: DiagnosticBuilder<'_, Self>) -> Self::EmitResult {
         db.emit_producing_nothing();
         crate::FatalError.raise()
     }
 }
 
 impl EmissionGuarantee for rustc_span::fatal_error::FatalError {
-    fn emit_producing_guarantee(db: &mut DiagnosticBuilder<'_, Self>) -> Self::EmitResult {
+    fn emit_producing_guarantee(db: DiagnosticBuilder<'_, Self>) -> Self::EmitResult {
         db.emit_producing_nothing();
         rustc_span::fatal_error::FatalError
     }
 }
 
-/// In general, the `DiagnosticBuilder` uses deref to allow access to
-/// the fields and methods of the embedded `diagnostic` in a
-/// transparent way. *However,* many of the methods are intended to
-/// be used in a chained way, and hence ought to return `self`. In
-/// that case, we can't just naively forward to the method on the
-/// `diagnostic`, because the return type would be a `&Diagnostic`
-/// instead of a `&DiagnosticBuilder<'a>`. This `forward!` macro makes
-/// it easy to declare such methods on the builder.
+/// `DiagnosticBuilder` impls `DerefMut`, which allows access to the fields and
+/// methods of the embedded `Diagnostic`. However, that doesn't allow method
+/// chaining at the `DiagnosticBuilder` level. Each use of this macro defines
+/// two builder methods at that level, both of which wrap the equivalent method
+/// in `Diagnostic`.
+/// - A `&mut self -> &mut Self` method, with the same name as the underlying
+///   `Diagnostic` method. It is mostly to modify existing diagnostics, either
+///   in a standalone fashion, e.g. `err.code(code)`, or in a chained fashion
+///   to make multiple modifications, e.g. `err.code(code).span(span)`.
+/// - A `self -> Self` method, with `_mv` suffix added (short for "move").
+///   It is mostly used in a chained fashion when producing a new diagnostic,
+///   e.g. `let err = struct_err(msg).code_mv(code)`, or when emitting a new
+///   diagnostic , e.g. `struct_err(msg).code_mv(code).emit()`.
+///
+/// Although the latter method can be used to modify an existing diagnostic,
+/// e.g. `err = err.code_mv(code)`, this should be avoided because the former
+/// method give shorter code, e.g. `err.code(code)`.
 macro_rules! forward {
-    // Forward pattern for &mut self -> &mut Self
     (
-        $(#[$attrs:meta])*
-        pub fn $n:ident(&mut self $(, $name:ident: $ty:ty)* $(,)?) -> &mut Self
+        ($n:ident, $n_mv:ident)($($name:ident: $ty:ty),* $(,)?)
     ) => {
-        $(#[$attrs])*
         #[doc = concat!("See [`Diagnostic::", stringify!($n), "()`].")]
-        pub fn $n(&mut self $(, $name: $ty)*) -> &mut Self {
-            self.diagnostic.$n($($name),*);
+        pub fn $n(&mut self, $($name: $ty),*) -> &mut Self {
+            self.diag.as_mut().unwrap().$n($($name),*);
+            self
+        }
+        #[doc = concat!("See [`Diagnostic::", stringify!($n), "()`].")]
+        pub fn $n_mv(mut self, $($name: $ty),*) -> Self {
+            self.diag.as_mut().unwrap().$n($($name),*);
             self
         }
     };
@@ -222,13 +202,13 @@ impl<G: EmissionGuarantee> Deref for DiagnosticBuilder<'_, G> {
     type Target = Diagnostic;
 
     fn deref(&self) -> &Diagnostic {
-        &self.diagnostic
+        self.diag.as_ref().unwrap()
     }
 }
 
 impl<G: EmissionGuarantee> DerefMut for DiagnosticBuilder<'_, G> {
     fn deref_mut(&mut self) -> &mut Diagnostic {
-        &mut self.diagnostic
+        self.diag.as_mut().unwrap()
     }
 }
 
@@ -242,20 +222,14 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> {
     /// Creates a new `DiagnosticBuilder` with an already constructed
     /// diagnostic.
     #[track_caller]
-    pub(crate) fn new_diagnostic(dcx: &'a DiagCtxt, diagnostic: Diagnostic) -> Self {
+    pub(crate) fn new_diagnostic(dcx: &'a DiagCtxt, diag: Diagnostic) -> Self {
         debug!("Created new diagnostic");
-        Self {
-            state: DiagnosticBuilderState::Emittable(dcx),
-            diagnostic: Box::new(diagnostic),
-            _marker: PhantomData,
-        }
+        Self { dcx, diag: Some(Box::new(diag)), _marker: PhantomData }
     }
 
-    /// 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`.
+    /// Emit and consume the diagnostic.
     #[track_caller]
-    pub fn emit(&mut self) -> G::EmitResult {
+    pub fn emit(self) -> G::EmitResult {
         G::emit_producing_guarantee(self)
     }
 
@@ -264,21 +238,17 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> {
     ///
     /// See `emit` and `delay_as_bug` for details.
     #[track_caller]
-    pub fn emit_unless(&mut self, delay: bool) -> G::EmitResult {
+    pub fn emit_unless(mut self, delay: bool) -> G::EmitResult {
         if delay {
             self.downgrade_to_delayed_bug();
         }
         self.emit()
     }
 
-    /// Cancel the diagnostic (a structured diagnostic must either be emitted or
+    /// Cancel and consume the diagnostic. (A diagnostic must either be emitted or
     /// cancelled or it will panic when dropped).
-    ///
-    /// This method takes `self` by-value to disallow calling `.emit()` on it,
-    /// which may be expected to *guarantee* the emission of an error, either
-    /// at the time of the call, or through a prior `.emit()` call.
     pub fn cancel(mut self) {
-        self.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation;
+        self.diag = None;
         drop(self);
     }
 
@@ -294,44 +264,21 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> {
     }
 
     /// Converts the builder to a `Diagnostic` for later emission,
-    /// unless dcx has disabled such buffering, or `.emit()` was called.
+    /// unless dcx has disabled such buffering.
     pub fn into_diagnostic(mut self) -> Option<(Diagnostic, &'a DiagCtxt)> {
-        let dcx = match self.state {
-            // No `.emit()` calls, the `&DiagCtxt` is still available.
-            DiagnosticBuilderState::Emittable(dcx) => dcx,
-            // `.emit()` was previously called, nothing we can do.
-            DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => {
-                return None;
-            }
-        };
-
-        if dcx.inner.lock().flags.dont_buffer_diagnostics
-            || dcx.inner.lock().flags.treat_err_as_bug.is_some()
-        {
+        let flags = self.dcx.inner.lock().flags;
+        if flags.dont_buffer_diagnostics || flags.treat_err_as_bug.is_some() {
             self.emit();
             return None;
         }
 
-        // Take the `Diagnostic` by replacing it with a dummy.
-        let dummy = Diagnostic::new(Level::Allow, DiagnosticMessage::from(""));
-        let diagnostic = std::mem::replace(&mut *self.diagnostic, dummy);
-
-        // Disable the ICE on `Drop`.
-        self.cancel();
+        let diag = self.take_diag();
 
         // Logging here is useful to help track down where in logs an error was
         // actually emitted.
-        debug!("buffer: diagnostic={:?}", diagnostic);
+        debug!("buffer: diag={:?}", diag);
 
-        Some((diagnostic, dcx))
-    }
-
-    /// Retrieves the [`DiagCtxt`] if available
-    pub fn dcx(&self) -> Option<&DiagCtxt> {
-        match self.state {
-            DiagnosticBuilderState::Emittable(dcx) => Some(dcx),
-            DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => None,
-        }
+        Some((diag, self.dcx))
     }
 
     /// Buffers the diagnostic for later emission,
@@ -351,170 +298,157 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> {
     /// In the meantime, though, callsites are required to deal with the "bug"
     /// locally in whichever way makes the most sense.
     #[track_caller]
-    pub fn delay_as_bug(&mut self) -> G::EmitResult {
+    pub fn delay_as_bug(mut self) -> G::EmitResult {
         self.downgrade_to_delayed_bug();
         self.emit()
     }
 
-    forward!(pub fn span_label(
-        &mut self,
+    forward!((span_label, span_label_mv)(
         span: Span,
-        label: impl Into<SubdiagnosticMessage>
-    ) -> &mut Self);
-    forward!(pub fn span_labels(
-        &mut self,
+        label: impl Into<SubdiagnosticMessage>,
+    ));
+    forward!((span_labels, span_labels_mv)(
         spans: impl IntoIterator<Item = Span>,
         label: &str,
-    ) -> &mut Self);
-    forward!(pub fn note_expected_found(
-        &mut self,
+    ));
+    forward!((note_expected_found, note_expected_found_mv)(
         expected_label: &dyn fmt::Display,
         expected: DiagnosticStyledString,
         found_label: &dyn fmt::Display,
         found: DiagnosticStyledString,
-    ) -> &mut Self);
-    forward!(pub fn note_expected_found_extra(
-        &mut self,
+    ));
+    forward!((note_expected_found_extra, note_expected_found_extra_mv)(
         expected_label: &dyn fmt::Display,
         expected: DiagnosticStyledString,
         found_label: &dyn fmt::Display,
         found: DiagnosticStyledString,
         expected_extra: &dyn fmt::Display,
         found_extra: &dyn fmt::Display,
-    ) -> &mut Self);
-    forward!(pub fn note(&mut self, msg: impl Into<SubdiagnosticMessage>) -> &mut Self);
-    forward!(pub fn note_once(&mut self, msg: impl Into<SubdiagnosticMessage>) -> &mut Self);
-    forward!(pub fn span_note(
-        &mut self,
+    ));
+    forward!((note, note_mv)(
+        msg: impl Into<SubdiagnosticMessage>,
+    ));
+    forward!((note_once, note_once_mv)(
+        msg: impl Into<SubdiagnosticMessage>,
+    ));
+    forward!((span_note, span_note_mv)(
         sp: impl Into<MultiSpan>,
         msg: impl Into<SubdiagnosticMessage>,
-    ) -> &mut Self);
-    forward!(pub fn span_note_once(
-        &mut self,
+    ));
+    forward!((span_note_once, span_note_once_mv)(
         sp: impl Into<MultiSpan>,
         msg: impl Into<SubdiagnosticMessage>,
-    ) -> &mut Self);
-    forward!(pub fn warn(&mut self, msg: impl Into<SubdiagnosticMessage>) -> &mut Self);
-    forward!(pub fn span_warn(
-        &mut self,
+    ));
+    forward!((warn, warn_mv)(
+        msg: impl Into<SubdiagnosticMessage>,
+    ));
+    forward!((span_warn, span_warn_mv)(
         sp: impl Into<MultiSpan>,
         msg: impl Into<SubdiagnosticMessage>,
-    ) -> &mut Self);
-    forward!(pub fn help(&mut self, msg: impl Into<SubdiagnosticMessage>) -> &mut Self);
-    forward!(pub fn help_once(&mut self, msg: impl Into<SubdiagnosticMessage>) -> &mut Self);
-    forward!(pub fn span_help(
-        &mut self,
+    ));
+    forward!((help, help_mv)(
+        msg: impl Into<SubdiagnosticMessage>,
+    ));
+    forward!((help_once, help_once_mv)(
+        msg: impl Into<SubdiagnosticMessage>,
+    ));
+    forward!((span_help, span_help_once_mv)(
         sp: impl Into<MultiSpan>,
         msg: impl Into<SubdiagnosticMessage>,
-    ) -> &mut Self);
-    forward!(pub fn is_lint(&mut self) -> &mut Self);
-    forward!(pub fn disable_suggestions(&mut self) -> &mut Self);
-    forward!(pub fn multipart_suggestion(
-        &mut self,
+    ));
+    forward!((multipart_suggestion, multipart_suggestion_mv)(
         msg: impl Into<SubdiagnosticMessage>,
         suggestion: Vec<(Span, String)>,
         applicability: Applicability,
-    ) -> &mut Self);
-    forward!(pub fn multipart_suggestion_verbose(
-        &mut self,
+    ));
+    forward!((multipart_suggestion_verbose, multipart_suggestion_verbose_mv)(
         msg: impl Into<SubdiagnosticMessage>,
         suggestion: Vec<(Span, String)>,
         applicability: Applicability,
-    ) -> &mut Self);
-    forward!(pub fn tool_only_multipart_suggestion(
-        &mut self,
+    ));
+    forward!((tool_only_multipart_suggestion, tool_only_multipart_suggestion_mv)(
         msg: impl Into<SubdiagnosticMessage>,
         suggestion: Vec<(Span, String)>,
         applicability: Applicability,
-    ) -> &mut Self);
-    forward!(pub fn span_suggestion(
-        &mut self,
+    ));
+    forward!((span_suggestion, span_suggestion_mv)(
         sp: Span,
         msg: impl Into<SubdiagnosticMessage>,
         suggestion: impl ToString,
         applicability: Applicability,
-    ) -> &mut Self);
-    forward!(pub fn span_suggestions(
-        &mut self,
+    ));
+    forward!((span_suggestions, span_suggestions_mv)(
         sp: Span,
         msg: impl Into<SubdiagnosticMessage>,
         suggestions: impl IntoIterator<Item = String>,
         applicability: Applicability,
-    ) -> &mut Self);
-    forward!(pub fn multipart_suggestions(
-        &mut self,
+    ));
+    forward!((multipart_suggestions, multipart_suggestions_mv)(
         msg: impl Into<SubdiagnosticMessage>,
         suggestions: impl IntoIterator<Item = Vec<(Span, String)>>,
         applicability: Applicability,
-    ) -> &mut Self);
-    forward!(pub fn span_suggestion_short(
-        &mut self,
+    ));
+    forward!((span_suggestion_short, span_suggestion_short_mv)(
         sp: Span,
         msg: impl Into<SubdiagnosticMessage>,
         suggestion: impl ToString,
         applicability: Applicability,
-    ) -> &mut Self);
-    forward!(pub fn span_suggestion_verbose(
-        &mut self,
+    ));
+    forward!((span_suggestion_verbose, span_suggestion_verbose_mv)(
         sp: Span,
         msg: impl Into<SubdiagnosticMessage>,
         suggestion: impl ToString,
         applicability: Applicability,
-    ) -> &mut Self);
-    forward!(pub fn span_suggestion_hidden(
-        &mut self,
+    ));
+    forward!((span_suggestion_hidden, span_suggestion_hidden_mv)(
         sp: Span,
         msg: impl Into<SubdiagnosticMessage>,
         suggestion: impl ToString,
         applicability: Applicability,
-    ) -> &mut Self);
-    forward!(pub fn tool_only_span_suggestion(
-        &mut self,
+    ));
+    forward!((tool_only_span_suggestion, tool_only_span_suggestion_mv)(
         sp: Span,
         msg: impl Into<SubdiagnosticMessage>,
         suggestion: impl ToString,
         applicability: Applicability,
-    ) -> &mut Self);
-    forward!(pub fn primary_message(&mut self, msg: impl Into<DiagnosticMessage>) -> &mut Self);
-    forward!(pub fn span(&mut self, sp: impl Into<MultiSpan>) -> &mut Self);
-    forward!(pub fn code(&mut self, s: DiagnosticId) -> &mut Self);
-    forward!(pub fn arg(
-        &mut self,
-        name: impl Into<Cow<'static, str>>,
-        arg: impl IntoDiagnosticArg,
-    ) -> &mut Self);
-    forward!(pub fn subdiagnostic(
-        &mut self,
-        subdiagnostic: impl crate::AddToDiagnostic
-    ) -> &mut Self);
+    ));
+    forward!((primary_message, primary_message_mv)(
+        msg: impl Into<DiagnosticMessage>,
+    ));
+    forward!((span, span_mv)(
+        sp: impl Into<MultiSpan>,
+    ));
+    forward!((code, code_mv)(
+        s: DiagnosticId,
+    ));
+    forward!((arg, arg_mv)(
+        name: impl Into<Cow<'static, str>>, arg: impl IntoDiagnosticArg,
+    ));
+    forward!((subdiagnostic, subdiagnostic_mv)(
+        subdiagnostic: impl crate::AddToDiagnostic,
+    ));
 }
 
 impl<G: EmissionGuarantee> Debug for DiagnosticBuilder<'_, G> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.diagnostic.fmt(f)
+        self.diag.fmt(f)
     }
 }
 
-/// Destructor bomb - a `DiagnosticBuilder` must be either emitted or cancelled
-/// or we emit a bug.
+/// Destructor bomb: every `DiagnosticBuilder` must be consumed (emitted,
+/// cancelled, etc.) or we emit a bug.
 impl<G: EmissionGuarantee> Drop for DiagnosticBuilder<'_, G> {
     fn drop(&mut self) {
-        match self.state {
-            // No `.emit()` or `.cancel()` calls.
-            DiagnosticBuilderState::Emittable(dcx) => {
-                if !panicking() {
-                    dcx.emit_diagnostic(Diagnostic::new(
-                        Level::Bug,
-                        DiagnosticMessage::from(
-                            "the following error was constructed but not emitted",
-                        ),
-                    ));
-                    dcx.emit_diagnostic_without_consuming(&mut self.diagnostic);
-                    panic!("error was constructed but not emitted");
-                }
+        match self.diag.take() {
+            Some(diag) if !panicking() => {
+                self.dcx.emit_diagnostic(Diagnostic::new(
+                    Level::Bug,
+                    DiagnosticMessage::from("the following error was constructed but not emitted"),
+                ));
+                self.dcx.emit_diagnostic(*diag);
+                panic!("error was constructed but not emitted");
             }
-            // `.emit()` was previously called, or maybe we're during `.cancel()`.
-            DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => {}
+            _ => {}
         }
     }
 }
@@ -522,11 +456,11 @@ impl<G: EmissionGuarantee> Drop for DiagnosticBuilder<'_, G> {
 #[macro_export]
 macro_rules! struct_span_err {
     ($dcx:expr, $span:expr, $code:ident, $($message:tt)*) => ({
-        $dcx.struct_span_err_with_code(
+        $dcx.struct_span_err(
             $span,
             format!($($message)*),
-            $crate::error_code!($code),
         )
+        .code_mv($crate::error_code!($code))
     })
 }