From 18f8c150051e9f4543ff800cf02091f73a0eaed4 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 17 Mar 2026 11:43:53 +1100 Subject: [PATCH] Rename various query cycle things. The existing names have bugged me for a while. Changes: - `CycleError` -> `Cycle`. Because we normally use "error" for a `Diag` with `Level::Error`, and this type is just a precursor to that. Also, many existing locals of this type are already named `cycle`. - `CycleError::cycle` -> `Cycle::stack`. Because it is a stack. And a couple of existing locals are already named `stack`. - `cycle_error` -> `find_and_handle_cycle`. Because that's what it does. The existing name is just a non-descript noun phrase. - `mk_cycle` -> `handle_cycle`. Because it doesn't make the cycle; the cycle is already made. It handles the cycle, which involves (a) creating the error, and (b) handling the error. - `report_cycle` -> `create_cycle_error`. Because that's what it does: creates the cycle error (i.e. the `Diag`). - `value_from_cycle_error` -> `handle_cycle_error_fn`. Because most cases don't produce a value, they just emit an error and quit. And the `_fn` suffix is for consistency with other vtable fns. - `from_cycle_error` -> `handle_cycle_error`. A similar story for this module. --- compiler/rustc_middle/src/queries.rs | 8 ++-- compiler/rustc_middle/src/query/job.rs | 6 +-- compiler/rustc_middle/src/query/mod.rs | 2 +- compiler/rustc_middle/src/query/plumbing.rs | 15 +++--- compiler/rustc_query_impl/src/execution.rs | 24 +++++----- ...m_cycle_error.rs => handle_cycle_error.rs} | 47 +++++++++---------- compiler/rustc_query_impl/src/job.rs | 24 +++++----- compiler/rustc_query_impl/src/lib.rs | 4 +- compiler/rustc_query_impl/src/query_impl.rs | 8 ++-- .../rustc_ty_utils/src/representability.rs | 2 +- 10 files changed, 66 insertions(+), 74 deletions(-) rename compiler/rustc_query_impl/src/{from_cycle_error.rs => handle_cycle_error.rs} (91%) diff --git a/compiler/rustc_middle/src/queries.rs b/compiler/rustc_middle/src/queries.rs index 4dd68db4f9135..23f77e11985f8 100644 --- a/compiler/rustc_middle/src/queries.rs +++ b/compiler/rustc_middle/src/queries.rs @@ -130,10 +130,10 @@ use crate::{mir, thir}; // `Providers` that the driver creates (using several `rustc_*` crates). // // The result type of each query must implement `Clone`. Additionally -// `ty::query::from_cycle_error::FromCycleError` can be implemented which produces an appropriate +// `QueryVTable::handle_cycle_error_fn` can be used to produce an appropriate // placeholder (error) value if the query resulted in a query cycle. -// Queries without a `FromCycleError` implementation will raise a fatal error on query -// cycles instead. +// Queries without a custom `handle_cycle_error_fn` implementation will raise a +// fatal error on query cycles instead. rustc_queries! { /// Caches the expansion of a derive proc macro, e.g. `#[derive(Serialize)]`. /// The key is: @@ -566,7 +566,7 @@ rustc_queries! { /// Checks whether a type is representable or infinitely sized // - // Infinitely sized types will cause a cycle. The `value_from_cycle_error` impl will print + // Infinitely sized types will cause a cycle. The query's `handle_cycle_error_fn` will print // a custom error about the infinite size and then abort compilation. (In the past we // recovered and continued, but in practice that leads to confusing subsequent error // messages about cycles that then abort.) diff --git a/compiler/rustc_middle/src/query/job.rs b/compiler/rustc_middle/src/query/job.rs index 3bf37a782ee8a..24c4daf9855d2 100644 --- a/compiler/rustc_middle/src/query/job.rs +++ b/compiler/rustc_middle/src/query/job.rs @@ -6,7 +6,7 @@ use std::sync::Arc; use parking_lot::{Condvar, Mutex}; use rustc_span::Span; -use crate::query::CycleError; +use crate::query::Cycle; use crate::ty::TyCtxt; /// A value uniquely identifying an active query job. @@ -59,7 +59,7 @@ pub struct QueryWaiter<'tcx> { pub parent: Option, pub condvar: Condvar, pub span: Span, - pub cycle: Mutex>>, + pub cycle: Mutex>>, } #[derive(Clone, Debug)] @@ -79,7 +79,7 @@ impl<'tcx> QueryLatch<'tcx> { tcx: TyCtxt<'tcx>, query: Option, span: Span, - ) -> Result<(), CycleError<'tcx>> { + ) -> Result<(), Cycle<'tcx>> { let mut waiters_guard = self.waiters.lock(); let Some(waiters) = &mut *waiters_guard else { return Ok(()); // already complete diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 1723a853ea750..713efed18efec 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -4,7 +4,7 @@ pub use self::caches::{DefIdCache, DefaultCache, QueryCache, SingleCache, VecCac pub use self::job::{QueryJob, QueryJobId, QueryLatch, QueryWaiter}; pub use self::keys::{AsLocalQueryKey, LocalCrate, QueryKey}; pub use self::plumbing::{ - ActiveKeyStatus, CycleError, EnsureMode, IntoQueryParam, QueryMode, QueryState, QuerySystem, + ActiveKeyStatus, Cycle, EnsureMode, IntoQueryParam, QueryMode, QueryState, QuerySystem, QueryVTable, TyCtxtAt, TyCtxtEnsureDone, TyCtxtEnsureOk, TyCtxtEnsureResult, }; pub use self::stack::QueryStackFrame; diff --git a/compiler/rustc_middle/src/query/plumbing.rs b/compiler/rustc_middle/src/query/plumbing.rs index e88c20f02537b..894d02f5e2aa9 100644 --- a/compiler/rustc_middle/src/query/plumbing.rs +++ b/compiler/rustc_middle/src/query/plumbing.rs @@ -51,12 +51,12 @@ pub enum ActiveKeyStatus<'tcx> { } #[derive(Debug)] -pub struct CycleError<'tcx> { +pub struct Cycle<'tcx> { /// The query and related span that uses the cycle. pub usage: Option>>, /// The span here corresponds to the reason for which this query was required. - pub cycle: Vec>>, + pub stack: Vec>>, } #[derive(Debug)] @@ -120,13 +120,10 @@ pub struct QueryVTable<'tcx, C: QueryCache> { /// Function pointer that handles a cycle error. `error` must be consumed, e.g. with `emit` (if /// it should be emitted) or `delay_as_bug` (if it need not be emitted because an alternative - /// error is created and emitted). - pub value_from_cycle_error: fn( - tcx: TyCtxt<'tcx>, - key: C::Key, - cycle_error: CycleError<'tcx>, - error: Diag<'_>, - ) -> C::Value, + /// error is created and emitted). A value may be returned, or (more commonly) the function may + /// just abort after emitting the error. + pub handle_cycle_error_fn: + fn(tcx: TyCtxt<'tcx>, key: C::Key, cycle: Cycle<'tcx>, error: Diag<'_>) -> C::Value, pub format_value: fn(&C::Value) -> String, diff --git a/compiler/rustc_query_impl/src/execution.rs b/compiler/rustc_query_impl/src/execution.rs index 1224ec0706cf0..31f41fd29a096 100644 --- a/compiler/rustc_query_impl/src/execution.rs +++ b/compiler/rustc_query_impl/src/execution.rs @@ -8,8 +8,8 @@ use rustc_data_structures::{outline, sharded, sync}; use rustc_errors::FatalError; use rustc_middle::dep_graph::{DepGraphData, DepNodeKey, SerializedDepNodeIndex}; use rustc_middle::query::{ - ActiveKeyStatus, CycleError, EnsureMode, QueryCache, QueryJob, QueryJobId, QueryKey, - QueryLatch, QueryMode, QueryState, QueryVTable, + ActiveKeyStatus, Cycle, EnsureMode, QueryCache, QueryJob, QueryJobId, QueryKey, QueryLatch, + QueryMode, QueryState, QueryVTable, }; use rustc_middle::ty::TyCtxt; use rustc_middle::verify_ich::incremental_verify_ich; @@ -17,7 +17,7 @@ use rustc_span::{DUMMY_SP, Span}; use tracing::warn; use crate::dep_graph::{DepNode, DepNodeIndex}; -use crate::job::{QueryJobInfo, QueryJobMap, find_cycle_in_stack, report_cycle}; +use crate::job::{QueryJobInfo, QueryJobMap, create_cycle_error, find_cycle_in_stack}; use crate::plumbing::{current_query_job, next_job_id, start_query}; use crate::query_impl::for_each_query_vtable; @@ -107,14 +107,14 @@ fn collect_active_query_jobs_inner<'tcx, C>( #[cold] #[inline(never)] -fn mk_cycle<'tcx, C: QueryCache>( +fn handle_cycle<'tcx, C: QueryCache>( query: &'tcx QueryVTable<'tcx, C>, tcx: TyCtxt<'tcx>, key: C::Key, - cycle_error: CycleError<'tcx>, + cycle: Cycle<'tcx>, ) -> C::Value { - let error = report_cycle(tcx, &cycle_error); - (query.value_from_cycle_error)(tcx, key, cycle_error, error) + let error = create_cycle_error(tcx, &cycle); + (query.handle_cycle_error_fn)(tcx, key, cycle, error) } /// Guard object representing the responsibility to execute a query job and @@ -193,7 +193,7 @@ where #[cold] #[inline(never)] -fn cycle_error<'tcx, C: QueryCache>( +fn find_and_handle_cycle<'tcx, C: QueryCache>( query: &'tcx QueryVTable<'tcx, C>, tcx: TyCtxt<'tcx>, key: C::Key, @@ -204,8 +204,8 @@ fn cycle_error<'tcx, C: QueryCache>( // We need the complete map to ensure we find a cycle to break. let job_map = collect_active_query_jobs(tcx, CollectActiveJobsKind::FullNoContention); - let error = find_cycle_in_stack(try_execute, job_map, ¤t_query_job(), span); - (mk_cycle(query, tcx, key, error), None) + let cycle = find_cycle_in_stack(try_execute, job_map, ¤t_query_job(), span); + (handle_cycle(query, tcx, key, cycle), None) } #[inline(always)] @@ -249,7 +249,7 @@ fn wait_for_query<'tcx, C: QueryCache>( (v, Some(index)) } - Err(cycle) => (mk_cycle(query, tcx, key, cycle), None), + Err(cycle) => (handle_cycle(query, tcx, key, cycle), None), } } @@ -333,7 +333,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, key, id, span) + find_and_handle_cycle(query, tcx, key, id, span) } } ActiveKeyStatus::Poisoned => FatalError.raise(), diff --git a/compiler/rustc_query_impl/src/from_cycle_error.rs b/compiler/rustc_query_impl/src/handle_cycle_error.rs similarity index 91% rename from compiler/rustc_query_impl/src/from_cycle_error.rs rename to compiler/rustc_query_impl/src/handle_cycle_error.rs index fa508419b52f2..7a6cde1b81fac 100644 --- a/compiler/rustc_query_impl/src/from_cycle_error.rs +++ b/compiler/rustc_query_impl/src/handle_cycle_error.rs @@ -10,7 +10,7 @@ use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_middle::dep_graph::DepKind; use rustc_middle::queries::{QueryVTables, TaggedQueryKey}; -use rustc_middle::query::CycleError; +use rustc_middle::query::Cycle; use rustc_middle::query::erase::erase_val; use rustc_middle::ty::layout::LayoutError; use rustc_middle::ty::{self, Ty, TyCtxt}; @@ -18,26 +18,26 @@ use rustc_middle::{bug, span_bug}; use rustc_span::def_id::{DefId, LocalDefId}; use rustc_span::{ErrorGuaranteed, Span}; -use crate::job::report_cycle; +use crate::job::create_cycle_error; pub(crate) fn specialize_query_vtables<'tcx>(vtables: &mut QueryVTables<'tcx>) { - vtables.fn_sig.value_from_cycle_error = |tcx, key, _, err| { + vtables.fn_sig.handle_cycle_error_fn = |tcx, key, _, err| { let guar = err.delay_as_bug(); erase_val(fn_sig(tcx, key, guar)) }; - vtables.check_representability.value_from_cycle_error = + vtables.check_representability.handle_cycle_error_fn = |tcx, _, cycle, _err| check_representability(tcx, cycle); - vtables.check_representability_adt_ty.value_from_cycle_error = + vtables.check_representability_adt_ty.handle_cycle_error_fn = |tcx, _, cycle, _err| check_representability(tcx, cycle); - vtables.variances_of.value_from_cycle_error = |tcx, _, cycle, err| { + vtables.variances_of.handle_cycle_error_fn = |tcx, _, cycle, err| { let _guar = err.delay_as_bug(); erase_val(variances_of(tcx, cycle)) }; - vtables.layout_of.value_from_cycle_error = |tcx, _, cycle, err| { + vtables.layout_of.handle_cycle_error_fn = |tcx, _, cycle, err| { let _guar = err.delay_as_bug(); erase_val(Err(layout_of(tcx, cycle))) } @@ -73,10 +73,10 @@ fn fn_sig<'tcx>( ))) } -fn check_representability<'tcx>(tcx: TyCtxt<'tcx>, cycle_error: CycleError<'tcx>) -> ! { +fn check_representability<'tcx>(tcx: TyCtxt<'tcx>, cycle: Cycle<'tcx>) -> ! { let mut item_and_field_ids = Vec::new(); let mut representable_ids = FxHashSet::default(); - for frame in &cycle_error.cycle { + for frame in &cycle.stack { if frame.node.dep_kind == DepKind::check_representability && let Some(field_id) = frame.node.def_id && let Some(field_id) = field_id.as_local() @@ -90,7 +90,7 @@ fn check_representability<'tcx>(tcx: TyCtxt<'tcx>, cycle_error: CycleError<'tcx> item_and_field_ids.push((item_id.expect_local(), field_id)); } } - for frame in &cycle_error.cycle { + for frame in &cycle.stack { if let TaggedQueryKey::check_representability_adt_ty(key) = frame.node.tagged_key && let Some(adt) = key.ty_adt_def() && let Some(def_id) = adt.did().as_local() @@ -105,11 +105,11 @@ fn check_representability<'tcx>(tcx: TyCtxt<'tcx>, cycle_error: CycleError<'tcx> guar.raise_fatal() } -fn variances_of<'tcx>(tcx: TyCtxt<'tcx>, cycle_error: CycleError<'tcx>) -> &'tcx [ty::Variance] { +fn variances_of<'tcx>(tcx: TyCtxt<'tcx>, cycle: Cycle<'tcx>) -> &'tcx [ty::Variance] { search_for_cycle_permutation( - &cycle_error.cycle, - |cycle| { - if let Some(frame) = cycle.get(0) + &cycle.stack, + |stack| { + if let Some(frame) = stack.get(0) && frame.node.dep_kind == DepKind::variances_of && let Some(def_id) = frame.node.def_id { @@ -121,7 +121,7 @@ fn variances_of<'tcx>(tcx: TyCtxt<'tcx>, cycle_error: CycleError<'tcx>) -> &'tcx }, || { span_bug!( - cycle_error.usage.as_ref().unwrap().span, + cycle.usage.as_ref().unwrap().span, "only `variances_of` returns `&[ty::Variance]`" ) }, @@ -147,14 +147,11 @@ fn search_for_cycle_permutation( otherwise() } -fn layout_of<'tcx>( - tcx: TyCtxt<'tcx>, - cycle_error: CycleError<'tcx>, -) -> &'tcx ty::layout::LayoutError<'tcx> { +fn layout_of<'tcx>(tcx: TyCtxt<'tcx>, cycle: Cycle<'tcx>) -> &'tcx ty::layout::LayoutError<'tcx> { let diag = search_for_cycle_permutation( - &cycle_error.cycle, - |cycle| { - if let TaggedQueryKey::layout_of(key) = cycle[0].node.tagged_key + &cycle.stack, + |stack| { + if let TaggedQueryKey::layout_of(key) = stack[0].node.tagged_key && let ty::Coroutine(def_id, _) = key.value.kind() && let Some(def_id) = def_id.as_local() && let def_kind = tcx.def_kind(def_id) @@ -178,7 +175,7 @@ fn layout_of<'tcx>( tcx.def_kind_descr_article(def_kind, def_id.to_def_id()), tcx.def_kind_descr(def_kind, def_id.to_def_id()), ); - for (i, frame) in cycle.iter().enumerate() { + for (i, frame) in stack.iter().enumerate() { let TaggedQueryKey::layout_of(frame_key) = frame.node.tagged_key else { continue; }; @@ -189,7 +186,7 @@ fn layout_of<'tcx>( continue; }; let frame_span = - frame.node.tagged_key.default_span(tcx, cycle[(i + 1) % cycle.len()].span); + frame.node.tagged_key.default_span(tcx, stack[(i + 1) % stack.len()].span); if frame_span.is_dummy() { continue; } @@ -223,7 +220,7 @@ fn layout_of<'tcx>( ControlFlow::Continue(()) } }, - || report_cycle(tcx, &cycle_error), + || create_cycle_error(tcx, &cycle), ); let guar = diag.emit(); diff --git a/compiler/rustc_query_impl/src/job.rs b/compiler/rustc_query_impl/src/job.rs index 0b989201a2e06..a2e12bbe9a650 100644 --- a/compiler/rustc_query_impl/src/job.rs +++ b/compiler/rustc_query_impl/src/job.rs @@ -6,9 +6,7 @@ use std::sync::Arc; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::{Diag, DiagCtxtHandle}; use rustc_hir::def::DefKind; -use rustc_middle::query::{ - CycleError, QueryJob, QueryJobId, QueryLatch, QueryStackFrame, QueryWaiter, -}; +use rustc_middle::query::{Cycle, QueryJob, QueryJobId, QueryLatch, QueryStackFrame, QueryWaiter}; use rustc_middle::ty::TyCtxt; use rustc_span::{DUMMY_SP, Span, respan}; @@ -57,29 +55,29 @@ pub(crate) fn find_cycle_in_stack<'tcx>( job_map: QueryJobMap<'tcx>, current_job: &Option, span: Span, -) -> CycleError<'tcx> { +) -> Cycle<'tcx> { // Find the waitee amongst `current_job` parents - let mut cycle = Vec::new(); + let mut stack = Vec::new(); let mut current_job = Option::clone(current_job); while let Some(job) = current_job { let info = &job_map.map[&job]; - cycle.push(respan(info.job.span, info.frame.clone())); + stack.push(respan(info.job.span, info.frame.clone())); if job == id { - cycle.reverse(); + stack.reverse(); // This is the end of the cycle // The span entry we included was for the usage // of the cycle itself, and not part of the cycle // Replace it with the span which caused the cycle to form - cycle[0].span = span; + stack[0].span = span; // Find out why the cycle itself was used let usage = try { let parent = info.job.parent?; respan(info.job.span, job_map.frame_of(parent).clone()) }; - return CycleError { usage, cycle }; + return Cycle { usage, stack }; } current_job = info.job.parent; @@ -316,9 +314,9 @@ fn remove_cycle<'tcx>( .map(|(span, job)| respan(span, job_map.frame_of(job).clone())); // Create the cycle error - let error = CycleError { + let error = Cycle { usage, - cycle: stack + stack: stack .iter() .map(|&(span, job)| respan(span, job_map.frame_of(job).clone())) .collect(), @@ -448,9 +446,9 @@ pub fn print_query_stack<'tcx>( #[inline(never)] #[cold] -pub(crate) fn report_cycle<'tcx>( +pub(crate) fn create_cycle_error<'tcx>( tcx: TyCtxt<'tcx>, - CycleError { usage, cycle: stack }: &CycleError<'tcx>, + Cycle { usage, stack }: &Cycle<'tcx>, ) -> Diag<'tcx> { assert!(!stack.is_empty()); diff --git a/compiler/rustc_query_impl/src/lib.rs b/compiler/rustc_query_impl/src/lib.rs index 2f69082db66e9..173a7b111f0ad 100644 --- a/compiler/rustc_query_impl/src/lib.rs +++ b/compiler/rustc_query_impl/src/lib.rs @@ -22,7 +22,7 @@ pub use crate::job::{QueryJobMap, break_query_cycles, print_query_stack}; mod dep_kind_vtables; mod error; mod execution; -mod from_cycle_error; +mod handle_cycle_error; mod job; mod plumbing; mod profiling_support; @@ -49,7 +49,7 @@ pub fn query_system<'tcx>( incremental: bool, ) -> QuerySystem<'tcx> { let mut query_vtables = query_impl::make_query_vtables(incremental); - from_cycle_error::specialize_query_vtables(&mut query_vtables); + handle_cycle_error::specialize_query_vtables(&mut query_vtables); QuerySystem { arenas: Default::default(), query_vtables, diff --git a/compiler/rustc_query_impl/src/query_impl.rs b/compiler/rustc_query_impl/src/query_impl.rs index caf4c7dde7e67..927f2ed2af7eb 100644 --- a/compiler/rustc_query_impl/src/query_impl.rs +++ b/compiler/rustc_query_impl/src/query_impl.rs @@ -175,10 +175,10 @@ macro_rules! define_queries { is_loadable_from_disk_fn: |_tcx, _key, _index| false, // The default just emits `err` and then aborts. - // `from_cycle_error::specialize_query_vtables` overwrites this default for - // certain queries. - value_from_cycle_error: |_tcx, _key, _cycle, err| { - $crate::from_cycle_error::default(err) + // `handle_cycle_error::specialize_query_vtables` overwrites this default + // for certain queries. + handle_cycle_error_fn: |_tcx, _key, _cycle, err| { + $crate::handle_cycle_error::default(err) }, #[cfg($no_hash)] diff --git a/compiler/rustc_ty_utils/src/representability.rs b/compiler/rustc_ty_utils/src/representability.rs index cef964ab139ae..195f27c617cea 100644 --- a/compiler/rustc_ty_utils/src/representability.rs +++ b/compiler/rustc_ty_utils/src/representability.rs @@ -58,7 +58,7 @@ fn check_representability_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) { // -> check_representability_adt_ty(Bar) // -> check_representability(Foo) // -// For the diagnostic output (in `Value::from_cycle_error`), we want to detect +// For the diagnostic output (in `check_representability`), we want to detect // that the `Foo` in the *second* field of the struct is culpable. This // requires traversing the HIR of the struct and calling `params_in_repr(Bar)`. // But we can't call params_in_repr for a given type unless it is known to be