diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index d7d306918fd0d..f0fa906db5e1b 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -249,20 +249,22 @@ internal compiler error: query cycle handler thread panicked, aborting process"; tls::enter_context(&tls::ImplicitCtxt::new(gcx), || { tls::with(|tcx| { // Accessing session globals is sound as they outlive `GlobalCtxt`. - // They are needed to hash query keys containing spans or symbols. - let job_map = rustc_span::set_session_globals_then( + // They are needed to hash query keys containing spans or symbols, + // and to print query descriptions while breaking the cycle, so the + // whole cycle search runs inside this scope. + rustc_span::set_session_globals_then( unsafe { &*(session_globals as *const SessionGlobals) }, || { // Ensure there were no errors collecting all active jobs. // We need the complete map to ensure we find a cycle to // break. - collect_active_query_jobs( + let job_map = collect_active_query_jobs( tcx, CollectActiveJobsKind::FullNoContention, - ) + ); + break_query_cycle(tcx, job_map, ®istry); }, ); - break_query_cycle(job_map, ®istry); }) }) }); diff --git a/compiler/rustc_query_impl/src/job.rs b/compiler/rustc_query_impl/src/job.rs index bf0493b29fd1e..e9cdcfb3f2f24 100644 --- a/compiler/rustc_query_impl/src/job.rs +++ b/compiler/rustc_query_impl/src/job.rs @@ -230,7 +230,11 @@ fn connected_to_root<'tcx>( } /// Processes a found query cycle into a `Cycle` -fn process_cycle<'tcx>(job_map: &QueryJobMap<'tcx>, stack: Vec<(Span, QueryJobId)>) -> Cycle<'tcx> { +fn process_cycle<'tcx>( + tcx: TyCtxt<'tcx>, + job_map: &QueryJobMap<'tcx>, + stack: Vec<(Span, QueryJobId)>, +) -> Cycle<'tcx> { // The stack is a vector of pairs of spans and queries; reverse it so that // the earlier entries require later entries let (mut spans, queries): (Vec<_>, Vec<_>) = stack.into_iter().rev().unzip(); @@ -277,11 +281,43 @@ fn process_cycle<'tcx>(job_map: &QueryJobMap<'tcx>, stack: Vec<(Span, QueryJobId }) .collect::>(); - // Pick an entry point, preferring ones with waiters + // The query jobs in the cycle carry no useful span of their own here (it is + // `DUMMY_SP`), but the cycle stack records, for each query, its incoming edge: + // the span where its predecessor in the cycle requested it. + let stack_span = |query: QueryJobId| { + stack.iter().find(|(_, q)| *q == query).map_or(DUMMY_SP, |&(span, _)| span) + }; + + // Pick an entry point with a stable ordering. The parallel deadlock handler + // collects active query jobs in a nondeterministic order, so both the order + // of `entry_points` and the original `entry_points[0]` fallback were racy, + // which made the reported cycle anchor change from run to run. + // + // Order by incoming-edge span first: for the recursive-definition cycles + // these errors come from, the entry point with the latest incoming-edge span + // is the query the single-threaded path anchors at, so this keeps the + // parallel output matching the committed `.stderr`. Span ties prefer an entry + // point with an outside waiter so the "cycle used when ..." note is still + // produced, then fall back to the query's stable description. + // + // This is a heuristic, not a guaranteed total order: two entry points that + // share both a span and a description still resolve arbitrarily, and there is + // no stable per-job key to order them by since `QueryJobId` is assigned in + // racy execution order. The cycles these tests exercise have distinct edge + // spans, so the anchor is deterministic in practice. + let description = |query: QueryJobId| job_map.tagged_key_of(query).description(tcx); let entry_point = entry_points .iter() - .find(|entry_point| entry_point.query_waiting_on_cycle.is_some()) - .unwrap_or(&entry_points[0]); + .max_by(|a, b| { + let (sa, sb) = (stack_span(a.query_in_cycle), stack_span(b.query_in_cycle)); + (sa.lo(), sa.hi()) + .cmp(&(sb.lo(), sb.hi())) + .then_with(|| { + a.query_waiting_on_cycle.is_some().cmp(&b.query_waiting_on_cycle.is_some()) + }) + .then_with(|| description(a.query_in_cycle).cmp(&description(b.query_in_cycle))) + }) + .expect("a query cycle must have at least one entry point"); // Shift the stack so that our entry point is first let entry_point_pos = stack.iter().position(|(_, query)| *query == entry_point.query_in_cycle); @@ -306,6 +342,7 @@ fn process_cycle<'tcx>(job_map: &QueryJobMap<'tcx>, stack: Vec<(Span, QueryJobId /// Looks for a query cycle starting at `query`. /// Returns a waiter to resume if a cycle is found. fn find_and_process_cycle<'tcx>( + tcx: TyCtxt<'tcx>, job_map: &QueryJobMap<'tcx>, query: QueryJobId, ) -> Option>> { @@ -315,7 +352,7 @@ fn find_and_process_cycle<'tcx>( find_cycle(job_map, query, DUMMY_SP, &mut stack, &mut visited) { // Create the cycle error - let error = process_cycle(job_map, stack); + let error = process_cycle(tcx, job_map, stack); // We unwrap `resumable` here since there must always be one // edge which is resumable / waited using a query latch @@ -341,12 +378,16 @@ fn find_and_process_cycle<'tcx>( /// There may be multiple cycles involved in a deadlock, but this only breaks one at a time so /// there will be multiple rounds through the deadlock handler if multiple cycles are present. #[allow(rustc::potential_query_instability)] -pub fn break_query_cycle<'tcx>(job_map: QueryJobMap<'tcx>, registry: &rustc_thread_pool::Registry) { +pub fn break_query_cycle<'tcx>( + tcx: TyCtxt<'tcx>, + job_map: QueryJobMap<'tcx>, + registry: &rustc_thread_pool::Registry, +) { // Look for a cycle starting at each query job let waiter = job_map .map .keys() - .find_map(|query| find_and_process_cycle(&job_map, *query)) + .find_map(|query| find_and_process_cycle(tcx, &job_map, *query)) .expect("unable to find a query cycle"); // Mark the thread we're about to wake up as unblocked. diff --git a/tests/ui/consts/recursive-zst-static.rs b/tests/ui/consts/recursive-zst-static.rs index 003707aeeab6f..853af6d70eb09 100644 --- a/tests/ui/consts/recursive-zst-static.rs +++ b/tests/ui/consts/recursive-zst-static.rs @@ -1,6 +1,6 @@ //@ revisions: default unleash //@[unleash]compile-flags: -Zunleash-the-miri-inside-of-you -//@ ignore-parallel-frontend query cycle + // This test ensures that we do not allow ZST statics to initialize themselves without ever // actually creating a value of that type. This is important, as the ZST may have private fields // that users can reasonably expect to only get initialized by their own code. Thus unsafe code diff --git a/tests/ui/cycle-trait/issue-12511.rs b/tests/ui/cycle-trait/issue-12511.rs index 1d33b03c57a2c..ea83e3fd9dc2f 100644 --- a/tests/ui/cycle-trait/issue-12511.rs +++ b/tests/ui/cycle-trait/issue-12511.rs @@ -1,7 +1,7 @@ trait T1 : T2 { //~^ ERROR cycle detected } -//@ ignore-parallel-frontend query cycle + trait T2 : T1 { } diff --git a/tests/ui/delegation/unsupported.rs b/tests/ui/delegation/unsupported.rs index 3349c4ec27ab1..a2ba2a71a91f4 100644 --- a/tests/ui/delegation/unsupported.rs +++ b/tests/ui/delegation/unsupported.rs @@ -3,7 +3,7 @@ //@ ignore-compare-mode-next-solver (explicit revisions) //@[next] compile-flags: -Znext-solver //@ check-fail -//@ ignore-parallel-frontend query cycle + // Next solver revision included because of trait-system-refactor-initiative#234. // If we end up in a query cycle, it should be okay as long as results are the same. diff --git a/tests/ui/infinite/infinite-trait-alias-recursion.rs b/tests/ui/infinite/infinite-trait-alias-recursion.rs index df884cca22b77..0bd4624de3f1e 100644 --- a/tests/ui/infinite/infinite-trait-alias-recursion.rs +++ b/tests/ui/infinite/infinite-trait-alias-recursion.rs @@ -1,5 +1,5 @@ #![feature(trait_alias)] -//@ ignore-parallel-frontend query cycle + trait T1 = T2; //~^ ERROR cycle detected when computing the implied predicates of `T1` diff --git a/tests/ui/infinite/infinite-type-alias-mutual-recursion.rs b/tests/ui/infinite/infinite-type-alias-mutual-recursion.rs index 24e1318ca3d7d..01e1ec8ab7b7f 100644 --- a/tests/ui/infinite/infinite-type-alias-mutual-recursion.rs +++ b/tests/ui/infinite/infinite-type-alias-mutual-recursion.rs @@ -1,5 +1,5 @@ //@ revisions: feature_old gated_old feature_new gated_new -//@ ignore-parallel-frontend query cycle + //@ ignore-compare-mode-next-solver (explicit revisions) //@ [feature_new] compile-flags: -Znext-solver //@ [gated_new] compile-flags: -Znext-solver diff --git a/tests/ui/issues/issue-34373.rs b/tests/ui/issues/issue-34373.rs index 02e1048e5a330..a9bd79f6830b8 100644 --- a/tests/ui/issues/issue-34373.rs +++ b/tests/ui/issues/issue-34373.rs @@ -1,5 +1,5 @@ #![allow(warnings)] -//@ ignore-parallel-frontend query cycle + trait Trait { fn foo(_: T) {} }