Repository: rust-lang/rust
Pull request: Move query-stack-frame spans into `QueryStackFrame`

- Follow-up to https://github.com/rust-lang/rust/pull/153907#discussion_r2936748691.
---

Code that previously used `QueryStackFrame` now uses `TaggedQueryKey` directly.

Code that previously used `Spanned<QueryStackFrame>` now uses `QueryStackFrame`, which includes a span.

This ends up bringing QueryStackFrame closer to the expectations of the code that actually consumes query cycles, and also avoids the awkward and misleading `respan` calls and `node` field accesses that had been required by `Spanned`.

r? nnethercote

Changed files:
filename: compiler/rustc_middle/src/query/plumbing.rs
patch: @@ -8,15 +8,14 @@ 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::{Span, Spanned};
+use rustc_span::Span;
 pub use sealed::IntoQueryParam;
 
 use crate::dep_graph::{DepKind, DepNodeIndex, SerializedDepNodeIndex};
 use crate::ich::StableHashingContext;
 use crate::queries::{ExternProviders, Providers, QueryArenas, QueryVTables, TaggedQueryKey};
 use crate::query::on_disk_cache::OnDiskCache;
-use crate::query::stack::QueryStackFrame;
-use crate::query::{QueryCache, QueryJob};
+use crate::query::{QueryCache, QueryJob, QueryStackFrame};
 use crate::ty::TyCtxt;
 
 /// For a particular query, keeps track of "active" keys, i.e. keys whose
@@ -53,10 +52,10 @@ pub enum ActiveKeyStatus<'tcx> {
 #[derive(Debug)]
 pub struct CycleError<'tcx> {
     /// The query and related span that uses the cycle.
-    pub usage: Option<Spanned<QueryStackFrame<'tcx>>>,
+    pub usage: Option<QueryStackFrame<'tcx>>,
 
     /// The span here corresponds to the reason for which this query was required.
-    pub cycle: Vec<Spanned<QueryStackFrame<'tcx>>>,
+    pub cycle: Vec<QueryStackFrame<'tcx>>,
 }
 
 #[derive(Debug)]
@@ -505,7 +504,7 @@ macro_rules! define_callbacks {
 
         /// Identifies a query by kind and key. This is in contrast to `QueryJobId` which is just a number.
         #[allow(non_camel_case_types)]
-        #[derive(Clone, Debug)]
+        #[derive(Clone, Copy, Debug)]
         pub enum TaggedQueryKey<'tcx> {
             $(
                 $name($name::Key<'tcx>),

filename: compiler/rustc_middle/src/query/stack.rs
patch: @@ -1,10 +1,14 @@
+use rustc_span::Span;
+
 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)]
+#[derive(Debug)]
 pub struct QueryStackFrame<'tcx> {
+    pub span: Span,
+
     /// The query and key of the query method call that this stack frame
     /// corresponds to.
     ///

filename: compiler/rustc_query_impl/src/execution.rs
patch: @@ -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, QueryStackFrame, QueryState, QueryVTable,
+    QueryLatch, QueryMode, QueryState, QueryVTable,
 };
 use rustc_middle::ty::TyCtxt;
 use rustc_middle::verify_ich::incremental_verify_ich;
@@ -75,8 +75,8 @@ fn collect_active_query_jobs_inner<'tcx, C>(
             if let ActiveKeyStatus::Started(job) = status {
                 // 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() });
+                let tagged_key = (query.create_tagged_key)(*key);
+                job_map.insert(job.id, QueryJobInfo { tagged_key, job: job.clone() });
             }
         }
     };

