Rollup merge of #153707 - nnethercote:rm-CycleErrorHandling, r=Zalathar

Remove `CycleErrorHandling`.

This PR removes the `cycle_*` query modifiers and `CycleErrorHandling`. Some good simplicity wins. Details in individual commits.

r? @Zalathar

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 @@ macro_rules! try_insert {
             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 @@ macro_rules! try_insert {
         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 @@ macro_rules! doc_link {
 
     doc_link!(
         arena_cache,
-        cycle_delay_bug,
         no_hash,
         anon,
         eval_always,

compiler/rustc_middle/src/queries.rs

@@ -564,13 +564,13 @@
     }
 
     /// Checks whether a type is representable or infinitely sized
-    query check_representability(key: LocalDefId) -> rustc_middle::ty::Representability {
+    //
+    // 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
@@ -580,9 +580,8 @@
 
     /// An implementation detail for the `check_representability` query. See that query for more
     /// details, particularly on the modifiers.
-    query check_representability_adt_ty(key: Ty<'tcx>) -> rustc_middle::ty::Representability {
+    query check_representability_adt_ty(key: Ty<'tcx>) {
         desc { "checking if `{}` is representable", key }
-        cycle_delay_bug
         anon
     }
 
@@ -1027,7 +1026,6 @@
         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.
@@ -1160,7 +1158,6 @@
         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.
@@ -1751,8 +1748,6 @@
     ) -> 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/erase.rs

@@ -383,7 +383,6 @@ impl Erasable for $ty {
     rustc_middle::ty::Destructor,
     rustc_middle::ty::fast_reject::SimplifiedType,
     rustc_middle::ty::ImplPolarity,
-    rustc_middle::ty::Representability,
     rustc_middle::ty::UnusedGenericParams,
     rustc_middle::ty::util::AlwaysRequiresDrop,
     rustc_middle::ty::Visibility<rustc_span::def_id::DefId>,

compiler/rustc_middle/src/query/mod.rs

@@ -4,8 +4,8 @@
 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::QueryStackFrame;
 pub use crate::queries::Providers;

compiler/rustc_middle/src/query/modifiers.rs

@@ -29,11 +29,6 @@
 /// identifier is available for use within the block, as is `tcx`.
 pub(crate) struct cache_on_disk_if;
 
-/// # `cycle_delay_bug` query modifier
-///
-/// If a dependency cycle is detected, emit a delayed bug instead of a normal error.
-pub(crate) struct cycle_delay_bug;
-
 /// # `depth_limit` query modifier
 ///
 /// Impose a recursion call depth limit on the query to prevent stack overflow.

compiler/rustc_middle/src/query/plumbing.rs

@@ -5,9 +5,10 @@
 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<'tcx> {
     /// The query and related span that uses the cycle.
@@ -98,8 +89,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,
 
@@ -127,12 +116,16 @@ 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>,
         key: C::Key,
         cycle_error: CycleError<'tcx>,
-        guar: ErrorGuaranteed,
+        error: Diag<'_>,
     ) -> C::Value,
+
     pub format_value: fn(&C::Value) -> String,
 
     pub create_tagged_key: fn(C::Key) -> TaggedQueryKey<'tcx>,
@@ -295,7 +288,6 @@ fn $name:ident($($K:tt)*) -> $V:ty
                     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_middle/src/ty/adt.rs

@@ -751,8 +751,3 @@ pub fn sizedness_constraint(
         if self.is_struct() { tcx.adt_sizedness_constraint((self.did(), sizedness)) } else { None }
     }
 }
-
-/// This type exists just so a `FromCycleError` impl can be made for the `check_representability`
-/// query.
-#[derive(Clone, Copy, Debug, HashStable)]
-pub struct Representability;

compiler/rustc_query_impl/src/dep_kind_vtables.rs

@@ -136,7 +136,6 @@ fn $name:ident($K:ty) -> $V:ty
                     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_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;
@@ -130,16 +130,7 @@ fn mk_cycle<'tcx, C: QueryCache>(
     cycle_error: CycleError<'tcx>,
 ) -> C::Value {
     let error = report_cycle(tcx, &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 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,41 +21,45 @@
 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<'tcx>,
-    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()
 }
 
@@ -84,11 +88,7 @@ fn fn_sig<'tcx>(
     )))
 }
 
