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

Simplification of query forcing

Extracted from #78780

Author: bors

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 {
-                        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
+                    force_query::<queries::$name<'_>, _>(tcx, dep_node)
                 }
 
                 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);
-
-            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;
-                    }
-                }
-            }
+            self.try_mark_parent_green(tcx, data, dep_dep_node_index, dep_node)?
         }
 
         // 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>>;