Change value_from_cycle_error signature.

Currently if a cycle error occurs the error is created, then handled
according to `CycleErrorHandling` (which comes from the `cycle_*` query
modifers), and then `value_from_cycle_error` is called.

This commit changes things so the error is created and then just passed
to `value_from_cycle_error`. This gives `value_from_cycle_error` full
control over how it's handled, eliminating the need for
`CycleErrorHandling`. This makes things simpler overall.

Author: nnethercote

compiler/rustc_macros/src/query.rs

@@ -143,7 +143,6 @@ struct QueryModifiers {
     anon: Option<Ident>,
     arena_cache: Option<Ident>,
     cache_on_disk_if: Option<CacheOnDiskIf>,
-    cycle_delay_bug: Option<Ident>,
     depth_limit: Option<Ident>,
     desc: Desc,
     eval_always: Option<Ident>,
@@ -157,7 +156,6 @@ fn parse_query_modifiers(input: ParseStream<'_>) -> Result<QueryModifiers> {
     let mut arena_cache = None;
     let mut cache_on_disk_if = None;
     let mut desc = None;
-    let mut cycle_delay_bug = None;
     let mut no_hash = None;
     let mut anon = None;
     let mut eval_always = None;
@@ -191,8 +189,6 @@ fn parse_query_modifiers(input: ParseStream<'_>) -> Result<QueryModifiers> {
             try_insert!(cache_on_disk_if = CacheOnDiskIf { modifier, block });
         } else if modifier == "arena_cache" {
             try_insert!(arena_cache = modifier);
-        } else if modifier == "cycle_delay_bug" {
-            try_insert!(cycle_delay_bug = modifier);
         } else if modifier == "no_hash" {
             try_insert!(no_hash = modifier);
         } else if modifier == "anon" {
@@ -216,7 +212,6 @@ fn parse_query_modifiers(input: ParseStream<'_>) -> Result<QueryModifiers> {
         arena_cache,
         cache_on_disk_if,
         desc,
-        cycle_delay_bug,
         no_hash,
         anon,
         eval_always,
@@ -251,7 +246,6 @@ fn make_modifiers_stream(query: &Query) -> proc_macro2::TokenStream {
         anon,
         arena_cache,
         cache_on_disk_if,
-        cycle_delay_bug,
         depth_limit,
         desc: _,
         eval_always,
@@ -264,13 +258,6 @@ fn make_modifiers_stream(query: &Query) -> proc_macro2::TokenStream {
     let anon = anon.is_some();
     let arena_cache = arena_cache.is_some();
     let cache_on_disk = cache_on_disk_if.is_some();
-
-    let cycle_error_handling = if cycle_delay_bug.is_some() {
-        quote! { DelayBug }
-    } else {
-        quote! { Error }
-    };
-
     let depth_limit = depth_limit.is_some();
     let eval_always = eval_always.is_some();
     let feedable = feedable.is_some();
@@ -289,7 +276,6 @@ fn make_modifiers_stream(query: &Query) -> proc_macro2::TokenStream {
         anon: #anon,
         arena_cache: #arena_cache,
         cache_on_disk: #cache_on_disk,
-        cycle_error_handling: #cycle_error_handling,
         depth_limit: #depth_limit,
         eval_always: #eval_always,
         feedable: #feedable,
@@ -402,7 +388,6 @@ fn add_to_analyzer_stream(query: &Query, analyzer_stream: &mut proc_macro2::Toke
 
     doc_link!(
         arena_cache,
-        cycle_delay_bug,
         no_hash,
         anon,
         eval_always,

compiler/rustc_middle/src/queries.rs

@@ -32,7 +32,6 @@
 //! - `arena_cache`: Use an arena for in-memory caching of the query result.
 //! - `cache_on_disk_if { ... }`: Cache the query result to disk if the provided block evaluates to
 //!   true. The query key identifier is available for use within the block, as is `tcx`.
-//! - `cycle_delay_bug`: If a dependency cycle is detected, emit a delayed bug instead of aborting immediately.
 //! - `no_hash`: Do not hash the query result for incremental compilation; just mark as dirty if recomputed.
 //! - `anon`: Make the query anonymous in the dependency graph (no dep node is created).
 //! - `eval_always`: Always evaluate the query, ignoring its dependencies and cached results.
@@ -580,13 +579,13 @@ 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
+    // 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.)
     query check_representability(key: LocalDefId) {
         desc { "checking if `{}` is representable", tcx.def_path_str(key) }
-        // Infinitely sized types will cause a cycle. The custom `FromCycleError` impl for
-        // `Representability` 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.)
-        cycle_delay_bug
         // We don't want recursive representability calls to be forced with
         // incremental compilation because, if a cycle occurs, we need the
         // entire cycle to be in memory for diagnostics. This means we can't
@@ -598,7 +597,6 @@ rustc_queries! {
     /// details, particularly on the modifiers.
     query check_representability_adt_ty(key: Ty<'tcx>) {
         desc { "checking if `{}` is representable", key }
-        cycle_delay_bug
         anon
     }
 
@@ -1043,7 +1041,6 @@ rustc_queries! {
         desc { "computing the variances of `{}`", tcx.def_path_str(def_id) }
         cache_on_disk_if { def_id.is_local() }
         separate_provide_extern
-        cycle_delay_bug
     }
 
     /// Gets a map with the inferred outlives-predicates of every item in the local crate.
@@ -1176,7 +1173,6 @@ rustc_queries! {
         desc { "computing function signature of `{}`", tcx.def_path_str(key) }
         cache_on_disk_if { key.is_local() }
         separate_provide_extern
-        cycle_delay_bug
     }
 
     /// Performs lint checking for the module.
@@ -1767,8 +1763,6 @@ rustc_queries! {
     ) -> Result<ty::layout::TyAndLayout<'tcx>, &'tcx ty::layout::LayoutError<'tcx>> {
         depth_limit
         desc { "computing layout of `{}`", key.value }
-        // we emit our own error during query cycle handling
-        cycle_delay_bug
     }
 
     /// Compute a `FnAbi` suitable for indirect calls, i.e. to `fn` pointers.

compiler/rustc_middle/src/query/mod.rs

@@ -4,8 +4,8 @@ pub use self::caches::{DefIdCache, DefaultCache, QueryCache, SingleCache, VecCac
 pub use self::job::{QueryInfo, QueryJob, QueryJobId, QueryLatch, QueryWaiter};
 pub use self::keys::{AsLocalQueryKey, LocalCrate, QueryKey};
 pub use self::plumbing::{
-    ActiveKeyStatus, CycleError, CycleErrorHandling, EnsureMode, IntoQueryParam, QueryMode,
-    QueryState, TyCtxtAt, TyCtxtEnsureDone, TyCtxtEnsureOk, TyCtxtEnsureResult,
+    ActiveKeyStatus, CycleError, EnsureMode, IntoQueryParam, QueryMode, QueryState, TyCtxtAt,
+    TyCtxtEnsureDone, TyCtxtEnsureOk, TyCtxtEnsureResult,
 };
 pub use self::stack::{QueryStackDeferred, QueryStackFrame, QueryStackFrameExtra};
 pub use crate::queries::Providers;

compiler/rustc_middle/src/query/modifiers.rs

@@ -23,11 +23,6 @@ pub(crate) struct arena_cache;
 /// Cache the query to disk if the `Block` returns true.
 pub(crate) struct cache_on_disk_if;
 
-/// # `cycle_delay_bug` query modifier
-///
-/// A cycle error results in a delay_bug call
-pub(crate) struct cycle_delay_bug;
-
 /// # `depth_limit` query modifier
 ///
 /// Whether the query has a call depth limit

compiler/rustc_middle/src/query/plumbing.rs

@@ -5,9 +5,10 @@ use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_data_structures::hash_table::HashTable;
 use rustc_data_structures::sharded::Sharded;
 use rustc_data_structures::sync::{AtomicU64, WorkerLocal};
+use rustc_errors::Diag;
 use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_hir::hir_id::OwnerId;
-use rustc_span::{ErrorGuaranteed, Span};
+use rustc_span::Span;
 pub use sealed::IntoQueryParam;
 
 use crate::dep_graph::{DepKind, DepNodeIndex, SerializedDepNodeIndex};
@@ -49,16 +50,6 @@ pub enum ActiveKeyStatus<'tcx> {
     Poisoned,
 }
 
-/// How a particular query deals with query cycle errors.
-///
-/// Inspected by the code that actually handles cycle errors, to decide what
-/// approach to use.
-#[derive(Copy, Clone)]
-pub enum CycleErrorHandling {
-    Error,
-    DelayBug,
-}
-
 #[derive(Clone, Debug)]
 pub struct CycleError<I = QueryStackFrameExtra> {
     /// The query and related span that uses the cycle.
@@ -107,8 +98,6 @@ pub struct QueryVTable<'tcx, C: QueryCache> {
     pub feedable: bool,
 
     pub dep_kind: DepKind,
-    /// How this query deals with query cycle errors.
-    pub cycle_error_handling: CycleErrorHandling,
     pub state: QueryState<'tcx, C::Key>,
     pub cache: C,
 
@@ -136,12 +125,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>,
-        key: C::Key,
-        cycle_error: CycleError,
-        guar: ErrorGuaranteed,
-    ) -> C::Value,
+    /// 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, error: Diag<'_>) -> C::Value,
+
     pub format_value: fn(&C::Value) -> String,
 
     /// Formats a human-readable description of this query and its key, as
@@ -308,7 +297,6 @@ macro_rules! define_callbacks {
                     anon: $anon:literal,
                     arena_cache: $arena_cache:literal,
                     cache_on_disk: $cache_on_disk:literal,
-                    cycle_error_handling: $cycle_error_handling:ident,
                     depth_limit: $depth_limit:literal,
                     eval_always: $eval_always:literal,
                     feedable: $feedable:literal,

compiler/rustc_query_impl/src/dep_kind_vtables.rs

@@ -136,7 +136,6 @@ macro_rules! define_dep_kind_vtables {
                     anon: $anon:literal,
                     arena_cache: $arena_cache:literal,
                     cache_on_disk: $cache_on_disk:literal,
-                    cycle_error_handling: $cycle_error_handling:ident,
                     depth_limit: $depth_limit:literal,
                     eval_always: $eval_always:literal,
                     feedable: $feedable:literal,

compiler/rustc_query_impl/src/execution.rs

@@ -9,8 +9,8 @@ use rustc_errors::FatalError;
 use rustc_middle::dep_graph::{DepGraphData, DepNodeKey, SerializedDepNodeIndex};
 use rustc_middle::query::plumbing::QueryVTable;
 use rustc_middle::query::{
-    ActiveKeyStatus, CycleError, CycleErrorHandling, EnsureMode, QueryCache, QueryJob, QueryJobId,
-    QueryKey, QueryLatch, QueryMode, QueryState,
+    ActiveKeyStatus, CycleError, EnsureMode, QueryCache, QueryJob, QueryJobId, QueryKey,
+    QueryLatch, QueryMode, QueryState,
 };
 use rustc_middle::ty::TyCtxt;
 use rustc_middle::verify_ich::incremental_verify_ich;
@@ -129,16 +129,7 @@ fn mk_cycle<'tcx, C: QueryCache>(
     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, key, cycle_error, guar)
-        }
-        CycleErrorHandling::DelayBug => {
-            let guar = error.delay_as_bug();
-            (query.value_from_cycle_error)(tcx, key, cycle_error, guar)
-        }
-    }
+    (query.value_from_cycle_error)(tcx, key, cycle_error, error)
 }
 
 /// Guard object representing the responsibility to execute a query job and

compiler/rustc_query_impl/src/from_cycle_error.rs

@@ -5,14 +5,14 @@ use std::ops::ControlFlow;
 
 use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::codes::*;
-use rustc_errors::{Applicability, MultiSpan, pluralize, struct_span_code_err};
+use rustc_errors::{Applicability, Diag, MultiSpan, pluralize, struct_span_code_err};
 use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Res};
 use rustc_middle::dep_graph::DepKind;
 use rustc_middle::queries::QueryVTables;
 use rustc_middle::query::CycleError;
 use rustc_middle::query::erase::erase_val;
-use rustc_middle::ty::layout::{LayoutError, TyAndLayout};
+use rustc_middle::ty::layout::LayoutError;
 use rustc_middle::ty::{self, Ty, TyCtxt};
 use rustc_middle::{bug, span_bug};
 use rustc_span::def_id::{DefId, LocalDefId};
@@ -21,37 +21,45 @@ 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)));
+    vtables.type_of.value_from_cycle_error = |tcx, _, _, err| {
+        let guar = err.emit();
+        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)));
+    vtables.type_of_opaque_hir_typeck.value_from_cycle_error = |tcx, _, _, err| {
+        let guar = err.emit();
+        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));
+    vtables.erase_and_anonymize_regions_ty.value_from_cycle_error = |tcx, _, _, err| {
+        let guar = err.emit();
+        erase_val(Ty::new_error(tcx, guar))
+    };
 
-    vtables.fn_sig.value_from_cycle_error = |tcx, key, _, guar| erase_val(fn_sig(tcx, key, guar));
+    vtables.fn_sig.value_from_cycle_error = |tcx, key, _, err| {
+        let guar = err.delay_as_bug();
+        erase_val(fn_sig(tcx, key, guar))
+    };
 
     vtables.check_representability.value_from_cycle_error =
-        |tcx, _, cycle, guar| check_representability(tcx, cycle, guar);
+        |tcx, _, cycle, _err| check_representability(tcx, cycle);
 
     vtables.check_representability_adt_ty.value_from_cycle_error =
-        |tcx, _, cycle, guar| check_representability(tcx, cycle, guar);
+        |tcx, _, cycle, _err| check_representability(tcx, cycle);
 
-    vtables.variances_of.value_from_cycle_error =
-        |tcx, _, cycle, guar| erase_val(variances_of(tcx, cycle, guar));
+    vtables.variances_of.value_from_cycle_error = |tcx, _, cycle, err| {
+        let _guar = err.delay_as_bug();
+        erase_val(variances_of(tcx, cycle))
+    };
 
-    vtables.layout_of.value_from_cycle_error =
-        |tcx, _, cycle, guar| erase_val(layout_of(tcx, cycle, guar));
+    vtables.layout_of.value_from_cycle_error = |tcx, _, cycle, err| {
+        let _guar = err.delay_as_bug();
+        erase_val(Err(layout_of(tcx, cycle)))
+    }
 }
 
