[WIP] Stop ignoring Span field when hashing some Idents

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_hir/src/hir.rs

@@ -205,7 +205,6 @@ impl Path<'_> {
 #[derive(Debug, HashStable_Generic)]
 pub struct PathSegment<'hir> {
     /// The identifier portion of this path segment.
-    #[stable_hasher(project(name))]
     pub ident: Ident,
     // `id` and `res` are optional. We currently only use these in save-analysis,
     // any path segments without these will not have save-analysis info and
@@ -850,7 +849,6 @@ pub struct PatField<'hir> {
     #[stable_hasher(ignore)]
     pub hir_id: HirId,
     /// The identifier for the field.
-    #[stable_hasher(project(name))]
     pub ident: Ident,
     /// The pattern the field is destructured to.
     pub pat: &'hir Pat<'hir>,
@@ -2113,7 +2111,6 @@ pub const FN_OUTPUT_NAME: Symbol = sym::Output;
 #[derive(Debug, HashStable_Generic)]
 pub struct TypeBinding<'hir> {
     pub hir_id: HirId,
-    #[stable_hasher(project(name))]
     pub ident: Ident,
     pub gen_args: &'hir GenericArgs<'hir>,
     pub kind: TypeBindingKind<'hir>,
@@ -2501,7 +2498,6 @@ pub struct EnumDef<'hir> {
 #[derive(Debug, HashStable_Generic)]
 pub struct Variant<'hir> {
     /// Name of the variant.
-    #[stable_hasher(project(name))]
     pub ident: Ident,
     /// Id of the variant (not the constructor, see `VariantData::ctor_hir_id()`).
     pub id: HirId,
@@ -2591,7 +2587,6 @@ impl VisibilityKind<'_> {
 #[derive(Debug, HashStable_Generic)]
 pub struct FieldDef<'hir> {
     pub span: Span,
-    #[stable_hasher(project(name))]
     pub ident: Ident,
     pub vis: Visibility<'hir>,
     pub hir_id: HirId,
@@ -2850,7 +2845,6 @@ impl ItemKind<'_> {
 #[derive(Encodable, Debug, HashStable_Generic)]
 pub struct TraitItemRef {
     pub id: TraitItemId,
-    #[stable_hasher(project(name))]
     pub ident: Ident,
     pub kind: AssocItemKind,
     pub span: Span,
@@ -2866,7 +2860,6 @@ pub struct TraitItemRef {
 #[derive(Debug, HashStable_Generic)]
 pub struct ImplItemRef {
     pub id: ImplItemId,
-    #[stable_hasher(project(name))]
     pub ident: Ident,
     pub kind: AssocItemKind,
     pub span: Span,
@@ -2905,7 +2898,6 @@ impl ForeignItemId {
 #[derive(Debug, HashStable_Generic)]
 pub struct ForeignItemRef {
     pub id: ForeignItemId,
-    #[stable_hasher(project(name))]
     pub ident: Ident,
     pub span: Span,
 }

compiler/rustc_middle/src/ty/adt.rs

@@ -4,6 +4,7 @@ use crate::ty::util::{Discr, IntTypeExt};
 use rustc_data_structures::captures::Captures;
 use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_data_structures::fx::FxHashMap;
+use rustc_data_structures::stable_hasher::HashingControls;
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use rustc_errors::ErrorReported;
 use rustc_hir as hir;
@@ -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/assoc.rs

@@ -43,7 +43,6 @@ impl AssocItemContainer {
 #[derive(Copy, Clone, Debug, PartialEq, HashStable, Eq, Hash)]
 pub struct AssocItem {
     pub def_id: DefId,
-    #[stable_hasher(project(name))]
     pub ident: Ident,
     pub kind: AssocKind,
     pub vis: Visibility,

compiler/rustc_middle/src/ty/impls_ty.rs

@@ -6,6 +6,7 @@ use crate::mir;
 use crate::ty;
 use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_data_structures::fx::FxHashMap;
+use rustc_data_structures::stable_hasher::HashingControls;
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher, ToStableHashKey};
 use rustc_query_system::ich::StableHashingContext;
 use std::cell::RefCell;
@@ -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

@@ -1502,7 +1502,6 @@ pub struct VariantDef {
     /// If this variant is a struct variant, then this is `None`.
     pub ctor_def_id: Option<DefId>,
     /// Variant or struct name.
-    #[stable_hasher(project(name))]
     pub ident: Ident,
     /// Discriminant of this variant.
     pub discr: VariantDiscr,

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/dep_graph/graph.rs

@@ -51,7 +51,7 @@ impl std::convert::From<DepNodeIndex> for QueryInvocationId {
     }
 }
 
-#[derive(PartialEq)]
+#[derive(PartialEq, Debug)]
 pub enum DepNodeColor {
     Red,
     Green(DepNodeIndex),
@@ -285,6 +285,7 @@ impl<K: DepKind> DepGraph<K> {
                 key
             );
 
+            tracing::info!("Inserting color: {:?} {:?}", key, color);
             data.colors.insert(prev_index, color);
         }
 
@@ -1156,6 +1157,7 @@ impl DepNodeColorMap {
     }
 
     fn insert(&self, index: SerializedDepNodeIndex, color: DepNodeColor) {
+        tracing::info!("Actually storing: {:?} {:?}", index, color);
         self.values[index].store(
             match color {
                 DepNodeColor::Red => COMPRESSED_RED,

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 self::hcx::StableHashingContext;
+pub use rustc_data_structures::stable_hasher::NodeIdHashingMode;
 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::HashingControls;
+use rustc_data_structures::stable_hasher::{HashStable, NodeIdHashingMode, StableHasher};
 use rustc_data_structures::sync::{Lock, Lrc};
 use rustc_data_structures::unhash::UnhashMap;
 use rustc_index::vec::IndexVec;
@@ -88,6 +89,20 @@ 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 +1459,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 +1509,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