Auto merge of #153694 - TKanX:bugfix/153391-cycle-error-key-param, r=nnethercote

fix(query): Pass Query Key to `value_from_cycle_error`



### Summary:

Pass the query key directly to `value_from_cycle_error` so that `FromCycleError` impls (notably `FnSig`) can use the recovered query's `DefId` instead of relying on `cycle[0]`, which is arbitrarily rotated by the parallel deadlock handler.

As suggested in [#153644 (comment)](#153644 (comment)).

Closes #153391 

r? @nnethercote 
cc @zetanumbers

Author: bors

compiler/rustc_middle/src/query/plumbing.rs

@@ -136,8 +136,12 @@ pub struct QueryVTable<'tcx, C: QueryCache> {
     /// For `no_hash` queries, this function pointer is None.
     pub hash_value_fn: Option<fn(&mut StableHashingContext<'_>, &C::Value) -> Fingerprint>,
 
-    pub value_from_cycle_error:
-        fn(tcx: TyCtxt<'tcx>, cycle_error: CycleError, guar: ErrorGuaranteed) -> C::Value,
+    pub value_from_cycle_error: fn(
+        tcx: TyCtxt<'tcx>,
+        key: C::Key,
+        cycle_error: CycleError,
+        guar: ErrorGuaranteed,
+    ) -> C::Value,
     pub format_value: fn(&C::Value) -> String,
 
     /// Formats a human-readable description of this query and its key, as

compiler/rustc_query_impl/src/execution.rs

@@ -125,17 +125,18 @@ where
 fn mk_cycle<'tcx, C: QueryCache>(
     query: &'tcx QueryVTable<'tcx, C>,
     tcx: TyCtxt<'tcx>,
+    key: C::Key,
     cycle_error: CycleError,
 ) -> C::Value {
     let error = report_cycle(tcx.sess, &cycle_error);
     match query.cycle_error_handling {
         CycleErrorHandling::Error => {
             let guar = error.emit();
-            (query.value_from_cycle_error)(tcx, cycle_error, guar)
+            (query.value_from_cycle_error)(tcx, key, cycle_error, guar)
         }
         CycleErrorHandling::DelayBug => {
             let guar = error.delay_as_bug();
-            (query.value_from_cycle_error)(tcx, cycle_error, guar)
+            (query.value_from_cycle_error)(tcx, key, cycle_error, guar)
         }
     }
 }
@@ -219,6 +220,7 @@ where
 fn cycle_error<'tcx, C: QueryCache>(
     query: &'tcx QueryVTable<'tcx, C>,
     tcx: TyCtxt<'tcx>,
+    key: C::Key,
     try_execute: QueryJobId,
     span: Span,
 ) -> (C::Value, Option<DepNodeIndex>) {
@@ -229,7 +231,7 @@ fn cycle_error<'tcx, C: QueryCache>(
         .expect("failed to collect active queries");
 
     let error = find_cycle_in_stack(try_execute, job_map, &current_query_job(), span);
-    (mk_cycle(query, tcx, error.lift()), None)
+    (mk_cycle(query, tcx, key, error.lift()), None)
 }
 
 #[inline(always)]
@@ -274,7 +276,7 @@ fn wait_for_query<'tcx, C: QueryCache>(
 
             (v, Some(index))
         }
-        Err(cycle) => (mk_cycle(query, tcx, cycle.lift()), None),
+        Err(cycle) => (mk_cycle(query, tcx, key, cycle.lift()), None),
     }
 }
 
@@ -337,7 +339,7 @@ fn try_execute_query<'tcx, C: QueryCache, const INCR: bool>(
 
                         // If we are single-threaded we know that we have cycle error,
                         // so we just return the error.
-                        cycle_error(query, tcx, id, span)
+                        cycle_error(query, tcx, key, id, span)
                     }
                 }
                 ActiveKeyStatus::Poisoned => FatalError.raise(),

compiler/rustc_query_impl/src/from_cycle_error.rs

@@ -15,34 +15,34 @@ use rustc_middle::query::erase::erase_val;
 use rustc_middle::ty::layout::{LayoutError, TyAndLayout};
 use rustc_middle::ty::{self, Ty, TyCtxt};
 use rustc_middle::{bug, span_bug};