-pub(crate) fn default<'tcx>(tcx: TyCtxt<'tcx>, cycle_error: CycleError, query_name: &str) -> ! {
-    let Some(guar) = tcx.sess.dcx().has_errors() else {
-        bug!(
-            "`from_cycle_error_default` on query `{query_name}` called without errors: {:#?}",
-            cycle_error.cycle,
-        );
-    };
+pub(crate) fn default(err: Diag<'_>) -> ! {
+    let guar = err.emit();
     guar.raise_fatal()
 }
 
@@ -80,11 +88,7 @@ fn fn_sig<'tcx>(
     )))
 }
 
-fn check_representability<'tcx>(
-    tcx: TyCtxt<'tcx>,
-    cycle_error: CycleError,
-    _guar: ErrorGuaranteed,
-) -> ! {
+fn check_representability<'tcx>(tcx: TyCtxt<'tcx>, cycle_error: CycleError) -> ! {
     let mut item_and_field_ids = Vec::new();
     let mut representable_ids = FxHashSet::default();
     for info in &cycle_error.cycle {
@@ -116,11 +120,7 @@ fn check_representability<'tcx>(
     guar.raise_fatal()
 }
 
-fn variances_of<'tcx>(
-    tcx: TyCtxt<'tcx>,
-    cycle_error: CycleError,
-    _guar: ErrorGuaranteed,
-) -> &'tcx [ty::Variance] {
+fn variances_of<'tcx>(tcx: TyCtxt<'tcx>, cycle_error: CycleError) -> &'tcx [ty::Variance] {
     search_for_cycle_permutation(
         &cycle_error.cycle,
         |cycle| {
@@ -165,8 +165,7 @@ fn search_for_cycle_permutation<Q, T>(
 fn layout_of<'tcx>(
     tcx: TyCtxt<'tcx>,
     cycle_error: CycleError,
-    _guar: ErrorGuaranteed,
-) -> Result<TyAndLayout<'tcx>, &'tcx ty::layout::LayoutError<'tcx>> {
+) -> &'tcx ty::layout::LayoutError<'tcx> {
     let diag = search_for_cycle_permutation(
         &cycle_error.cycle,
         |cycle| {
@@ -243,7 +242,7 @@ fn layout_of<'tcx>(
     );
 
     let guar = diag.emit();
-    Err(tcx.arena.alloc(LayoutError::Cycle(guar)))
+    tcx.arena.alloc(LayoutError::Cycle(guar))
 }
 
 // item_and_field_ids should form a cycle where each field contains the