From e04ae80c5ac7fab7d01aee9ad97ef109fe7f024d Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 9 Feb 2026 17:32:48 +1100 Subject: [PATCH 1/2] Rename `JobOwner` to `ActiveJobGuard` This commit also adds and updates some relevant comments. --- compiler/rustc_middle/src/ty/context/tls.rs | 3 +-- compiler/rustc_query_impl/src/execution.rs | 25 +++++++++++++-------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_middle/src/ty/context/tls.rs b/compiler/rustc_middle/src/ty/context/tls.rs index fa9995898ac20..a06f927928205 100644 --- a/compiler/rustc_middle/src/ty/context/tls.rs +++ b/compiler/rustc_middle/src/ty/context/tls.rs @@ -16,8 +16,7 @@ pub struct ImplicitCtxt<'a, 'tcx> { /// The current `TyCtxt`. pub tcx: TyCtxt<'tcx>, - /// The current query job, if any. This is updated by `JobOwner::start` in - /// `ty::query::plumbing` when executing a query. + /// The current query job, if any. pub query: Option, /// Used to prevent queries from calling too deeply. diff --git a/compiler/rustc_query_impl/src/execution.rs b/compiler/rustc_query_impl/src/execution.rs index ba442d8a0081d..7bed6ee937db0 100644 --- a/compiler/rustc_query_impl/src/execution.rs +++ b/compiler/rustc_query_impl/src/execution.rs @@ -83,9 +83,12 @@ pub(crate) fn gather_active_jobs_inner<'tcx, K: Copy>( Some(()) } -/// A type representing the responsibility to execute the job in the `job` field. -/// This will poison the relevant query if dropped. -struct JobOwner<'tcx, K> +/// Guard object representing the responsibility to execute a query job and +/// mark it as completed. +/// +/// This will poison the relevant query key if it is dropped without calling +/// [`Self::complete`]. +struct ActiveJobGuard<'tcx, K> where K: Eq + Hash + Copy, { @@ -137,12 +140,12 @@ fn handle_cycle_error<'tcx, C: QueryCache, const FLAGS: QueryFlags>( } } -impl<'tcx, K> JobOwner<'tcx, K> +impl<'tcx, K> ActiveJobGuard<'tcx, K> where K: Eq + Hash + Copy, { /// Completes the query by updating the query cache with the `result`, - /// signals the waiter and forgets the JobOwner, so it won't poison the query + /// signals the waiter, and forgets the guard so it won't poison the query. fn complete(self, cache: &C, key_hash: u64, result: C::Value, dep_node_index: DepNodeIndex) where C: QueryCache, @@ -174,7 +177,7 @@ where } } -impl<'tcx, K> Drop for JobOwner<'tcx, K> +impl<'tcx, K> Drop for ActiveJobGuard<'tcx, K> where K: Eq + Hash + Copy, { @@ -342,11 +345,13 @@ fn execute_job<'tcx, C: QueryCache, const FLAGS: QueryFlags, const INCR: bool>( id: QueryJobId, dep_node: Option, ) -> (C::Value, Option) { - // Use `JobOwner` so the query will be poisoned if executing it panics. - let job_owner = JobOwner { state, key }; + // Set up a guard object that will automatically poison the query if a + // panic occurs while executing the query (or any intermediate plumbing). + let job_guard = ActiveJobGuard { state, key }; debug_assert_eq!(qcx.tcx.dep_graph.is_fully_enabled(), INCR); + // Delegate to another function to actually execute the query job. let (result, dep_node_index) = if INCR { execute_job_incr(query, qcx, qcx.tcx.dep_graph.data().unwrap(), key, dep_node, id) } else { @@ -388,7 +393,9 @@ fn execute_job<'tcx, C: QueryCache, const FLAGS: QueryFlags, const INCR: bool>( } } } - job_owner.complete(cache, key_hash, result, dep_node_index); + + // Tell the guard to perform completion bookkeeping, and also to not poison the query. + job_guard.complete(cache, key_hash, result, dep_node_index); (result, Some(dep_node_index)) } From c475bdaa53441f52977ce79ac2e16b807dcb006d Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 9 Feb 2026 17:36:29 +1100 Subject: [PATCH 2/2] Include `key_hash` in `ActiveJobGuard` This value is a previously-computed hash of the key, so it makes sense to bundle it with the key inside the guard, since the guard will need it on completion anyway. --- compiler/rustc_query_impl/src/execution.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_query_impl/src/execution.rs b/compiler/rustc_query_impl/src/execution.rs index 7bed6ee937db0..88604c91d0259 100644 --- a/compiler/rustc_query_impl/src/execution.rs +++ b/compiler/rustc_query_impl/src/execution.rs @@ -94,6 +94,7 @@ where { state: &'tcx QueryState<'tcx, K>, key: K, + key_hash: u64, } #[cold] @@ -146,14 +147,13 @@ where { /// Completes the query by updating the query cache with the `result`, /// signals the waiter, and forgets the guard so it won't poison the query. - fn complete(self, cache: &C, key_hash: u64, result: C::Value, dep_node_index: DepNodeIndex) + fn complete(self, cache: &C, result: C::Value, dep_node_index: DepNodeIndex) where C: QueryCache, { - let key = self.key; - let state = self.state; - - // Forget ourself so our destructor won't poison the query + // Forget ourself so our destructor won't poison the query. + // (Extract fields by value first to make sure we don't leak anything.) + let Self { state, key, key_hash }: Self = self; mem::forget(self); // Mark as complete before we remove the job from the active state @@ -185,11 +185,10 @@ where #[cold] fn drop(&mut self) { // Poison the query so jobs waiting on it panic. - let state = self.state; + let Self { state, key, key_hash } = *self; let job = { - let key_hash = sharded::make_hash(&self.key); let mut shard = state.active.lock_shard_by_hash(key_hash); - match shard.find_entry(key_hash, equivalent_key(&self.key)) { + match shard.find_entry(key_hash, equivalent_key(&key)) { Err(_) => panic!(), Ok(occupied) => { let ((key, value), vacant) = occupied.remove(); @@ -347,7 +346,7 @@ fn execute_job<'tcx, C: QueryCache, const FLAGS: QueryFlags, const INCR: bool>( ) -> (C::Value, Option) { // Set up a guard object that will automatically poison the query if a // panic occurs while executing the query (or any intermediate plumbing). - let job_guard = ActiveJobGuard { state, key }; + let job_guard = ActiveJobGuard { state, key, key_hash }; debug_assert_eq!(qcx.tcx.dep_graph.is_fully_enabled(), INCR); @@ -395,7 +394,7 @@ fn execute_job<'tcx, C: QueryCache, const FLAGS: QueryFlags, const INCR: bool>( } // Tell the guard to perform completion bookkeeping, and also to not poison the query. - job_guard.complete(cache, key_hash, result, dep_node_index); + job_guard.complete(cache, result, dep_node_index); (result, Some(dep_node_index)) }