Fix ErrorGuaranteed unsoundness with stash/steal.

compiler/rustc_errors/src/lib.rs

@@ -429,6 +429,10 @@ struct DiagCtxtInner {
     /// The number of non-lint errors that have been emitted, including duplicates.
     err_count: usize,
 
+    /// 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.
@@ -598,6 +602,7 @@ impl DiagCtxt {
                 flags: DiagCtxtFlags { can_emit_warnings: true, ..Default::default() },
                 lint_err_count: 0,
                 err_count: 0,
+                stashed_err_count: 0,
                 deduplicated_err_count: 0,
                 deduplicated_warn_count: 0,
                 has_printed: false,
@@ -654,6 +659,7 @@ impl DiagCtxt {
         let mut inner = self.inner.borrow_mut();
         inner.lint_err_count = 0;
         inner.err_count = 0;
+        inner.stashed_err_count = 0;
         inner.deduplicated_err_count = 0;
         inner.deduplicated_warn_count = 0;
         inner.has_printed = false;
@@ -675,10 +681,8 @@ impl DiagCtxt {
         let key = (span.with_parent(None), key);
 
         if diag.is_error() {
-            if diag.is_lint.is_some() {
-                inner.lint_err_count += 1;
-            } else {
-                inner.err_count += 1;
+            if diag.is_lint.is_none() {
+                inner.stashed_err_count += 1;
             }
         }
 
@@ -694,10 +698,8 @@ impl DiagCtxt {
         let key = (span.with_parent(None), key);
         let diag = inner.stashed_diagnostics.remove(&key)?;
         if diag.is_error() {
-            if diag.is_lint.is_some() {
-                inner.lint_err_count -= 1;
-            } else {
-                inner.err_count -= 1;
+            if diag.is_lint.is_none() {
+                inner.stashed_err_count -= 1;
             }
         }
         Some(DiagnosticBuilder::new_diagnostic(self, diag))
@@ -922,13 +924,22 @@ impl DiagCtxt {
         self.struct_bug(msg).emit()
     }
 
-    /// This excludes lint errors and delayed bugs.
+    /// This excludes lint errors, delayed bugs, and stashed errors.
     #[inline]
     pub fn err_count(&self) -> usize {
         self.inner.borrow().err_count
     }
 
-    /// This excludes lint errors and delayed bugs.
+    /// 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
+    }
+
+    /// This excludes lint errors, delayed bugs, and stashed errors.
     pub fn has_errors(&self) -> Option<ErrorGuaranteed> {
         self.inner.borrow().has_errors().then(|| {
             // FIXME(nnethercote) find a way to store an `ErrorGuaranteed`.
@@ -937,8 +948,8 @@ impl DiagCtxt {
         })
     }
 
-    /// This excludes delayed bugs. Unless absolutely necessary, prefer
-    /// `has_errors` to this method.
+    /// This excludes delayed bugs and stashed errors. Unless absolutely
+    /// necessary, prefer `has_errors` to this method.
     pub fn has_errors_or_lint_errors(&self) -> Option<ErrorGuaranteed> {
         let inner = self.inner.borrow();
         let result = inner.has_errors() || inner.lint_err_count > 0;
@@ -949,8 +960,8 @@ impl DiagCtxt {
         })
     }
 
-    /// Unless absolutely necessary, prefer `has_errors` or
-    /// `has_errors_or_lint_errors` to this method.
+    /// This excludes stashed errors. Unless absolutely necessary, prefer
+    /// `has_errors` or `has_errors_or_lint_errors` to this method.
     pub fn has_errors_or_lint_errors_or_delayed_bugs(&self) -> Option<ErrorGuaranteed> {
         let inner = self.inner.borrow();
         let result =
@@ -1224,10 +1235,8 @@ impl DiagCtxtInner {
         for (_, diag) in std::mem::take(&mut self.stashed_diagnostics).into_iter() {
             // Decrement the count tracking the stash; emitting will increment it.
             if diag.is_error() {
-                if diag.is_lint.is_some() {
-                    self.lint_err_count -= 1;
-                } else {
-                    self.err_count -= 1;
+                if diag.is_lint.is_none() {
+                    self.stashed_err_count -= 1;
                 }
             } else {
                 // Unless they're forced, don't flush stashed warnings when

compiler/rustc_hir_typeck/src/writeback.rs

@@ -753,10 +753,14 @@ impl<'cx, 'tcx> Resolver<'cx, 'tcx> {
     }
 
     fn report_error(&self, p: impl Into<ty::GenericArg<'tcx>>) -> ErrorGuaranteed {
-        match self.fcx.dcx().has_errors() {
-            Some(e) => e,
-            None => self
-                .fcx
+        if let Some(guar) = self.fcx.dcx().has_errors() {
+            guar
+        } else if self.fcx.dcx().stashed_err_count() > 0 {
+            // Without this case we sometimes get uninteresting and extraneous
+            // "type annotations needed" errors.
+            self.fcx.dcx().delayed_bug("error in Resolver")
+        } else {
+            self.fcx
                 .err_ctxt()
                 .emit_inference_failure_err(
                     self.fcx.tcx.hir().body_owner_def_id(self.body.id()),
@@ -765,7 +769,7 @@ impl<'cx, 'tcx> Resolver<'cx, 'tcx> {
                     E0282,
                     false,
                 )
-                .emit(),
+                .emit()
         }
     }
 

compiler/rustc_infer/src/infer/at.rs

@@ -87,6 +87,7 @@ impl<'tcx> InferCtxt<'tcx> {
             reported_signature_mismatch: self.reported_signature_mismatch.clone(),
             tainted_by_errors: self.tainted_by_errors.clone(),
             err_count_on_creation: self.err_count_on_creation,
+            stashed_err_count_on_creation: self.stashed_err_count_on_creation,
             universe: self.universe.clone(),
             intercrate,
             next_trait_solver: self.next_trait_solver,

compiler/rustc_infer/src/infer/mod.rs

@@ -306,6 +306,12 @@ pub struct InferCtxt<'tcx> {
     // FIXME(matthewjasper) Merge into `tainted_by_errors`
     err_count_on_creation: usize,
 
+    /// Track how many errors were stashed when this infcx is created.
+    /// Used for the same purpose as `err_count_on_creation`, even
+    /// though it's weaker because the count can go up and down.
+    // FIXME(matthewjasper) Merge into `tainted_by_errors`
+    stashed_err_count_on_creation: usize,
+
     /// What is the innermost universe we have created? Starts out as
     /// `UniverseIndex::root()` but grows from there as we enter
     /// universal quantifiers.
@@ -711,6 +717,7 @@ impl<'tcx> InferCtxtBuilder<'tcx> {
             reported_signature_mismatch: Default::default(),
             tainted_by_errors: Cell::new(None),
             err_count_on_creation: tcx.dcx().err_count(),
+            stashed_err_count_on_creation: tcx.dcx().stashed_err_count(),
             universe: Cell::new(ty::UniverseIndex::ROOT),
             intercrate,
             next_trait_solver,
@@ -1261,26 +1268,24 @@ impl<'tcx> InferCtxt<'tcx> {
     /// inference variables, regionck errors).
     #[must_use = "this method does not have any side effects"]
     pub fn tainted_by_errors(&self) -> Option<ErrorGuaranteed> {
-        debug!(
-            "is_tainted_by_errors(err_count={}, err_count_on_creation={}, \
-             tainted_by_errors={})",
-            self.dcx().err_count(),
-            self.err_count_on_creation,
-            self.tainted_by_errors.get().is_some()
-        );
-
-        if let Some(e) = self.tainted_by_errors.get() {
-            return Some(e);
-        }
-
-        if self.dcx().err_count() > self.err_count_on_creation {
-            // errors reported since this infcx was made
-            let e = self.dcx().has_errors().unwrap();
-            self.set_tainted_by_errors(e);
-            return Some(e);
+        if let Some(guar) = self.tainted_by_errors.get() {
+            Some(guar)
+        } else if self.dcx().err_count() > self.err_count_on_creation {
+            // Errors reported since this infcx was made.
+            let guar = self.dcx().has_errors().unwrap();
+            self.set_tainted_by_errors(guar);
+            Some(guar)
+        } else if self.dcx().stashed_err_count() > self.stashed_err_count_on_creation {
+            // Errors stashed since this infcx was made. Not entirely reliable
+            // because the count of stashed errors can go down. But without
+            // this case we get a moderate number of uninteresting and
+            // extraneous "type annotations needed" errors.
+            let guar = self.dcx().delayed_bug("tainted_by_errors: stashed bug awaiting emission");
+            self.set_tainted_by_errors(guar);
+            Some(guar)
+        } else {
+            None
         }
-
-        None
     }
 
     /// Set the "tainted by errors" flag to true. We call this when we

compiler/rustc_interface/src/passes.rs

@@ -778,6 +778,10 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> {
     // kindck is gone now). -nmatsakis
     if let Some(reported) = sess.dcx().has_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"));
     }
 
     sess.time("misc_checking_3", || {