Auto merge of #101303 - jyn514:jnelson/handle-cycle-enum, r=cjgillot

Make `HandleCycleError` an enum instead of a macro-generated closure

Helps with https://github.com/rust-lang/rust/issues/96524. Based on https://github.com/rust-lang/rust/pull/100943 to avoid merge conflicts, so it looks larger than it is (only the last commit is relevant).

cc https://rust-lang.zulipchat.com/#narrow/stream/241847-t-compiler.2Fwg-incr-comp/topic/Moving.20.60Value.60.20to.20rustc_query_system.20.2396524

r? `@cjgillot`

compiler/rustc_middle/src/lib.rs

@@ -96,6 +96,7 @@
 pub mod thir;
 pub mod traits;
 pub mod ty;
+mod values;
 
 pub mod util {
     pub mod bug;

compiler/rustc_middle/src/values.rs

@@ -1,39 +1,28 @@
-use super::QueryCtxt;
-use rustc_middle::ty::{self, AdtSizedConstraint, Ty};
+use rustc_middle::ty::{self, AdtSizedConstraint, Ty, TyCtxt};
+use rustc_query_system::Value;
 
-pub(super) trait Value<'tcx>: Sized {
-    fn from_cycle_error(tcx: QueryCtxt<'tcx>) -> Self;
-}
-
-impl<'tcx, T> Value<'tcx> for T {
-    default fn from_cycle_error(tcx: QueryCtxt<'tcx>) -> T {
-        tcx.sess.abort_if_errors();
-        bug!("Value::from_cycle_error called without errors");
-    }
-}
-
-impl<'tcx> Value<'tcx> for Ty<'_> {
-    fn from_cycle_error(tcx: QueryCtxt<'tcx>) -> Self {
+impl<'tcx> Value<TyCtxt<'tcx>> for Ty<'_> {
+    fn from_cycle_error(tcx: TyCtxt<'tcx>) -> Self {
         // SAFETY: This is never called when `Self` is not `Ty<'tcx>`.
         // FIXME: Represent the above fact in the trait system somehow.
         unsafe { std::mem::transmute::<Ty<'tcx>, Ty<'_>>(tcx.ty_error()) }
     }
 }
 
-impl<'tcx> Value<'tcx> for ty::SymbolName<'_> {
-    fn from_cycle_error(tcx: QueryCtxt<'tcx>) -> Self {
+impl<'tcx> Value<TyCtxt<'tcx>> for ty::SymbolName<'_> {
+    fn from_cycle_error(tcx: TyCtxt<'tcx>) -> Self {
         // SAFETY: This is never called when `Self` is not `SymbolName<'tcx>`.
         // FIXME: Represent the above fact in the trait system somehow.
         unsafe {
             std::mem::transmute::<ty::SymbolName<'tcx>, ty::SymbolName<'_>>(ty::SymbolName::new(
-                *tcx, "<error>",
+                tcx, "<error>",
             ))
         }
     }
 }
 
-impl<'tcx> Value<'tcx> for AdtSizedConstraint<'_> {
-    fn from_cycle_error(tcx: QueryCtxt<'tcx>) -> Self {
+impl<'tcx> Value<TyCtxt<'tcx>> for AdtSizedConstraint<'_> {
+    fn from_cycle_error(tcx: TyCtxt<'tcx>) -> Self {
         // SAFETY: This is never called when `Self` is not `AdtSizedConstraint<'tcx>`.
         // FIXME: Represent the above fact in the trait system somehow.
         unsafe {
@@ -44,8 +33,8 @@ fn from_cycle_error(tcx: QueryCtxt<'tcx>) -> Self {
     }
 }
 
-impl<'tcx> Value<'tcx> for ty::Binder<'_, ty::FnSig<'_>> {
-    fn from_cycle_error(tcx: QueryCtxt<'tcx>) -> Self {
+impl<'tcx> Value<TyCtxt<'tcx>> for ty::Binder<'_, ty::FnSig<'_>> {
+    fn from_cycle_error(tcx: TyCtxt<'tcx>) -> Self {
         let err = tcx.ty_error();
         // FIXME(compiler-errors): It would be nice if we could get the
         // query key, so we could at least generate a fn signature that

compiler/rustc_query_impl/src/lib.rs

@@ -34,9 +34,6 @@
 mod keys;
 use keys::Key;
 
-mod values;
-use self::values::Value;
-
 pub use rustc_query_system::query::QueryConfig;
 pub(crate) use rustc_query_system::query::{QueryDescription, QueryVTable};
 

compiler/rustc_query_impl/src/plumbing.rs

@@ -18,6 +18,7 @@
     force_query, QueryConfig, QueryContext, QueryDescription, QueryJobId, QueryMap,
     QuerySideEffects, QueryStackFrame,
 };
+use rustc_query_system::Value;
 use std::any::Any;
 use std::num::NonZeroU64;
 use thin_vec::ThinVec;
@@ -174,21 +175,17 @@ pub fn try_print_query_stack(
 }
 
 macro_rules! handle_cycle_error {
-    ([][$tcx: expr, $error:expr]) => {{
-        $error.emit();
-        Value::from_cycle_error($tcx)
+    ([]) => {{
+        rustc_query_system::HandleCycleError::Error
     }};
-    ([(fatal_cycle) $($rest:tt)*][$tcx:expr, $error:expr]) => {{
-        $error.emit();
-        $tcx.sess.abort_if_errors();
-        unreachable!()
+    ([(fatal_cycle) $($rest:tt)*]) => {{
+        rustc_query_system::HandleCycleError::Fatal
     }};
-    ([(cycle_delay_bug) $($rest:tt)*][$tcx:expr, $error:expr]) => {{
-        $error.delay_as_bug();
-        Value::from_cycle_error($tcx)
+    ([(cycle_delay_bug) $($rest:tt)*]) => {{
+        rustc_query_system::HandleCycleError::DelayBug
     }};
-    ([$other:tt $($modifiers:tt)*][$($args:tt)*]) => {
-        handle_cycle_error!([$($modifiers)*][$($args)*])
+    ([$other:tt $($modifiers:tt)*]) => {
+        handle_cycle_error!([$($modifiers)*])
     };
 }
 
@@ -320,6 +317,7 @@ fn force_from_dep_node<'tcx, Q>(tcx: TyCtxt<'tcx>, dep_node: DepNode) -> bool
 where
     Q: QueryDescription<QueryCtxt<'tcx>>,
     Q::Key: DepNodeParams<TyCtxt<'tcx>>,
+    Q::Value: Value<TyCtxt<'tcx>>,
 {
     if let Some(key) = Q::Key::recover(tcx, &dep_node) {
         #[cfg(debug_assertions)]
@@ -418,7 +416,7 @@ fn make_vtable(tcx: QueryCtxt<'tcx>, key: &Self::Key) ->
                     depth_limit: depth_limit!([$($modifiers)*]),
                     dep_kind: dep_graph::DepKind::$name,
                     hash_result: hash_result!([$($modifiers)*]),
-                    handle_cycle_error: |tcx, mut error| handle_cycle_error!([$($modifiers)*][tcx, error]),
+                    handle_cycle_error: handle_cycle_error!([$($modifiers)*]),
                     compute,
                     cache_on_disk,
                     try_load_from_disk: Self::TRY_LOAD_FROM_DISK,

compiler/rustc_query_system/src/error.rs

@@ -12,6 +12,13 @@ fn add_to_diagnostic(self, diag: &mut rustc_errors::Diagnostic) {
     }
 }
 
+#[derive(Copy, Clone)]
+pub enum HandleCycleError {
+    Error,
+    Fatal,
+    DelayBug,
+}
+
 #[derive(SessionSubdiagnostic)]
 pub enum StackCount {
     #[note(query_system::cycle_stack_single)]

compiler/rustc_query_system/src/lib.rs

@@ -20,3 +20,7 @@
 mod error;
 pub mod ich;
 pub mod query;
+mod values;
+
+pub use error::HandleCycleError;
+pub use values::Value;

compiler/rustc_query_system/src/query/config.rs

@@ -2,12 +2,12 @@
 
 use crate::dep_graph::DepNode;
 use crate::dep_graph::SerializedDepNodeIndex;
+use crate::error::HandleCycleError;
 use crate::ich::StableHashingContext;
 use crate::query::caches::QueryCache;
 use crate::query::{QueryContext, QueryState};
 
 use rustc_data_structures::fingerprint::Fingerprint;
-use rustc_errors::{DiagnosticBuilder, ErrorGuaranteed};
 use std::fmt::Debug;
 use std::hash::Hash;
 
@@ -19,6 +19,7 @@ pub trait QueryConfig {
     type Stored: Clone;
 }
 
+#[derive(Copy, Clone)]
 pub struct QueryVTable<CTX: QueryContext, K, V> {
     pub anon: bool,
     pub dep_kind: CTX::DepKind,
@@ -28,7 +29,7 @@ pub struct QueryVTable<CTX: QueryContext, K, V> {
 
     pub compute: fn(CTX::DepContext, K) -> V,
     pub hash_result: Option<fn(&mut StableHashingContext<'_>, &V) -> Fingerprint>,
-    pub handle_cycle_error: fn(CTX, DiagnosticBuilder<'_, ErrorGuaranteed>) -> V,
+    pub handle_cycle_error: HandleCycleError,
     pub try_load_from_disk: Option<fn(CTX, SerializedDepNodeIndex) -> Option<V>>,
 }
 

compiler/rustc_query_system/src/query/plumbing.rs

@@ -7,6 +7,8 @@
 use crate::query::config::{QueryDescription, QueryVTable};
 use crate::query::job::{report_cycle, QueryInfo, QueryJob, QueryJobId, QueryJobInfo};
 use crate::query::{QueryContext, QueryMap, QuerySideEffects, QueryStackFrame};
+use crate::values::Value;
+use crate::HandleCycleError;
 use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_data_structures::fx::FxHashMap;
 #[cfg(parallel_compiler)]
@@ -118,19 +120,46 @@ struct JobOwner<'tcx, K>
 fn mk_cycle<CTX, V, R>(
     tcx: CTX,
     error: CycleError,
-    handle_cycle_error: fn(CTX, DiagnosticBuilder<'_, ErrorGuaranteed>) -> V,
+    handler: HandleCycleError,
     cache: &dyn crate::query::QueryStorage<Value = V, Stored = R>,
 ) -> R
 where
     CTX: QueryContext,
-    V: std::fmt::Debug,
+    V: std::fmt::Debug + Value<CTX::DepContext>,
     R: Clone,
 {
     let error = report_cycle(tcx.dep_context().sess(), error);
-    let value = handle_cycle_error(tcx, error);
+    let value = handle_cycle_error(*tcx.dep_context(), error, handler);
     cache.store_nocache(value)
 }
 
+fn handle_cycle_error<CTX, V>(
+    tcx: CTX,
+    mut error: DiagnosticBuilder<'_, ErrorGuaranteed>,
+    handler: HandleCycleError,
+) -> V
+where
+    CTX: DepContext,
+    V: Value<CTX>,
+{
+    use HandleCycleError::*;
+    match handler {
+        Error => {
+            error.emit();
+            Value::from_cycle_error(tcx)
+        }
+        Fatal => {
+            error.emit();
+            tcx.sess().abort_if_errors();
+            unreachable!()
+        }
+        DelayBug => {
+            error.delay_as_bug();
+            Value::from_cycle_error(tcx)
+        }
+    }
+}
+
 impl<'tcx, K> JobOwner<'tcx, K>
 where
     K: Eq + Hash + Clone,
@@ -336,6 +365,7 @@ fn try_execute_query<CTX, C>(
 where
     C: QueryCache,
     C::Key: Clone + DepNodeParams<CTX::DepContext>,
+    C::Value: Value<CTX::DepContext>,
     CTX: QueryContext,
 {
     match JobOwner::<'_, C::Key>::try_start(&tcx, state, span, key.clone()) {
@@ -686,6 +716,7 @@ pub fn get_query<Q, CTX>(tcx: CTX, span: Span, key: Q::Key, mode: QueryMode) ->
 where
     Q: QueryDescription<CTX>,
     Q::Key: DepNodeParams<CTX::DepContext>,
+    Q::Value: Value<CTX::DepContext>,
     CTX: QueryContext,
 {
     let query = Q::make_vtable(tcx, &key);
@@ -718,6 +749,7 @@ pub fn force_query<Q, CTX>(tcx: CTX, key: Q::Key, dep_node: DepNode<CTX::DepKind
 where
     Q: QueryDescription<CTX>,
     Q::Key: DepNodeParams<CTX::DepContext>,
+    Q::Value: Value<CTX::DepContext>,
     CTX: QueryContext,
 {
     // We may be concurrently trying both execute and force a query.

compiler/rustc_query_system/src/values.rs

@@ -0,0 +1,14 @@
+use crate::dep_graph::DepContext;
+
+pub trait Value<CTX: DepContext>: Sized {
+    fn from_cycle_error(tcx: CTX) -> Self;
+}
+
+impl<CTX: DepContext, T> Value<CTX> for T {
+    default fn from_cycle_error(tcx: CTX) -> T {
+        tcx.sess().abort_if_errors();
+        // Ideally we would use `bug!` here. But bug! is only defined in rustc_middle, and it's
+        // non-trivial to define it earlier.
+        panic!("Value::from_cycle_error called without errors");
+    }
+}