Rollup merge of #120699 - nnethercote:rm-useless-TRACK_DIAGNOSTIC-calls, r=oli-obk

Document `TRACK_DIAGNOSTIC` calls.

r? ```````@cjgillot```````

2 files changed, 93 insertions, 71 deletions

diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs
index 57b8df52f4b..4f033e3fefa 100644
--- a/compiler/rustc_errors/src/emitter.rs
+++ b/compiler/rustc_errors/src/emitter.rs
@@ -2086,7 +2086,7 @@ impl HumanEmitter {
                 }
                 if !self.short_message {
                     for child in children {
-                        assert!(child.level.can_be_top_or_sub().1);
+                        assert!(child.level.can_be_subdiag());
                         let span = &child.span;
                         if let Err(err) = self.emit_messages_default_inner(
                             span,
diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs
index 286d4621850..723f13dbe8d 100644
--- a/compiler/rustc_errors/src/lib.rs
+++ b/compiler/rustc_errors/src/lib.rs
@@ -526,12 +526,15 @@ pub enum StashKey {
     UndeterminedMacroResolution,
 }
 
-fn default_track_diagnostic(diag: DiagInner, f: &mut dyn FnMut(DiagInner)) {
+fn default_track_diagnostic<R>(diag: DiagInner, f: &mut dyn FnMut(DiagInner) -> R) -> R {
     (*f)(diag)
 }
 
-pub static TRACK_DIAGNOSTIC: AtomicRef<fn(DiagInner, &mut dyn FnMut(DiagInner))> =
-    AtomicRef::new(&(default_track_diagnostic as _));
+/// Diagnostics emitted by `DiagCtxtInner::emit_diagnostic` are passed through this function. Used
+/// for tracking by incremental, to replay diagnostics as necessary.
+pub static TRACK_DIAGNOSTIC: AtomicRef<
+    fn(DiagInner, &mut dyn FnMut(DiagInner) -> Option<ErrorGuaranteed>) -> Option<ErrorGuaranteed>,
+> = AtomicRef::new(&(default_track_diagnostic as _));
 
 #[derive(Copy, Clone, Default)]
 pub struct DiagCtxtFlags {
@@ -1422,74 +1425,103 @@ impl DiagCtxtInner {
 
     // Return value is only `Some` if the level is `Error` or `DelayedBug`.
     fn emit_diagnostic(&mut self, mut diagnostic: DiagInner) -> Option<ErrorGuaranteed> {
-        assert!(diagnostic.level.can_be_top_or_sub().0);
-
-        if let Some(expectation_id) = diagnostic.level.get_expectation_id() {
-            // The `LintExpectationId` can be stable or unstable depending on when it was created.
-            // Diagnostics created before the definition of `HirId`s are unstable and can not yet
-            // be stored. Instead, they are buffered until the `LintExpectationId` is replaced by
-            // a stable one by the `LintLevelsBuilder`.
-            if let LintExpectationId::Unstable { .. } = expectation_id {
-                self.unstable_expect_diagnostics.push(diagnostic);
-                return None;
-            }
-            self.suppressed_expected_diag = true;
-            self.fulfilled_expectations.insert(expectation_id.normalize());
-        }
-
         if diagnostic.has_future_breakage() {
             // Future breakages aren't emitted if they're `Level::Allow`,
             // but they still need to be constructed and stashed below,
             // so they'll trigger the must_produce_diag check.
-            self.suppressed_expected_diag = true;
+            assert!(matches!(diagnostic.level, Error | Warning | Allow));
             self.future_breakage_diagnostics.push(diagnostic.clone());
         }
 
-        // Note that because this comes before the `match` below,
-        // `-Zeagerly-emit-delayed-bugs` continues to work even after we've
-        // issued an error and stopped recording new delayed bugs.
-        if diagnostic.level == DelayedBug && self.flags.eagerly_emit_delayed_bugs {
-            diagnostic.level = Error;
-        }
-
+        // We call TRACK_DIAGNOSTIC with an empty closure for the cases that
+        // return early *and* have some kind of side-effect, except where
+        // noted.
         match diagnostic.level {
-            // This must come after the possible promotion of `DelayedBug` to
-            // `Error` above.
-            Fatal | Error if self.treat_next_err_as_bug() => {
-                diagnostic.level = Bug;
+            Bug => {}
+            Fatal | Error => {
+                if self.treat_next_err_as_bug() {
+                    // `Fatal` and `Error` can be promoted to `Bug`.
+                    diagnostic.level = Bug;
+                }
             }
             DelayedBug => {
-                // If we have already emitted at least one error, we don't need
-                // to record the delayed bug, because it'll never be used.
-                return if let Some(guar) = self.has_errors() {
-                    Some(guar)
+                // Note that because we check these conditions first,
+                // `-Zeagerly-emit-delayed-bugs` and `-Ztreat-err-as-bug`
+                // continue to work even after we've issued an error and
+                // stopped recording new delayed bugs.
+                if self.flags.eagerly_emit_delayed_bugs {
+                    // `DelayedBug` can be promoted to `Error` or `Bug`.
+                    if self.treat_next_err_as_bug() {
+                        diagnostic.level = Bug;
+                    } else {
+                        diagnostic.level = Error;
+                    }
                 } else {
-                    let backtrace = std::backtrace::Backtrace::capture();
-                    // This `unchecked_error_guaranteed` is valid. It is where the
-                    // `ErrorGuaranteed` for delayed bugs originates. See
-                    // `DiagCtxtInner::drop`.
-                    #[allow(deprecated)]
-                    let guar = ErrorGuaranteed::unchecked_error_guaranteed();
-                    self.delayed_bugs
-                        .push((DelayedDiagInner::with_backtrace(diagnostic, backtrace), guar));
-                    Some(guar)
-                };
+                    // If we have already emitted at least one error, we don't need
+                    // to record the delayed bug, because it'll never be used.
+                    return if let Some(guar) = self.has_errors() {
+                        Some(guar)
+                    } else {
+                        // No `TRACK_DIAGNOSTIC` call is needed, because the
+                        // incremental session is deleted if there is a delayed
+                        // bug. This also saves us from cloning the diagnostic.
+                        let backtrace = std::backtrace::Backtrace::capture();
+                        // This `unchecked_error_guaranteed` is valid. It is where the
+                        // `ErrorGuaranteed` for delayed bugs originates. See
+                        // `DiagCtxtInner::drop`.
+                        #[allow(deprecated)]
+                        let guar = ErrorGuaranteed::unchecked_error_guaranteed();
+                        self.delayed_bugs
+                            .push((DelayedDiagInner::with_backtrace(diagnostic, backtrace), guar));
+                        Some(guar)
+                    };
+                }
+            }
+            ForceWarning(None) => {} // `ForceWarning(Some(...))` is below, with `Expect`
+            Warning => {
+                if !self.flags.can_emit_warnings {
+                    // We are not emitting warnings.
+                    if diagnostic.has_future_breakage() {
+                        // The side-effect is at the top of this method.
+                        TRACK_DIAGNOSTIC(diagnostic, &mut |_| None);
+                    }
+                    return None;
+                }
             }
-            Warning if !self.flags.can_emit_warnings => {
+            Note | Help | FailureNote => {}
+            OnceNote | OnceHelp => panic!("bad level: {:?}", diagnostic.level),
+            Allow => {
+                // Nothing emitted for allowed lints.
                 if diagnostic.has_future_breakage() {
-                    (*TRACK_DIAGNOSTIC)(diagnostic, &mut |_| {});
+                    // The side-effect is at the top of this method.
+                    TRACK_DIAGNOSTIC(diagnostic, &mut |_| None);
+                    self.suppressed_expected_diag = true;
                 }
                 return None;
             }
-            Allow | Expect(_) => {
-                (*TRACK_DIAGNOSTIC)(diagnostic, &mut |_| {});
-                return None;
+            Expect(expect_id) | ForceWarning(Some(expect_id)) => {
+                // Diagnostics created before the definition of `HirId`s are
+                // unstable and can not yet be stored. Instead, they are
+                // buffered until the `LintExpectationId` is replaced by a
+                // stable one by the `LintLevelsBuilder`.
+                if let LintExpectationId::Unstable { .. } = expect_id {
+                    // We don't call TRACK_DIAGNOSTIC because we wait for the
+                    // unstable ID to be updated, whereupon the diagnostic will
+                    // be passed into this method again.
+                    self.unstable_expect_diagnostics.push(diagnostic);
+                    return None;
+                }
+                self.fulfilled_expectations.insert(expect_id.normalize());
+                if let Expect(_) = diagnostic.level {
+                    // Nothing emitted here for expected lints.
+                    TRACK_DIAGNOSTIC(diagnostic, &mut |_| None);
+                    self.suppressed_expected_diag = true;
+                    return None;
+                }
             }
-            _ => {}
         }
 
-        let mut guaranteed = None;
-        (*TRACK_DIAGNOSTIC)(diagnostic, &mut |mut diagnostic| {
+        TRACK_DIAGNOSTIC(diagnostic, &mut |mut diagnostic| {
             if let Some(code) = diagnostic.code {
                 self.emitted_diagnostic_codes.insert(code);
             }
@@ -1552,17 +1584,17 @@ impl DiagCtxtInner {
                 // `ErrorGuaranteed` for errors and lint errors originates.
                 #[allow(deprecated)]
                 let guar = ErrorGuaranteed::unchecked_error_guaranteed();
-                guaranteed = Some(guar);
                 if is_lint {
                     self.lint_err_guars.push(guar);
                 } else {
                     self.err_guars.push(guar);
                 }
                 self.panic_if_treat_err_as_bug();
+                Some(guar)
+            } else {
+                None
             }
-        });
-
-        guaranteed
+        })
     }
 
     fn treat_err_as_bug(&self) -> bool {
@@ -1863,23 +1895,13 @@ impl Level {
         matches!(*self, FailureNote)
     }
 
-    pub fn get_expectation_id(&self) -> Option<LintExpectationId> {
-        match self {
-            Expect(id) | ForceWarning(Some(id)) => Some(*id),
-            _ => None,
-        }
-    }
-
-    // Can this level be used in a top-level diagnostic message and/or a
-    // subdiagnostic message?
-    fn can_be_top_or_sub(&self) -> (bool, bool) {
+    // Can this level be used in a subdiagnostic message?
+    fn can_be_subdiag(&self) -> bool {
         match self {
             Bug | DelayedBug | Fatal | Error | ForceWarning(_) | FailureNote | Allow
-            | Expect(_) => (true, false),
-
-            Warning | Note | Help => (true, true),
+            | Expect(_) => false,
 
-            OnceNote | OnceHelp => (false, true),
+            Warning | Note | Help | OnceNote | OnceHelp => true,
         }
     }
 }