Skip to content

Commit a5aeac2

Browse files
committed
Inline and remove execute_job.
`execute_job` has a single call site in `try_execute_query`. This commit inlines and removes `execute_job`, but also puts the part that checks feedable results in its own separate function, `check_feedable`, because it's a nicely separate piece of logic. The big win here is that all the code dealing with the `QueryState` is now together in `try_execute_query`: get the lock, do the lookup, drop the lock, create the job guard, and complete the job guard. Previously these steps were split across two functions which I found hard to follow. This commit purely moves code around, there are no other changes.
1 parent 43584c3 commit a5aeac2

1 file changed

Lines changed: 55 additions & 57 deletions

File tree

compiler/rustc_query_impl/src/execution.rs

Lines changed: 55 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,28 @@ fn try_execute_query<'tcx, C: QueryCache, const INCR: bool>(
317317
// Drop the lock before we start executing the query
318318
drop(state_lock);
319319

320-
execute_job::<C, INCR>(query, tcx, key, key_hash, id, dep_node)
320+
// Set up a guard object that will automatically poison the query if a
321+
// panic occurs while executing the query (or any intermediate plumbing).
322+
let job_guard = ActiveJobGuard { state: &query.state, key, key_hash };
323+
324+
debug_assert_eq!(tcx.dep_graph.is_fully_enabled(), INCR);
325+
326+
// Delegate to another function to actually execute the query job.
327+
let (value, dep_node_index) = if INCR {
328+
execute_job_incr(query, tcx, key, dep_node, id)
329+
} else {
330+
execute_job_non_incr(query, tcx, key, id)
331+
};
332+
333+
let cache = &query.cache;
334+
if query.feedable {
335+
check_feedable(tcx, query, key, &value);
336+
}
337+
338+
// Tell the guard to perform completion bookkeeping, and also to not poison the query.
339+
job_guard.complete(cache, value, dep_node_index);
340+
341+
(value, Some(dep_node_index))
321342
}
322343
Entry::Occupied(mut entry) => {
323344
match &mut entry.get_mut().1 {
@@ -346,67 +367,44 @@ fn try_execute_query<'tcx, C: QueryCache, const INCR: bool>(
346367
}
347368

348369
#[inline(always)]
349-
fn execute_job<'tcx, C: QueryCache, const INCR: bool>(
350-
query: &'tcx QueryVTable<'tcx, C>,
370+
fn check_feedable<'tcx, C: QueryCache>(
351371
tcx: TyCtxt<'tcx>,
372+
query: &'tcx QueryVTable<'tcx, C>,
352373
key: C::Key,
353-
key_hash: u64,
354-
id: QueryJobId,
355-
dep_node: Option<DepNode>,
356-
) -> (C::Value, Option<DepNodeIndex>) {
357-
// Set up a guard object that will automatically poison the query if a
358-
// panic occurs while executing the query (or any intermediate plumbing).
359-
let job_guard = ActiveJobGuard { state: &query.state, key, key_hash };
360-
361-
debug_assert_eq!(tcx.dep_graph.is_fully_enabled(), INCR);
362-
363-
// Delegate to another function to actually execute the query job.
364-
let (value, dep_node_index) = if INCR {
365-
execute_job_incr(query, tcx, key, dep_node, id)
366-
} else {
367-
execute_job_non_incr(query, tcx, key, id)
368-
};
369-
370-
let cache = &query.cache;
371-
if query.feedable {
372-
// We should not compute queries that also got a value via feeding.
373-
// This can't happen, as query feeding adds the very dependencies to the fed query
374-
// as its feeding query had. So if the fed query is red, so is its feeder, which will
375-
// get evaluated first, and re-feed the query.
376-
if let Some((cached_value, _)) = cache.lookup(&key) {
377-
let Some(hash_value_fn) = query.hash_value_fn else {
378-
panic!(
379-
"no_hash fed query later has its value computed.\n\
380-
Remove `no_hash` modifier to allow recomputation.\n\
381-
The already cached value: {}",
382-
(query.format_value)(&cached_value)
383-
);
384-
};
374+
value: &C::Value,
375+
) {
376+
// We should not compute queries that also got a value via feeding.
377+
// This can't happen, as query feeding adds the very dependencies to the fed query
378+
// as its feeding query had. So if the fed query is red, so is its feeder, which will
379+
// get evaluated first, and re-feed the query.
380+
if let Some((cached_value, _)) = query.cache.lookup(&key) {
381+
let Some(hash_value_fn) = query.hash_value_fn else {
382+
panic!(
383+
"no_hash fed query later has its value computed.\n\
384+
Remove `no_hash` modifier to allow recomputation.\n\
385+
The already cached value: {}",
386+
(query.format_value)(&cached_value)
387+
);
388+
};
385389

386-
let (old_hash, new_hash) = tcx.with_stable_hashing_context(|mut hcx| {
387-
(hash_value_fn(&mut hcx, &cached_value), hash_value_fn(&mut hcx, &value))
388-
});
389-
let formatter = query.format_value;
390-
if old_hash != new_hash {
391-
// We have an inconsistency. This can happen if one of the two
392-
// results is tainted by errors.
393-
assert!(
394-
tcx.dcx().has_errors().is_some(),
395-
"Computed query value for {:?}({:?}) is inconsistent with fed value,\n\
396-
computed={:#?}\nfed={:#?}",
397-
query.dep_kind,
398-
key,
399-
formatter(&value),
400-
formatter(&cached_value),
401-
);
402-
}
390+
let (old_hash, new_hash) = tcx.with_stable_hashing_context(|mut hcx| {
391+
(hash_value_fn(&mut hcx, &cached_value), hash_value_fn(&mut hcx, value))
392+
});
393+
let formatter = query.format_value;
394+
if old_hash != new_hash {
395+
// We have an inconsistency. This can happen if one of the two
396+
// results is tainted by errors.
397+
assert!(
398+
tcx.dcx().has_errors().is_some(),
399+
"Computed query value for {:?}({:?}) is inconsistent with fed value,\n\
400+
computed={:#?}\nfed={:#?}",
401+
query.dep_kind,
402+
key,
403+
formatter(value),
404+
formatter(&cached_value),
405+
);
403406
}
404407
}
405-
406-
// Tell the guard to perform completion bookkeeping, and also to not poison the query.
407-
job_guard.complete(cache, value, dep_node_index);
408-
409-
(value, Some(dep_node_index))
410408
}
411409

412410
// Fast path for when incr. comp. is off.

0 commit comments

Comments
 (0)