Auto merge of #95524 - oli-obk:cached_stable_hash_cleanups, r=nnethercote

Cached stable hash cleanups

r? `@nnethercote`

Add a sanity assertion in debug mode to check that the cached hashes are actually the ones we get if we compute the hash each time.

Add a new data structure that bundles all the hash-caching work to make it easier to re-use it for different interned data structures

Author: bors

compiler/rustc_data_structures/src/intern.rs

@@ -4,6 +4,8 @@ use std::hash::{Hash, Hasher};
 use std::ops::Deref;
 use std::ptr;
 
+use crate::fingerprint::Fingerprint;
+
 mod private {
     #[derive(Clone, Copy, Debug)]
     pub struct PrivateZst;
@@ -108,5 +110,87 @@ where
     }
 }
 
+/// A helper trait so that `Interned` things can cache stable hashes reproducibly.
+pub trait InternedHashingContext {
+    fn with_def_path_and_no_spans(&mut self, f: impl FnOnce(&mut Self));
+}
+
+/// A helper type that you can wrap round your own type in order to automatically
+/// cache the stable hash on creation and not recompute it whenever the stable hash
+/// of the type is computed.
+/// This is only done in incremental mode. You can also opt out of caching by using
+/// StableHash::ZERO for the hash, in which case the hash gets computed each time.
+/// This is useful if you have values that you intern but never (can?) use for stable
+/// hashing.
+#[derive(Copy, Clone)]
+pub struct WithStableHash<T> {
+    pub internee: T,
+    pub stable_hash: Fingerprint,
+}
+
+impl<T: PartialEq> PartialEq for WithStableHash<T> {
+    #[inline]
+    fn eq(&self, other: &Self) -> bool {
+        self.internee.eq(&other.internee)
+    }
+}
+
+impl<T: Eq> Eq for WithStableHash<T> {}
+
+impl<T: Ord> PartialOrd for WithStableHash<T> {
+    fn partial_cmp(&self, other: &WithStableHash<T>) -> Option<Ordering> {
+        Some(self.internee.cmp(&other.internee))
+    }
+}
+
+impl<T: Ord> Ord for WithStableHash<T> {
+    fn cmp(&self, other: &WithStableHash<T>) -> Ordering {
+        self.internee.cmp(&other.internee)
+    }
+}
+
+impl<T> Deref for WithStableHash<T> {
+    type Target = T;
+
+    #[inline]
+    fn deref(&self) -> &T {
+        &self.internee
+    }
+}
+
+impl<T: Hash> Hash for WithStableHash<T> {
+    #[inline]
+    fn hash<H: Hasher>(&self, s: &mut H) {
+        self.internee.hash(s)
+    }
+}
+
+impl<T: HashStable<CTX>, CTX: InternedHashingContext> HashStable<CTX> for WithStableHash<T> {
+    fn hash_stable(&self, hcx: &mut CTX, hasher: &mut StableHasher) {
+        if self.stable_hash == Fingerprint::ZERO || cfg!(debug_assertions) {
+            // No cached hash available. This can only mean that incremental is disabled.
+            // We don't cache stable hashes in non-incremental mode, because they are used
+            // so rarely that the performance actually suffers.
+
+            // We need to build the hash as if we cached it and then hash that hash, as
+            // otherwise the hashes will differ between cached and non-cached mode.
+            let stable_hash: Fingerprint = {
+                let mut hasher = StableHasher::new();
+                hcx.with_def_path_and_no_spans(|hcx| self.internee.hash_stable(hcx, &mut hasher));
+                hasher.finish()
+            };
+            if cfg!(debug_assertions) && self.stable_hash != Fingerprint::ZERO {
+                assert_eq!(
+                    stable_hash, self.stable_hash,
+                    "cached stable hash does not match freshly computed stable hash"
+                );
+            }
+            stable_hash.hash_stable(hcx, hasher);
+        } else {
+            self.stable_hash.hash_stable(hcx, hasher);
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests;

compiler/rustc_infer/src/infer/outlives/env.rs

@@ -2,7 +2,6 @@ use crate::infer::free_regions::FreeRegionMap;
 use crate::infer::{GenericKind, InferCtxt};
 use crate::traits::query::OutlivesBound;
 use rustc_data_structures::fx::FxHashMap;
-use rustc_data_structures::intern::Interned;
 use rustc_hir as hir;
 use rustc_middle::ty::{self, ReEarlyBound, ReFree, ReVar, Region};
 
@@ -164,12 +163,6 @@ impl<'a, 'tcx> OutlivesEnvironment<'tcx> {
         for outlives_bound in outlives_bounds {
             debug!("add_outlives_bounds: outlives_bound={:?}", outlives_bound);
             match outlives_bound {
-                OutlivesBound::RegionSubRegion(
-                    r_a @ (Region(Interned(ReEarlyBound(_), _)) | Region(Interned(ReFree(_), _))),
-                    Region(Interned(ReVar(vid_b), _)),
-                ) => {
-                    infcx.expect("no infcx provided but region vars found").add_given(r_a, *vid_b);
-                }
                 OutlivesBound::RegionSubParam(r_a, param_b) => {
                     self.region_bound_pairs_accum.push((r_a, GenericKind::Param(param_b)));
                 }
@@ -178,17 +171,23 @@ impl<'a, 'tcx> OutlivesEnvironment<'tcx> {
                         .push((r_a, GenericKind::Projection(projection_b)));
                 }
                 OutlivesBound::RegionSubRegion(r_a, r_b) => {
-                    // In principle, we could record (and take
-                    // advantage of) every relationship here, but
-                    // we are also free not to -- it simply means
-                    // strictly less that we can successfully type
-                    // check. Right now we only look for things
-                    // relationships between free regions. (It may
-                    // also be that we should revise our inference
-                    // system to be more general and to make use
-                    // of *every* relationship that arises here,
-                    // but presently we do not.)
-                    self.free_region_map.relate_regions(r_a, r_b);
+                    if let (ReEarlyBound(_) | ReFree(_), ReVar(vid_b)) = (r_a.kind(), r_b.kind()) {
+                        infcx
+                            .expect("no infcx provided but region vars found")
+                            .add_given(r_a, vid_b);
+                    } else {
+                        // In principle, we could record (and take
+                        // advantage of) every relationship here, but
+                        // we are also free not to -- it simply means
+                        // strictly less that we can successfully type
+                        // check. Right now we only look for things
+                        // relationships between free regions. (It may
+                        // also be that we should revise our inference
+                        // system to be more general and to make use
+                        // of *every* relationship that arises here,
+                        // but presently we do not.)
+                        self.free_region_map.relate_regions(r_a, r_b);
+                    }
                 }
             }
         }

compiler/rustc_middle/src/arena.rs

@@ -87,7 +87,7 @@ macro_rules! arena_types {
             [] hir_id_set: rustc_hir::HirIdSet,
 
             // Interned types
-            [] tys: rustc_middle::ty::TyS<'tcx>,
+            [] tys: rustc_data_structures::intern::WithStableHash<rustc_middle::ty::TyS<'tcx>>,
             [] predicates: rustc_middle::ty::PredicateS<'tcx>,
             [] consts: rustc_middle::ty::ConstS<'tcx>,
 

compiler/rustc_middle/src/ty/context.rs

@@ -26,7 +26,7 @@ use crate::ty::{
 use rustc_ast as ast;
 use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
-use rustc_data_structures::intern::Interned;
+use rustc_data_structures::intern::{Interned, WithStableHash};
 use rustc_data_structures::memmap::Mmap;
 use rustc_data_structures::profiling::SelfProfilerRef;
 use rustc_data_structures::sharded::{IntoPointer, ShardedHashMap};
@@ -105,7 +105,7 @@ pub struct CtxtInterners<'tcx> {
 
     // Specifically use a speedy hash algorithm for these hash sets, since
     // they're accessed quite often.
-    type_: InternedSet<'tcx, TyS<'tcx>>,
+    type_: InternedSet<'tcx, WithStableHash<TyS<'tcx>>>,
     substs: InternedSet<'tcx, InternalSubsts<'tcx>>,
     canonical_var_infos: InternedSet<'tcx, List<CanonicalVarInfo<'tcx>>>,
     region: InternedSet<'tcx, RegionKind>,
@@ -178,10 +178,11 @@ impl<'tcx> CtxtInterners<'tcx> {
                         kind,
                         flags: flags.flags,
                         outer_exclusive_binder: flags.outer_exclusive_binder,
-                        stable_hash,
                     };
 
-                    InternedInSet(self.arena.alloc(ty_struct))
+                    InternedInSet(
+                        self.arena.alloc(WithStableHash { internee: ty_struct, stable_hash }),
+                    )
                 })
                 .0,
         ))
