Merge from rustc

2 files changed, 197 insertions, 91 deletions

diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs
index 01f36ad6a78..1c820bfd01f 100644
--- a/compiler/rustc_errors/src/diagnostic.rs
+++ b/compiler/rustc_errors/src/diagnostic.rs
@@ -108,6 +108,25 @@ impl EmissionGuarantee for rustc_span::fatal_error::FatalError {
 
 /// Trait implemented by error types. This is rarely implemented manually. Instead, use
 /// `#[derive(Diagnostic)]` -- see [rustc_macros::Diagnostic].
+///
+/// When implemented manually, it should be generic over the emission
+/// guarantee, i.e.:
+/// ```ignore (fragment)
+/// impl<'a, G: EmissionGuarantee> IntoDiagnostic<'a, G> for Foo { ... }
+/// ```
+/// rather than being specific:
+/// ```ignore (fragment)
+/// impl<'a> IntoDiagnostic<'a> for Bar { ... }  // the default type param is `ErrorGuaranteed`
+/// impl<'a> IntoDiagnostic<'a, ()> for Baz { ... }
+/// ```
+/// There are two reasons for this.
+/// - A diagnostic like `Foo` *could* be emitted at any level -- `level` is
+///   passed in to `into_diagnostic` from outside. Even if in practice it is
+///   always emitted at a single level, we let the diagnostic creation/emission
+///   site determine the level (by using `create_err`, `emit_warn`, etc.)
+///   rather than the `IntoDiagnostic` impl.
+/// - Derived impls are always generic, and it's good for the hand-written
+///   impls to be consistent with them.
 #[rustc_diagnostic_item = "IntoDiagnostic"]
 pub trait IntoDiagnostic<'a, G: EmissionGuarantee = ErrorGuaranteed> {
     /// Write out as a diagnostic out of `DiagCtxt`.
@@ -1289,11 +1308,9 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> {
         drop(self);
     }
 
-    /// Stashes diagnostic for possible later improvement in a different,
-    /// later stage of the compiler. The diagnostic can be accessed with
-    /// the provided `span` and `key` through [`DiagCtxt::steal_diagnostic()`].
-    pub fn stash(mut self, span: Span, key: StashKey) {
-        self.dcx.stash_diagnostic(span, key, self.take_diag());
+    /// See `DiagCtxt::stash_diagnostic` for details.
+    pub fn stash(mut self, span: Span, key: StashKey) -> Option<ErrorGuaranteed> {
+        self.dcx.stash_diagnostic(span, key, self.take_diag())
     }
 
     /// Delay emission of this diagnostic as a bug.
diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs
index a5e6aed13eb..0a533833e64 100644
--- a/compiler/rustc_errors/src/lib.rs
+++ b/compiler/rustc_errors/src/lib.rs
@@ -434,10 +434,6 @@ struct DiagCtxtInner {
     /// The delayed bugs and their error guarantees.
     delayed_bugs: Vec<(DelayedDiagInner, ErrorGuaranteed)>,
 
-    /// The number of stashed errors. Unlike the other counts, this can go up
-    /// and down, so it doesn't guarantee anything.
-    stashed_err_count: usize,
-
     /// The error count shown to the user at the end.
     deduplicated_err_count: usize,
     /// The warning count shown to the user at the end.
@@ -475,7 +471,7 @@ struct DiagCtxtInner {
     /// add more information). All stashed diagnostics must be emitted with
     /// `emit_stashed_diagnostics` by the time the `DiagCtxtInner` is dropped,
     /// otherwise an assertion failure will occur.
-    stashed_diagnostics: FxIndexMap<(Span, StashKey), DiagInner>,
+    stashed_diagnostics: FxIndexMap<(Span, StashKey), (DiagInner, Option<ErrorGuaranteed>)>,
 
     future_breakage_diagnostics: Vec<DiagInner>,
 
@@ -559,10 +555,18 @@ pub struct DiagCtxtFlags {
 
 impl Drop for DiagCtxtInner {
     fn drop(&mut self) {
-        // Any stashed diagnostics should have been handled by
-        // `emit_stashed_diagnostics` by now.
-        assert!(self.stashed_diagnostics.is_empty());
+        // For tools using `interface::run_compiler` (e.g. rustc, rustdoc)
+        // stashed diagnostics will have already been emitted. But for others
+        // that don't use `interface::run_compiler` (e.g. rustfmt, some clippy
+        // lints) this fallback is necessary.
+        //
+        // Important: it is sound to produce an `ErrorGuaranteed` when stashing
+        // errors because they are guaranteed to be emitted here or earlier.
+        self.emit_stashed_diagnostics();
 
+        // Important: it is sound to produce an `ErrorGuaranteed` when emitting
+        // delayed bugs because they are guaranteed to be emitted here if
+        // necessary.
         if self.err_guars.is_empty() {
             self.flush_delayed()
         }
@@ -608,7 +612,6 @@ impl DiagCtxt {
                 err_guars: Vec::new(),
                 lint_err_guars: Vec::new(),
                 delayed_bugs: Vec::new(),
-                stashed_err_count: 0,
                 deduplicated_err_count: 0,
                 deduplicated_warn_count: 0,
                 emitter,
@@ -669,7 +672,6 @@ impl DiagCtxt {
             err_guars,
             lint_err_guars,
             delayed_bugs,
-            stashed_err_count,
             deduplicated_err_count,
             deduplicated_warn_count,
             emitter: _,
@@ -692,7 +694,6 @@ impl DiagCtxt {
         *err_guars = Default::default();
         *lint_err_guars = Default::default();
         *delayed_bugs = Default::default();
-        *stashed_err_count = 0;
         *deduplicated_err_count = 0;
         *deduplicated_warn_count = 0;
         *must_produce_diag = false;
@@ -708,39 +709,111 @@ impl DiagCtxt {
         *fulfilled_expectations = Default::default();
     }
 
-    /// Stash a given diagnostic with the given `Span` and [`StashKey`] as the key.
-    /// Retrieve a stashed diagnostic with `steal_diagnostic`.
-    pub fn stash_diagnostic(&self, span: Span, key: StashKey, diag: DiagInner) {
-        let mut inner = self.inner.borrow_mut();
-
-        let key = (span.with_parent(None), key);
-
-        if diag.is_error() {
-            if diag.is_lint.is_none() {
-                inner.stashed_err_count += 1;
-            }
-        }
+    /// Stashes a diagnostic for possible later improvement in a different,
+    /// later stage of the compiler. Possible actions depend on the diagnostic
+    /// level:
+    /// - Level::Error: immediately counted as an error that has occurred, because it
+    ///   is guaranteed to be emitted eventually. Can be later accessed with the
+    ///   provided `span` and `key` through
+    ///   [`DiagCtxt::try_steal_modify_and_emit_err`] or
+    ///   [`DiagCtxt::try_steal_replace_and_emit_err`]. These do not allow
+    ///   cancellation or downgrading of the error. Returns
+    ///   `Some(ErrorGuaranteed)`.
+    /// - Level::Warning and lower (i.e. !is_error()): can be accessed with the
+    ///   provided `span` and `key` through [`DiagCtxt::steal_non_err()`]. This
+    ///   allows cancelling and downgrading of the diagnostic. Returns `None`.
+    /// - Others: not allowed, will trigger a panic.
+    pub fn stash_diagnostic(
+        &self,
+        span: Span,
+        key: StashKey,
+        diag: DiagInner,
+    ) -> Option<ErrorGuaranteed> {
+        let guar = if diag.level() == Level::Error {
+            // This `unchecked_error_guaranteed` is valid. It is where the
+            // `ErrorGuaranteed` for stashed errors originates. See
+            // `DiagCtxtInner::drop`.
+            #[allow(deprecated)]
+            Some(ErrorGuaranteed::unchecked_error_guaranteed())
+        } else if !diag.is_error() {
+            None
+        } else {
+            self.span_bug(span, format!("invalid level in `stash_diagnostic`: {}", diag.level));
+        };
 
         // FIXME(Centril, #69537): Consider reintroducing panic on overwriting a stashed diagnostic
         // if/when we have a more robust macro-friendly replacement for `(span, key)` as a key.
         // See the PR for a discussion.
-        inner.stashed_diagnostics.insert(key, diag);
+        let key = (span.with_parent(None), key);
+        self.inner.borrow_mut().stashed_diagnostics.insert(key, (diag, guar));
+
+        guar
     }
 
-    /// Steal a previously stashed diagnostic with the given `Span` and [`StashKey`] as the key.
-    pub fn steal_diagnostic(&self, span: Span, key: StashKey) -> Option<Diag<'_, ()>> {
-        let mut inner = self.inner.borrow_mut();
+    /// Steal a previously stashed non-error diagnostic with the given `Span`
+    /// and [`StashKey`] as the key. Panics if the found diagnostic is an
+    /// error.
+    pub fn steal_non_err(&self, span: Span, key: StashKey) -> Option<Diag<'_, ()>> {
         let key = (span.with_parent(None), key);
         // FIXME(#120456) - is `swap_remove` correct?
-        let diag = inner.stashed_diagnostics.swap_remove(&key)?;
-        if diag.is_error() {
-            if diag.is_lint.is_none() {
-                inner.stashed_err_count -= 1;
-            }
-        }
+        let (diag, guar) = self.inner.borrow_mut().stashed_diagnostics.swap_remove(&key)?;
+        assert!(!diag.is_error());
+        assert!(guar.is_none());
         Some(Diag::new_diagnostic(self, diag))
     }
 
+    /// Steals a previously stashed error with the given `Span` and
+    /// [`StashKey`] as the key, modifies it, and emits it. Returns `None` if
+    /// no matching diagnostic is found. Panics if the found diagnostic's level
+    /// isn't `Level::Error`.
+    pub fn try_steal_modify_and_emit_err<F>(
+        &self,
+        span: Span,
+        key: StashKey,
+        mut modify_err: F,
+    ) -> Option<ErrorGuaranteed>
+    where
+        F: FnMut(&mut Diag<'_>),
+    {
+        let key = (span.with_parent(None), key);
+        // FIXME(#120456) - is `swap_remove` correct?
+        let err = self.inner.borrow_mut().stashed_diagnostics.swap_remove(&key);
+        err.map(|(err, guar)| {
+            // The use of `::<ErrorGuaranteed>` is safe because level is `Level::Error`.
+            assert_eq!(err.level, Level::Error);
+            assert!(guar.is_some());
+            let mut err = Diag::<ErrorGuaranteed>::new_diagnostic(self, err);
+            modify_err(&mut err);
+            assert_eq!(err.level, Level::Error);
+            err.emit()
+        })
+    }
+
+    /// Steals a previously stashed error with the given `Span` and
+    /// [`StashKey`] as the key, cancels it if found, and emits `new_err`.
+    /// Panics if the found diagnostic's level isn't `Level::Error`.
+    pub fn try_steal_replace_and_emit_err(
+        &self,
+        span: Span,
+        key: StashKey,
+        new_err: Diag<'_>,
+    ) -> ErrorGuaranteed {
+        let key = (span.with_parent(None), key);
+        // FIXME(#120456) - is `swap_remove` correct?
+        let old_err = self.inner.borrow_mut().stashed_diagnostics.swap_remove(&key);
+        match old_err {
+            Some((old_err, guar)) => {
+                assert_eq!(old_err.level, Level::Error);
+                assert!(guar.is_some());
+                // Because `old_err` has already been counted, it can only be
+                // safely cancelled because the `new_err` supplants it.
+                Diag::<ErrorGuaranteed>::new_diagnostic(self, old_err).cancel();
+            }
+            None => {}
+        };
+        new_err.emit()
+    }
+
     pub fn has_stashed_diagnostic(&self, span: Span, key: StashKey) -> bool {
         self.inner.borrow().stashed_diagnostics.get(&(span.with_parent(None), key)).is_some()
     }
@@ -750,41 +823,40 @@ impl DiagCtxt {
         self.inner.borrow_mut().emit_stashed_diagnostics()
     }
 
-    /// This excludes lint errors, delayed bugs and stashed errors.
+    /// This excludes lint errors, and delayed bugs.
     #[inline]
     pub fn err_count_excluding_lint_errs(&self) -> usize {
-        self.inner.borrow().err_guars.len()
+        let inner = self.inner.borrow();
+        inner.err_guars.len()
+            + inner
+                .stashed_diagnostics
+                .values()
+                .filter(|(diag, guar)| guar.is_some() && diag.is_lint.is_none())
+                .count()
     }
 
-    /// This excludes delayed bugs and stashed errors.
+    /// This excludes delayed bugs.
     #[inline]
     pub fn err_count(&self) -> usize {
         let inner = self.inner.borrow();
-        inner.err_guars.len() + inner.lint_err_guars.len()
-    }
-
-    /// This excludes normal errors, lint errors, and delayed bugs. Unless
-    /// absolutely necessary, avoid using this. It's dubious because stashed
-    /// errors can later be cancelled, so the presence of a stashed error at
-    /// some point of time doesn't guarantee anything -- there are no
-    /// `ErrorGuaranteed`s here.
-    pub fn stashed_err_count(&self) -> usize {
-        self.inner.borrow().stashed_err_count
+        inner.err_guars.len()
+            + inner.lint_err_guars.len()
+            + inner.stashed_diagnostics.values().filter(|(_diag, guar)| guar.is_some()).count()
     }
 
-    /// This excludes lint errors, delayed bugs, and stashed errors. Unless
-    /// absolutely necessary, prefer `has_errors` to this method.
+    /// This excludes lint errors and delayed bugs. Unless absolutely
+    /// necessary, prefer `has_errors` to this method.
     pub fn has_errors_excluding_lint_errors(&self) -> Option<ErrorGuaranteed> {
         self.inner.borrow().has_errors_excluding_lint_errors()
     }
 
-    /// This excludes delayed bugs and stashed errors.
+    /// This excludes delayed bugs.
     pub fn has_errors(&self) -> Option<ErrorGuaranteed> {
         self.inner.borrow().has_errors()
     }
 
-    /// This excludes stashed errors. Unless absolutely necessary, prefer
-    /// `has_errors` to this method.
+    /// This excludes nothing. Unless absolutely necessary, prefer `has_errors`
+    /// to this method.
     pub fn has_errors_or_delayed_bugs(&self) -> Option<ErrorGuaranteed> {
         self.inner.borrow().has_errors_or_delayed_bugs()
     }
@@ -869,10 +941,10 @@ impl DiagCtxt {
         }
     }
 
-    /// This excludes delayed bugs and stashed errors. Used for early aborts
-    /// after errors occurred -- e.g. because continuing in the face of errors is
-    /// likely to lead to bad results, such as spurious/uninteresting
-    /// additional errors -- when returning an error `Result` is difficult.
+    /// This excludes delayed bugs. Used for early aborts after errors occurred
+    /// -- e.g. because continuing in the face of errors is likely to lead to
+    /// bad results, such as spurious/uninteresting additional errors -- when
+    /// returning an error `Result` is difficult.
     pub fn abort_if_errors(&self) {
         if self.has_errors().is_some() {
             FatalError.raise();
@@ -956,7 +1028,7 @@ impl DiagCtxt {
         inner
             .stashed_diagnostics
             .values_mut()
-            .for_each(|diag| diag.update_unstable_expectation_id(unstable_to_stable));
+            .for_each(|(diag, _guar)| diag.update_unstable_expectation_id(unstable_to_stable));
         inner
             .future_breakage_diagnostics
             .iter_mut()
@@ -1263,12 +1335,8 @@ impl DiagCtxtInner {
     fn emit_stashed_diagnostics(&mut self) -> Option<ErrorGuaranteed> {
         let mut guar = None;
         let has_errors = !self.err_guars.is_empty();
-        for (_, diag) in std::mem::take(&mut self.stashed_diagnostics).into_iter() {
-            if diag.is_error() {
-                if diag.is_lint.is_none() {
-                    self.stashed_err_count -= 1;
-                }
-            } else {
+        for (_, (diag, _guar)) in std::mem::take(&mut self.stashed_diagnostics).into_iter() {
+            if !diag.is_error() {
                 // Unless they're forced, don't flush stashed warnings when
                 // there are errors, to avoid causing warning overload. The
                 // stash would've been stolen already if it were important.
@@ -1327,7 +1395,8 @@ impl DiagCtxtInner {
                 } else {
                     let backtrace = std::backtrace::Backtrace::capture();
                     // This `unchecked_error_guaranteed` is valid. It is where the
-                    // `ErrorGuaranteed` for delayed bugs originates.
+                    // `ErrorGuaranteed` for delayed bugs originates. See
+                    // `DiagCtxtInner::drop`.
                     #[allow(deprecated)]
                     let guar = ErrorGuaranteed::unchecked_error_guaranteed();
                     self.delayed_bugs
@@ -1439,11 +1508,31 @@ impl DiagCtxtInner {
     }
 
     fn has_errors_excluding_lint_errors(&self) -> Option<ErrorGuaranteed> {
-        self.err_guars.get(0).copied()
+        self.err_guars.get(0).copied().or_else(|| {
+            if let Some((_diag, guar)) = self
+                .stashed_diagnostics
+                .values()
+                .find(|(diag, guar)| guar.is_some() && diag.is_lint.is_none())
+            {
+                *guar
+            } else {
+                None
+            }
+        })
     }
 
     fn has_errors(&self) -> Option<ErrorGuaranteed> {
-        self.has_errors_excluding_lint_errors().or_else(|| self.lint_err_guars.get(0).copied())
+        self.err_guars.get(0).copied().or_else(|| self.lint_err_guars.get(0).copied()).or_else(
+            || {
+                if let Some((_diag, guar)) =
+                    self.stashed_diagnostics.values().find(|(_diag, guar)| guar.is_some())
+                {
+                    *guar
+                } else {
+                    None
+                }
+            },
+        )
     }
 
     fn has_errors_or_delayed_bugs(&self) -> Option<ErrorGuaranteed> {
@@ -1491,14 +1580,26 @@ impl DiagCtxtInner {
         let bugs: Vec<_> =
             std::mem::take(&mut self.delayed_bugs).into_iter().map(|(b, _)| b).collect();
 
-        // If backtraces are enabled, also print the query stack
         let backtrace = std::env::var_os("RUST_BACKTRACE").map_or(true, |x| &x != "0");
-        for (i, bug) in bugs.into_iter().enumerate() {
-            if let Some(file) = self.ice_file.as_ref()
-                && let Ok(mut out) = std::fs::File::options().create(true).append(true).open(file)
-            {
-                let _ = write!(
-                    &mut out,
+        let decorate = backtrace || self.ice_file.is_none();
+        let mut out = self
+            .ice_file
+            .as_ref()
+            .and_then(|file| std::fs::File::options().create(true).append(true).open(file).ok());
+
+        // Put the overall explanation before the `DelayedBug`s, to frame them
+        // better (e.g. separate warnings from them). Also, use notes, which
+        // don't count as errors, to avoid possibly triggering
+        // `-Ztreat-err-as-bug`, which we don't want.
+        let note1 = "no errors encountered even though delayed bugs were created";
+        let note2 = "those delayed bugs will now be shown as internal compiler errors";
+        self.emit_diagnostic(DiagInner::new(Note, note1));
+        self.emit_diagnostic(DiagInner::new(Note, note2));
+
+        for bug in bugs {
+            if let Some(out) = &mut out {
+                _ = write!(
+                    out,
                     "delayed bug: {}\n{}\n",
                     bug.inner
                         .messages
@@ -1509,21 +1610,9 @@ impl DiagCtxtInner {
                 );
             }
 
-            if i == 0 {
-                // Put the overall explanation before the `DelayedBug`s, to
-                // frame them better (e.g. separate warnings from them). Also,
-                // make it a note so it doesn't count as an error, because that
-                // could trigger `-Ztreat-err-as-bug`, which we don't want.
-                let note1 = "no errors encountered even though delayed bugs were created";
-                let note2 = "those delayed bugs will now be shown as internal compiler errors";
-                self.emit_diagnostic(DiagInner::new(Note, note1));
-                self.emit_diagnostic(DiagInner::new(Note, note2));
-            }
-
-            let mut bug =
-                if backtrace || self.ice_file.is_none() { bug.decorate(self) } else { bug.inner };
+            let mut bug = if decorate { bug.decorate(self) } else { bug.inner };
 
-            // "Undelay" the delayed bugs (into plain `Bug`s).
+            // "Undelay" the delayed bugs into plain bugs.
             if bug.level != DelayedBug {
                 // NOTE(eddyb) not panicking here because we're already producing
                 // an ICE, and the more information the merrier.