Fix hashing cache issue

Author: Aaron1011

compiler/rustc_data_structures/src/stable_hasher.rs

@@ -1,3 +1,4 @@
+use crate::fx::FxHashMap;
 use crate::sip128::SipHasher128;
 use rustc_index::bit_set;
 use rustc_index::vec;
@@ -584,3 +585,28 @@ fn stable_hash_reduce<HCX, I, C, F>(
         }
     }
 }
+
+#[derive(PartialEq, Eq, Clone, Copy, Hash, Debug)]
+pub enum NodeIdHashingMode {
+    Ignore,
+    HashDefPath,
+}
+
+/// Controls what data we do or not not hash.
+/// Whenever a `HashStable` implementation caches its
+/// result, it needs to include `HashingControls` as part
+/// of the key, to ensure that is does not produce an incorrect
+/// result (for example, using a `Fingerprint` produced while
+/// hashing `Span`s when a `Fingeprint` without `Span`s is
+/// being requested)
+#[derive(Clone, Hash, Eq, PartialEq, Debug)]
+pub struct HashingControls {
+    pub hash_spans: bool,
+    pub node_id_hashing_mode: NodeIdHashingMode,
+}
+
+/// A cache for use by `HashStable` implementations. It includes
+/// a `HashingControls` as part of the key, to prevent using
+/// a result computed under one `HashingControls` with a different
+/// `HashingControls`
+pub type StableHashCache<K, V> = FxHashMap<(K, HashingControls), V>;

compiler/rustc_middle/src/ty/adt.rs

@@ -5,6 +5,7 @@ use rustc_data_structures::captures::Captures;
 use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
+use rustc_data_structures::stable_hasher::HashingControls;
 use rustc_errors::ErrorReported;
 use rustc_hir as hir;
 use rustc_hir::def::{CtorKind, DefKind, Res};
