Rollup merge of rust-lang#151203 - revert-QueryStackFrame-split, r=oli-obk

Revert `QueryStackFrame` split

PR rust-lang#138672 fixed a query cycle OOM reported in rust-lang#124901. The fix involved delaying computation of some query stack frame elements and was very complex. It involved the addition of two new types, the addition of a generic `I` parameter to eleven(!) other types, a `PhantomData` field, and even required an unsafe transmute of a closure. [This comment](rust-lang#124901 (comment)) had suggested a much simpler fix, but it was ignored. The PR also failed to add a test case.

This PR adds a test case, reverts the complex fix, applies the simpler fix, and does a few other minor cleanups.

r? @oli-obk

Author: matthiaskrgr

compiler/rustc_macros/src/query.rs

@@ -93,7 +93,7 @@ struct QueryModifiers {
     cache: Option<(Option<Pat>, Block)>,
 
     /// A cycle error for this query aborting the compilation with a fatal error.
-    fatal_cycle: Option<Ident>,
+    cycle_fatal: Option<Ident>,
 
     /// A cycle error results in a delay_bug call
     cycle_delay_bug: Option<Ident>,
@@ -136,7 +136,7 @@ fn parse_query_modifiers(input: ParseStream<'_>) -> Result<QueryModifiers> {
     let mut arena_cache = None;
     let mut cache = None;
     let mut desc = None;
-    let mut fatal_cycle = None;
+    let mut cycle_fatal = None;
     let mut cycle_delay_bug = None;
     let mut cycle_stash = None;
     let mut no_hash = None;
@@ -189,8 +189,8 @@ fn parse_query_modifiers(input: ParseStream<'_>) -> Result<QueryModifiers> {
             try_insert!(cache = (args, block));
         } else if modifier == "arena_cache" {
             try_insert!(arena_cache = modifier);
-        } else if modifier == "fatal_cycle" {
-            try_insert!(fatal_cycle = modifier);
+        } else if modifier == "cycle_fatal" {
+            try_insert!(cycle_fatal = modifier);
         } else if modifier == "cycle_delay_bug" {
             try_insert!(cycle_delay_bug = modifier);
         } else if modifier == "cycle_stash" {
@@ -220,7 +220,7 @@ fn parse_query_modifiers(input: ParseStream<'_>) -> Result<QueryModifiers> {
         arena_cache,
         cache,
         desc,
-        fatal_cycle,
+        cycle_fatal,
         cycle_delay_bug,
         cycle_stash,
         no_hash,
@@ -366,8 +366,8 @@ pub(super) fn rustc_queries(input: TokenStream) -> TokenStream {
         }
 
         passthrough!(
-            fatal_cycle,
             arena_cache,
+            cycle_fatal,
             cycle_delay_bug,
             cycle_stash,
             no_hash,

compiler/rustc_middle/src/query/mod.rs

@@ -28,7 +28,7 @@
 //! - `desc { ... }`: Sets the human-readable description for diagnostics and profiling. Required for every query.
 //! - `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.
-//! - `fatal_cycle`: If a dependency cycle is detected, abort compilation with a fatal error.
+//! - `cycle_fatal`: If a dependency cycle is detected, abort compilation with a fatal error.
 //! - `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.
@@ -88,7 +88,7 @@ use rustc_index::IndexVec;
 use rustc_lint_defs::LintId;
 use rustc_macros::rustc_queries;
 use rustc_query_system::ich::StableHashingContext;
-use rustc_query_system::query::{QueryMode, QueryStackDeferred, QueryState};
+use rustc_query_system::query::{QueryMode, QueryState};
 use rustc_session::Limits;
 use rustc_session::config::{EntryFnType, OptLevel, OutputFilenames, SymbolManglingVersion};
 use rustc_session::cstore::{
@@ -161,7 +161,7 @@ pub mod plumbing;
 // The result type of each query must implement `Clone`, and additionally
 // `ty::query::values::Value`, which produces an appropriate placeholder
 // (error) value if the query resulted in a query cycle.
-// Queries marked with `fatal_cycle` do not need the latter implementation,
+// Queries marked with `cycle_fatal` do not need the latter implementation,
 // as they will raise an fatal error on query cycles instead.
 rustc_queries! {
     /// Caches the expansion of a derive proc macro, e.g. `#[derive(Serialize)]`.
@@ -596,7 +596,7 @@ rustc_queries! {
     }
 
     query is_panic_runtime(_: CrateNum) -> bool {
-        fatal_cycle
+        cycle_fatal
         desc { "checking if the crate is_panic_runtime" }
         separate_provide_extern
     }
@@ -1327,7 +1327,7 @@ rustc_queries! {
     /// Return the set of (transitive) callees that may result in a recursive call to `key`,
     /// if we were able to walk all callees.
     query mir_callgraph_cyclic(key: LocalDefId) -> &'tcx Option<UnordSet<LocalDefId>> {
-        fatal_cycle
+        cycle_fatal
         arena_cache
         desc { |tcx|
             "computing (transitive) callees of `{}` that may recurse",
@@ -1338,7 +1338,7 @@ rustc_queries! {
 
     /// Obtain all the calls into other local functions
     query mir_inliner_callees(key: ty::InstanceKind<'tcx>) -> &'tcx [(DefId, GenericArgsRef<'tcx>)] {
-        fatal_cycle
+        cycle_fatal
         desc { |tcx|
             "computing all local function calls in `{}`",
             tcx.def_path_str(key.def_id()),
@@ -1834,31 +1834,31 @@ rustc_queries! {
     }
 
     query is_compiler_builtins(_: CrateNum) -> bool {
-        fatal_cycle
+        cycle_fatal
         desc { "checking if the crate is_compiler_builtins" }
         separate_provide_extern
     }
     query has_global_allocator(_: CrateNum) -> bool {
         // This query depends on untracked global state in CStore
         eval_always
-        fatal_cycle
+        cycle_fatal
         desc { "checking if the crate has_global_allocator" }
         separate_provide_extern
     }
     query has_alloc_error_handler(_: CrateNum) -> bool {
         // This query depends on untracked global state in CStore
         eval_always
-        fatal_cycle
+        cycle_fatal
         desc { "checking if the crate has_alloc_error_handler" }
         separate_provide_extern
     }
     query has_panic_handler(_: CrateNum) -> bool {
-        fatal_cycle
+        cycle_fatal
         desc { "checking if the crate has_panic_handler" }
         separate_provide_extern
     }
     query is_profiler_runtime(_: CrateNum) -> bool {
-        fatal_cycle
+        cycle_fatal
         desc { "checking if a crate is `#![profiler_runtime]`" }
         separate_provide_extern
     }
@@ -1867,22 +1867,22 @@ rustc_queries! {
         cache_on_disk_if { true }
     }
     query required_panic_strategy(_: CrateNum) -> Option<PanicStrategy> {
-        fatal_cycle
+        cycle_fatal
         desc { "getting a crate's required panic strategy" }
         separate_provide_extern
     }
     query panic_in_drop_strategy(_: CrateNum) -> PanicStrategy {
-        fatal_cycle
+        cycle_fatal
         desc { "getting a crate's configured panic-in-drop strategy" }
         separate_provide_extern
     }
     query is_no_builtins(_: CrateNum) -> bool {
-        fatal_cycle
+        cycle_fatal
         desc { "getting whether a crate has `#![no_builtins]`" }
         separate_provide_extern
     }
     query symbol_mangling_version(_: CrateNum) -> SymbolManglingVersion {
-        fatal_cycle
+        cycle_fatal
         desc { "getting a crate's symbol mangling version" }
         separate_provide_extern
     }

compiler/rustc_middle/src/query/plumbing.rs

@@ -278,7 +278,7 @@ macro_rules! define_callbacks {
                     ($V)
                 );
 
-                /// This function takes `ProvidedValue` and coverts it to an erased `Value` by
+                /// This function takes `ProvidedValue` and converts it to an erased `Value` by
                 /// allocating it on an arena if the query has the `arena_cache` modifier. The
                 /// value is then erased and returned. This will happen when computing the query
                 /// using a provider or decoding a stored result.
@@ -431,7 +431,7 @@ macro_rules! define_callbacks {
         #[derive(Default)]
         pub struct QueryStates<'tcx> {
             $(
-                pub $name: QueryState<$($K)*, QueryStackDeferred<'tcx>>,
+                pub $name: QueryState<$($K)*>,
             )*
         }
 
@@ -532,7 +532,7 @@ macro_rules! define_feedable {
 // The result type of each query must implement `Clone`, and additionally
 // `ty::query::values::Value`, which produces an appropriate placeholder
 // (error) value if the query resulted in a query cycle.
-// Queries marked with `fatal_cycle` do not need the latter implementation,
+// Queries marked with `cycle_fatal` do not need the latter implementation,
 // as they will raise an fatal error on query cycles instead.
 
 mod sealed {

compiler/rustc_middle/src/values.rs

@@ -88,7 +88,7 @@ impl<'tcx> Value<TyCtxt<'tcx>> for Representability {
             if info.query.dep_kind == dep_kinds::representability
                 && let Some(field_id) = info.query.def_id
                 && let Some(field_id) = field_id.as_local()
-                && let Some(DefKind::Field) = info.query.info.def_kind
+                && let Some(DefKind::Field) = info.query.def_kind
             {
                 let parent_id = tcx.parent(field_id.to_def_id());
                 let item_id = match tcx.def_kind(parent_id) {
@@ -224,7 +224,7 @@ impl<'tcx, T> Value<TyCtxt<'tcx>> for Result<T, &'_ ty::layout::LayoutError<'_>>
                             continue;
                         };
                         let frame_span =
-                            frame.query.info.default_span(cycle[(i + 1) % cycle.len()].span);
+                            frame.query.default_span(cycle[(i + 1) % cycle.len()].span);
                         if frame_span.is_dummy() {
                             continue;
                         }

compiler/rustc_query_impl/src/lib.rs

@@ -21,8 +21,8 @@ use rustc_middle::ty::TyCtxt;
 use rustc_query_system::dep_graph::SerializedDepNodeIndex;
 use rustc_query_system::ich::StableHashingContext;
 use rustc_query_system::query::{
-    CycleError, HashResult, QueryCache, QueryConfig, QueryMap, QueryMode, QueryStackDeferred,
-    QueryState, get_query_incr, get_query_non_incr,
+    CycleError, HashResult, QueryCache, QueryConfig, QueryMap, QueryMode, QueryState,
+    get_query_incr, get_query_non_incr,
 };
 use rustc_query_system::{HandleCycleError, Value};
 use rustc_span::{ErrorGuaranteed, Span};
@@ -79,10 +79,7 @@ where
     }
 
     #[inline(always)]
-    fn query_state<'a>(
-        self,
-        qcx: QueryCtxt<'tcx>,
-    ) -> &'a QueryState<Self::Key, QueryStackDeferred<'tcx>>
+    fn query_state<'a>(self, qcx: QueryCtxt<'tcx>) -> &'a QueryState<Self::Key>
     where
         QueryCtxt<'tcx>: 'a,
     {
@@ -91,7 +88,7 @@ where
         unsafe {
             &*(&qcx.tcx.query_system.states as *const QueryStates<'tcx>)
                 .byte_add(self.dynamic.query_state)
-                .cast::<QueryState<Self::Key, QueryStackDeferred<'tcx>>>()
+                .cast::<QueryState<Self::Key>>()
         }
     }