@@ -2048,23 +2049,23 @@ impl<'tcx, T: 'tcx + ?Sized> IntoPointer for InternedInSet<'tcx, T> {
 }
 
 #[allow(rustc::usage_of_ty_tykind)]
-impl<'tcx> Borrow<TyKind<'tcx>> for InternedInSet<'tcx, TyS<'tcx>> {
+impl<'tcx> Borrow<TyKind<'tcx>> for InternedInSet<'tcx, WithStableHash<TyS<'tcx>>> {
     fn borrow<'a>(&'a self) -> &'a TyKind<'tcx> {
         &self.0.kind
     }
 }
 
-impl<'tcx> PartialEq for InternedInSet<'tcx, TyS<'tcx>> {
-    fn eq(&self, other: &InternedInSet<'tcx, TyS<'tcx>>) -> bool {
+impl<'tcx> PartialEq for InternedInSet<'tcx, WithStableHash<TyS<'tcx>>> {
+    fn eq(&self, other: &InternedInSet<'tcx, WithStableHash<TyS<'tcx>>>) -> bool {
         // The `Borrow` trait requires that `x.borrow() == y.borrow()` equals
         // `x == y`.
         self.0.kind == other.0.kind
     }
 }
 
-impl<'tcx> Eq for InternedInSet<'tcx, TyS<'tcx>> {}
+impl<'tcx> Eq for InternedInSet<'tcx, WithStableHash<TyS<'tcx>>> {}
 
