From 12a8a7e86412949abc2e289a709d8858683885c0 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 17 Mar 2026 21:41:44 +1100 Subject: [PATCH 1/2] Use closures more consistently in `dep_graph.rs`. This file has several methods that take a `FnOnce() -> R` closure: - `DepGraph::with_ignore` - `DepGraph::with_query_deserialization` - `DepGraph::with_anon_task` - `DepGraphData::with_anon_task_inner` It also has two methods that take a faux closure via an `A` argument and a `fn(TyCtxt<'tcx>, A) -> R` argument: - DepGraph::with_task - DepGraphData::with_task The rationale is that the faux closure exercises tight control over what state they have access to. This seems silly when (a) they are passed a `TyCtxt`, and (b) when similar nearby functions take real closures. And they are more awkward to use, e.g. requiring multiple arguments to be gathered into a tuple. This commit changes the faux closures to real closures. The commit also changes the types and names of the closures from the awkward `op: OP` to the more standard `f: F`. --- .../rustc_codegen_cranelift/src/driver/aot.rs | 18 +++-- compiler/rustc_codegen_gcc/src/base.rs | 7 +- compiler/rustc_codegen_llvm/src/base.rs | 3 +- compiler/rustc_metadata/src/rmeta/encoder.rs | 3 +- compiler/rustc_middle/src/dep_graph/graph.rs | 78 ++++++++----------- compiler/rustc_query_impl/src/execution.rs | 3 +- 6 files changed, 50 insertions(+), 62 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/driver/aot.rs b/compiler/rustc_codegen_cranelift/src/driver/aot.rs index 79a3214568082..012d8caec16f8 100644 --- a/compiler/rustc_codegen_cranelift/src/driver/aot.rs +++ b/compiler/rustc_codegen_cranelift/src/driver/aot.rs @@ -380,11 +380,9 @@ fn codegen_cgu_content( fn module_codegen( tcx: TyCtxt<'_>, - (global_asm_config, cgu_name, token): ( - Arc, - rustc_span::Symbol, - ConcurrencyLimiterToken, - ), + global_asm_config: Arc, + cgu_name: rustc_span::Symbol, + token: ConcurrencyLimiterToken, ) -> OngoingModuleCodegen { let mut module = make_module(tcx.sess, cgu_name.as_str().to_string()); @@ -513,8 +511,14 @@ pub(crate) fn run_aot(tcx: TyCtxt<'_>) -> Box { let (module, _) = tcx.dep_graph.with_task( dep_node, tcx, - (global_asm_config.clone(), cgu.name(), concurrency_limiter.acquire(tcx.dcx())), - module_codegen, + || { + module_codegen( + tcx, + global_asm_config.clone(), + cgu.name(), + concurrency_limiter.acquire(tcx.dcx()), + ) + }, Some(rustc_middle::dep_graph::hash_result), ); IntoDynSyncSend(module) diff --git a/compiler/rustc_codegen_gcc/src/base.rs b/compiler/rustc_codegen_gcc/src/base.rs index d1637dd663bb7..8c29cfe4a0ac4 100644 --- a/compiler/rustc_codegen_gcc/src/base.rs +++ b/compiler/rustc_codegen_gcc/src/base.rs @@ -83,8 +83,7 @@ pub fn compile_codegen_unit( let (module, _) = tcx.dep_graph.with_task( dep_node, tcx, - (cgu_name, target_info, lto_supported), - module_codegen, + || module_codegen(tcx, cgu_name, target_info, lto_supported), Some(dep_graph::hash_result), ); let time_to_codegen = start_time.elapsed(); @@ -96,7 +95,9 @@ pub fn compile_codegen_unit( fn module_codegen( tcx: TyCtxt<'_>, - (cgu_name, target_info, lto_supported): (Symbol, LockedTargetInfo, bool), + cgu_name: Symbol, + target_info: LockedTargetInfo, + lto_supported: bool, ) -> ModuleCodegen { let cgu = tcx.codegen_unit(cgu_name); // Instantiate monomorphizations without filling out definitions yet... diff --git a/compiler/rustc_codegen_llvm/src/base.rs b/compiler/rustc_codegen_llvm/src/base.rs index d00e70638b45a..f87571700e2f2 100644 --- a/compiler/rustc_codegen_llvm/src/base.rs +++ b/compiler/rustc_codegen_llvm/src/base.rs @@ -65,8 +65,7 @@ pub(crate) fn compile_codegen_unit( let (module, _) = tcx.dep_graph.with_task( dep_node, tcx, - cgu_name, - module_codegen, + || module_codegen(tcx, cgu_name), Some(dep_graph::hash_result), ); let time_to_codegen = start_time.elapsed(); diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index fbc7232f3a27d..77083d00527f8 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -2474,8 +2474,7 @@ pub fn encode_metadata(tcx: TyCtxt<'_>, path: &Path, ref_path: Option<&Path>) { tcx.dep_graph.with_task( dep_node, tcx, - path, - |tcx, path| { + || { with_encode_metadata_header(tcx, path, |ecx| { // Encode all the entries and extra information in the crate, // culminating in the `CrateRoot` which points to all of it. diff --git a/compiler/rustc_middle/src/dep_graph/graph.rs b/compiler/rustc_middle/src/dep_graph/graph.rs index d410d9a48cd10..3530f18974c3d 100644 --- a/compiler/rustc_middle/src/dep_graph/graph.rs +++ b/compiler/rustc_middle/src/dep_graph/graph.rs @@ -52,6 +52,7 @@ pub enum QuerySideEffect { /// the side effect dep node as a dependency. CheckFeature { symbol: Symbol }, } + #[derive(Clone)] pub struct DepGraph { data: Option>, @@ -215,11 +216,11 @@ impl DepGraph { } } - pub fn with_ignore(&self, op: OP) -> R + pub fn with_ignore(&self, f: F) -> R where - OP: FnOnce() -> R, + F: FnOnce() -> R, { - with_deps(TaskDepsRef::Ignore, op) + with_deps(TaskDepsRef::Ignore, f) } /// Used to wrap the deserialization of a query result from disk, @@ -268,87 +269,72 @@ impl DepGraph { /// in the query infrastructure, and is not currently needed by the /// decoding of any query results. Should the need arise in the future, /// we should consider extending the query system with this functionality. - pub fn with_query_deserialization(&self, op: OP) -> R + pub fn with_query_deserialization(&self, f: F) -> R where - OP: FnOnce() -> R, + F: FnOnce() -> R, { - with_deps(TaskDepsRef::Forbid, op) + with_deps(TaskDepsRef::Forbid, f) } #[inline(always)] - pub fn with_task<'tcx, A: Debug, R>( + pub fn with_task<'tcx, F, R>( &self, dep_node: DepNode, tcx: TyCtxt<'tcx>, - task_arg: A, - task_fn: fn(tcx: TyCtxt<'tcx>, task_arg: A) -> R, + f: F, hash_result: Option, &R) -> Fingerprint>, - ) -> (R, DepNodeIndex) { + ) -> (R, DepNodeIndex) + where + F: FnOnce() -> R, + { match self.data() { - Some(data) => data.with_task(dep_node, tcx, task_arg, task_fn, hash_result), - None => (task_fn(tcx, task_arg), self.next_virtual_depnode_index()), + Some(data) => data.with_task(dep_node, tcx, f, hash_result), + None => (f(), self.next_virtual_depnode_index()), } } - pub fn with_anon_task<'tcx, OP, R>( + pub fn with_anon_task<'tcx, F, R>( &self, tcx: TyCtxt<'tcx>, dep_kind: DepKind, - op: OP, + f: F, ) -> (R, DepNodeIndex) where - OP: FnOnce() -> R, + F: FnOnce() -> R, { match self.data() { Some(data) => { - let (result, index) = data.with_anon_task_inner(tcx, dep_kind, op); + let (result, index) = data.with_anon_task_inner(tcx, dep_kind, f); self.read_index(index); (result, index) } - None => (op(), self.next_virtual_depnode_index()), + None => (f(), self.next_virtual_depnode_index()), } } } impl DepGraphData { - /// Starts a new dep-graph task. Dep-graph tasks are specified - /// using a free function (`task`) and **not** a closure -- this - /// is intentional because we want to exercise tight control over - /// what state they have access to. In particular, we want to - /// prevent implicit 'leaks' of tracked state into the task (which - /// could then be read without generating correct edges in the - /// dep-graph -- see the [rustc dev guide] for more details on - /// the dep-graph). - /// - /// Therefore, the task function takes a `TyCtxt`, plus exactly one - /// additional argument, `task_arg`. The additional argument type can be - /// `()` if no argument is needed, or a tuple if multiple arguments are - /// needed. - /// - /// [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/queries/incremental-compilation.html #[inline(always)] - pub fn with_task<'tcx, A: Debug, R>( + pub fn with_task<'tcx, F, R>( &self, dep_node: DepNode, tcx: TyCtxt<'tcx>, - task_arg: A, - task_fn: fn(tcx: TyCtxt<'tcx>, task_arg: A) -> R, + f: F, hash_result: Option, &R) -> Fingerprint>, - ) -> (R, DepNodeIndex) { + ) -> (R, DepNodeIndex) + where + F: FnOnce() -> R, + { // If the following assertion triggers, it can have two reasons: // 1. Something is wrong with DepNode creation, either here or // in `DepGraph::try_mark_green()`. // 2. Two distinct query keys get mapped to the same `DepNode` // (see for example #48923). self.assert_dep_node_not_yet_allocated_in_current_session(tcx.sess, &dep_node, || { - format!( - "forcing query with already existing `DepNode`\n\ - - query-key: {task_arg:?}\n\ - - dep-node: {dep_node:?}" - ) + format!("forcing query with already existing `DepNode`: {dep_node:?}") }); - let with_deps = |task_deps| with_deps(task_deps, || task_fn(tcx, task_arg)); + let with_deps = |task_deps| with_deps(task_deps, f); let (result, edges) = if tcx.is_eval_always(dep_node.kind) { (with_deps(TaskDepsRef::EvalAlways), EdgesVec::new()) } else { @@ -377,14 +363,14 @@ impl DepGraphData { /// FIXME: This could perhaps return a `WithDepNode` to ensure that the /// user of this function actually performs the read; we'll have to see /// how to make that work with `anon` in `execute_job_incr`, though. - pub fn with_anon_task_inner<'tcx, OP, R>( + pub fn with_anon_task_inner<'tcx, F, R>( &self, tcx: TyCtxt<'tcx>, dep_kind: DepKind, - op: OP, + f: F, ) -> (R, DepNodeIndex) where - OP: FnOnce() -> R, + F: FnOnce() -> R, { debug_assert!(!tcx.is_eval_always(dep_kind)); @@ -395,7 +381,7 @@ impl DepGraphData { None, 128, )); - let result = with_deps(TaskDepsRef::Allow(&task_deps), op); + let result = with_deps(TaskDepsRef::Allow(&task_deps), f); let task_deps = task_deps.into_inner(); let reads = task_deps.reads; diff --git a/compiler/rustc_query_impl/src/execution.rs b/compiler/rustc_query_impl/src/execution.rs index 1224ec0706cf0..5a3249f65f538 100644 --- a/compiler/rustc_query_impl/src/execution.rs +++ b/compiler/rustc_query_impl/src/execution.rs @@ -465,8 +465,7 @@ fn execute_job_incr<'tcx, C: QueryCache>( dep_graph_data.with_task( dep_node, tcx, - (query, key), - |tcx, (query, key)| (query.invoke_provider_fn)(tcx, key), + || (query.invoke_provider_fn)(tcx, key), query.hash_value_fn, ) }); From b9f081584a6c94fbe8ffc7fe8ed523fa1453bef4 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 18 Mar 2026 08:30:31 +1100 Subject: [PATCH 2/2] Use APITs for closure arguments in `dep_graph.rs`. As requested by reviewers. --- compiler/rustc_middle/src/dep_graph/graph.rs | 55 +++++++------------- 1 file changed, 19 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_middle/src/dep_graph/graph.rs b/compiler/rustc_middle/src/dep_graph/graph.rs index 3530f18974c3d..36c2a157e4a13 100644 --- a/compiler/rustc_middle/src/dep_graph/graph.rs +++ b/compiler/rustc_middle/src/dep_graph/graph.rs @@ -216,10 +216,7 @@ impl DepGraph { } } - pub fn with_ignore(&self, f: F) -> R - where - F: FnOnce() -> R, - { + pub fn with_ignore(&self, f: impl FnOnce() -> R) -> R { with_deps(TaskDepsRef::Ignore, f) } @@ -269,39 +266,30 @@ impl DepGraph { /// in the query infrastructure, and is not currently needed by the /// decoding of any query results. Should the need arise in the future, /// we should consider extending the query system with this functionality. - pub fn with_query_deserialization(&self, f: F) -> R - where - F: FnOnce() -> R, - { + pub fn with_query_deserialization(&self, f: impl FnOnce() -> R) -> R { with_deps(TaskDepsRef::Forbid, f) } #[inline(always)] - pub fn with_task<'tcx, F, R>( + pub fn with_task<'tcx, R>( &self, dep_node: DepNode, tcx: TyCtxt<'tcx>, - f: F, + f: impl FnOnce() -> R, hash_result: Option, &R) -> Fingerprint>, - ) -> (R, DepNodeIndex) - where - F: FnOnce() -> R, - { + ) -> (R, DepNodeIndex) { match self.data() { Some(data) => data.with_task(dep_node, tcx, f, hash_result), None => (f(), self.next_virtual_depnode_index()), } } - pub fn with_anon_task<'tcx, F, R>( + pub fn with_anon_task<'tcx, R>( &self, tcx: TyCtxt<'tcx>, dep_kind: DepKind, - f: F, - ) -> (R, DepNodeIndex) - where - F: FnOnce() -> R, - { + f: impl FnOnce() -> R, + ) -> (R, DepNodeIndex) { match self.data() { Some(data) => { let (result, index) = data.with_anon_task_inner(tcx, dep_kind, f); @@ -315,16 +303,13 @@ impl DepGraph { impl DepGraphData { #[inline(always)] - pub fn with_task<'tcx, F, R>( + pub fn with_task<'tcx, R>( &self, dep_node: DepNode, tcx: TyCtxt<'tcx>, - f: F, + f: impl FnOnce() -> R, hash_result: Option, &R) -> Fingerprint>, - ) -> (R, DepNodeIndex) - where - F: FnOnce() -> R, - { + ) -> (R, DepNodeIndex) { // If the following assertion triggers, it can have two reasons: // 1. Something is wrong with DepNode creation, either here or // in `DepGraph::try_mark_green()`. @@ -363,15 +348,12 @@ impl DepGraphData { /// FIXME: This could perhaps return a `WithDepNode` to ensure that the /// user of this function actually performs the read; we'll have to see /// how to make that work with `anon` in `execute_job_incr`, though. - pub fn with_anon_task_inner<'tcx, F, R>( + pub fn with_anon_task_inner<'tcx, R>( &self, tcx: TyCtxt<'tcx>, dep_kind: DepKind, - f: F, - ) -> (R, DepNodeIndex) - where - F: FnOnce() -> R, - { + f: impl FnOnce() -> R, + ) -> (R, DepNodeIndex) { debug_assert!(!tcx.is_eval_always(dep_kind)); // Large numbers of reads are common enough here that pre-sizing `read_set` @@ -832,10 +814,11 @@ impl DepGraph { #[cfg(debug_assertions)] #[inline(always)] - pub(crate) fn register_dep_node_debug_str(&self, dep_node: DepNode, debug_str_gen: F) - where - F: FnOnce() -> String, - { + pub(crate) fn register_dep_node_debug_str( + &self, + dep_node: DepNode, + debug_str_gen: impl FnOnce() -> String, + ) { // Early queries (e.g., `-Z query-dep-graph` on empty crates) can reach here // before the graph is initialized. Return early to prevent an ICE. let data = match &self.data {