Rollup merge of #153907 - Zalathar:stack-frame, r=nnethercote

Remove redundant fields from `QueryStackFrame`

- Follow-up to #153867 (comment)
---

Now that QueryStackFrame holds a TaggedQueryKey, it turns out that all of its other fields are redundant and can be removed:

- An explicit dep-kind is redundant here. If we want to find the query name, or check for specific queries, we can inspect the `TaggedQueryKey` instead.
- Similarly, in cases where the key is some kind of `DepId`, we can extract it directly from the `TaggedQueryKey`.

r? nnethercote

Author: Zalathar

compiler/rustc_middle/src/query/plumbing.rs

@@ -513,6 +513,18 @@ macro_rules! define_callbacks {
         }
 
         impl<'tcx> TaggedQueryKey<'tcx> {
+            /// Returns the name of the query this key is tagged with.
+            ///
+            /// This is useful for error/debug output, but don't use it to check for
+            /// specific query names. Instead, match on the `TaggedQueryKey` variant.
+            pub fn query_name(&self) -> &'static str {
+                match self {
+                    $(
+                        TaggedQueryKey::$name(_) => stringify!($name),
+                    )*
+                }
+            }
+
             /// Formats a human-readable description of this query and its key, as
             /// specified by the `desc` query modifier.
             ///

compiler/rustc_middle/src/query/stack.rs

@@ -1,16 +1,14 @@
-use std::fmt::Debug;
-
-use rustc_span::def_id::DefId;
-
-use crate::dep_graph::DepKind;
 use crate::queries::TaggedQueryKey;
 
 /// Description of a frame in the query stack.
 ///
 /// This is mostly used in case of cycles for error reporting.
 #[derive(Clone, Debug)]
 pub struct QueryStackFrame<'tcx> {
+    /// The query and key of the query method call that this stack frame
+    /// corresponds to.
+    ///
+    /// Code that doesn't care about the specific key can still use this to
+    /// check which query it's for, or obtain the query's name.
     pub tagged_key: TaggedQueryKey<'tcx>,
-    pub dep_kind: DepKind,
-    pub def_id: Option<DefId>,
 }

compiler/rustc_query_impl/src/execution.rs

@@ -9,7 +9,7 @@ use rustc_errors::FatalError;
 use rustc_middle::dep_graph::{DepGraphData, DepNodeKey, SerializedDepNodeIndex};
 use rustc_middle::query::{
     ActiveKeyStatus, CycleError, EnsureMode, QueryCache, QueryJob, QueryJobId, QueryKey,
-    QueryLatch, QueryMode, QueryState, QueryVTable,
+    QueryLatch, QueryMode, QueryStackFrame, QueryState, QueryVTable,
 };
 use rustc_middle::ty::TyCtxt;
 use rustc_middle::verify_ich::incremental_verify_ich;
@@ -73,8 +73,9 @@ fn collect_active_query_jobs_inner<'tcx, C>(
     let mut collect_shard_jobs = |shard: &HashTable<(C::Key, ActiveKeyStatus<'tcx>)>| {
         for (key, status) in shard.iter() {
             if let ActiveKeyStatus::Started(job) = status {
-                // This function is safe to call with the shard locked because it is very simple.
-                let frame = crate::plumbing::create_query_stack_frame(query, *key);
+                // It's fine to call `create_tagged_key` with the shard locked,
+                // because it's just a `TaggedQueryKey` variant constructor.
+                let frame = QueryStackFrame { tagged_key: (query.create_tagged_key)(*key) };
                 job_map.insert(job.id, QueryJobInfo { frame, job: job.clone() });
             }
         }

compiler/rustc_query_impl/src/from_cycle_error.rs

@@ -8,7 +8,6 @@ use rustc_errors::codes::*;
 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, TaggedQueryKey};
 use rustc_middle::query::CycleError;
 use rustc_middle::query::erase::erase_val;
