Ensure that Fingerprint caching respects hashing configuration

compiler/rustc_data_structures/src/stable_hasher.rs

@@ -583,3 +583,22 @@ 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,
+}

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/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_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};
@@ -26,20 +27,15 @@ fn compute_ignored_attr_names() -> FxHashSet<Symbol> {
 pub struct StableHashingContext<'a> {
  definitions: &'a Definitions,
  cstore: &'a dyn CrateStore,
+ // The value of `-Z incremental-ignore-spans`.
+ // This field should only be used by `debug_opts_incremental_ignore_span`
+ incremental_ignore_spans: bool,
  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`.
@@ -70,10 +66,13 @@ impl<'a> StableHashingContext<'a> {
  body_resolver: BodyResolver::Forbidden,
  definitions,
  cstore,
+ incremental_ignore_spans: sess.opts.debugging_opts.incremental_ignore_spans,
  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 +132,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 +144,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 +182,11 @@ impl<'a> StableHashingContext<'a> {
  }
  IGNORED_ATTRIBUTES.with(|attrs| attrs.contains(&name))
  }
+
+ #[inline]
+ pub fn hashing_controls(&self) -> HashingControls {
+ self.hashing_controls.clone()
+ }
 }
 
 impl<'a> HashStable<StableHashingContext<'a>> for ast::NodeId {
@@ -195,7 +199,12 @@ 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]
+ fn debug_opts_incremental_ignore_spans(&self) -> bool {
+ self.incremental_ignore_spans
  }
 
  #[inline]
@@ -215,6 +224,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

@@ -11,7 +11,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.
  }
@@ -89,12 +89,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,6 +32,7 @@ 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::HashingControls;
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use rustc_data_structures::sync::{Lock, Lrc};
 use rustc_data_structures::unhash::UnhashMap;
@@ -88,6 +89,33 @@ 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) {
+ match ctx.hashing_controls() {
+ // Ideally, we would also check that `node_id_hashing_mode` was always
+ // `NodeIdHashingMode::HashDefPath`. However, we currently end up hashing
+ // `Span`s in this mode, and there's not an easy way to change that.
+ // All of the span-related data that we hash is pretty self-contained
+ // (in particular, we don't hash any `HirId`s), so this shouldn't result
+ // in any caching problems.
+ // FIXME: Enforce that we don't end up transitively hashing any `HirId`s,
+ // or ensure that this method is always invoked with the same
+ // `NodeIdHashingMode`
+ //
+ // 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.
+ HashingControls { hash_spans, node_id_hashing_mode: _ }
+ if hash_spans == !ctx.debug_opts_incremental_ignore_spans() => {}
+ other => panic!("Attempted hashing of {msg} with non-default HashingControls: {:?}", other),
+ }
+}
+
 /// 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 +1472,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 +1522,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;
@@ -2057,11 +2058,15 @@ impl InnerSpan {
 pub trait HashStableContext {
  fn def_path_hash(&self, def_id: DefId) -> DefPathHash;
  fn hash_spans(&self) -> bool;
+ /// Accesses `sess.opts.debugging_opts.incremental_ignore_spans` since
+ /// we don't have easy access to a `Session`
+ fn debug_opts_incremental_ignore_spans(&self) -> bool;
  fn def_span(&self, def_id: LocalDefId) -> Span;
  fn span_data_to_lines_and_cols(
  &mut self,
  span: &SpanData,
  ) -> Option<(Lrc<SourceFile>, usize, BytePos, usize, BytePos)>;
+ fn hashing_controls(&self) -> HashingControls;
 }
 
 impl<CTX> HashStable<CTX> for Span

compiler/rustc_symbol_mangling/src/legacy.rs

@@ -113,29 +113,29 @@ 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(&mut 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);
+ });
+ });
  });
 
  // 64 bits should be enough to avoid collisions.