Stable hashing cleanups

Cargo.lock

@@ -4570,7 +4570,6 @@ dependencies = [
  "rustc_macros",
  "rustc_metadata",
  "rustc_middle",
- "rustc_query_system",
  "rustc_session",
  "rustc_span",
  "smallvec",

compiler/rustc_data_structures/src/stable_hasher.rs

@@ -607,7 +607,7 @@ where
 /// result (for example, using a `Fingerprint` produced while
 /// hashing `Span`s when a `Fingerprint` without `Span`s is
 /// being requested)
-#[derive(Clone, Hash, Eq, PartialEq, Debug)]
+#[derive(Clone, Copy, Hash, Eq, PartialEq, Debug)]
 pub struct HashingControls {
     pub hash_spans: bool,
 }

compiler/rustc_hir/src/stable_hash_impls.rs

@@ -11,9 +11,7 @@ use crate::lints::DelayedLints;
 /// Requirements for a `StableHashingContext` to be used in this crate.
 /// This is a hack to allow using the `HashStable_Generic` derive macro
 /// instead of implementing everything in `rustc_middle`.
-pub trait HashStableContext: rustc_ast::HashStableContext + rustc_abi::HashStableContext {
-    fn hash_attr_id(&mut self, id: &HashIgnoredAttrId, hasher: &mut StableHasher);
-}
+pub trait HashStableContext: rustc_ast::HashStableContext + rustc_abi::HashStableContext {}
 
 impl<HirCtx: crate::HashStableContext> ToStableHashKey<HirCtx> for BodyId {
     type KeyType = (DefPathHash, ItemLocalId);
@@ -104,7 +102,7 @@ impl<HirCtx: crate::HashStableContext> HashStable<HirCtx> for Crate<'_> {
 }
 
 impl<HirCtx: crate::HashStableContext> HashStable<HirCtx> for HashIgnoredAttrId {
-    fn hash_stable(&self, hcx: &mut HirCtx, hasher: &mut StableHasher) {
-        hcx.hash_attr_id(self, hasher)
+    fn hash_stable(&self, _hcx: &mut HirCtx, _hasher: &mut StableHasher) {
+        /* we don't hash HashIgnoredAttrId, we ignore them */
     }
 }

compiler/rustc_middle/src/ty/impls_ty.rs

@@ -20,6 +20,9 @@ where
     T: HashStable<StableHashingContext<'a>>,
 {
     fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) {
+        // Note: this cache makes an *enormous* performance difference on certain benchmarks. E.g.
+        // without it, compiling `diesel-2.2.10` can be 74% slower, and compiling
+        // `deeply-nested-multi` can be ~4,000x slower(!)
         thread_local! {
             static CACHE: RefCell<FxHashMap<(*const (), HashingControls), Fingerprint>> =
                 RefCell::new(Default::default());

compiler/rustc_query_system/src/ich/hcx.rs

@@ -1,13 +1,18 @@
-use rustc_ast as ast;
-use rustc_data_structures::stable_hasher::{HashStable, HashingControls, StableHasher};
+use rustc_data_structures::stable_hasher::HashingControls;
 use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_hir::definitions::DefPathHash;
 use rustc_session::Session;
 use rustc_session::cstore::Untracked;
 use rustc_span::source_map::SourceMap;
-use rustc_span::{BytePos, CachingSourceMapView, DUMMY_SP, SourceFile, Span, SpanData, Symbol};
+use rustc_span::{BytePos, CachingSourceMapView, DUMMY_SP, SourceFile, Span, SpanData};
 
-use crate::ich;
+// Very often, we are hashing something that does not need the `CachingSourceMapView`, so we
+// initialize it lazily.
+#[derive(Clone)]
+enum CachingSourceMap<'a> {
+    Unused(&'a SourceMap),
+    InUse(CachingSourceMapView<'a>),
+}
 
 /// This is the context state available during incr. comp. hashing. It contains
 /// enough information to transform `DefId`s and `HirId`s into stable `DefPath`s (i.e.,
@@ -19,10 +24,7 @@ pub struct StableHashingContext<'a> {
     // The value of `-Z incremental-ignore-spans`.
     // This field should only be used by `unstable_opts_incremental_ignore_span`
     incremental_ignore_spans: bool,
-    // Very often, we are hashing something that does not need the
-    // `CachingSourceMapView`, so we initialize it lazily.
-    raw_source_map: &'a SourceMap,
-    caching_source_map: Option<CachingSourceMapView<'a>>,
+    caching_source_map: CachingSourceMap<'a>,
     hashing_controls: HashingControls,
 }
 
@@ -34,8 +36,7 @@ impl<'a> StableHashingContext<'a> {
         StableHashingContext {
             untracked,
             incremental_ignore_spans: sess.opts.unstable_opts.incremental_ignore_spans,
-            caching_source_map: None,
-            raw_source_map: sess.source_map(),
+            caching_source_map: CachingSourceMap::Unused(sess.source_map()),
             hashing_controls: HashingControls { hash_spans: hash_spans_initial },
         }
     }
@@ -49,62 +50,35 @@ impl<'a> StableHashingContext<'a> {
     }
 
     #[inline]
-    pub fn def_path_hash(&self, def_id: DefId) -> DefPathHash {
-        if let Some(def_id) = def_id.as_local() {
-            self.local_def_path_hash(def_id)
-        } else {
-            self.untracked.cstore.read().def_path_hash(def_id)
-        }
-    }
-
-    #[inline]
-    pub fn local_def_path_hash(&self, def_id: LocalDefId) -> DefPathHash {
-        self.untracked.definitions.read().def_path_hash(def_id)
-    }
-
-    #[inline]
-    pub fn source_map(&mut self) -> &mut CachingSourceMapView<'a> {
+    fn source_map(&mut self) -> &mut CachingSourceMapView<'a> {
         match self.caching_source_map {
-            Some(ref mut sm) => sm,
-            ref mut none => {
-                *none = Some(CachingSourceMapView::new(self.raw_source_map));
-                none.as_mut().unwrap()
+            CachingSourceMap::InUse(ref mut sm) => sm,
+            CachingSourceMap::Unused(sm) => {
+                self.caching_source_map = CachingSourceMap::InUse(CachingSourceMapView::new(sm));
+                self.source_map() // this recursive call will hit the `InUse` case
             }
         }
     }
 
-    #[inline]
-    pub fn is_ignored_attr(&self, name: Symbol) -> bool {
-        ich::IGNORED_ATTRIBUTES.contains(&name)
-    }
-
     #[inline]
     pub fn hashing_controls(&self) -> HashingControls {
-        self.hashing_controls.clone()
-    }
-}
-
-impl<'a> HashStable<StableHashingContext<'a>> for ast::NodeId {
-    #[inline]
-    fn hash_stable(&self, _: &mut StableHashingContext<'a>, _: &mut StableHasher) {
-        panic!("Node IDs should not appear in incremental state");
+        self.hashing_controls
     }
 }
 
 impl<'a> rustc_span::HashStableContext for StableHashingContext<'a> {
-    #[inline]
-    fn hash_spans(&self) -> bool {
-        self.hashing_controls.hash_spans
-    }
-
     #[inline]
     fn unstable_opts_incremental_ignore_spans(&self) -> bool {
         self.incremental_ignore_spans
     }
 
     #[inline]
     fn def_path_hash(&self, def_id: DefId) -> DefPathHash {
-        self.def_path_hash(def_id)
+        if let Some(def_id) = def_id.as_local() {
+            self.untracked.definitions.read().def_path_hash(def_id)
+        } else {
+            self.untracked.cstore.read().def_path_hash(def_id)
+        }
     }
 
     #[inline]
@@ -122,8 +96,11 @@ impl<'a> rustc_span::HashStableContext for StableHashingContext<'a> {
 
     #[inline]
     fn hashing_controls(&self) -> HashingControls {
-        self.hashing_controls.clone()
+        self.hashing_controls
     }
 }
 
+impl<'a> rustc_abi::HashStableContext for StableHashingContext<'a> {}
+impl<'a> rustc_ast::HashStableContext for StableHashingContext<'a> {}
+impl<'a> rustc_hir::HashStableContext for StableHashingContext<'a> {}
 impl<'a> rustc_session::HashStableContext for StableHashingContext<'a> {}

compiler/rustc_query_system/src/ich/impls_syntax.rs

@@ -2,14 +2,18 @@
 //! from various crates in no particular order.
 
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
-use rustc_hir::{self as hir, HashIgnoredAttrId};
-use rustc_span::SourceFile;
+use rustc_span::{SourceFile, Symbol, sym};
 use smallvec::SmallVec;
+use {rustc_ast as ast, rustc_hir as hir};
 
 use crate::ich::StableHashingContext;
 
-impl<'ctx> rustc_abi::HashStableContext for StableHashingContext<'ctx> {}
-impl<'ctx> rustc_ast::HashStableContext for StableHashingContext<'ctx> {}
+impl<'a> HashStable<StableHashingContext<'a>> for ast::NodeId {
+    #[inline]
+    fn hash_stable(&self, _: &mut StableHashingContext<'a>, _: &mut StableHasher) {
+        panic!("Node IDs should not appear in incremental state");
+    }
+}
 
 impl<'a> HashStable<StableHashingContext<'a>> for [hir::Attribute] {
     fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) {
@@ -24,7 +28,7 @@ impl<'a> HashStable<StableHashingContext<'a>> for [hir::Attribute] {
             .filter(|attr| {
                 attr.is_doc_comment().is_none()
                     // FIXME(jdonszelmann) have a better way to handle ignored attrs
-                    && !attr.name().is_some_and(|ident| hcx.is_ignored_attr(ident))
+                    && !attr.name().is_some_and(|ident| is_ignored_attr(ident))
             })
             .collect();
 
@@ -35,10 +39,18 @@ impl<'a> HashStable<StableHashingContext<'a>> for [hir::Attribute] {
     }
 }
 
-impl<'ctx> rustc_hir::HashStableContext for StableHashingContext<'ctx> {
-    fn hash_attr_id(&mut self, _id: &HashIgnoredAttrId, _hasher: &mut StableHasher) {
-        /* we don't hash HashIgnoredAttrId, we ignore them */
-    }
+#[inline]
+fn is_ignored_attr(name: Symbol) -> bool {
+    const IGNORED_ATTRIBUTES: &[Symbol] = &[
+        sym::cfg_trace, // FIXME(#138844) should this really be ignored?
+        sym::rustc_if_this_changed,
+        sym::rustc_then_this_would_need,
+        sym::rustc_clean,
+        sym::rustc_partition_reused,
+        sym::rustc_partition_codegened,
+        sym::rustc_expected_cgu_reuse,
+    ];
+    IGNORED_ATTRIBUTES.contains(&name)
 }
 
 impl<'a> HashStable<StableHashingContext<'a>> for SourceFile {

compiler/rustc_query_system/src/ich/mod.rs

@@ -1,18 +1,6 @@
 //! ICH - Incremental Compilation Hash
 
-use rustc_span::{Symbol, sym};
-
 pub use self::hcx::StableHashingContext;
 
 mod hcx;
 mod impls_syntax;
-
-pub const IGNORED_ATTRIBUTES: &[Symbol] = &[
-    sym::cfg_trace, // FIXME should this really be ignored?
-    sym::rustc_if_this_changed,
-    sym::rustc_then_this_would_need,
-    sym::rustc_clean,
-    sym::rustc_partition_reused,
-    sym::rustc_partition_codegened,
-    sym::rustc_expected_cgu_reuse,
-];

compiler/rustc_resolve/Cargo.toml

@@ -21,7 +21,6 @@ rustc_index = { path = "../rustc_index" }
 rustc_macros = { path = "../rustc_macros" }
 rustc_metadata = { path = "../rustc_metadata" }
 rustc_middle = { path = "../rustc_middle" }
-rustc_query_system = { path = "../rustc_query_system" }
 rustc_session = { path = "../rustc_session" }
 rustc_span = { path = "../rustc_span" }
 smallvec = { version = "1.8.1", features = ["union", "may_dangle"] }

compiler/rustc_resolve/src/lib.rs

@@ -72,7 +72,6 @@ use rustc_middle::ty::{
     self, DelegationFnSig, DelegationInfo, Feed, MainDefinition, RegisteredTools,
     ResolverAstLowering, ResolverGlobalCtxt, TyCtxt, TyCtxtFeed, Visibility,
 };
-use rustc_query_system::ich::StableHashingContext;
 use rustc_session::config::CrateType;
 use rustc_session::lint::builtin::PRIVATE_MACRO_USE;
 use rustc_span::hygiene::{ExpnId, LocalExpnId, MacroKind, SyntaxContext, Transparency};
@@ -1839,10 +1838,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         ResolverOutputs { global_ctxt, ast_lowering }
     }
 
-    fn create_stable_hashing_context(&self) -> StableHashingContext<'_> {
-        StableHashingContext::new(self.tcx.sess, self.tcx.untracked())
-    }
-
     fn cstore(&self) -> FreezeReadGuard<'_, CStore> {
         CStore::from_tcx(self.tcx)
     }

compiler/rustc_resolve/src/macros.rs

@@ -222,17 +222,19 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> {
     ) -> LocalExpnId {
         let parent_module =
             parent_module_id.map(|module_id| self.local_def_id(module_id).to_def_id());
-        let expn_id = LocalExpnId::fresh(
-            ExpnData::allow_unstable(
-                ExpnKind::AstPass(pass),
-                call_site,
-                self.tcx.sess.edition(),
-                features.into(),
-                None,
-                parent_module,
-            ),
-            self.create_stable_hashing_context(),
-        );
+        let expn_id = self.tcx.with_stable_hashing_context(|hcx| {
+            LocalExpnId::fresh(
+                ExpnData::allow_unstable(
+                    ExpnKind::AstPass(pass),
+                    call_site,
+                    self.tcx.sess.edition(),
+                    features.into(),
+                    None,
+                    parent_module,
+                ),
+                hcx,
+            )
+        });
 
         let parent_scope =
             parent_module.map_or(self.empty_module, |def_id| self.expect_module(def_id));
@@ -322,17 +324,19 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> {
 
         let span = invoc.span();
         let def_id = if deleg_impl.is_some() { None } else { res.opt_def_id() };
-        invoc_id.set_expn_data(
-            ext.expn_data(
-                parent_scope.expansion,
-                span,
-                fast_print_path(path),
-                kind,
-                def_id,
-                def_id.map(|def_id| self.macro_def_scope(def_id).nearest_parent_mod()),
-            ),
-            self.create_stable_hashing_context(),
-        );
+        self.tcx.with_stable_hashing_context(|hcx| {
+            invoc_id.set_expn_data(
+                ext.expn_data(
+                    parent_scope.expansion,
+                    span,
+                    fast_print_path(path),
+                    kind,
+                    def_id,
+                    def_id.map(|def_id| self.macro_def_scope(def_id).nearest_parent_mod()),
+                ),
+                hcx,
+            )
+        });
 
         Ok(ext)
     }

compiler/rustc_span/src/hygiene.rs

@@ -132,19 +132,21 @@ impl !PartialOrd for LocalExpnId {}
 /// to maintain separate versions of `ExpnData` hashes for each permutation
 /// of `HashingControls` settings.
 fn assert_default_hashing_controls(ctx: &impl HashStableContext, msg: &str) {
-    match ctx.hashing_controls() {
-        // Note that we require that `hash_spans` be set according to the global
-        // `-Z incremental-ignore-spans` option. Normally, this option is disabled,
-        // which will cause us to require that this method always be called with `Span` hashing
-        // enabled.
-        //
-        // Span hashing can also be disabled without `-Z incremental-ignore-spans`.
-        // This is the case for instance when building a hash for name mangling.
-        // Such configuration must not be used for metadata.
-        HashingControls { hash_spans }
-            if hash_spans != ctx.unstable_opts_incremental_ignore_spans() => {}
-        other => panic!("Attempted hashing of {msg} with non-default HashingControls: {other:?}"),
-    }
+    let hashing_controls = ctx.hashing_controls();
+    let HashingControls { hash_spans } = hashing_controls;
+
+    // Note that we require that `hash_spans` be the inverse of the global
+    // `-Z incremental-ignore-spans` option. Normally, this option is disabled,
+    // in which case `hash_spans` must be true.
+    //
+    // Span hashing can also be disabled without `-Z incremental-ignore-spans`.
+    // This is the case for instance when building a hash for name mangling.
+    // Such configuration must not be used for metadata.
+    assert_eq!(
+        hash_spans,
+        !ctx.unstable_opts_incremental_ignore_spans(),
+        "Attempted hashing of {msg} with non-default HashingControls: {hashing_controls:?}"
+    );
 }
 
 /// A unique hash value associated to an expansion.

compiler/rustc_span/src/lib.rs

@@ -2799,7 +2799,6 @@ impl InnerSpan {
 /// instead of implementing everything in rustc_middle.
 pub trait HashStableContext {
     fn def_path_hash(&self, def_id: DefId) -> DefPathHash;
-    fn hash_spans(&self) -> bool;
     /// Accesses `sess.opts.unstable_opts.incremental_ignore_spans` since
     /// we don't have easy access to a `Session`
     fn unstable_opts_incremental_ignore_spans(&self) -> bool;
@@ -2832,7 +2831,7 @@ where
         const TAG_INVALID_SPAN: u8 = 1;
         const TAG_RELATIVE_SPAN: u8 = 2;
 
-        if !ctx.hash_spans() {
+        if !ctx.hashing_controls().hash_spans {
             return;
         }