-fn check_representability<'tcx>(
-    tcx: TyCtxt<'tcx>,
-    cycle_error: CycleError<'tcx>,
-    _guar: ErrorGuaranteed,
-) -> ! {
+fn check_representability<'tcx>(tcx: TyCtxt<'tcx>, cycle_error: CycleError<'tcx>) -> ! {
     let mut item_and_field_ids = Vec::new();
     let mut representable_ids = FxHashSet::default();
     for info in &cycle_error.cycle {
@@ -120,11 +120,7 @@ fn check_representability<'tcx>(
     guar.raise_fatal()
 }
 
-fn variances_of<'tcx>(
-    tcx: TyCtxt<'tcx>,
-    cycle_error: CycleError<'tcx>,
-    _guar: ErrorGuaranteed,
-) -> &'tcx [ty::Variance] {
+fn variances_of<'tcx>(tcx: TyCtxt<'tcx>, cycle_error: CycleError<'tcx>) -> &'tcx [ty::Variance] {
     search_for_cycle_permutation(
         &cycle_error.cycle,
         |cycle| {
@@ -169,8 +165,7 @@ fn search_for_cycle_permutation<Q, T>(
 fn layout_of<'tcx>(
     tcx: TyCtxt<'tcx>,
     cycle_error: CycleError<'tcx>,
-    _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| {
@@ -247,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

compiler/rustc_query_impl/src/plumbing.rs

@@ -297,7 +297,6 @@ fn $name:ident($K:ty) -> $V:ty
                     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,
@@ -411,8 +410,6 @@ pub(crate) fn make_query_vtable<'tcx>(incremental: bool)
                     depth_limit: $depth_limit,
                     feedable: $feedable,
                     dep_kind: dep_graph::DepKind::$name,
-                    cycle_error_handling:
-                        rustc_middle::query::CycleErrorHandling::$cycle_error_handling,
                     state: Default::default(),
                     cache: Default::default(),
 
@@ -448,8 +445,11 @@ pub(crate) fn make_query_vtable<'tcx>(incremental: bool)
                     #[cfg(not($cache_on_disk))]
                     is_loadable_from_disk_fn: |_tcx, _key, _index| false,
 
-                    value_from_cycle_error: |tcx, _, cycle, _| {
-                        $crate::from_cycle_error::default(tcx, cycle, stringify!($name))
+                    // 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)
                     },
 
                     #[cfg($no_hash)]

compiler/rustc_ty_utils/src/representability.rs

@@ -2,7 +2,7 @@
 use rustc_index::bit_set::DenseBitSet;
 use rustc_middle::bug;
 use rustc_middle::query::Providers;
-use rustc_middle::ty::{self, Representability, Ty, TyCtxt};
+use rustc_middle::ty::{self, Ty, TyCtxt};
 use rustc_span::def_id::LocalDefId;
 
 pub(crate) fn provide(providers: &mut Providers) {
@@ -14,7 +14,7 @@ pub(crate) fn provide(providers: &mut Providers) {
     };
 }
 
-fn check_representability(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Representability {
+fn check_representability(tcx: TyCtxt<'_>, def_id: LocalDefId) {
     match tcx.def_kind(def_id) {
         DefKind::Struct | DefKind::Union | DefKind::Enum => {
             for variant in tcx.adt_def(def_id).variants() {
@@ -28,7 +28,6 @@ fn check_representability(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Representabili
         }
         def_kind => bug!("unexpected {def_kind:?}"),
     }
-    Representability
 }
 
 fn check_representability_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) {
@@ -67,7 +66,7 @@ fn check_representability_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) {
 // Looking at the query cycle above, we know that `Bar` is representable
 // because `check_representability_adt_ty(Bar<..>)` is in the cycle and
 // `check_representability(Bar)` is *not* in the cycle.
-fn check_representability_adt_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Representability {
+fn check_representability_adt_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) {
     let ty::Adt(adt, args) = ty.kind() else { bug!("expected adt") };
     if let Some(def_id) = adt.did().as_local() {
         let _ = tcx.check_representability(def_id);
@@ -82,7 +81,6 @@ fn check_representability_adt_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Repre
             }
         }
     }
-    Representability
 }
 
 fn params_in_repr(tcx: TyCtxt<'_>, def_id: LocalDefId) -> DenseBitSet<u32> {