-impl<'tcx> Hash for InternedInSet<'tcx, TyS<'tcx>> {
+impl<'tcx> Hash for InternedInSet<'tcx, WithStableHash<TyS<'tcx>>> {
     fn hash<H: Hasher>(&self, s: &mut H) {
         // The `Borrow` trait requires that `x.borrow().hash(s) == x.hash(s)`.
         self.0.kind.hash(s)

compiler/rustc_middle/src/ty/layout.rs

@@ -5,7 +5,6 @@ use crate::ty::subst::Subst;
 use crate::ty::{self, subst::SubstsRef, ReprOptions, Ty, TyCtxt, TypeFoldable};
 use rustc_ast as ast;
 use rustc_attr as attr;
-use rustc_data_structures::intern::Interned;
 use rustc_hir as hir;
 use rustc_hir::lang_items::LangItem;
 use rustc_index::bit_set::BitSet;
@@ -503,42 +502,34 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
                 }
 
                 // Two non-ZST fields, and they're both scalars.
-                (
-                    Some((
-                        i,
-                        &TyAndLayout {
-                            layout: Layout(Interned(&LayoutS { abi: Abi::Scalar(a), .. }, _)),
-                            ..
-                        },
-                    )),
-                    Some((
-                        j,
-                        &TyAndLayout {
-                            layout: Layout(Interned(&LayoutS { abi: Abi::Scalar(b), .. }, _)),
-                            ..
-                        },
-                    )),
-                    None,
-                ) => {
-                    // Order by the memory placement, not source order.
-                    let ((i, a), (j, b)) =
-                        if offsets[i] < offsets[j] { ((i, a), (j, b)) } else { ((j, b), (i, a)) };
-                    let pair = self.scalar_pair(a, b);
-                    let pair_offsets = match pair.fields {
-                        FieldsShape::Arbitrary { ref offsets, ref memory_index } => {
-                            assert_eq!(memory_index, &[0, 1]);
-                            offsets
+                (Some((i, a)), Some((j, b)), None) => {
+                    match (a.abi, b.abi) {
+                        (Abi::Scalar(a), Abi::Scalar(b)) => {
+                            // Order by the memory placement, not source order.
+                            let ((i, a), (j, b)) = if offsets[i] < offsets[j] {
+                                ((i, a), (j, b))
+                            } else {
+                                ((j, b), (i, a))
+                            };
+                            let pair = self.scalar_pair(a, b);
+                            let pair_offsets = match pair.fields {
+                                FieldsShape::Arbitrary { ref offsets, ref memory_index } => {
+                                    assert_eq!(memory_index, &[0, 1]);
+                                    offsets
+                                }
+                                _ => bug!(),
+                            };
+                            if offsets[i] == pair_offsets[0]
+                                && offsets[j] == pair_offsets[1]
+                                && align == pair.align
+                                && size == pair.size
+                            {
+                                // We can use `ScalarPair` only when it matches our
+                                // already computed layout (including `#[repr(C)]`).
+                                abi = pair.abi;
+                            }
                         }
-                        _ => bug!(),
-                    };
-                    if offsets[i] == pair_offsets[0]
-                        && offsets[j] == pair_offsets[1]
-                        && align == pair.align
-                        && size == pair.size
-                    {
-                        // We can use `ScalarPair` only when it matches our
-                        // already computed layout (including `#[repr(C)]`).
-                        abi = pair.abi;
+                        _ => {}
                     }
                 }
 
@@ -791,10 +782,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
                     }
 
                     // Extract the number of elements from the layout of the array field:
-                    let Ok(TyAndLayout {
-                        layout: Layout(Interned(LayoutS { fields: FieldsShape::Array { count, .. }, .. }, _)),
-                        ..
-                    }) = self.layout_of(f0_ty) else {
+                    let FieldsShape::Array { count, .. } = self.layout_of(f0_ty)?.layout.fields() else {
                         return Err(LayoutError::Unknown(ty));
                     };