@@ -77,11 +76,10 @@ 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 frame in &cycle_error.cycle {
-        if frame.node.dep_kind == DepKind::check_representability
-            && let Some(field_id) = frame.node.def_id
-            && let Some(field_id) = field_id.as_local()
-            && let Some(DefKind::Field) = frame.node.tagged_key.def_kind(tcx)
+        if let TaggedQueryKey::check_representability(def_id) = frame.node.tagged_key
+            && tcx.def_kind(def_id) == DefKind::Field
         {
+            let field_id: LocalDefId = def_id;
             let parent_id = tcx.parent(field_id.to_def_id());
             let item_id = match tcx.def_kind(parent_id) {
                 DefKind::Variant => tcx.parent(parent_id),
@@ -110,8 +108,7 @@ fn variances_of<'tcx>(tcx: TyCtxt<'tcx>, cycle_error: CycleError<'tcx>) -> &'tcx
         &cycle_error.cycle,
         |cycle| {
             if let Some(frame) = cycle.get(0)
-                && frame.node.dep_kind == DepKind::variances_of
-                && let Some(def_id) = frame.node.def_id
+                && let TaggedQueryKey::variances_of(def_id) = frame.node.tagged_key
             {
                 let n = tcx.generics_of(def_id).own_params.len();
                 ControlFlow::Break(tcx.arena.alloc_from_iter(iter::repeat_n(ty::Bivariant, n)))

compiler/rustc_query_impl/src/job.rs

@@ -1,11 +1,12 @@
 use std::io::Write;
-use std::iter;
 use std::ops::ControlFlow;
 use std::sync::Arc;
+use std::{iter, mem};
 
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_errors::{Diag, DiagCtxtHandle};
 use rustc_hir::def::DefKind;
+use rustc_middle::queries::TaggedQueryKey;
 use rustc_middle::query::{
     CycleError, QueryJob, QueryJobId, QueryLatch, QueryStackFrame, QueryWaiter,
 };
@@ -88,8 +89,8 @@ pub(crate) fn find_cycle_in_stack<'tcx>(
     panic!("did not find a cycle")
 }
 
-/// Finds the job closest to the root with a `DepKind` matching the `DepKind` of `id` and returns
-/// information about it.
+/// Finds the query job closest to the root that is for the same query method as `id`
+/// (but not necessarily the same query key), and returns information about it.
 #[cold]
 #[inline(never)]
 pub(crate) fn find_dep_kind_root<'tcx>(
@@ -99,12 +100,14 @@ pub(crate) fn find_dep_kind_root<'tcx>(
 ) -> (Span, String, usize) {
     let mut depth = 1;
     let mut info = &job_map.map[&id];
-    let dep_kind = info.frame.dep_kind;
+    // Two query stack frames are for the same query method if they have the same
+    // `TaggedQueryKey` discriminant.
+    let expected_query = mem::discriminant::<TaggedQueryKey<'tcx>>(&info.frame.tagged_key);
     let mut last_info = info;
 
     while let Some(id) = info.job.parent {
         info = &job_map.map[&id];
-        if info.frame.dep_kind == dep_kind {
+        if mem::discriminant(&info.frame.tagged_key) == expected_query {
             depth += 1;
             last_info = info;
         }
@@ -420,8 +423,8 @@ pub fn print_query_stack<'tcx>(
         if Some(count_printed) < limit_frames || limit_frames.is_none() {
             // Only print to stderr as many stack frames as `num_frames` when present.
             dcx.struct_failure_note(format!(
-                "#{} [{:?}] {}",
-                count_printed, query_info.frame.dep_kind, description
+                "#{count_printed} [{query_name}] {description}",
+                query_name = query_info.frame.tagged_key.query_name(),
             ))
             .with_span(query_info.job.span)
             .emit();
@@ -431,8 +434,8 @@ pub fn print_query_stack<'tcx>(
         if let Some(ref mut file) = file {
             let _ = writeln!(
                 file,
-                "#{} [{:?}] {}",
-                count_total, query_info.frame.dep_kind, description
+                "#{count_total} [{query_name}] {description}",
+                query_name = query_info.frame.tagged_key.query_name(),
             );
         }
 

compiler/rustc_query_impl/src/plumbing.rs

@@ -1,8 +1,6 @@
 use std::num::NonZero;
 
-use rustc_data_structures::sync::{DynSend, DynSync};
 use rustc_data_structures::unord::UnordMap;
-use rustc_hir::def_id::DefId;
 use rustc_hir::limit::Limit;
 use rustc_index::Idx;
 use rustc_middle::bug;
@@ -13,9 +11,7 @@ use rustc_middle::query::erase::{Erasable, Erased};
 use rustc_middle::query::on_disk_cache::{
     AbsoluteBytePos, CacheDecoder, CacheEncoder, EncodedDepNodeIndex,
 };
-use rustc_middle::query::{
-    QueryCache, QueryJobId, QueryKey, QueryMode, QueryStackFrame, QueryVTable, erase,
-};
+use rustc_middle::query::{QueryCache, QueryJobId, QueryMode, QueryVTable, erase};
 use rustc_middle::ty::TyCtxt;
 use rustc_middle::ty::codec::TyEncoder;
 use rustc_middle::ty::tls::{self, ImplicitCtxt};
@@ -83,23 +79,6 @@ pub(crate) fn start_query<R>(
     })
 }
 
-pub(crate) fn create_query_stack_frame<'tcx, C>(
-    vtable: &'tcx QueryVTable<'tcx, C>,
-    key: C::Key,
-) -> QueryStackFrame<'tcx>
-where
-    C: QueryCache<Key: QueryKey + DynSend + DynSync>,
-    QueryVTable<'tcx, C>: DynSync,
-{
-    let def_id: Option<DefId> = key.key_as_def_id();
-
-    QueryStackFrame {
-        tagged_key: (vtable.create_tagged_key)(key),
-        dep_kind: vtable.dep_kind,
-        def_id,
-    }
-}
-
 pub(crate) fn encode_query_values<'tcx>(
     tcx: TyCtxt<'tcx>,
     encoder: &mut CacheEncoder<'_, 'tcx>,