Auto merge of #89343 - Mark-Simulacrum:no-args-queries, r=cjgillot

Refactor fingerprint reconstruction

This PR replaces can_reconstruct_query_key with fingerprint_style, which returns the style of the fingerprint for that query. This allows us to avoid trying to extract a DefId (or equivalent) from keys which *are* reconstructible because they're () but not as DefIds.

This is done with the goal of fixing -Zdump-dep-graph, which seems to have broken a while ago (I didn't try to bisect). Currently even on a `fn main() {}` file it'll ICE (you need to also pass -Zquery-dep-graph for it to work at all), and this patch indirectly fixes the cause of that ICE. This also adds a test for it continuing to work.

Author: bors

compiler/rustc_middle/src/dep_graph/dep_node.rs

@@ -63,6 +63,7 @@ use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, CRATE_DEF_INDEX};
 use rustc_hir::definitions::DefPathHash;
 use rustc_hir::HirId;
+use rustc_query_system::dep_graph::FingerprintStyle;
 use rustc_span::symbol::Symbol;
 use std::hash::Hash;
 
@@ -89,9 +90,9 @@ pub struct DepKindStruct {
 
     /// Whether the query key can be recovered from the hashed fingerprint.
     /// See [DepNodeParams] trait for the behaviour of each key type.
-    // FIXME: Make this a simple boolean once DepNodeParams::can_reconstruct_query_key
+    // FIXME: Make this a simple boolean once DepNodeParams::fingerprint_style
     // can be made a specialized associated const.
-    can_reconstruct_query_key: fn() -> bool,
+    fingerprint_style: fn() -> FingerprintStyle,
 }
 
 impl std::ops::Deref for DepKind {
@@ -103,14 +104,14 @@ impl std::ops::Deref for DepKind {
 
 impl DepKind {
     #[inline(always)]
-    pub fn can_reconstruct_query_key(&self) -> bool {
+    pub fn fingerprint_style(&self) -> FingerprintStyle {
         // Only fetch the DepKindStruct once.
         let data: &DepKindStruct = &**self;
         if data.is_anon {
-            return false;
+            return FingerprintStyle::Opaque;
         }
 
-        (data.can_reconstruct_query_key)()
+        (data.fingerprint_style)()
     }
 }
 
@@ -151,38 +152,39 @@ macro_rules! contains_eval_always_attr {
 pub mod dep_kind {
     use super::*;
     use crate::ty::query::query_keys;
+    use rustc_query_system::dep_graph::FingerprintStyle;
 
     // We use this for most things when incr. comp. is turned off.
     pub const Null: DepKindStruct = DepKindStruct {
         has_params: false,
         is_anon: false,
         is_eval_always: false,
 
-        can_reconstruct_query_key: || true,
+        fingerprint_style: || FingerprintStyle::Unit,
     };
 
     pub const TraitSelect: DepKindStruct = DepKindStruct {
         has_params: false,
         is_anon: true,
         is_eval_always: false,
 
-        can_reconstruct_query_key: || true,
+        fingerprint_style: || FingerprintStyle::Unit,
     };
 
     pub const CompileCodegenUnit: DepKindStruct = DepKindStruct {
         has_params: true,
         is_anon: false,
         is_eval_always: false,
 
-        can_reconstruct_query_key: || false,
+        fingerprint_style: || FingerprintStyle::Opaque,
     };
 
     pub const CompileMonoItem: DepKindStruct = DepKindStruct {
         has_params: true,
         is_anon: false,
         is_eval_always: false,
 
-        can_reconstruct_query_key: || false,
+        fingerprint_style: || FingerprintStyle::Opaque,
     };
 
     macro_rules! define_query_dep_kinds {
@@ -196,16 +198,16 @@ pub mod dep_kind {
                 const is_eval_always: bool = contains_eval_always_attr!($($attrs)*);
 
                 #[inline(always)]
-                fn can_reconstruct_query_key() -> bool {
+                fn fingerprint_style() -> rustc_query_system::dep_graph::FingerprintStyle {
                     <query_keys::$variant<'_> as DepNodeParams<TyCtxt<'_>>>
-                        ::can_reconstruct_query_key()
+                        ::fingerprint_style()
                 }
 
                 DepKindStruct {
                     has_params,
                     is_anon,
                     is_eval_always,
-                    can_reconstruct_query_key,
+                    fingerprint_style,
                 }
             };)*
         );
@@ -320,7 +322,7 @@ impl DepNodeExt for DepNode {
     /// method will assert that the given DepKind actually requires a
     /// single DefId/DefPathHash parameter.
     fn from_def_path_hash(def_path_hash: DefPathHash, kind: DepKind) -> DepNode {
-        debug_assert!(kind.can_reconstruct_query_key() && kind.has_params);
+        debug_assert!(kind.fingerprint_style() == FingerprintStyle::DefPathHash);
         DepNode { kind, hash: def_path_hash.0.into() }
     }
 
@@ -335,7 +337,7 @@ impl DepNodeExt for DepNode {
     /// refers to something from the previous compilation session that
     /// has been removed.
     fn extract_def_id(&self, tcx: TyCtxt<'tcx>) -> Option<DefId> {
-        if self.kind.can_reconstruct_query_key() {
+        if self.kind.fingerprint_style() == FingerprintStyle::DefPathHash {
             Some(
                 tcx.on_disk_cache
                     .as_ref()?
@@ -350,14 +352,16 @@ impl DepNodeExt for DepNode {
     fn from_label_string(label: &str, def_path_hash: DefPathHash) -> Result<DepNode, ()> {
         let kind = dep_kind_from_label_string(label)?;
 
-        if !kind.can_reconstruct_query_key() {
-            return Err(());
-        }
-
-        if kind.has_params {
-            Ok(DepNode::from_def_path_hash(def_path_hash, kind))
-        } else {
-            Ok(DepNode::new_no_params(kind))
+        match kind.fingerprint_style() {
+            FingerprintStyle::Opaque => Err(()),
+            FingerprintStyle::Unit => {
+                if !kind.has_params {
+                    Ok(DepNode::new_no_params(kind))
+                } else {
+                    Err(())
+                }
+            }
+            FingerprintStyle::DefPathHash => Ok(DepNode::from_def_path_hash(def_path_hash, kind)),
         }
     }
 
@@ -369,8 +373,8 @@ impl DepNodeExt for DepNode {
 
 impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for () {
     #[inline(always)]
-    fn can_reconstruct_query_key() -> bool {
-        true
+    fn fingerprint_style() -> FingerprintStyle {
+        FingerprintStyle::Unit
     }
 
     fn to_fingerprint(&self, _: TyCtxt<'tcx>) -> Fingerprint {
@@ -384,8 +388,8 @@ impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for () {
 
 impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for DefId {
     #[inline(always)]
-    fn can_reconstruct_query_key() -> bool {
-        true
+    fn fingerprint_style() -> FingerprintStyle {
+        FingerprintStyle::DefPathHash
     }
 
     fn to_fingerprint(&self, tcx: TyCtxt<'tcx>) -> Fingerprint {
@@ -403,8 +407,8 @@ impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for DefId {
 
 impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for LocalDefId {
     #[inline(always)]
-    fn can_reconstruct_query_key() -> bool {
-        true
+    fn fingerprint_style() -> FingerprintStyle {
+        FingerprintStyle::DefPathHash
     }
 
     fn to_fingerprint(&self, tcx: TyCtxt<'tcx>) -> Fingerprint {
@@ -422,8 +426,8 @@ impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for LocalDefId {
 
 impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for CrateNum {
     #[inline(always)]
-    fn can_reconstruct_query_key() -> bool {
-        true
+    fn fingerprint_style() -> FingerprintStyle {
+        FingerprintStyle::DefPathHash
     }
 
     fn to_fingerprint(&self, tcx: TyCtxt<'tcx>) -> Fingerprint {
@@ -442,8 +446,8 @@ impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for CrateNum {
 
 impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for (DefId, DefId) {
     #[inline(always)]
-    fn can_reconstruct_query_key() -> bool {
-        false
+    fn fingerprint_style() -> FingerprintStyle {
+        FingerprintStyle::Opaque
     }
 
     // We actually would not need to specialize the implementation of this
@@ -467,8 +471,8 @@ impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for (DefId, DefId) {
 
 impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for HirId {
     #[inline(always)]
-    fn can_reconstruct_query_key() -> bool {
-        false
+    fn fingerprint_style() -> FingerprintStyle {
+        FingerprintStyle::Opaque
     }
 
     // We actually would not need to specialize the implementation of this

compiler/rustc_middle/src/dep_graph/mod.rs

@@ -25,8 +25,8 @@ impl rustc_query_system::dep_graph::DepKind for DepKind {
     const NULL: Self = DepKind::Null;
 
     #[inline(always)]
-    fn can_reconstruct_query_key(&self) -> bool {
-        DepKind::can_reconstruct_query_key(self)
+    fn fingerprint_style(&self) -> rustc_query_system::dep_graph::FingerprintStyle {
+        DepKind::fingerprint_style(self)
     }
 
     #[inline(always)]

compiler/rustc_query_impl/src/plumbing.rs

@@ -428,6 +428,7 @@ macro_rules! define_queries {
             use rustc_middle::ty::query::query_keys;
             use rustc_query_system::dep_graph::DepNodeParams;
             use rustc_query_system::query::{force_query, QueryDescription};
+            use rustc_query_system::dep_graph::FingerprintStyle;
 
             // We use this for most things when incr. comp. is turned off.
             pub const Null: QueryStruct = QueryStruct {
@@ -454,9 +455,9 @@ macro_rules! define_queries {
                 const is_anon: bool = is_anon!([$($modifiers)*]);
 
                 #[inline(always)]
-                fn can_reconstruct_query_key() -> bool {
+                fn fingerprint_style() -> FingerprintStyle {
                     <query_keys::$name<'_> as DepNodeParams<TyCtxt<'_>>>
-                        ::can_reconstruct_query_key()
+                        ::fingerprint_style()
                 }
 
                 fn recover<'tcx>(tcx: TyCtxt<'tcx>, dep_node: &DepNode) -> Option<query_keys::$name<'tcx>> {
@@ -472,7 +473,7 @@ macro_rules! define_queries {
                         return
                     }
 
-                    if !can_reconstruct_query_key() {
+                    if !fingerprint_style().reconstructible() {
                         return
                     }
 

compiler/rustc_query_system/src/dep_graph/dep_node.rs

@@ -42,7 +42,7 @@
 //!   `DefId` it was computed from. In other cases, too much information gets
 //!   lost during fingerprint computation.
 
-use super::{DepContext, DepKind};
+use super::{DepContext, DepKind, FingerprintStyle};
 use crate::ich::StableHashingContext;
 
 use rustc_data_structures::fingerprint::{Fingerprint, PackedFingerprint};
@@ -75,7 +75,7 @@ impl<K: DepKind> DepNode<K> {
 
         #[cfg(debug_assertions)]
         {
-            if !kind.can_reconstruct_query_key()
+            if !kind.fingerprint_style().reconstructible()
                 && (tcx.sess().opts.debugging_opts.incremental_info
                     || tcx.sess().opts.debugging_opts.query_dep_graph)
             {
@@ -94,7 +94,7 @@ impl<K: DepKind> fmt::Debug for DepNode<K> {
 }
 
 pub trait DepNodeParams<Ctxt: DepContext>: fmt::Debug + Sized {
-    fn can_reconstruct_query_key() -> bool;
+    fn fingerprint_style() -> FingerprintStyle;
 
     /// This method turns the parameters of a DepNodeConstructor into an opaque
     /// Fingerprint to be used in DepNode.
@@ -111,7 +111,7 @@ pub trait DepNodeParams<Ctxt: DepContext>: fmt::Debug + Sized {
     /// This method tries to recover the query key from the given `DepNode`,
     /// something which is needed when forcing `DepNode`s during red-green
     /// evaluation. The query system will only call this method if
-    /// `can_reconstruct_query_key()` is `true`.
+    /// `fingerprint_style()` is not `FingerprintStyle::Opaque`.
     /// It is always valid to return `None` here, in which case incremental
     /// compilation will treat the query as having changed instead of forcing it.
     fn recover(tcx: Ctxt, dep_node: &DepNode<Ctxt::DepKind>) -> Option<Self>;
@@ -122,8 +122,8 @@ where
     T: for<'a> HashStable<StableHashingContext<'a>> + fmt::Debug,
 {
     #[inline]
-    default fn can_reconstruct_query_key() -> bool {
-        false
+    default fn fingerprint_style() -> FingerprintStyle {
+        FingerprintStyle::Opaque
     }
 
     default fn to_fingerprint(&self, tcx: Ctxt) -> Fingerprint {

compiler/rustc_query_system/src/dep_graph/mod.rs

@@ -50,6 +50,27 @@ impl<T: DepContext> HasDepContext for T {
     }
 }
 
+/// Describes the contents of the fingerprint generated by a given query.
+#[derive(PartialEq, Eq, Copy, Clone)]
+pub enum FingerprintStyle {
+    /// The fingerprint is actually a DefPathHash.
+    DefPathHash,
+    /// Query key was `()` or equivalent, so fingerprint is just zero.
+    Unit,
+    /// Some opaque hash.
+    Opaque,
+}
+
+impl FingerprintStyle {
+    #[inline]
+    pub fn reconstructible(self) -> bool {
+        match self {
+            FingerprintStyle::DefPathHash | FingerprintStyle::Unit => true,
+            FingerprintStyle::Opaque => false,
+        }
+    }
+}
+
 /// Describe the different families of dependency nodes.
 pub trait DepKind: Copy + fmt::Debug + Eq + Hash + Send + Encodable<FileEncoder> + 'static {
     const NULL: Self;
@@ -73,5 +94,5 @@ pub trait DepKind: Copy + fmt::Debug + Eq + Hash + Send + Encodable<FileEncoder>
     where
         OP: for<'a> FnOnce(Option<&'a Lock<TaskDeps<Self>>>);
 
-    fn can_reconstruct_query_key(&self) -> bool;
+    fn fingerprint_style(&self) -> FingerprintStyle;
 }

compiler/rustc_query_system/src/query/plumbing.rs

@@ -540,7 +540,7 @@ where
         // We always expect to find a cached result for things that
         // can be forced from `DepNode`.
         debug_assert!(
-            !dep_node.kind.can_reconstruct_query_key() || result.is_some(),
+            !dep_node.kind.fingerprint_style().reconstructible() || result.is_some(),
             "missing on-disk cache entry for {:?}",
             dep_node
         );
@@ -778,7 +778,7 @@ where
         return false;
     }
 
-    if !<Q::Key as DepNodeParams<CTX::DepContext>>::can_reconstruct_query_key() {
+    if !<Q::Key as DepNodeParams<CTX::DepContext>>::fingerprint_style().reconstructible() {
         return false;
     }
 

src/test/run-make/dep-graph/Makefile

@@ -0,0 +1,12 @@
+-include ../../run-make-fulldeps/tools.mk
+
+# ignore-cross-compile
+
+# Just verify that we successfully run and produce dep graphs when requested.
+
+all:
+	RUST_DEP_GRAPH=$(TMPDIR)/dep-graph $(RUSTC) \
+        -Cincremental=$(TMPDIR)/incr \
+        -Zquery-dep-graph -Zdump-dep-graph foo.rs
+	test -f $(TMPDIR)/dep-graph.txt
+	test -f $(TMPDIR)/dep-graph.dot

src/test/run-make/dep-graph/foo.rs

@@ -0,0 +1 @@
+fn main() {}