Auto merge of #85319 - cjgillot:query-simp, r=Mark-Simulacrum

Simplification of query forcing

Extracted from #78780

compiler/rustc_query_impl/src/lib.rs

@@ -26,7 +26,7 @@ use rustc_middle::ty::query::{query_keys, query_storage, query_stored, query_val
 use rustc_middle::ty::query::{Providers, QueryEngine};
 use rustc_middle::ty::{self, TyCtxt};
 use rustc_serialize::opaque;
-use rustc_span::{Span, DUMMY_SP};
+use rustc_span::Span;
 
 #[macro_use]
 mod plumbing;

compiler/rustc_query_impl/src/plumbing.rs

@@ -14,7 +14,7 @@ use rustc_data_structures::sync::Lock;
 use rustc_data_structures::thin_vec::ThinVec;
 use rustc_errors::Diagnostic;
 use rustc_serialize::opaque;
-use rustc_span::def_id::{DefId, LocalDefId};
+use rustc_span::def_id::LocalDefId;
 
 #[derive(Copy, Clone)]
 pub struct QueryCtxt<'tcx> {
@@ -25,6 +25,7 @@ pub struct QueryCtxt<'tcx> {
 impl<'tcx> std::ops::Deref for QueryCtxt<'tcx> {
     type Target = TyCtxt<'tcx>;
 
+    #[inline]
     fn deref(&self) -> &Self::Target {
         &self.tcx
     }
@@ -42,10 +43,6 @@ impl HasDepContext for QueryCtxt<'tcx> {
 }
 
 impl QueryContext for QueryCtxt<'tcx> {
-    fn def_path_str(&self, def_id: DefId) -> String {
-        self.tcx.def_path_str(def_id)
-    }
-
     fn current_query_job(&self) -> Option<QueryJobId<Self::DepKind>> {
         tls::with_related_context(**self, |icx| icx.query)
     }
@@ -457,20 +454,7 @@ macro_rules! define_queries {
                 }
 
                 fn force_from_dep_node(tcx: QueryCtxt<'_>, dep_node: &DepNode) -> bool {
-                    if is_anon {
+                    force_query::<queries::$name<'_>, _>(tcx, dep_node)
-                        return false;
-                    }
-
-                    if !can_reconstruct_query_key() {
-                        return false;
-                    }
-
-                    if let Some(key) = recover(*tcx, dep_node) {
-                        force_query::<queries::$name<'_>, _>(tcx, key, DUMMY_SP, *dep_node);
-                        return true;
-                    }
-
-                    false
                 }
 
                 fn try_load_from_on_disk_cache(tcx: QueryCtxt<'_>, dep_node: &DepNode) {

compiler/rustc_query_system/src/dep_graph/graph.rs

@@ -487,6 +487,117 @@ impl<K: DepKind> DepGraph<K> {
         }
     }
 
+    fn try_mark_parent_green<Ctxt: QueryContext<DepKind = K>>(
+        &self,
+        tcx: Ctxt,
+        data: &DepGraphData<K>,
+        parent_dep_node_index: SerializedDepNodeIndex,
+        dep_node: &DepNode<K>,
+    ) -> Option<()> {
+        let dep_dep_node_color = data.colors.get(parent_dep_node_index);
+        let dep_dep_node = &data.previous.index_to_node(parent_dep_node_index);
+
+        match dep_dep_node_color {
+            Some(DepNodeColor::Green(_)) => {
+                // This dependency has been marked as green before, we are
+                // still fine and can continue with checking the other
+                // dependencies.
+                debug!(
+                    "try_mark_previous_green({:?}) --- found dependency {:?} to \
+                            be immediately green",
+                    dep_node, dep_dep_node,
+                );
+                return Some(());
+            }
+            Some(DepNodeColor::Red) => {
+                // We found a dependency the value of which has changed
+                // compared to the previous compilation session. We cannot
+                // mark the DepNode as green and also don't need to bother
+                // with checking any of the other dependencies.
+                debug!(
+                    "try_mark_previous_green({:?}) - END - dependency {:?} was immediately red",
+                    dep_node, dep_dep_node,
+                );
+                return None;
+            }
+            None => {}
+        }
+
+        // We don't know the state of this dependency. If it isn't
+        // an eval_always node, let's try to mark it green recursively.
+        if !dep_dep_node.kind.is_eval_always() {
+            debug!(
+                "try_mark_previous_green({:?}) --- state of dependency {:?} ({}) \
+                                 is unknown, trying to mark it green",
+                dep_node, dep_dep_node, dep_dep_node.hash,
+            );
+
+            let node_index =
+                self.try_mark_previous_green(tcx, data, parent_dep_node_index, dep_dep_node);
+            if node_index.is_some() {
+                debug!(
+                    "try_mark_previous_green({:?}) --- managed to MARK dependency {:?} as green",
+                    dep_node, dep_dep_node
+                );
+                return Some(());
+            }
+        }
+
+        // We failed to mark it green, so we try to force the query.
+        debug!(
+            "try_mark_previous_green({:?}) --- trying to force dependency {:?}",
+            dep_node, dep_dep_node
+        );
+        if !tcx.try_force_from_dep_node(dep_dep_node) {
+            // The DepNode could not be forced.
+            debug!(
+                "try_mark_previous_green({:?}) - END - dependency {:?} could not be forced",
+                dep_node, dep_dep_node
+            );
+            return None;
+        }
+
+        let dep_dep_node_color = data.colors.get(parent_dep_node_index);
+
+        match dep_dep_node_color {
+            Some(DepNodeColor::Green(_)) => {
+                debug!(
+                    "try_mark_previous_green({:?}) --- managed to FORCE dependency {:?} to green",
+                    dep_node, dep_dep_node
+                );
+                return Some(());
+            }
+            Some(DepNodeColor::Red) => {
+                debug!(
+                    "try_mark_previous_green({:?}) - END - dependency {:?} was red after forcing",
+                    dep_node, dep_dep_node
+                );
+                return None;
+            }
+            None => {}
+        }
+
+        if !tcx.dep_context().sess().has_errors_or_delayed_span_bugs() {
+            panic!("try_mark_previous_green() - Forcing the DepNode should have set its color")
+        }
+
+        // If the query we just forced has resulted in
+        // some kind of compilation error, we cannot rely on
+        // the dep-node color having been properly updated.
+        // This means that the query system has reached an
+        // invalid state. We let the compiler continue (by
+        // returning `None`) so it can emit error messages
+        // and wind down, but rely on the fact that this
+        // invalid state will not be persisted to the
+        // incremental compilation cache because of
+        // compilation errors being present.
+        debug!(
+            "try_mark_previous_green({:?}) - END - dependency {:?} resulted in compilation error",
+            dep_node, dep_dep_node
+        );
+        return None;
+    }
+
     /// Try to mark a dep-node which existed in the previous compilation session as green.
     fn try_mark_previous_green<Ctxt: QueryContext<DepKind = K>>(
         &self,
@@ -511,123 +622,7 @@ impl<K: DepKind> DepGraph<K> {
         let prev_deps = data.previous.edge_targets_from(prev_dep_node_index);
 
         for &dep_dep_node_index in prev_deps {
-            let dep_dep_node_color = data.colors.get(dep_dep_node_index);
+            self.try_mark_parent_green(tcx, data, dep_dep_node_index, dep_node)?
-
-            match dep_dep_node_color {
-                Some(DepNodeColor::Green(_)) => {
-                    // This dependency has been marked as green before, we are
-                    // still fine and can continue with checking the other
-                    // dependencies.
-                    debug!(
-                        "try_mark_previous_green({:?}) --- found dependency {:?} to \
-                            be immediately green",
-                        dep_node,
-                        data.previous.index_to_node(dep_dep_node_index)
-                    );
-                }
-                Some(DepNodeColor::Red) => {
-                    // We found a dependency the value of which has changed
-                    // compared to the previous compilation session. We cannot
-                    // mark the DepNode as green and also don't need to bother
-                    // with checking any of the other dependencies.
-                    debug!(
-                        "try_mark_previous_green({:?}) - END - dependency {:?} was \
-                            immediately red",
-                        dep_node,
-                        data.previous.index_to_node(dep_dep_node_index)
-                    );
-                    return None;
-                }
-                None => {
-                    let dep_dep_node = &data.previous.index_to_node(dep_dep_node_index);
-
-                    // We don't know the state of this dependency. If it isn't
-                    // an eval_always node, let's try to mark it green recursively.
-                    if !dep_dep_node.kind.is_eval_always() {
-                        debug!(
-                            "try_mark_previous_green({:?}) --- state of dependency {:?} ({}) \
-                                 is unknown, trying to mark it green",
-                            dep_node, dep_dep_node, dep_dep_node.hash,
-                        );
-
-                        let node_index = self.try_mark_previous_green(
-                            tcx,
-                            data,
-                            dep_dep_node_index,
-                            dep_dep_node,
-                        );
-                        if node_index.is_some() {
-                            debug!(
-                                "try_mark_previous_green({:?}) --- managed to MARK \
-                                    dependency {:?} as green",
-                                dep_node, dep_dep_node
-                            );
-                            continue;
-                        }
-                    }
-
-                    // We failed to mark it green, so we try to force the query.
-                    debug!(
-                        "try_mark_previous_green({:?}) --- trying to force \
-                            dependency {:?}",
-                        dep_node, dep_dep_node
-                    );
-                    if tcx.try_force_from_dep_node(dep_dep_node) {
-                        let dep_dep_node_color = data.colors.get(dep_dep_node_index);
-
-                        match dep_dep_node_color {
-                            Some(DepNodeColor::Green(_)) => {
-                                debug!(
-                                    "try_mark_previous_green({:?}) --- managed to \
-                                        FORCE dependency {:?} to green",
-                                    dep_node, dep_dep_node
-                                );
-                            }
-                            Some(DepNodeColor::Red) => {
-                                debug!(
-                                    "try_mark_previous_green({:?}) - END - \
-                                        dependency {:?} was red after forcing",
-                                    dep_node, dep_dep_node
-                                );
-                                return None;
-                            }
-                            None => {
-                                if !tcx.dep_context().sess().has_errors_or_delayed_span_bugs() {
-                                    panic!(
-                                        "try_mark_previous_green() - Forcing the DepNode \
-                                          should have set its color"
-                                    )
-                                } else {
-                                    // If the query we just forced has resulted in
-                                    // some kind of compilation error, we cannot rely on
-                                    // the dep-node color having been properly updated.
-                                    // This means that the query system has reached an
-                                    // invalid state. We let the compiler continue (by
-                                    // returning `None`) so it can emit error messages
-                                    // and wind down, but rely on the fact that this
-                                    // invalid state will not be persisted to the
-                                    // incremental compilation cache because of
-                                    // compilation errors being present.
-                                    debug!(
-                                        "try_mark_previous_green({:?}) - END - \
-                                            dependency {:?} resulted in compilation error",
-                                        dep_node, dep_dep_node
-                                    );
-                                    return None;
-                                }
-                            }
-                        }
-                    } else {
-                        // The DepNode could not be forced.
-                        debug!(
-                            "try_mark_previous_green({:?}) - END - dependency {:?} \
-                                could not be forced",
-                            dep_node, dep_dep_node
-                        );
-                        return None;
-                    }
-                }
-            }
         }
 
         // If we got here without hitting a `return` that means that all
@@ -796,7 +791,7 @@ impl<K: DepKind> DepGraph<K> {
         }
     }
 
-    fn next_virtual_depnode_index(&self) -> DepNodeIndex {
+    pub(crate) fn next_virtual_depnode_index(&self) -> DepNodeIndex {
         let index = self.virtual_dep_node_index.fetch_add(1, Relaxed);
         DepNodeIndex::from_u32(index)
     }

compiler/rustc_query_system/src/query/mod.rs

@@ -19,7 +19,6 @@ use crate::dep_graph::{DepNode, DepNodeIndex, HasDepContext, SerializedDepNodeIn
 use rustc_data_structures::sync::Lock;
 use rustc_data_structures::thin_vec::ThinVec;
 use rustc_errors::Diagnostic;
-use rustc_span::def_id::DefId;
 use rustc_span::Span;
 
 /// Description of a frame in the query stack.
@@ -64,9 +63,6 @@ impl QueryStackFrame {
 }
 
 pub trait QueryContext: HasDepContext {
-    /// Get string representation from DefPath.
-    fn def_path_str(&self, def_id: DefId) -> String;
-
     /// Get the query information from the TLS context.
     fn current_query_job(&self) -> Option<QueryJobId<Self::DepKind>>;
 

compiler/rustc_query_system/src/query/plumbing.rs

@@ -2,7 +2,7 @@
 //! generate the actual methods on tcx which find and execute the provider,
 //! manage the caches, and so forth.
 
-use crate::dep_graph::{DepContext, DepKind, DepNode};
+use crate::dep_graph::{DepContext, DepKind, DepNode, DepNodeParams};
 use crate::dep_graph::{DepNodeIndex, SerializedDepNodeIndex};
 use crate::query::caches::QueryCache;
 use crate::query::config::{QueryDescription, QueryVtable, QueryVtableExt};
@@ -19,7 +19,7 @@ use rustc_data_structures::thin_vec::ThinVec;
 #[cfg(not(parallel_compiler))]
 use rustc_errors::DiagnosticBuilder;
 use rustc_errors::{Diagnostic, FatalError};
-use rustc_span::Span;
+use rustc_span::{Span, DUMMY_SP};
 use std::collections::hash_map::Entry;
 use std::fmt::Debug;
 use std::hash::{Hash, Hasher};
@@ -431,7 +431,7 @@ fn try_execute_query<CTX, C>(
 ) -> C::Stored
 where
     C: QueryCache,
-    C::Key: crate::dep_graph::DepNodeParams<CTX::DepContext>,
+    C::Key: DepNodeParams<CTX::DepContext>,
     CTX: QueryContext,
 {
     let job = match JobOwner::<'_, CTX::DepKind, C>::try_start(
@@ -452,11 +452,15 @@ where
         }
     };
 
-    // Fast path for when incr. comp. is off. `to_dep_node` is
+    let dep_graph = tcx.dep_context().dep_graph();
-    // expensive for some `DepKind`s.
+
-    if !tcx.dep_context().dep_graph().is_fully_enabled() {
+    // Fast path for when incr. comp. is off.
-        let null_dep_node = DepNode::new_no_params(DepKind::NULL);
+    if !dep_graph.is_fully_enabled() {
-        return force_query_with_job(tcx, key, job, null_dep_node, query).0;
+        let prof_timer = tcx.dep_context().profiler().query_provider();
+        let result = tcx.start_query(job.id, None, || query.compute(tcx, key));
+        let dep_node_index = dep_graph.next_virtual_depnode_index();
+        prof_timer.finish_with_query_invocation_id(dep_node_index.into());
+        return job.complete(result, dep_node_index);
     }
 
     if query.anon {
@@ -464,17 +468,14 @@ where
 
         let ((result, dep_node_index), diagnostics) = with_diagnostics(|diagnostics| {
             tcx.start_query(job.id, diagnostics, || {
-                tcx.dep_context().dep_graph().with_anon_task(
+                dep_graph
-                    *tcx.dep_context(),
+                    .with_anon_task(*tcx.dep_context(), query.dep_kind, || query.compute(tcx, key))
-                    query.dep_kind,
-                    || query.compute(tcx, key),
-                )
             })
         });
 
         prof_timer.finish_with_query_invocation_id(dep_node_index.into());
 
-        tcx.dep_context().dep_graph().read_index(dep_node_index);
+        dep_graph.read_index(dep_node_index);
 
         if unlikely!(!diagnostics.is_empty()) {
             tcx.store_diagnostics_for_anon_node(dep_node_index, diagnostics);
@@ -490,7 +491,7 @@ where
         // promoted to the current session during
         // `try_mark_green()`, so we can ignore them here.
         let loaded = tcx.start_query(job.id, None, || {
-            let marked = tcx.dep_context().dep_graph().try_mark_green_and_read(tcx, &dep_node);
+            let marked = dep_graph.try_mark_green_and_read(tcx, &dep_node);
             marked.map(|(prev_dep_node_index, dep_node_index)| {
                 (
                     load_from_disk_and_cache_in_memory(
@@ -511,7 +512,7 @@ where
     }
 
     let (result, dep_node_index) = force_query_with_job(tcx, key, job, dep_node, query);
-    tcx.dep_context().dep_graph().read_index(dep_node_index);
+    dep_graph.read_index(dep_node_index);
     result
 }
 
@@ -693,7 +694,7 @@ fn get_query_impl<CTX, C>(
 where
     CTX: QueryContext,
     C: QueryCache,
-    C::Key: crate::dep_graph::DepNodeParams<CTX::DepContext>,
+    C::Key: DepNodeParams<CTX::DepContext>,
 {
     try_execute_query(tcx, state, cache, span, key, lookup, query)
 }
@@ -743,15 +744,28 @@ fn force_query_impl<CTX, C>(
     tcx: CTX,
     state: &QueryState<CTX::DepKind, C::Key>,
     cache: &QueryCacheStore<C>,
-    key: C::Key,
-    span: Span,
     dep_node: DepNode<CTX::DepKind>,
     query: &QueryVtable<CTX, C::Key, C::Value>,
-) where
+) -> bool
+where
     C: QueryCache,
-    C::Key: crate::dep_graph::DepNodeParams<CTX::DepContext>,
+    C::Key: DepNodeParams<CTX::DepContext>,
     CTX: QueryContext,
 {
+    debug_assert!(!query.anon);
+
+    if !<C::Key as DepNodeParams<CTX::DepContext>>::can_reconstruct_query_key() {
+        return false;
+    }
+
+    let key = if let Some(key) =
+        <C::Key as DepNodeParams<CTX::DepContext>>::recover(*tcx.dep_context(), &dep_node)
+    {
+        key
+    } else {
+        return false;
+    };
+
     // We may be concurrently trying both execute and force a query.
     // Ensure that only one of them runs the query.
     let cached = cache.cache.lookup(cache, &key, |_, index| {
@@ -765,7 +779,7 @@ fn force_query_impl<CTX, C>(
     });
 
     let lookup = match cached {
-        Ok(()) => return,
+        Ok(()) => return true,
         Err(lookup) => lookup,
     };
 
@@ -773,17 +787,20 @@ fn force_query_impl<CTX, C>(
         tcx,
         state,
         cache,
-        span,
+        DUMMY_SP,
         key.clone(),
         lookup,
         query,
     ) {
         TryGetJob::NotYetStarted(job) => job,
-        TryGetJob::Cycle(_) => return,
+        TryGetJob::Cycle(_) => return true,
         #[cfg(parallel_compiler)]
-        TryGetJob::JobCompleted(_) => return,
+        TryGetJob::JobCompleted(_) => return true,
     };
+
     force_query_with_job(tcx, key, job, dep_node, query);
+
+    true
 }
 
 pub enum QueryMode {
@@ -800,7 +817,7 @@ pub fn get_query<Q, CTX>(
 ) -> Option<Q::Stored>
 where
     Q: QueryDescription<CTX>,
-    Q::Key: crate::dep_graph::DepNodeParams<CTX::DepContext>,
+    Q::Key: DepNodeParams<CTX::DepContext>,
     CTX: QueryContext,
 {
     let query = &Q::VTABLE;
@@ -816,11 +833,15 @@ where
     Some(value)
 }
 
-pub fn force_query<Q, CTX>(tcx: CTX, key: Q::Key, span: Span, dep_node: DepNode<CTX::DepKind>)
+pub fn force_query<Q, CTX>(tcx: CTX, dep_node: &DepNode<CTX::DepKind>) -> bool
 where
     Q: QueryDescription<CTX>,
-    Q::Key: crate::dep_graph::DepNodeParams<CTX::DepContext>,
+    Q::Key: DepNodeParams<CTX::DepContext>,
     CTX: QueryContext,
 {
-    force_query_impl(tcx, Q::query_state(tcx), Q::query_cache(tcx), key, span, dep_node, &Q::VTABLE)
+    if Q::ANON {
+        return false;
+    }
+
+    force_query_impl(tcx, Q::query_state(tcx), Q::query_cache(tcx), *dep_node, &Q::VTABLE)
 }