Rollup merge of rust-lang#153979 - nnethercote:rename-cycle-things, r=petrochenkov

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::frames`. Because it is a sequence of frames, and we want to avoid `Cycle::cycle` (and `cycle.cycle`).

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

r? @petrochenkov

Author: JonathanBrouwer

compiler/rustc_middle/src/queries.rs

@@ -131,10 +131,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:
@@ -577,7 +577,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.)

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<QueryJobId>,
     pub condvar: Condvar,
     pub span: Span,
-    pub cycle: Mutex<Option<CycleError<'tcx>>>,
+    pub cycle: Mutex<Option<Cycle<'tcx>>>,
 }
 
 #[derive(Clone, Debug)]
@@ -79,7 +79,7 @@ impl<'tcx> QueryLatch<'tcx> {
         tcx: TyCtxt<'tcx>,
         query: Option<QueryJobId>,
         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

compiler/rustc_middle/src/query/mod.rs

@@ -5,8 +5,8 @@ pub use self::into_query_key::IntoQueryKey;
 pub use self::job::{QueryJob, QueryJobId, QueryLatch, QueryWaiter};
 pub use self::keys::{AsLocalQueryKey, LocalCrate, QueryKey};
 pub use self::plumbing::{
-    ActiveKeyStatus, CycleError, EnsureMode, QueryMode, QueryState, QuerySystem, QueryVTable,
-    TyCtxtAt, TyCtxtEnsureDone, TyCtxtEnsureOk, TyCtxtEnsureResult,
+    ActiveKeyStatus, Cycle, EnsureMode, QueryMode, QueryState, QuerySystem, QueryVTable, TyCtxtAt,
+    TyCtxtEnsureDone, TyCtxtEnsureOk, TyCtxtEnsureResult,
 };
 pub use self::stack::QueryStackFrame;
 pub use crate::queries::Providers;

compiler/rustc_middle/src/query/plumbing.rs

@@ -47,12 +47,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<QueryStackFrame<'tcx>>,
 
     /// The span here corresponds to the reason for which this query was required.
-    pub cycle: Vec<QueryStackFrame<'tcx>>,
+    pub frames: Vec<QueryStackFrame<'tcx>>,
 }
 
 #[derive(Debug)]
@@ -111,13 +111,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,
 

compiler/rustc_query_impl/src/execution.rs

@@ -8,16 +8,16 @@ 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;
 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, loadable_from_disk, next_job_id, start_query};
 use crate::query_impl::for_each_query_vtable;
 
@@ -108,14 +108,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
@@ -194,7 +194,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,
@@ -205,8 +205,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, &current_query_job(), span);
-    (mk_cycle(query, tcx, key, error), None)
+    let cycle = find_cycle_in_stack(try_execute, job_map, &current_query_job(), span);
+    (handle_cycle(query, tcx, key, cycle), None)
 }
 
 #[inline(always)]
@@ -250,7 +250,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),
     }
 }
 
@@ -334,7 +334,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(),

…rustc_query_impl/src/from_cycle_error.rs …stc_query_impl/src/handle_cycle_error.rscompiler/rustc_query_impl/src/from_cycle_error.rs renamed to compiler/rustc_query_impl/src/handle_cycle_error.rs compiler/rustc_query_impl/src/from_cycle_error.rs renamed to compiler/rustc_query_impl/src/handle_cycle_error.rs

@@ -10,33 +10,33 @@ use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Res};
 use rustc_middle::bug;
 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};
 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, key, _, err| {
+    vtables.variances_of.handle_cycle_error_fn = |tcx, key, _, err| {
         let _guar = err.delay_as_bug();
         erase_val(variances_of(tcx, key))
     };
 
-    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)))
     }
@@ -72,10 +72,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.frames {
         if let TaggedQueryKey::check_representability(def_id) = frame.tagged_key
             && tcx.def_kind(def_id) == DefKind::Field
         {
@@ -88,7 +88,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.frames {
         if let TaggedQueryKey::check_representability_adt_ty(key) = frame.tagged_key
             && let Some(adt) = key.ty_adt_def()
             && let Some(def_id) = adt.did().as_local()
@@ -127,14 +127,11 @@ fn search_for_cycle_permutation<Q, T>(
     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].tagged_key
+        &cycle.frames,
+        |frames| {
+            if let TaggedQueryKey::layout_of(key) = frames[0].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)
@@ -158,7 +155,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 frames.iter().enumerate() {
                     let TaggedQueryKey::layout_of(frame_key) = frame.tagged_key else {
                         continue;
                     };
@@ -169,7 +166,7 @@ fn layout_of<'tcx>(
                         continue;
                     };
                     let frame_span =
-                        frame.tagged_key.default_span(tcx, cycle[(i + 1) % cycle.len()].span);
+                        frame.tagged_key.default_span(tcx, frames[(i + 1) % frames.len()].span);
                     if frame_span.is_dummy() {
                         continue;
                     }
@@ -203,7 +200,7 @@ fn layout_of<'tcx>(
                 ControlFlow::Continue(())
             }
         },
-        || report_cycle(tcx, &cycle_error),
+        || create_cycle_error(tcx, &cycle),
     );
 
     let guar = diag.emit();

compiler/rustc_query_impl/src/job.rs

@@ -7,9 +7,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_errors::{Diag, DiagCtxtHandle};
 use rustc_hir::def::DefKind;
 use rustc_middle::queries::TaggedQueryKey;
-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};
 
@@ -58,29 +56,28 @@ pub(crate) fn find_cycle_in_stack<'tcx>(
     job_map: QueryJobMap<'tcx>,
     current_job: &Option<QueryJobId>,
     span: Span,
-) -> CycleError<'tcx> {
-    // Find the waitee amongst `current_job` parents
-    let mut cycle = Vec::new();
+) -> Cycle<'tcx> {
+    // Find the waitee amongst `current_job` parents.
+    let mut frames = Vec::new();
     let mut current_job = Option::clone(current_job);
 
     while let Some(job) = current_job {
         let info = &job_map.map[&job];
-        cycle.push(QueryStackFrame { span: info.job.span, tagged_key: info.tagged_key });
+        frames.push(QueryStackFrame { span: info.job.span, tagged_key: info.tagged_key });
 
         if job == id {
-            cycle.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;
-            // Find out why the cycle itself was used
+            frames.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.
+            frames[0].span = span;
+            // Find out why the cycle itself was used.
             let usage = try {
                 let parent = info.job.parent?;
                 QueryStackFrame { span: info.job.span, tagged_key: job_map.tagged_key_of(parent) }
             };
-            return CycleError { usage, cycle };
+            return Cycle { usage, frames };
         }
 
         current_job = info.job.parent;
@@ -319,9 +316,9 @@ fn remove_cycle<'tcx>(
             .map(|(span, job)| QueryStackFrame { span, tagged_key: job_map.tagged_key_of(job) });
 
         // Create the cycle error
-        let error = CycleError {
+        let error = Cycle {
             usage,
-            cycle: stack
+            frames: stack
                 .iter()
                 .map(|&(span, job)| QueryStackFrame {
                     span,
@@ -454,27 +451,27 @@ 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, frames }: &Cycle<'tcx>,
 ) -> Diag<'tcx> {
-    assert!(!stack.is_empty());
+    assert!(!frames.is_empty());
 
-    let span = stack[0].tagged_key.default_span(tcx, stack[1 % stack.len()].span);
+    let span = frames[0].tagged_key.default_span(tcx, frames[1 % frames.len()].span);
 
     let mut cycle_stack = Vec::new();
 
     use crate::error::StackCount;
-    let stack_bottom = stack[0].tagged_key.description(tcx);
-    let stack_count = if stack.len() == 1 {
+    let stack_bottom = frames[0].tagged_key.description(tcx);
+    let stack_count = if frames.len() == 1 {
         StackCount::Single { stack_bottom: stack_bottom.clone() }
     } else {
         StackCount::Multiple { stack_bottom: stack_bottom.clone() }
     };
 
-    for i in 1..stack.len() {
-        let frame = &stack[i];
-        let span = frame.tagged_key.default_span(tcx, stack[(i + 1) % stack.len()].span);
+    for i in 1..frames.len() {
+        let frame = &frames[i];
+        let span = frame.tagged_key.default_span(tcx, frames[(i + 1) % frames.len()].span);
         cycle_stack
             .push(crate::error::CycleStack { span, desc: frame.tagged_key.description(tcx) });
     }
@@ -484,12 +481,12 @@ pub(crate) fn report_cycle<'tcx>(
         usage: usage.tagged_key.description(tcx),
     });
 
-    let alias = if stack
+    let alias = if frames
         .iter()
         .all(|frame| frame.tagged_key.def_kind(tcx) == Some(DefKind::TyAlias))
     {
         Some(crate::error::Alias::Ty)
-    } else if stack.iter().all(|frame| frame.tagged_key.def_kind(tcx) == Some(DefKind::TraitAlias))
+    } else if frames.iter().all(|frame| frame.tagged_key.def_kind(tcx) == Some(DefKind::TraitAlias))
     {
         Some(crate::error::Alias::Trait)
     } else {