Skip to content

Commit da3f196

Browse files
committed
Remove special-cased stable hashing for HIR module
All other 'containers' (e.g. `impl` blocks) hashed their contents in the normal, order-dependent way. However, `Mod` was hashing its contents in a (sort-of) order-independent way. However, the exact order is exposed to consumers through `Mod.item_ids`, and through query results like `hir_module_items`. Therefore, stable hashing needs to take the order of items into account, to avoid fingerprint ICEs. Unforuntately, I was unable to directly build a reproducer for the ICE, due to the behavior of `Fingerprint::combine_commutative`. This operation swaps the upper and lower `u64` when constructing the result, which makes the function non-associative. Since we start the hashing of module items by combining `Fingerprint::ZERO` with the first item, it's difficult to actually build an example where changing the order of module items leaves the final hash unchanged. However, this appears to have been hit in practice in #92218 While we're not able to reproduce it, the fact that proc-macros are involved (which can give an entire module the same span, preventing any span-related invalidations) makes me confident that the root cause of that issue is our method of hashing module items. This PR removes all of the special handling for `Mod`, instead deriving a `HashStable` implementation. This makes `Mod` consistent with other 'contains' like `Impl`, which hash their contents through the typical derive of `HashStable`.
1 parent 489296d commit da3f196

File tree

4 files changed

+31
-33
lines changed

4 files changed

+31
-33
lines changed

Diff for: compiler/rustc_hir/src/hir.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2484,7 +2484,7 @@ impl FnRetTy<'_> {
24842484
}
24852485
}
24862486

2487-
#[derive(Encodable, Debug)]
2487+
#[derive(Encodable, Debug, HashStable_Generic)]
24882488
pub struct Mod<'hir> {
24892489
/// A span from the first token past `{` to the last token until `}`.
24902490
/// For `mod foo;`, the inner span ranges from the first token

Diff for: compiler/rustc_hir/src/stable_hash_impls.rs

+1-8
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher, ToStableHas
22

33
use crate::hir::{
44
AttributeMap, BodyId, Crate, Expr, ForeignItem, ForeignItemId, ImplItem, ImplItemId, Item,
5-
ItemId, Mod, OwnerNodes, TraitCandidate, TraitItem, TraitItemId, Ty, VisibilityKind,
5+
ItemId, OwnerNodes, TraitCandidate, TraitItem, TraitItemId, Ty, VisibilityKind,
66
};
77
use crate::hir_id::{HirId, ItemLocalId};
88
use rustc_span::def_id::DefPathHash;
@@ -16,7 +16,6 @@ pub trait HashStableContext:
1616
fn hash_hir_id(&mut self, _: HirId, hasher: &mut StableHasher);
1717
fn hash_body_id(&mut self, _: BodyId, hasher: &mut StableHasher);
1818
fn hash_reference_to_item(&mut self, _: HirId, hasher: &mut StableHasher);
19-
fn hash_hir_mod(&mut self, _: &Mod<'_>, hasher: &mut StableHasher);
2019
fn hash_hir_expr(&mut self, _: &Expr<'_>, hasher: &mut StableHasher);
2120
fn hash_hir_ty(&mut self, _: &Ty<'_>, hasher: &mut StableHasher);
2221
fn hash_hir_visibility_kind(&mut self, _: &VisibilityKind<'_>, hasher: &mut StableHasher);
@@ -132,12 +131,6 @@ impl<HirCtx: crate::HashStableContext> HashStable<HirCtx> for TraitItemId {
132131
}
133132
}
134133

135-
impl<HirCtx: crate::HashStableContext> HashStable<HirCtx> for Mod<'_> {
136-
fn hash_stable(&self, hcx: &mut HirCtx, hasher: &mut StableHasher) {
137-
hcx.hash_hir_mod(self, hasher)
138-
}
139-
}
140-
141134
impl<HirCtx: crate::HashStableContext> HashStable<HirCtx> for Expr<'_> {
142135
fn hash_stable(&self, hcx: &mut HirCtx, hasher: &mut StableHasher) {
143136
hcx.hash_hir_expr(self, hasher)

Diff for: compiler/rustc_query_system/src/ich/impls_hir.rs

+1-24
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@
33
44
use crate::ich::hcx::BodyResolver;
55
use crate::ich::{NodeIdHashingMode, StableHashingContext};
6-
use rustc_data_structures::fingerprint::Fingerprint;
7-
use rustc_data_structures::stable_hasher::{HashStable, StableHasher, ToStableHashKey};
6+
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
87
use rustc_hir as hir;
98
use std::mem;
109

@@ -47,28 +46,6 @@ impl<'ctx> rustc_hir::HashStableContext for StableHashingContext<'ctx> {
4746
})
4847
}
4948

50-
#[inline]
51-
fn hash_hir_mod(&mut self, module: &hir::Mod<'_>, hasher: &mut StableHasher) {
52-
let hcx = self;
53-
let hir::Mod { inner: ref inner_span, ref item_ids } = *module;
54-
55-
inner_span.hash_stable(hcx, hasher);
56-
57-
// Combining the `DefPathHash`s directly is faster than feeding them
58-
// into the hasher. Because we use a commutative combine, we also don't
59-
// have to sort the array.
60-
let item_ids_hash = item_ids
61-
.iter()
62-
.map(|id| {
63-
let def_path_hash = id.to_stable_hash_key(hcx);
64-
def_path_hash.0
65-
})
66-
.fold(Fingerprint::ZERO, |a, b| a.combine_commutative(b));
67-
68-
item_ids.len().hash_stable(hcx, hasher);
69-
item_ids_hash.hash_stable(hcx, hasher);
70-
}
71-
7249
fn hash_hir_expr(&mut self, expr: &hir::Expr<'_>, hasher: &mut StableHasher) {
7350
self.while_hashing_hir_bodies(true, |hcx| {
7451
let hir::Expr { hir_id: _, ref span, ref kind } = *expr;

Diff for: src/test/incremental/hash-module-order.rs

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// revisions: rpass1 rpass2
2+
// compile-flags: -Z incremental-ignore-spans -Z query-dep-graph
3+
4+
// Tests that module hashing depends on the order of the items
5+
// (since the order is exposed through `Mod.item_ids`).
6+
// Changing the order of items (while keeping `Span`s the same)
7+
// should still result in `hir_owner` being invalidated.
8+
// Note that it's possible to keep the spans unchanged using
9+
// a proc-macro (e.g. producing the module via `quote!`)
10+
// but we use `-Z incremental-ignore-spans` for simplicity
11+
12+
#![feature(rustc_attrs)]
13+
14+
#[cfg(rpass1)]
15+
#[rustc_clean(cfg="rpass1",except="hir_owner")]
16+
mod foo {
17+
struct First;
18+
struct Second;
19+
}
20+
21+
#[cfg(rpass2)]
22+
#[rustc_clean(cfg="rpass2",except="hir_owner")]
23+
mod foo {
24+
struct Second;
25+
struct First;
26+
}
27+
28+
fn main() {}

0 commit comments

Comments
 (0)