@@ -136,12 +137,13 @@ impl Hash for AdtDef {
 impl<'a> HashStable<StableHashingContext<'a>> for AdtDef {
     fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) {
         thread_local! {
-            static CACHE: RefCell<FxHashMap<usize, Fingerprint>> = Default::default();
+            static CACHE: RefCell<FxHashMap<(usize, HashingControls), Fingerprint>> = Default::default();
         }
 
         let hash: Fingerprint = CACHE.with(|cache| {
             let addr = self as *const AdtDef as usize;
-            *cache.borrow_mut().entry(addr).or_insert_with(|| {
+            let hashing_controls = hcx.hashing_controls();
+            *cache.borrow_mut().entry((addr, hashing_controls)).or_insert_with(|| {
                 let ty::AdtDef { did, ref variants, ref flags, ref repr } = *self;
 
                 let mut hasher = StableHasher::new();

compiler/rustc_middle/src/ty/impls_ty.rs

@@ -7,6 +7,7 @@ use crate::ty;
 use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher, ToStableHashKey};
+use rustc_data_structures::stable_hasher::HashingControls;
 use rustc_query_system::ich::StableHashingContext;
 use std::cell::RefCell;
 use std::mem;
@@ -17,12 +18,12 @@ where
 {
     fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) {
         thread_local! {
-            static CACHE: RefCell<FxHashMap<(usize, usize), Fingerprint>> =
+            static CACHE: RefCell<FxHashMap<(usize, usize, HashingControls), Fingerprint>> =
                 RefCell::new(Default::default());
         }
 
         let hash = CACHE.with(|cache| {
-            let key = (self.as_ptr() as usize, self.len());
+            let key = (self.as_ptr() as usize, self.len(), hcx.hashing_controls());
             if let Some(&hash) = cache.borrow().get(&key) {
                 return hash;
             }

compiler/rustc_middle/src/ty/mod.rs

@@ -1597,6 +1597,7 @@ pub enum VariantDiscr {
 #[derive(Debug, HashStable, TyEncodable, TyDecodable)]
 pub struct FieldDef {
     pub did: DefId,
+    #[stable_hasher(project(name))]
     pub ident: Ident,
     pub vis: Visibility,
 }

compiler/rustc_middle/src/ty/util.rs

@@ -137,7 +137,9 @@ impl<'tcx> TyCtxt<'tcx> {
 
         hcx.while_hashing_spans(false, |hcx| {
             hcx.with_node_id_hashing_mode(NodeIdHashingMode::HashDefPath, |hcx| {
+                tracing::info!("Hashing:      {:?}", ty);
                 ty.hash_stable(hcx, &mut hasher);
+                tracing::info!("Done hashing: {:?}", ty);
             });
         });
         hasher.finish()

compiler/rustc_query_system/src/ich/hcx.rs

@@ -3,6 +3,7 @@ use rustc_ast as ast;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::sorted_map::SortedMap;
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
+use rustc_data_structures::stable_hasher::{HashingControls, NodeIdHashingMode};
 use rustc_data_structures::sync::Lrc;
 use rustc_hir as hir;
 use rustc_hir::def_id::{DefId, LocalDefId};
@@ -27,19 +28,11 @@ pub struct StableHashingContext<'a> {
     definitions: &'a Definitions,
     cstore: &'a dyn CrateStore,
     pub(super) body_resolver: BodyResolver<'a>,
-    hash_spans: bool,
-    pub(super) node_id_hashing_mode: NodeIdHashingMode,
-
     // 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>>,
-}
-
-#[derive(PartialEq, Eq, Clone, Copy)]
-pub enum NodeIdHashingMode {
-    Ignore,
-    HashDefPath,
+    pub(super) hashing_controls: HashingControls,
 }
 
 /// The `BodyResolver` allows mapping a `BodyId` to the corresponding `hir::Body`.
@@ -72,8 +65,10 @@ impl<'a> StableHashingContext<'a> {
             cstore,
             caching_source_map: None,
             raw_source_map: sess.source_map(),
-            hash_spans: hash_spans_initial,
-            node_id_hashing_mode: NodeIdHashingMode::HashDefPath,
+            hashing_controls: HashingControls {
+                hash_spans: hash_spans_initial,
+                node_id_hashing_mode: NodeIdHashingMode::HashDefPath,
+            }
         }
     }
 
@@ -133,10 +128,10 @@ impl<'a> StableHashingContext<'a> {
 
     #[inline]
     pub fn while_hashing_spans<F: FnOnce(&mut Self)>(&mut self, hash_spans: bool, f: F) {
-        let prev_hash_spans = self.hash_spans;
-        self.hash_spans = hash_spans;
+        let prev_hash_spans = self.hashing_controls.hash_spans;
+        self.hashing_controls.hash_spans = hash_spans;
         f(self);
-        self.hash_spans = prev_hash_spans;
+        self.hashing_controls.hash_spans = prev_hash_spans;
     }
 
     #[inline]
@@ -145,10 +140,10 @@ impl<'a> StableHashingContext<'a> {
         mode: NodeIdHashingMode,
         f: F,
     ) {
-        let prev = self.node_id_hashing_mode;
-        self.node_id_hashing_mode = mode;
+        let prev = self.hashing_controls.node_id_hashing_mode;
+        self.hashing_controls.node_id_hashing_mode = mode;
         f(self);
-        self.node_id_hashing_mode = prev;
+        self.hashing_controls.node_id_hashing_mode = prev;
     }
 
     #[inline]
@@ -183,6 +178,10 @@ impl<'a> StableHashingContext<'a> {
         }
         IGNORED_ATTRIBUTES.with(|attrs| attrs.contains(&name))
     }
+
+    pub fn hashing_controls(&self) -> HashingControls {
+        self.hashing_controls.clone()
+    }
 }
 
 impl<'a> HashStable<StableHashingContext<'a>> for ast::NodeId {
@@ -195,7 +194,7 @@ impl<'a> HashStable<StableHashingContext<'a>> for ast::NodeId {
 impl<'a> rustc_span::HashStableContext for StableHashingContext<'a> {
     #[inline]
     fn hash_spans(&self) -> bool {
-        self.hash_spans
+        self.hashing_controls.hash_spans
     }
 
     #[inline]
@@ -215,6 +214,11 @@ impl<'a> rustc_span::HashStableContext for StableHashingContext<'a> {
     ) -> Option<(Lrc<SourceFile>, usize, BytePos, usize, BytePos)> {
         self.source_map().span_data_to_lines_and_cols(span)
     }
+
+    #[inline]
+    fn hashing_controls(&self) -> HashingControls {
+        self.hashing_controls.clone()
+    }
 }
 
 impl<'a> rustc_session::HashStableContext for StableHashingContext<'a> {}

compiler/rustc_query_system/src/ich/impls_hir.rs

@@ -12,7 +12,7 @@ impl<'ctx> rustc_hir::HashStableContext for StableHashingContext<'ctx> {
     #[inline]
     fn hash_hir_id(&mut self, hir_id: hir::HirId, hasher: &mut StableHasher) {
         let hcx = self;
-        match hcx.node_id_hashing_mode {
+        match hcx.hashing_controls.node_id_hashing_mode {
             NodeIdHashingMode::Ignore => {
                 // Don't do anything.
             }
@@ -112,12 +112,12 @@ impl<'ctx> rustc_hir::HashStableContext for StableHashingContext<'ctx> {
 
     #[inline]
     fn hash_hir_item_like<F: FnOnce(&mut Self)>(&mut self, f: F) {
-        let prev_hash_node_ids = self.node_id_hashing_mode;
-        self.node_id_hashing_mode = NodeIdHashingMode::Ignore;
+        let prev_hash_node_ids = self.hashing_controls.node_id_hashing_mode;
+        self.hashing_controls.node_id_hashing_mode = NodeIdHashingMode::Ignore;
 
         f(self);
 
-        self.node_id_hashing_mode = prev_hash_node_ids;
+        self.hashing_controls.node_id_hashing_mode = prev_hash_node_ids;
     }
 
     #[inline]

compiler/rustc_query_system/src/ich/mod.rs

@@ -1,6 +1,7 @@
 //! ICH - Incremental Compilation Hash
 
-pub use self::hcx::{NodeIdHashingMode, StableHashingContext};
+pub use rustc_data_structures::stable_hasher::NodeIdHashingMode;
+pub use self::hcx::{StableHashingContext};
 use rustc_span::symbol::{sym, Symbol};
 
 mod hcx;

compiler/rustc_span/src/hygiene.rs

@@ -32,7 +32,8 @@ use crate::{HashStableContext, Span, DUMMY_SP};
 use crate::def_id::{CrateNum, DefId, StableCrateId, CRATE_DEF_ID, LOCAL_CRATE};
 use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
-use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
+use rustc_data_structures::stable_hasher::{HashStable, StableHasher, NodeIdHashingMode};
+use rustc_data_structures::stable_hasher::HashingControls;
 use rustc_data_structures::sync::{Lock, Lrc};
 use rustc_data_structures::unhash::UnhashMap;
 use rustc_index::vec::IndexVec;
@@ -88,6 +89,22 @@ rustc_index::newtype_index! {
     }
 }
 
+// Assert that the provided `HashStableContext` is configured with the 'default'
+// `HashingControls`. We should always have bailed out before getting to here
+// with a non-default mode. With this check in place, we can avoid the need
+// to maintain separate versions of `ExpnData` hashes for each permutation
+// of `HashingControls` settings.
+fn assert_default_hashing_controls<CTX: HashStableContext>(ctx: &CTX, msg: &str) {
+    let default = HashingControls {
+        hash_spans: true,
+        node_id_hashing_mode: NodeIdHashingMode::HashDefPath
+    };
+    let current = ctx.hashing_controls();
+    if current != default {
+        //panic!("Attempted hashing of {msg} with non-default HashingControls: {:?}", current);
+    }
+}
+
 /// A unique hash value associated to an expansion.
 #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, Encodable, Decodable, HashStable_Generic)]
 pub struct ExpnHash(Fingerprint);
@@ -1444,6 +1461,7 @@ fn update_disambiguator(expn_data: &mut ExpnData, mut ctx: impl HashStableContex
         "Already set disambiguator for ExpnData: {:?}",
         expn_data
     );
+    assert_default_hashing_controls(&ctx, "ExpnData (disambiguator)");
     let mut expn_hash = expn_data.hash_expn(&mut ctx);
 
     let disambiguator = HygieneData::with(|data| {
@@ -1493,6 +1511,7 @@ impl<CTX: HashStableContext> HashStable<CTX> for SyntaxContext {
 
 impl<CTX: HashStableContext> HashStable<CTX> for ExpnId {
     fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) {
+        assert_default_hashing_controls(ctx, "ExpnId");
         let hash = if *self == ExpnId::root() {
             // Avoid fetching TLS storage for a trivial often-used value.
             Fingerprint::ZERO

compiler/rustc_span/src/lib.rs

@@ -42,6 +42,7 @@ pub mod hygiene;
 use hygiene::Transparency;
 pub use hygiene::{DesugaringKind, ExpnKind, MacroKind};
 pub use hygiene::{ExpnData, ExpnHash, ExpnId, LocalExpnId, SyntaxContext};
+use rustc_data_structures::stable_hasher::HashingControls;
 pub mod def_id;
 use def_id::{CrateNum, DefId, DefPathHash, LocalDefId, LOCAL_CRATE};
 pub mod lev_distance;
@@ -2062,6 +2063,7 @@ pub trait HashStableContext {
         &mut self,
         span: &SpanData,
     ) -> Option<(Lrc<SourceFile>, usize, BytePos, usize, BytePos)>;
+    fn hashing_controls(&self) -> HashingControls;
 }
 
 impl<CTX> HashStable<CTX> for Span
@@ -2087,6 +2089,8 @@ where
             return;
         }
 
+        tracing::info!("Hashing span: {:?}", self);
+
         let span = self.data_untracked();
         span.ctxt.hash_stable(ctx, hasher);
         span.parent.hash_stable(ctx, hasher);

compiler/rustc_symbol_mangling/src/legacy.rs

@@ -113,29 +113,30 @@ fn get_symbol_hash<'tcx>(
         hcx.while_hashing_spans(false, |hcx| {
             hcx.with_node_id_hashing_mode(NodeIdHashingMode::HashDefPath, |hcx| {
                 item_type.hash_stable(hcx, &mut hasher);
-            });
-        });
 
-        // If this is a function, we hash the signature as well.
-        // This is not *strictly* needed, but it may help in some
-        // situations, see the `run-make/a-b-a-linker-guard` test.
-        if let ty::FnDef(..) = item_type.kind() {
-            item_type.fn_sig(tcx).hash_stable(&mut hcx, &mut hasher);
-        }
+                // If this is a function, we hash the signature as well.
+                // This is not *strictly* needed, but it may help in some
+                // situations, see the `run-make/a-b-a-linker-guard` test.
+                if let ty::FnDef(..) = item_type.kind() {
+                    item_type.fn_sig(tcx).hash_stable(hcx, &mut hasher);
+                }
 
-        // also include any type parameters (for generic items)
-        substs.hash_stable(&mut hcx, &mut hasher);
+                // also include any type parameters (for generic items)
+                substs.hash_stable(hcx, &mut hasher);
 
-        if let Some(instantiating_crate) = instantiating_crate {
-            tcx.def_path_hash(instantiating_crate.as_def_id())
-                .stable_crate_id()
-                .hash_stable(&mut hcx, &mut hasher);
-        }
+                if let Some(instantiating_crate) = instantiating_crate {
+                    tcx.def_path_hash(instantiating_crate.as_def_id())
+                        .stable_crate_id()
+                        .hash_stable(hcx, &mut hasher);
+                }
+
+                // We want to avoid accidental collision between different types of instances.
+                // Especially, `VtableShim`s and `ReifyShim`s may overlap with their original
+                // instances without this.
+                discriminant(&instance.def).hash_stable(hcx, &mut hasher);
 
-        // We want to avoid accidental collision between different types of instances.
-        // Especially, `VtableShim`s and `ReifyShim`s may overlap with their original
-        // instances without this.
-        discriminant(&instance.def).hash_stable(&mut hcx, &mut hasher);
+            });
+        });
     });
 
     // 64 bits should be enough to avoid collisions.

src/test/ui/macros/assert-macro-owned.rs

@@ -4,6 +4,10 @@
 
 #![allow(non_fmt_panics)]
 
+use std::any::TypeId;
+
 fn main() {
+    println!("String:       {:?}", TypeId::of::<String>());
+    println!("&'static str: {:?}", TypeId::of::<&'static str>());
     assert!(false, "test-assert-owned".to_string());
 }