filename: compiler/rustc_query_impl/src/from_cycle_error.rs
patch: @@ -76,7 +76,7 @@ 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 let TaggedQueryKey::check_representability(def_id) = frame.node.tagged_key
+        if let TaggedQueryKey::check_representability(def_id) = frame.tagged_key
             && tcx.def_kind(def_id) == DefKind::Field
         {
             let field_id: LocalDefId = def_id;
@@ -89,7 +89,7 @@ fn check_representability<'tcx>(tcx: TyCtxt<'tcx>, cycle_error: CycleError<'tcx>
         }
     }
     for frame in &cycle_error.cycle {
-        if let TaggedQueryKey::check_representability_adt_ty(key) = frame.node.tagged_key
+        if let TaggedQueryKey::check_representability_adt_ty(key) = frame.tagged_key
             && let Some(adt) = key.ty_adt_def()
             && let Some(def_id) = adt.did().as_local()
             && !item_and_field_ids.iter().any(|&(id, _)| id == def_id)
@@ -134,7 +134,7 @@ fn layout_of<'tcx>(
     let diag = search_for_cycle_permutation(
         &cycle_error.cycle,
         |cycle| {
-            if let TaggedQueryKey::layout_of(key) = cycle[0].node.tagged_key
+            if let TaggedQueryKey::layout_of(key) = cycle[0].tagged_key
                 && let ty::Coroutine(def_id, _) = key.value.kind()
                 && let Some(def_id) = def_id.as_local()
                 && let def_kind = tcx.def_kind(def_id)
@@ -159,7 +159,7 @@ fn layout_of<'tcx>(
                     tcx.def_kind_descr(def_kind, def_id.to_def_id()),
                 );
                 for (i, frame) in cycle.iter().enumerate() {
-                    let TaggedQueryKey::layout_of(frame_key) = frame.node.tagged_key else {
+                    let TaggedQueryKey::layout_of(frame_key) = frame.tagged_key else {
                         continue;
                     };
                     let &ty::Coroutine(frame_def_id, _) = frame_key.value.kind() else {
@@ -169,7 +169,7 @@ fn layout_of<'tcx>(
                         continue;
                     };
                     let frame_span =
-                        frame.node.tagged_key.default_span(tcx, cycle[(i + 1) % cycle.len()].span);
+                        frame.tagged_key.default_span(tcx, cycle[(i + 1) % cycle.len()].span);
                     if frame_span.is_dummy() {
                         continue;
                     }

filename: compiler/rustc_query_impl/src/job.rs
patch: @@ -11,7 +11,7 @@ use rustc_middle::query::{
     CycleError, QueryJob, QueryJobId, QueryLatch, QueryStackFrame, QueryWaiter,
 };
 use rustc_middle::ty::TyCtxt;
-use rustc_span::{DUMMY_SP, Span, respan};
+use rustc_span::{DUMMY_SP, Span};
 
 use crate::{CollectActiveJobsKind, collect_active_query_jobs};
 
@@ -30,8 +30,8 @@ impl<'tcx> QueryJobMap<'tcx> {
         self.map.insert(id, info);
     }
 
-    fn frame_of(&self, id: QueryJobId) -> &QueryStackFrame<'tcx> {
-        &self.map[&id].frame
+    fn tagged_key_of(&self, id: QueryJobId) -> TaggedQueryKey<'tcx> {
+        self.map[&id].tagged_key
     }
 
     fn span_of(&self, id: QueryJobId) -> Span {
@@ -49,7 +49,7 @@ impl<'tcx> QueryJobMap<'tcx> {
 
 #[derive(Debug)]
 pub(crate) struct QueryJobInfo<'tcx> {
-    pub(crate) frame: QueryStackFrame<'tcx>,
+    pub(crate) tagged_key: TaggedQueryKey<'tcx>,
     pub(crate) job: QueryJob<'tcx>,
 }
 
@@ -65,7 +65,7 @@ pub(crate) fn find_cycle_in_stack<'tcx>(
 
     while let Some(job) = current_job {
         let info = &job_map.map[&job];
-        cycle.push(respan(info.job.span, info.frame.clone()));
+        cycle.push(QueryStackFrame { span: info.job.span, tagged_key: info.tagged_key });
 
         if job == id {
             cycle.reverse();
@@ -78,7 +78,7 @@ pub(crate) fn find_cycle_in_stack<'tcx>(
             // Find out why the cycle itself was used
             let usage = try {
                 let parent = info.job.parent?;
-                respan(info.job.span, job_map.frame_of(parent).clone())
+                QueryStackFrame { span: info.job.span, tagged_key: job_map.tagged_key_of(parent) }
             };
             return CycleError { usage, cycle };
         }
@@ -100,19 +100,19 @@ pub(crate) fn find_dep_kind_root<'tcx>(
 ) -> (Span, String, usize) {
     let mut depth = 1;
     let mut info = &job_map.map[&id];
-    // Two query stack frames are for the same query method if they have the same
+    // Two query jobs 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 expected_query = mem::discriminant::<TaggedQueryKey<'tcx>>(&info.tagged_key);
     let mut last_info = info;
 
     while let Some(id) = info.job.parent {
         info = &job_map.map[&id];
-        if mem::discriminant(&info.frame.tagged_key) == expected_query {
+        if mem::discriminant(&info.tagged_key) == expected_query {
             depth += 1;
             last_info = info;
         }
     }
-    (last_info.job.span, last_info.frame.tagged_key.description(tcx), depth)
+    (last_info.job.span, last_info.tagged_key.description(tcx), depth)
 }
 
 /// The locaton of a resumable waiter. The usize is the index into waiters in the query's latch.
@@ -316,14 +316,17 @@ fn remove_cycle<'tcx>(
 
         let usage = entry_point
             .query_waiting_on_cycle
-            .map(|(span, job)| respan(span, job_map.frame_of(job).clone()));
+            .map(|(span, job)| QueryStackFrame { span, tagged_key: job_map.tagged_key_of(job) });
 
         // Create the cycle error
         let error = CycleError {
             usage,
             cycle: stack
                 .iter()
-                .map(|&(span, job)| respan(span, job_map.frame_of(job).clone()))
+                .map(|&(span, job)| QueryStackFrame {
+                    span,
+                    tagged_key: job_map.tagged_key_of(job),
+                })
                 .collect(),
         };
 
@@ -419,12 +422,12 @@ pub fn print_query_stack<'tcx>(
         let Some(query_info) = job_map.map.get(&query) else {
             break;
         };
-        let description = query_info.frame.tagged_key.description(tcx);
+        let description = query_info.tagged_key.description(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_name}] {description}",
-                query_name = query_info.frame.tagged_key.query_name(),
+                query_name = query_info.tagged_key.query_name(),
             ))
             .with_span(query_info.job.span)
             .emit();
@@ -435,7 +438,7 @@ pub fn print_query_stack<'tcx>(
             let _ = writeln!(
                 file,
                 "#{count_total} [{query_name}] {description}",
-                query_name = query_info.frame.tagged_key.query_name(),
+                query_name = query_info.tagged_key.query_name(),
             );
         }
 
@@ -457,37 +460,36 @@ pub(crate) fn report_cycle<'tcx>(
 ) -> Diag<'tcx> {
     assert!(!stack.is_empty());
 
