Bubble up post-monomorphization errors instead of reading the global count

`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.

Instead of consulting the global count, bubble up an `ErrorGuaranteed` from the
two places that actually emit post-monomorphization errors during collection:
const-eval failures in `eval_constant` and the feature-dependent ABI checks in
`check_mono_item`. `items_of_instance` now returns whether such an error was
encountered, and `collect_items_rec` uses that to decide whether to emit the
note. This is unaffected by what other threads are doing.

The `large_assignments` lint in `move_check` is the one collection-time
diagnostic that is a lint rather than a hard error, so it never promotes to an
`ErrorGuaranteed` and is intentionally not bubbled up; its tests confirm no
instantiation note is expected.

Author: xmakro

compiler/rustc_middle/src/queries.rs

@@ -2680,11 +2680,11 @@ rustc_queries! {
     /// Perform monomorphization-time checking on this item.
     /// This is used for lints/errors that can only be checked once the instance is fully
     /// monomorphized.
-    query check_mono_item(key: ty::Instance<'tcx>) {
+    query check_mono_item(key: ty::Instance<'tcx>) -> Result<(), ErrorGuaranteed> {
         desc { "monomorphization-time checking" }
     }
 
-    query items_of_instance(key: (ty::Instance<'tcx>, CollectionMode)) -> Result<(&'tcx [Spanned<MonoItem<'tcx>>], &'tcx [Spanned<MonoItem<'tcx>>]), NormalizationErrorInMono> {
+    query items_of_instance(key: (ty::Instance<'tcx>, CollectionMode)) -> Result<(&'tcx [Spanned<MonoItem<'tcx>>], &'tcx [Spanned<MonoItem<'tcx>>], Option<ErrorGuaranteed>), NormalizationErrorInMono> {
         desc { "collecting items used by `{}`", key.0 }
         cache_on_disk
     }

compiler/rustc_middle/src/query/erase.rs

@@ -193,7 +193,7 @@ impl_erasable_for_types_with_no_type_params! {
     Result<&'_ traits::ImplSource<'_, ()>, traits::CodegenObligationError>,
     Result<&'_ ty::List<Ty<'_>>, ty::util::AlwaysRequiresDrop>,
     Result<(&'_ Steal<thir::Thir<'_>>, thir::ExprId), ErrorGuaranteed>,
-    Result<(&'_ [Spanned<MonoItem<'_>>], &'_ [Spanned<MonoItem<'_>>]), NormalizationErrorInMono>,
+    Result<(&'_ [Spanned<MonoItem<'_>>], &'_ [Spanned<MonoItem<'_>>], Option<ErrorGuaranteed>), NormalizationErrorInMono>,
     Result<(), ErrorGuaranteed>,
     Result<Option<ty::EarlyBinder<'_, ty::Const<'_>>>, ErrorGuaranteed>,
     Result<Option<ty::Instance<'_>>, ErrorGuaranteed>,

compiler/rustc_monomorphize/src/collector.rs

@@ -232,7 +232,7 @@ use rustc_middle::ty::{
 use rustc_middle::util::Providers;
 use rustc_middle::{bug, span_bug};
 use rustc_session::config::{DebugInfo, EntryFnType};
-use rustc_span::{DUMMY_SP, Span, Spanned, dummy_spanned, respan};
+use rustc_span::{DUMMY_SP, ErrorGuaranteed, Span, Spanned, dummy_spanned, respan};
 use tracing::{debug, instrument, trace};
 
 use crate::errors::{
@@ -408,12 +408,12 @@ fn collect_items_rec<'tcx>(
     // source. If the cause is in another crate, the goal here is to quickly locate which mono
     // item in the current crate is ultimately responsible for causing the error.
     //
-    // To give at least _some_ context to the user: while collecting mono items, we check the
-    // error count. If it has changed, a PME occurred, and we trigger some diagnostics about the
-    // current step of mono items collection.
-    //
-    // 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();
+    // To give at least _some_ context to the user: while collecting mono items, we track whether a
+    // post-monomorphization error was encountered while processing this item. Rather than reading
+    // the global error count (which is racy under the parallel front-end, since errors emitted on
+    // other threads would be miscounted), we bubble up an `ErrorGuaranteed` from the places that
+    // actually emit such errors (const-eval and the ABI checks) through `items_of_instance`.
+    let mut encountered_error: Option<ErrorGuaranteed> = None;
 
     // 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
@@ -472,7 +472,8 @@ fn collect_items_rec<'tcx>(
             ));
 
             rustc_data_structures::stack::ensure_sufficient_stack(|| {
-                let Ok((used, mentioned)) = tcx.items_of_instance((instance, mode)) else {
+                let Ok((used, mentioned, used_error)) = tcx.items_of_instance((instance, mode))
+                else {
                     // Normalization errors here are usually due to trait solving overflow.
                     // FIXME: I assume that there are few type errors at post-analysis stage, but not
                     // entirely sure.
@@ -488,6 +489,7 @@ fn collect_items_rec<'tcx>(
                         def_path_str,
                     });
                 };
+                encountered_error = encountered_error.or(used_error);
                 used_items.extend(used.into_iter().copied());
                 mentioned_items.extend(mentioned.into_iter().copied());
             });
@@ -538,7 +540,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 encountered_error.is_some()
         && starting_item.node.is_generic_fn()
         && starting_item.node.is_user_defined()
     {
@@ -686,6 +688,10 @@ struct MirUsedCollector<'a, 'tcx> {
     /// Note that this contains *not-monomorphized* items!
     used_mentioned_items: &'a mut UnordSet<MentionedItem<'tcx>>,
     instance: Instance<'tcx>,
+    /// Set if collection encountered an already-reported post-monomorphization error (e.g. a
+    /// const-eval failure). Bubbled up so callers can emit the "while instantiating" note without
+    /// consulting the global error count.
+    encountered_error: Option<ErrorGuaranteed>,
 }
 
 impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> {
@@ -715,8 +721,9 @@ impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> {
                 "collection encountered polymorphic constant: {:?}",
                 const_
             ),
-            Err(err @ ErrorHandled::Reported(..)) => {
+            Err(err @ ErrorHandled::Reported(info, _span)) => {
                 err.emit_note(self.tcx);
+                self.encountered_error = self.encountered_error.or(Some(info.into()));
                 return None;
             }
         }
@@ -1303,14 +1310,14 @@ fn collect_items_of_instance<'tcx>(
     tcx: TyCtxt<'tcx>,
     instance: Instance<'tcx>,
     mode: CollectionMode,
-) -> Result<(MonoItems<'tcx>, MonoItems<'tcx>), NormalizationErrorInMono> {
+) -> Result<(MonoItems<'tcx>, MonoItems<'tcx>, Option<ErrorGuaranteed>), NormalizationErrorInMono> {
     // This item is getting monomorphized, do mono-time checks.
     let body = tcx.instance_mir(instance.def);
     // Plenty of code paths later assume that everything can be normalized. So we have to check
     // normalization first.
     // We choose to emit the error outside to provide helpful diagnostics.
     check_normalization_error(tcx, instance, body)?;
-    tcx.ensure_ok().check_mono_item(instance);
+    let mono_check_error = tcx.check_mono_item(instance).err();
 
     // Naively, in "used" collection mode, all functions get added to *both* `used_items` and
     // `mentioned_items`. Mentioned items processing will then notice that they have already been
@@ -1331,6 +1338,7 @@ fn collect_items_of_instance<'tcx>(
         used_items: &mut used_items,
         used_mentioned_items: &mut used_mentioned_items,
         instance,
+        encountered_error: mono_check_error,
     };
 
     if mode == CollectionMode::UsedItems {
@@ -1361,22 +1369,24 @@ fn collect_items_of_instance<'tcx>(
         }
     }
 
-    Ok((used_items, mentioned_items))
+    let encountered_error = collector.encountered_error;
+    Ok((used_items, mentioned_items, encountered_error))
 }
 
 fn items_of_instance<'tcx>(
     tcx: TyCtxt<'tcx>,
     (instance, mode): (Instance<'tcx>, CollectionMode),
 ) -> Result<
-    (&'tcx [Spanned<MonoItem<'tcx>>], &'tcx [Spanned<MonoItem<'tcx>>]),
+    (&'tcx [Spanned<MonoItem<'tcx>>], &'tcx [Spanned<MonoItem<'tcx>>], Option<ErrorGuaranteed>),
     NormalizationErrorInMono,
 > {
-    let (used_items, mentioned_items) = collect_items_of_instance(tcx, instance, mode)?;
+    let (used_items, mentioned_items, encountered_error) =
+        collect_items_of_instance(tcx, instance, mode)?;
 
     let used_items = tcx.arena.alloc_from_iter(used_items);
     let mentioned_items = tcx.arena.alloc_from_iter(mentioned_items);
 
-    Ok((used_items, mentioned_items))
+    Ok((used_items, mentioned_items, encountered_error))
 }
 
 /// `item` must be already monomorphized.

compiler/rustc_monomorphize/src/mono_checks/abi_check.rs

@@ -5,7 +5,7 @@ use rustc_hir::{CRATE_HIR_ID, HirId};
 use rustc_middle::mir::{self, Location, traversal};
 use rustc_middle::ty::{self, Instance, InstanceKind, Ty, TyCtxt};
 use rustc_span::def_id::DefId;
-use rustc_span::{DUMMY_SP, Span, Symbol, sym};
+use rustc_span::{DUMMY_SP, ErrorGuaranteed, Span, Symbol, sym};
 use rustc_target::callconv::{FnAbi, PassMode};
 
 use crate::errors;
@@ -57,7 +57,8 @@ fn do_check_simd_vector_abi<'tcx>(
     def_id: DefId,
     is_call: bool,
     loc: impl Fn() -> (Span, HirId),
-) {
+) -> Option<ErrorGuaranteed> {
+    let mut res = None;
     let codegen_attrs = tcx.codegen_fn_attrs(def_id);
     let have_feature = |feat: Symbol| {
         let target_feats = tcx.sess.unstable_target_features.contains(&feat);
@@ -74,23 +75,25 @@ fn do_check_simd_vector_abi<'tcx>(
                     Some((_, feature)) => feature,
                     None => {
                         let (span, _hir_id) = loc();
-                        tcx.dcx().emit_err(errors::AbiErrorUnsupportedVectorType {
-                            span,
-                            ty: arg_abi.layout.ty,
-                            is_call,
-                        });
+                        res = res.or(Some(tcx.dcx().emit_err(
+                            errors::AbiErrorUnsupportedVectorType {
+                                span,
+                                ty: arg_abi.layout.ty,
+                                is_call,
+                            },
+                        )));
                         continue;
                     }
                 };
                 if !feature.is_empty() && !have_feature(Symbol::intern(feature)) {
                     let (span, _hir_id) = loc();
-                    tcx.dcx().emit_err(errors::AbiErrorDisabledVectorType {
+                    res = res.or(Some(tcx.dcx().emit_err(errors::AbiErrorDisabledVectorType {
                         span,
                         required_feature: feature,
                         ty: arg_abi.layout.ty,
                         is_call,
                         is_scalable: false,
-                    });
+                    })));
                 }
             }
             UsesVectorRegisters::ScalableVector => {
@@ -101,13 +104,13 @@ fn do_check_simd_vector_abi<'tcx>(
                 };
                 if !required_feature.is_empty() && !have_feature(Symbol::intern(required_feature)) {
                     let (span, _) = loc();
-                    tcx.dcx().emit_err(errors::AbiErrorDisabledVectorType {
+                    res = res.or(Some(tcx.dcx().emit_err(errors::AbiErrorDisabledVectorType {
                         span,
                         required_feature,
                         ty: arg_abi.layout.ty,
                         is_call,
                         is_scalable: true,
-                    });
+                    })));
                 }
             }
             UsesVectorRegisters::No => {
@@ -118,13 +121,14 @@ fn do_check_simd_vector_abi<'tcx>(
     // The `vectorcall` ABI is special in that it requires SSE2 no matter which types are being passed.
     if abi.conv == CanonAbi::X86(X86Call::Vectorcall) && !have_feature(sym::sse2) {
         let (span, _hir_id) = loc();
-        tcx.dcx().emit_err(errors::AbiRequiredTargetFeature {
+        res = res.or(Some(tcx.dcx().emit_err(errors::AbiRequiredTargetFeature {
             span,
             required_feature: "sse2",
             abi: "vectorcall",
             is_call,
-        });
+        })));
     }
+    res
 }
 
 /// Emit an error when a non-rustic ABI has unsized parameters.
@@ -136,42 +140,47 @@ fn do_check_unsized_params<'tcx>(
     fn_abi: &FnAbi<'tcx, Ty<'tcx>>,
     is_call: bool,
     loc: impl Fn() -> (Span, HirId),
-) {
+) -> Option<ErrorGuaranteed> {
     // Unsized parameters are allowed with the (unstable) "Rust" (and similar) ABIs.
     if fn_abi.conv.is_rustic_abi() {
-        return;
+        return None;
     }
 
+    let mut res = None;
     for arg_abi in fn_abi.args.iter() {
         if !arg_abi.layout.layout.is_sized() {
             let (span, _hir_id) = loc();
-            tcx.dcx().emit_err(errors::AbiErrorUnsupportedUnsizedParameter {
+            res = res.or(Some(tcx.dcx().emit_err(errors::AbiErrorUnsupportedUnsizedParameter {
                 span,
                 ty: arg_abi.layout.ty,
                 is_call,
-            });
+            })));
         }
     }
+    res
 }
 
 /// Checks the ABI of an Instance, emitting an error when:
 ///
 /// - a non-rustic ABI uses unsized parameters
 /// - the signature requires target features that are not enabled
-fn check_instance_abi<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) {
+fn check_instance_abi<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    instance: Instance<'tcx>,
+) -> Option<ErrorGuaranteed> {
     let typing_env = ty::TypingEnv::fully_monomorphized();
     let ty = instance.ty(tcx, typing_env);
     if ty.is_fn() && ty.fn_sig(tcx).abi() == ExternAbi::Unadjusted {
         // We disable all checks for the unadjusted ABI to allow linking to arbitrary LLVM
         // intrinsics
-        return;
+        return None;
     }
     let Ok(abi) = tcx.fn_abi_of_instance(typing_env.as_query_input((instance, ty::List::empty())))
     else {
         // An error will be reported during codegen if we cannot determine the ABI of this
         // function.
         tcx.dcx().delayed_bug("ABI computation failure should lead to compilation failure");
-        return;
+        return None;
     };
     // Unlike the call-site check, we do also check "Rust" ABI functions here.
     // This should never trigger, *except* if we start making use of vector registers
@@ -186,8 +195,10 @@ fn check_instance_abi<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) {
             def_id.as_local().map(|did| tcx.local_def_id_to_hir_id(did)).unwrap_or(CRATE_HIR_ID),
         )
     };
-    do_check_unsized_params(tcx, abi, /*is_call*/ false, loc);
-    do_check_simd_vector_abi(tcx, abi, instance.def_id(), /*is_call*/ false, loc);
+    // Call both checks unconditionally for their diagnostic side effects before combining.
+    let unsized_res = do_check_unsized_params(tcx, abi, /*is_call*/ false, loc);
+    let simd_res = do_check_simd_vector_abi(tcx, abi, instance.def_id(), /*is_call*/ false, loc);
+    unsized_res.or(simd_res)
 }
 
 /// Check the ABI at a call site, emitting an error when:
@@ -199,14 +210,14 @@ fn check_call_site_abi<'tcx>(
     callee: Ty<'tcx>,
     caller: InstanceKind<'tcx>,
     loc: impl Fn() -> (Span, HirId) + Copy,
-) {
+) -> Option<ErrorGuaranteed> {
     let extern_abi = callee.fn_sig(tcx).abi();
     if extern_abi.is_rustic_abi() || extern_abi == ExternAbi::Unadjusted {
         // We directly handle the soundness of Rust ABIs -- so let's skip the majority of
         // call sites to avoid a perf regression.
         // We disable all checks for the unadjusted ABI to allow linking to arbitrary LLVM
         // intrinsics
-        return;
+        return None;
     }
     let typing_env = ty::TypingEnv::fully_monomorphized();
     let callee_abi = match *callee.kind() {
@@ -216,7 +227,7 @@ fn check_call_site_abi<'tcx>(
         ty::FnDef(def_id, args) => {
             // Intrinsics are handled separately by the compiler.
             if tcx.intrinsic(def_id).is_some() {
-                return;
+                return None;
             }
             let instance = ty::Instance::expect_resolve(tcx, typing_env, def_id, args, DUMMY_SP);
             tcx.fn_abi_of_instance(typing_env.as_query_input((instance, ty::List::empty())))
@@ -228,13 +239,21 @@ fn check_call_site_abi<'tcx>(
 
     let Ok(callee_abi) = callee_abi else {
         // ABI failed to compute; this will not get through codegen.
-        return;
+        return None;
     };
-    do_check_unsized_params(tcx, callee_abi, /*is_call*/ true, loc);
-    do_check_simd_vector_abi(tcx, callee_abi, caller.def_id(), /*is_call*/ true, loc);
+    // Call both checks unconditionally for their diagnostic side effects before combining.
+    let unsized_res = do_check_unsized_params(tcx, callee_abi, /*is_call*/ true, loc);
+    let simd_res =
+        do_check_simd_vector_abi(tcx, callee_abi, caller.def_id(), /*is_call*/ true, loc);
+    unsized_res.or(simd_res)
 }
 
-fn check_callees_abi<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>, body: &mir::Body<'tcx>) {
+fn check_callees_abi<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    instance: Instance<'tcx>,
+    body: &mir::Body<'tcx>,
+) -> Option<ErrorGuaranteed> {
+    let mut res = None;
     // Check all function call terminators.
     for (bb, _data) in traversal::mono_reachable(body, tcx, instance) {
         let terminator = body.basic_blocks[bb].terminator();
@@ -247,7 +266,7 @@ fn check_callees_abi<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>, body: &m
                     ty::TypingEnv::fully_monomorphized(),
                     ty::EarlyBinder::bind(callee_ty),
                 );
-                check_call_site_abi(tcx, callee_ty, body.source.instance, || {
+                res = res.or(check_call_site_abi(tcx, callee_ty, body.source.instance, || {
                     let loc = Location {
                         block: bb,
                         statement_index: body.basic_blocks[bb].statements.len(),
@@ -259,18 +278,21 @@ fn check_callees_abi<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>, body: &m
                             .lint_root(&body.source_scopes)
                             .unwrap_or(CRATE_HIR_ID),
                     )
-                });
+                }));
             }
             _ => {}
         }
     }
+    res
 }
 
 pub(crate) fn check_feature_dependent_abi<'tcx>(
     tcx: TyCtxt<'tcx>,
     instance: Instance<'tcx>,
     body: &'tcx mir::Body<'tcx>,
-) {
-    check_instance_abi(tcx, instance);
-    check_callees_abi(tcx, instance, body);
+) -> Option<ErrorGuaranteed> {
+    // Call both checks unconditionally for their diagnostic side effects before combining.
+    let instance_res = check_instance_abi(tcx, instance);
+    let callees_res = check_callees_abi(tcx, instance, body);
+    instance_res.or(callees_res)
 }