Attribute post-monomorphization error notes by emitting thread

collect_items_rec snapshots the error count before processing a mono item
and compares it afterwards to decide whether to attach the "the above error
was encountered while instantiating ..." note. The mono item graph is walked
in parallel, so the global error count can be bumped by an error emitted while
collecting a different item on another thread between the two reads, which
makes an unrelated item (often lang_start) get blamed.

Record the thread that emits each error alongside its ErrorGuaranteed in
err_guars, lint_err_guars, and the stashed-diagnostics map, and add
err_count_on_current_thread, which counts only the errors whose recorded
thread is the calling thread. The collector uses it for the snapshot and
comparison, so the delta is not affected by errors emitted on other threads.
In serial compilation it is equivalent to a delta of err_count.

Author: xmakro

compiler/rustc_errors/src/lib.rs

@@ -24,6 +24,7 @@ use std::io::Write;
 use std::num::NonZero;
 use std::ops::DerefMut;
 use std::path::{Path, PathBuf};
+use std::thread::ThreadId;
 use std::{assert_matches, fmt, panic};
 
 use Level::*;
@@ -297,11 +298,12 @@ impl<'a> std::ops::Deref for DiagCtxtHandle<'a> {
 struct DiagCtxtInner {
     flags: DiagCtxtFlags,
 
-    /// The error guarantees from all emitted errors. The length gives the error count.
-    err_guars: Vec<ErrorGuaranteed>,
-    /// The error guarantee from all emitted lint errors. The length gives the
-    /// lint error count.
-    lint_err_guars: Vec<ErrorGuaranteed>,
+    /// The error guarantees from all emitted errors, each paired with the
+    /// thread that emitted it. The length gives the error count.
+    err_guars: Vec<(ErrorGuaranteed, ThreadId)>,
+    /// The error guarantee from all emitted lint errors, each paired with the
+    /// thread that emitted it. The length gives the lint error count.
+    lint_err_guars: Vec<(ErrorGuaranteed, ThreadId)>,
     /// The delayed bugs and their error guarantees.
     delayed_bugs: Vec<(DelayedDiagInner, ErrorGuaranteed)>,
 
@@ -343,7 +345,7 @@ struct DiagCtxtInner {
     /// `emit_stashed_diagnostics` by the time the `DiagCtxtInner` is dropped,
     /// otherwise an assertion failure will occur.
     stashed_diagnostics:
-        FxIndexMap<StashKey, FxIndexMap<Span, (DiagInner, Option<ErrorGuaranteed>)>>,
+        FxIndexMap<StashKey, FxIndexMap<Span, (DiagInner, Option<(ErrorGuaranteed, ThreadId)>)>>,
 
     future_breakage_diagnostics: Vec<DiagInner>,
 
@@ -608,12 +610,10 @@ impl<'a> DiagCtxtHandle<'a> {
         // 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.
-        self.inner
-            .borrow_mut()
-            .stashed_diagnostics
-            .entry(key)
-            .or_default()
-            .insert(span.with_parent(None), (diag, guar));
+        self.inner.borrow_mut().stashed_diagnostics.entry(key).or_default().insert(
+            span.with_parent(None),
+            (diag, guar.map(|guar| (guar, std::thread::current().id()))),
+        );
 
         guar
     }
@@ -714,6 +714,27 @@ impl<'a> DiagCtxtHandle<'a> {
                 .sum::<usize>()
     }
 
+    /// The number of errors that have been emitted on the *current thread*.
+    ///
+    /// Like [`DiagCtxtHandle::err_count`], but only counts errors whose recorded
+    /// emitting thread is the calling thread. It is meant for code that takes a
+    /// before/after delta to detect whether a unit of work emitted an error, and
+    /// must not be misled by errors emitted on other threads at the same time
+    /// (#154260). In serial compilation it is equivalent to taking a delta of
+    /// `err_count`.
+    pub fn err_count_on_current_thread(&self) -> usize {
+        let inner = self.inner.borrow();
+        let current = std::thread::current().id();
+        inner.err_guars.iter().filter(|(_, thread)| *thread == current).count()
+            + inner.lint_err_guars.iter().filter(|(_, thread)| *thread == current).count()
+            + inner
+                .stashed_diagnostics
+                .values()
+                .flat_map(|stashed| stashed.values())
+                .filter(|(_, guar)| matches!(guar, Some((_, thread)) if *thread == current))
+                .count()
+    }
+
     /// 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> {
@@ -879,7 +900,8 @@ impl<'a> DiagCtxtHandle<'a> {
             // This `unchecked_error_guaranteed` is valid. It is where the
             // `ErrorGuaranteed` for unused_extern errors originates.
             #[allow(deprecated)]
-            inner.lint_err_guars.push(ErrorGuaranteed::unchecked_error_guaranteed());
+            let guar = ErrorGuaranteed::unchecked_error_guaranteed();
+            inner.lint_err_guars.push((guar, std::thread::current().id()));
             inner.panic_if_treat_err_as_bug();
         }
 
@@ -1347,13 +1369,17 @@ impl DiagCtxtInner {
                 // `ErrorGuaranteed` for errors and lint errors originates.
                 #[allow(deprecated)]
                 let guar = ErrorGuaranteed::unchecked_error_guaranteed();
+                // Record which thread emitted the error so that
+                // `err_count_on_current_thread` can attribute it correctly when
+                // the mono item graph is collected in parallel (#154260).
+                let thread = std::thread::current().id();
                 if is_lint {
-                    self.lint_err_guars.push(guar);
+                    self.lint_err_guars.push((guar, thread));
                 } else {
                     if let Some(taint) = taint {
                         taint.set(Some(guar));
                     }
-                    self.err_guars.push(guar);
+                    self.err_guars.push((guar, thread));
                 }
                 self.panic_if_treat_err_as_bug();
                 Some(guar)
@@ -1377,28 +1403,30 @@ impl DiagCtxtInner {
     }
 
     fn has_errors_excluding_lint_errors(&self) -> Option<ErrorGuaranteed> {
-        self.err_guars.get(0).copied().or_else(|| {
+        self.err_guars.get(0).map(|(guar, _)| *guar).or_else(|| {
             if let Some((_diag, guar)) = self
                 .stashed_diagnostics
                 .values()
                 .flat_map(|stashed_diagnostics| stashed_diagnostics.values())
                 .find(|(diag, guar)| guar.is_some() && diag.is_lint.is_none())
             {
-                *guar
+                (*guar).map(|(guar, _)| guar)
             } else {
                 None
             }
         })
     }
 
     fn has_errors(&self) -> Option<ErrorGuaranteed> {
-        self.err_guars.get(0).copied().or_else(|| self.lint_err_guars.get(0).copied()).or_else(
-            || {
+        self.err_guars
+            .get(0)
+            .map(|(guar, _)| *guar)
+            .or_else(|| self.lint_err_guars.get(0).map(|(guar, _)| *guar))
+            .or_else(|| {
                 self.stashed_diagnostics.values().find_map(|stashed_diagnostics| {
-                    stashed_diagnostics.values().find_map(|(_, guar)| *guar)
+                    stashed_diagnostics.values().find_map(|(_, guar)| (*guar).map(|(guar, _)| guar))
                 })
-            },
-        )
+            })
     }
 
     fn has_errors_or_delayed_bugs(&self) -> Option<ErrorGuaranteed> {

compiler/rustc_monomorphize/src/collector.rs

@@ -412,8 +412,14 @@ fn collect_items_rec<'tcx>(
     // error count. If it has changed, a PME occurred, and we trigger some diagnostics about the
     // current step of mono items collection.
     //
+    // We use the per-thread error count rather than the global one: the mono item graph is walked
+    // in parallel (see the `par_for_each_in` in `collect_crate_mono_items`), so the global count
+    // can be bumped by an unrelated error emitted on another thread between the two reads, which
+    // would then wrongly blame this item (#154260). The per-thread count is not affected by errors
+    // emitted on other threads, so the delta reflects work done on this thread.
+    //
     // FIXME: don't rely on global state, instead bubble up errors. Note: this is very hard to do.
-    let error_count = tcx.dcx().err_count();
+    let error_count = tcx.dcx().err_count_on_current_thread();
 
     // In `mentioned_items` we collect items that were mentioned in this MIR but possibly do not
     // need to be monomorphized. This is done to ensure that optimizing away function calls does not
@@ -538,7 +544,7 @@ fn collect_items_rec<'tcx>(
 
     // Check for PMEs and emit a diagnostic if one happened. To try to show relevant edges of the
     // mono item graph.
-    if tcx.dcx().err_count() > error_count
+    if tcx.dcx().err_count_on_current_thread() > error_count
         && starting_item.node.is_generic_fn()
         && starting_item.node.is_user_defined()
     {