-    let span = stack[0].node.tagged_key.default_span(tcx, stack[1 % stack.len()].span);
+    let span = stack[0].tagged_key.default_span(tcx, stack[1 % stack.len()].span);
 
     let mut cycle_stack = Vec::new();
 
     use crate::error::StackCount;
-    let stack_bottom = stack[0].node.tagged_key.description(tcx);
+    let stack_bottom = stack[0].tagged_key.description(tcx);
     let stack_count = if stack.len() == 1 {
         StackCount::Single { stack_bottom: stack_bottom.clone() }
     } else {
         StackCount::Multiple { stack_bottom: stack_bottom.clone() }
     };
 
     for i in 1..stack.len() {
-        let node = &stack[i].node;
-        let span = node.tagged_key.default_span(tcx, stack[(i + 1) % stack.len()].span);
-        cycle_stack.push(crate::error::CycleStack { span, desc: node.tagged_key.description(tcx) });
+        let frame = &stack[i];
+        let span = frame.tagged_key.default_span(tcx, stack[(i + 1) % stack.len()].span);
+        cycle_stack
+            .push(crate::error::CycleStack { span, desc: frame.tagged_key.description(tcx) });
     }
 
     let cycle_usage = usage.as_ref().map(|usage| crate::error::CycleUsage {
-        span: usage.node.tagged_key.default_span(tcx, usage.span),
-        usage: usage.node.tagged_key.description(tcx),
+        span: usage.tagged_key.default_span(tcx, usage.span),
+        usage: usage.tagged_key.description(tcx),
     });
 
     let alias = if stack
         .iter()
-        .all(|entry| matches!(entry.node.tagged_key.def_kind(tcx), Some(DefKind::TyAlias)))
+        .all(|frame| frame.tagged_key.def_kind(tcx) == Some(DefKind::TyAlias))
     {
         Some(crate::error::Alias::Ty)
-    } else if stack
-        .iter()
-        .all(|entry| entry.node.tagged_key.def_kind(tcx) == Some(DefKind::TraitAlias))
+    } else if stack.iter().all(|frame| frame.tagged_key.def_kind(tcx) == Some(DefKind::TraitAlias))
     {
         Some(crate::error::Alias::Trait)
     } else {