[stable-mir failure] XXX: stashed fixes

    The remove error counts in the ones with delayed bugs: because we no
    longer flush delayed bugs in `codegen_and_link`
    (do that in a precursor?)

    ---

    Require explicit calls to `emit_stashed_diagnostic`.

    `DiagCtxtInner::drop` will emit any remaining stashed diagnostics. But
    we also have explicit calls to `emit_stashed_diagnostics` in several
    places. All this makes it difficult to understand where the
    responsibility for emitting stashed diagnostics lies.

    As it happens, the call in `DiagCtxtInner::drop` isn't necessary. So
    this commit replaces that call with an assertion that there are no
    stashed diagnostics left. This makes things clearer. In particular,
    numerous `DiagCtxt`s are created during compilation, but
    stashing/stealing only occurs in the main one, which is the one used for
    `print_error_count`.

Author: nnethercote

compiler/rustc_errors/src/lib.rs

@@ -476,9 +476,10 @@ struct DiagCtxtInner {
     emitted_diagnostics: FxHashSet<Hash128>,
 
     /// Stashed diagnostics emitted in one stage of the compiler that may be
-    /// stolen by other stages (e.g. to improve them and add more information).
-    /// The stashed diagnostics count towards the total error count.
-    /// When `.abort_if_errors()` is called, these are also emitted.
+    /// stolen and emitted/cancelled by other stages (e.g. to improve them and
+    /// 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), Diagnostic>,
 
     future_breakage_diagnostics: Vec<Diagnostic>,
@@ -563,7 +564,9 @@ pub struct DiagCtxtFlags {
 
 impl Drop for DiagCtxtInner {
     fn drop(&mut self) {
-        self.emit_stashed_diagnostics();
+        // Any stashed diagnostics should have been handled by
+        // `emit_stashed_diagnostics` by now.
+        assert!(self.stashed_diagnostics.is_empty());
 
         if self.err_guars.is_empty() {
             self.flush_delayed()
@@ -755,7 +758,7 @@ impl DiagCtxt {
     }
 
     /// Emit all stashed diagnostics.
-    pub fn emit_stashed_diagnostics(&self) {
+    pub fn emit_stashed_diagnostics(&self) -> Option<ErrorGuaranteed> {
         self.inner.borrow_mut().emit_stashed_diagnostics()
     }
 
@@ -801,8 +804,6 @@ impl DiagCtxt {
     pub fn print_error_count(&self, registry: &Registry) {
         let mut inner = self.inner.borrow_mut();
 
-        inner.emit_stashed_diagnostics();
-
         if inner.treat_err_as_bug() {
             return;
         }
@@ -877,9 +878,7 @@ impl DiagCtxt {
     }
 
     pub fn abort_if_errors(&self) {
-        let mut inner = self.inner.borrow_mut();
-        inner.emit_stashed_diagnostics();
-        if !inner.err_guars.is_empty() {
+        if self.has_errors().is_some() {
             FatalError.raise();
         }
     }
@@ -1280,7 +1279,8 @@ impl DiagCtxt {
 // `DiagCtxtInner::foo`.
 impl DiagCtxtInner {
     /// Emit all stashed diagnostics.
-    fn emit_stashed_diagnostics(&mut self) {
+    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() {
@@ -1295,8 +1295,9 @@ impl DiagCtxtInner {
                     continue;
                 }
             }
-            self.emit_diagnostic(diag);
+            guar = guar.or(self.emit_diagnostic(diag));
         }
+        guar
     }
 
     // Return value is only `Some` if the level is `Error` or `DelayedBug`.
@@ -1489,6 +1490,11 @@ impl DiagCtxtInner {
     }
 
     fn flush_delayed(&mut self) {
+        // Stashed diagnostics must be emitted before delayed bugs are flushed.
+        // Otherwise, we might ICE prematurely when errors would have
+        // eventually happened.
+        assert!(self.stashed_diagnostics.is_empty());
+
         if self.delayed_bugs.is_empty() {
             return;
         }

compiler/rustc_interface/src/interface.rs

@@ -423,18 +423,43 @@ pub fn run_compiler<R: Send>(config: Config, f: impl FnOnce(&Compiler) -> R + Se
                 Compiler { sess, codegen_backend, override_queries: config.override_queries };
 
             rustc_span::set_source_map(compiler.sess.parse_sess.clone_source_map(), move || {
-                let r = {
-                    let _sess_abort_error = defer(|| {
-                        compiler.sess.finish_diagnostics(&config.registry);
+                // There are two paths out of `f`.
+                // - Normal exit.
+                // - Panic, e.g. triggered by `abort_if_errors`.
+                //
+                // We must run `finish_diagnostics` in both cases.
+                let res = {
+                    // If `f` panics, `finish_diagnostics` will run during
+                    // unwinding because of the `defer`.
+                    let mut guar = None;
+                    let sess_abort_guard = defer(|| {
+                        guar = compiler.sess.finish_diagnostics(&config.registry);
                     });
 
-                    f(&compiler)
+                    let res = f(&compiler);
+
+                    // If `f` doesn't panic, `finish_diagnostics` will run
+                    // normally when `sess_abort_guard` is dropped.
+                    drop(sess_abort_guard);
+
+                    // If `finish_diagnostics` emits errors (e.g. stashed
+                    // errors) we can't return an error directly, because the
+                    // return type of this function is `R`, not `Result<R, E>`.
+                    // But we need to communicate the errors' existence to the
+                    // caller, otherwise the caller might mistakenly think that
+                    // no errors occurred and return a zero exit code. So we
+                    // abort (panic) instead, similar to if `f` had panicked.
+                    if guar.is_some() {
+                        compiler.sess.dcx().abort_if_errors();
+                    }
+
+                    res
                 };
 
                 let prof = compiler.sess.prof.clone();
-
                 prof.generic_activity("drop_compiler").run(move || drop(compiler));
-                r
+
+                res
             })
         },
     )

compiler/rustc_interface/src/passes.rs

@@ -772,12 +772,11 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> {
     // lot of annoying errors in the ui tests (basically,
     // lint warnings and so on -- kindck used to do this abort, but
     // kindck is gone now). -nmatsakis
-    if let Some(reported) = sess.dcx().has_errors_excluding_lint_errors() {
-        return Err(reported);
-    } else if sess.dcx().stashed_err_count() > 0 {
-        // Without this case we sometimes get delayed bug ICEs and I don't
-        // understand why. -nnethercote
-        return Err(sess.dcx().delayed_bug("some stashed error is waiting for use"));
+    //
+    // But we exclude lint errors from this, because lint errors are typically
+    // less serious and we're more likely to want to continue (#87337).
+    if let Some(guar) = sess.dcx().has_errors_excluding_lint_errors() {
+        return Err(guar);
     }
 
     sess.time("misc_checking_3", || {

compiler/rustc_interface/src/queries.rs

@@ -222,12 +222,12 @@ impl<'tcx> Queries<'tcx> {
 
     pub fn codegen_and_build_linker(&'tcx self) -> Result<Linker> {
         self.global_ctxt()?.enter(|tcx| {
-            // Don't do code generation if there were any errors
-            self.compiler.sess.compile_status()?;
-
-            // If we have any delayed bugs, for example because we created TyKind::Error earlier,
-            // it's likely that codegen will only cause more ICEs, obscuring the original problem
-            self.compiler.sess.dcx().flush_delayed();
+            // Don't do code generation if there were any errors. Likewise if
+            // there were any delayed bugs, because codegen will likely cause
+            // more ICEs, obscuring the original problem.
+            if let Some(guar) = self.compiler.sess.dcx().has_errors_or_delayed_bugs() {
+                return Err(guar);
+            }
 
             // Hook for UI tests.
             Self::check_for_rustc_errors_attr(tcx);

compiler/rustc_passes/src/dead.rs

@@ -237,7 +237,10 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
     ) {
         let variant = match self.typeck_results().node_type(lhs.hir_id).kind() {
             ty::Adt(adt, _) => adt.variant_of_res(res),
-            _ => span_bug!(lhs.span, "non-ADT in tuple struct pattern"),
+            _ => {
+                self.tcx.dcx().span_delayed_bug(lhs.span, "non-ADT in tuple struct pattern");
+                return;
+            }
         };
         let dotdot = dotdot.as_opt_usize().unwrap_or(pats.len());
         let first_n = pats.iter().enumerate().take(dotdot);

compiler/rustc_session/src/session.rs

@@ -258,7 +258,8 @@ impl Session {
         }
     }
 
-    fn check_miri_unleashed_features(&self) {
+    fn check_miri_unleashed_features(&self) -> Option<ErrorGuaranteed> {
+        let mut guar = None;
         let unleashed_features = self.miri_unleashed_features.lock();
         if !unleashed_features.is_empty() {
             let mut must_err = false;
@@ -279,18 +280,22 @@ impl Session {
             // If we should err, make sure we did.
             if must_err && self.dcx().has_errors().is_none() {
                 // We have skipped a feature gate, and not run into other errors... reject.
-                self.dcx().emit_err(errors::NotCircumventFeature);
+                guar = Some(self.dcx().emit_err(errors::NotCircumventFeature));
             }
         }
+        guar
     }
 
     /// Invoked all the way at the end to finish off diagnostics printing.
-    pub fn finish_diagnostics(&self, registry: &Registry) {
-        self.check_miri_unleashed_features();
+    pub fn finish_diagnostics(&self, registry: &Registry) -> Option<ErrorGuaranteed> {
+        let mut guar = None;
+        guar = guar.or(self.check_miri_unleashed_features());
+        guar = guar.or(self.dcx().emit_stashed_diagnostics());
         self.dcx().print_error_count(registry);
         if self.opts.json_future_incompat {
             self.dcx().emit_future_breakage_report();
         }
+        guar
     }
 
     /// Returns true if the crate is a testing one.
@@ -314,7 +319,6 @@ impl Session {
 
     pub fn compile_status(&self) -> Result<(), ErrorGuaranteed> {
         if let Some(reported) = self.dcx().has_errors() {
-            self.dcx().emit_stashed_diagnostics();
             Err(reported)
         } else {
             Ok(())

tests/ui/impl-trait/equality-in-canonical-query.clone.stderr

@@ -21,5 +21,3 @@ LL |     same_output(foo, rpit);
 
 query stack during panic:
 end of query stack
-error: aborting due to 2 previous errors
-

tests/ui/inference/issue-80409.no-compat.stderr

@@ -2,5 +2,3 @@ error: internal compiler error: error performing ParamEnvAnd { param_env: ParamE
    |
    = query stack during panic:
 end of query stack
-error: aborting due to 1 previous error
-

tests/ui/type-alias-impl-trait/rpit_tait_equality_in_canonical_query.current.stderr

@@ -21,5 +21,3 @@ LL |     query(get_rpit);
 
 query stack during panic:
 end of query stack
-error: aborting due to 2 previous errors
-