-use rustc_span::def_id::LocalDefId;
+use rustc_span::def_id::{DefId, LocalDefId};
 use rustc_span::{ErrorGuaranteed, Span};
 
 use crate::job::report_cycle;
 
 pub(crate) fn specialize_query_vtables<'tcx>(vtables: &mut QueryVTables<'tcx>) {
     vtables.type_of.value_from_cycle_error =
-        |tcx, _, guar| erase_val(ty::EarlyBinder::bind(Ty::new_error(tcx, guar)));
+        |tcx, _, _, guar| erase_val(ty::EarlyBinder::bind(Ty::new_error(tcx, guar)));
 
     vtables.type_of_opaque_hir_typeck.value_from_cycle_error =
-        |tcx, _, guar| erase_val(ty::EarlyBinder::bind(Ty::new_error(tcx, guar)));
+        |tcx, _, _, guar| erase_val(ty::EarlyBinder::bind(Ty::new_error(tcx, guar)));
 
     vtables.erase_and_anonymize_regions_ty.value_from_cycle_error =
-        |tcx, _, guar| erase_val(Ty::new_error(tcx, guar));
+        |tcx, _, _, guar| erase_val(Ty::new_error(tcx, guar));
 
-    vtables.fn_sig.value_from_cycle_error = |tcx, cycle, guar| erase_val(fn_sig(tcx, cycle, guar));
+    vtables.fn_sig.value_from_cycle_error = |tcx, key, _, guar| erase_val(fn_sig(tcx, key, guar));
 
     vtables.check_representability.value_from_cycle_error =
-        |tcx, cycle, guar| check_representability(tcx, cycle, guar);
+        |tcx, _, cycle, guar| check_representability(tcx, cycle, guar);
 
     vtables.check_representability_adt_ty.value_from_cycle_error =
-        |tcx, cycle, guar| check_representability(tcx, cycle, guar);
+        |tcx, _, cycle, guar| check_representability(tcx, cycle, guar);
 
     vtables.variances_of.value_from_cycle_error =
-        |tcx, cycle, guar| erase_val(variances_of(tcx, cycle, guar));
+        |tcx, _, cycle, guar| erase_val(variances_of(tcx, cycle, guar));
 
     vtables.layout_of.value_from_cycle_error =
-        |tcx, cycle, guar| erase_val(layout_of(tcx, cycle, guar));
+        |tcx, _, cycle, guar| erase_val(layout_of(tcx, cycle, guar));
 }
 
 pub(crate) fn default<'tcx>(tcx: TyCtxt<'tcx>, cycle_error: CycleError, query_name: &str) -> ! {
@@ -57,15 +57,12 @@ pub(crate) fn default<'tcx>(tcx: TyCtxt<'tcx>, cycle_error: CycleError, query_na
 
 fn fn_sig<'tcx>(
     tcx: TyCtxt<'tcx>,
-    cycle_error: CycleError,
+    def_id: DefId,
     guar: ErrorGuaranteed,
 ) -> ty::EarlyBinder<'tcx, ty::PolyFnSig<'tcx>> {
     let err = Ty::new_error(tcx, guar);
 
-    let arity = if let Some(info) = cycle_error.cycle.get(0)
-        && info.frame.dep_kind == DepKind::fn_sig
-        && let Some(def_id) = info.frame.def_id
-        && let Some(node) = tcx.hir_get_if_local(def_id)
+    let arity = if let Some(node) = tcx.hir_get_if_local(def_id)
         && let Some(sig) = node.fn_sig()
     {
         sig.decl.inputs.len()

compiler/rustc_query_impl/src/plumbing.rs

@@ -487,7 +487,7 @@ macro_rules! define_queries {
                     #[cfg(not($cache_on_disk))]
                     is_loadable_from_disk_fn: |_tcx, _key, _index| false,
 
-                    value_from_cycle_error: |tcx, cycle, _| {
+                    value_from_cycle_error: |tcx, _, cycle, _| {
                         $crate::from_cycle_error::default(tcx, cycle, stringify!($name))
                     },
 

tests/ui/parallel-rustc/fn-sig-cycle-ice-153391.rs

@@ -0,0 +1,18 @@
+// Regression test for #153391.
+//
+//@ edition:2024
+//@ compile-flags: -Z threads=16
+//@ compare-output-by-lines
+//@ ignore-test (#142063)
+
+trait A {
+    fn g() -> B;
+    //~^ ERROR expected a type, found a trait
+}
+
+trait B {
+    fn bar(&self, x: &A);
+    //~^ ERROR expected a type, found a trait
+}
+
+fn main() {}

tests/ui/parallel-rustc/fn-sig-cycle-ice-153391.stderr

@@ -0,0 +1,31 @@
+error[E0782]: expected a type, found a trait
+  --> $DIR/fn-sig-cycle-ice-153391.rs:8:15
+   |
+LL |     fn g() -> B;
+   |               ^
+   |
+help: `B` is dyn-incompatible, use `impl B` to return an opaque type, as long as you return a single underlying type
+   |
+LL |     fn g() -> impl B;
+   |               ++++
+
+error[E0782]: expected a type, found a trait
+  --> $DIR/fn-sig-cycle-ice-153391.rs:13:23
+   |
+LL |     fn bar(&self, x: &A);
+   |                       ^
+   |
+   = note: `A` is dyn-incompatible, otherwise a trait object could be used
+help: use a new generic type parameter, constrained by `A`
+   |
+LL -     fn bar(&self, x: &A);
+LL +     fn bar<T: A>(&self, x: &T);
+   |
+help: you can also use an opaque type, but users won't be able to specify the type parameter when calling the `fn`, having to rely exclusively on type inference
+   |
+LL |     fn bar(&self, x: &impl A);
+   |                       ++++
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0782`.