diff options
| -rw-r--r-- | compiler/rustc_query_system/src/dep_graph/graph.rs | 58 | ||||
| -rw-r--r-- | compiler/rustc_query_system/src/query/plumbing.rs | 15 |
2 files changed, 64 insertions, 9 deletions
diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index 64dcc6f37f1..630c76764c7 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -171,6 +171,57 @@ impl<K: DepKind> DepGraph<K> { K::with_deps(None, op) } + /// Used to wrap the deserialization of a query result from disk, + /// This method enforces that no new `DepNodes` are created during + /// query result deserialization. + /// + /// Enforcing this makes the query dep graph simpler - all nodes + /// must be created during the query execution, and should be + /// created from inside the 'body' of a query (the implementation + /// provided by a particular compiler crate). + /// + /// Consider the case of three queries `A`, `B`, and `C`, where + /// `A` invokes `B` and `B` invokes `C`: + /// + /// `A -> B -> C` + /// + /// Suppose that decoding the result of query `B` required invoking + /// a query `D`. If we did not create a fresh `TaskDeps` when + /// decoding `B`, we might would still be using the `TaskDeps` for query `A` + /// (if we needed to re-execute `A`). This would cause us to create + /// a new edge `A -> D`. If this edge did not previously + /// exist in the `DepGraph`, then we could end up with a different + /// `DepGraph` at the end of compilation, even if there were no + /// meaningful changes to the overall program (e.g. a newline was added). + /// In addition, this edge might cause a subsequent compilation run + /// to try to force `D` before marking other necessary nodes green. If + /// `D` did not exist in the new compilation session, then we might + /// get an ICE. Normally, we would have tried (and failed) to mark + /// some other query green (e.g. `item_children`) which was used + /// to obtain `D`, which would prevent us from ever trying to force + /// a non-existent `D`. + /// + /// It might be possible to enforce that all `DepNode`s read during + /// deserialization already exist in the previous `DepGraph`. In + /// the above example, we would invoke `D` during the deserialization + /// of `B`. Since we correctly create a new `TaskDeps` from the decoding + /// of `B`, this would result in an edge `B -> D`. If that edge already + /// existed (with the same `DepPathHash`es), then it should be correct + /// to allow the invocation of the query to proceed during deserialization + /// of a query result. However, this would require additional complexity + /// 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<OP, R>(&self, op: OP) -> R + where + OP: FnOnce() -> R, + { + let mut deps = TaskDeps::default(); + deps.read_allowed = false; + let deps = Lock::new(deps); + K::with_deps(Some(&deps), op) + } + /// 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 @@ -1121,7 +1172,12 @@ pub struct TaskDeps<K> { reads: EdgesVec, read_set: FxHashSet<DepNodeIndex>, phantom_data: PhantomData<DepNode<K>>, - pub read_allowed: bool, + /// Whether or not we allow `DepGraph::read_index` to run. + /// This is normally true, except inside `with_query_deserialization`, + /// where it set to `false` to enforce that no new `DepNode` edges are + /// created. See the documentation of `with_query_deserialization` for + /// more details. + read_allowed: bool, } impl<K> Default for TaskDeps<K> { diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index cd31c5b3f08..192da6735fc 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -2,8 +2,7 @@ //! generate the actual methods on tcx which find and execute the provider, //! manage the caches, and so forth. -use crate::dep_graph::DepKind; -use crate::dep_graph::{DepContext, DepNode, DepNodeIndex, DepNodeParams, TaskDeps}; +use crate::dep_graph::{DepContext, DepNode, DepNodeIndex, DepNodeParams}; use crate::query::caches::QueryCache; use crate::query::config::{QueryDescription, QueryVtable}; use crate::query::job::{ @@ -516,12 +515,12 @@ where if query.cache_on_disk { let prof_timer = tcx.dep_context().profiler().incr_cache_loading(); - let mut deps = TaskDeps::default(); - deps.read_allowed = false; - let deps = Lock::new(deps); - let result = CTX::DepKind::with_deps(Some(&deps), || { - query.try_load_from_disk(tcx, prev_dep_node_index) - }); + // The call to `with_query_deserialization` enforces that no new `DepNodes` + // are created during deserialization. See the docs of that method for more + // details. + let result = dep_graph + .with_query_deserialization(|| query.try_load_from_disk(tcx, prev_dep_node_index)); + prof_timer.finish_with_query_invocation_id(dep_node_index.into()); if let Some(result) = result { |