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,8 +143,6 @@ struct QueryModifiers {
     anon: Option<Ident>,
     arena_cache: Option<Ident>,
     cache_on_disk_if: Option<CacheOnDiskIf>,
-    cycle_delay_bug: Option<Ident>,
-    cycle_stash: Option<Ident>,
     depth_limit: Option<Ident>,
     desc: Desc,
     eval_always: Option<Ident>,
@@ -158,8 +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 cycle_stash = None;
     let mut no_hash = None;
     let mut anon = None;
     let mut eval_always = None;
@@ -193,10 +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 == "cycle_stash" {
-            try_insert!(cycle_stash = modifier);
         } else if modifier == "no_hash" {
             try_insert!(no_hash = modifier);
         } else if modifier == "anon" {
@@ -220,8 +212,6 @@ fn parse_query_modifiers(input: ParseStream<'_>) -> Result<QueryModifiers> {
         arena_cache,
         cache_on_disk_if,
         desc,
-        cycle_delay_bug,
-        cycle_stash,
         no_hash,
         anon,
         eval_always,
@@ -256,8 +246,6 @@ fn make_modifiers_stream(query: &Query) -> proc_macro2::TokenStream {
         anon,
         arena_cache,
         cache_on_disk_if,
-        cycle_delay_bug,
-        cycle_stash,
         depth_limit,
         desc: _,
         eval_always,
@@ -270,15 +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 if cycle_stash.is_some() {
-        quote! { Stash }
-    } else {
-        quote! { Error }
-    };
-
     let depth_limit = depth_limit.is_some();
     let eval_always = eval_always.is_some();
     let feedable = feedable.is_some();
@@ -297,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,
@@ -410,8 +388,6 @@ fn add_to_analyzer_stream(query: &Query, analyzer_stream: &mut proc_macro2::Toke
 
     doc_link!(
         arena_cache,
-        cycle_delay_bug,
-        cycle_stash,
         no_hash,
         anon,
         eval_always,

compiler/rustc_middle/src/queries.rs

@@ -32,8 +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.
-//! - `cycle_stash`: If a dependency cycle is detected, stash the error for later handling.
 //! - `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.
@@ -354,7 +352,6 @@ rustc_queries! {
             "computing type of opaque `{path}`",
             path = tcx.def_path_str(key),
         }
-        cycle_stash
     }
     query type_of_opaque_hir_typeck(key: LocalDefId) -> ty::EarlyBinder<'tcx, Ty<'tcx>> {
         desc {
@@ -588,14 +585,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) }
-        // njn: update comment
-        // 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
@@ -607,7 +603,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
     }
 
@@ -1052,7 +1047,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.
@@ -1185,7 +1179,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.
@@ -1776,8 +1769,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,16 +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;
-
-/// # `cycle_stash` query modifier
-///
-/// A cycle error results in a stashed cycle error that can be unstashed and canceled later
-pub(crate) struct cycle_stash;
-
 /// # `depth_limit` query modifier
 ///
 /// Whether the query has a call depth limit

compiler/rustc_middle/src/query/plumbing.rs

@@ -5,6 +5,7 @@ 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_macros::HashStable;
@@ -50,17 +51,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,
-    Stash,
-}
-
 #[derive(Clone, Debug)]
 pub struct CycleError<I = QueryStackFrameExtra> {
     /// The query and related span that uses the cycle.
@@ -109,8 +99,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,
 
@@ -138,8 +126,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>,
 
+    /// 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>, cycle_error: CycleError, guar: ErrorGuaranteed) -> C::Value,
+        fn(tcx: TyCtxt<'tcx>, 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
@@ -306,7 +298,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

@@ -5,12 +5,12 @@ use rustc_data_structures::hash_table::{Entry, HashTable};
 use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_data_structures::sync::{DynSend, DynSync};
 use rustc_data_structures::{outline, sharded, sync};
-use rustc_errors::{FatalError, StashKey};
+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;
@@ -101,26 +101,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, cycle_error, guar)
-        }
-        CycleErrorHandling::DelayBug => {
-            let guar = error.delay_as_bug();
-            (query.value_from_cycle_error)(tcx, cycle_error, guar)
-        }
-        CycleErrorHandling::Stash => {
-            let guar = if let Some(root) = cycle_error.cycle.first()
-                && let Some(span) = root.frame.info.span
-            {
-                error.stash(span, StashKey::Cycle).unwrap()
-            } else {
-                error.emit()
-            };
-            (query.value_from_cycle_error)(tcx, cycle_error, guar)
-        }
-    }
+    (query.value_from_cycle_error)(tcx, cycle_error, error)
 }
 
 /// Guard object representing the responsibility to execute a query job and