Skip to content

Commit 4f5db2c

Browse files
committed
Disambiguate expansions per DepNode.
1 parent cdd8e9d commit 4f5db2c

File tree

3 files changed

+75
-32
lines changed

3 files changed

+75
-32
lines changed

compiler/rustc_middle/src/query/on_disk_cache.rs

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -630,20 +630,7 @@ impl<'a, 'tcx> Decodable<CacheDecoder<'a, 'tcx>> for ExpnId {
630630

631631
let data: ExpnData = decoder
632632
.with_position(pos.to_usize(), |decoder| decode_tagged(decoder, TAG_EXPN_DATA));
633-
let expn_id = rustc_span::hygiene::register_local_expn_id(data, hash);
634-
635-
#[cfg(debug_assertions)]
636-
{
637-
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
638-
let local_hash = decoder.tcx.with_stable_hashing_context(|mut hcx| {
639-
let mut hasher = StableHasher::new();
640-
expn_id.expn_data().hash_stable(&mut hcx, &mut hasher);
641-
hasher.finish()
642-
});
643-
debug_assert_eq!(hash.local_hash(), local_hash);
644-
}
645-
646-
expn_id
633+
rustc_span::hygiene::register_local_expn_id(data, hash)
647634
} else {
648635
let index_guess = decoder.foreign_expn_data[&hash];
649636
decoder.tcx.cstore_untracked().expn_hash_to_expn_id(

compiler/rustc_middle/src/ty/context.rs

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
pub mod tls;
66

77
use crate::arena::Arena;
8-
use crate::dep_graph::{DepGraph, DepKindStruct};
8+
use crate::dep_graph::{CurrentDepNode, DepGraph, DepKindStruct, DepNode};
99
use crate::infer::canonical::CanonicalVarInfo;
1010
use crate::lint::struct_lint_level;
1111
use crate::metadata::ModChild;
@@ -902,19 +902,41 @@ impl<'tcx> TyCtxt<'tcx> {
902902
}
903903
}
904904

905+
const ENSURE_FAKE_DEP_NODE_SYNC: () = if std::mem::size_of::<rustc_span::hygiene::FakeDepNode>()
906+
!= std::mem::size_of::<DepNode>()
907+
{
908+
panic!(
909+
"rustc_span::hygiene::FakeDepNode has come out of sync with DepNode and should be updated."
910+
)
911+
};
912+
905913
impl<'tcx> TyCtxt<'tcx> {
906914
#[instrument(level = "trace", skip(self), ret)]
907915
pub fn create_expansion(self, expn_data: ExpnData) -> LocalExpnId {
916+
let current_node = tls::with_related_context(self, |icx| icx.current_node);
917+
let CurrentDepNode::Regular(DepNode { kind, hash }) = current_node else {
918+
bug!("creating an expansion outside of a query")
919+
};
920+
let _ = ENSURE_FAKE_DEP_NODE_SYNC;
921+
let fake_dep_node = rustc_span::hygiene::FakeDepNode { kind: kind as _, hash };
908922
self.with_stable_hashing_context(|ctx| {
909-
LocalExpnId::create_untracked_expansion(expn_data, ctx)
923+
LocalExpnId::create_untracked_expansion(expn_data, fake_dep_node, ctx)
910924
})
911925
}
912926

913927
/// Fill an empty expansion. This method must not be used outside of the resolver.
914928
#[inline]
915929
#[instrument(level = "trace", skip(self))]
916930
pub fn finalize_expansion(self, expn_id: LocalExpnId, expn_data: ExpnData) {
917-
self.with_stable_hashing_context(|ctx| expn_id.set_untracked_expn_data(expn_data, ctx));
931+
let current_node = tls::with_related_context(self, |icx| icx.current_node);
932+
let CurrentDepNode::Regular(DepNode { kind, hash }) = current_node else {
933+
bug!("creating an expansion outside of a query")
934+
};
935+
let _ = ENSURE_FAKE_DEP_NODE_SYNC;
936+
let fake_dep_node = rustc_span::hygiene::FakeDepNode { kind: kind as _, hash };
937+
self.with_stable_hashing_context(|ctx| {
938+
expn_id.set_untracked_expn_data(expn_data, fake_dep_node, ctx)
939+
});
918940
}
919941

920942
/// Reuses the span but adds information like the kind of the desugaring and features that are

compiler/rustc_span/src/hygiene.rs

Lines changed: 49 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ use crate::with_session_globals;
3030
use crate::{HashStableContext, Span, DUMMY_SP};
3131

3232
use crate::def_id::{CrateNum, DefId, StableCrateId, CRATE_DEF_ID, LOCAL_CRATE};
33-
use rustc_data_structures::fingerprint::Fingerprint;
33+
use rustc_data_structures::fingerprint::{Fingerprint, PackedFingerprint};
3434
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
3535
use rustc_data_structures::stable_hasher::HashingControls;
3636
use rustc_data_structures::stable_hasher::{Hash64, HashStable, StableHasher};
@@ -195,34 +195,44 @@ impl LocalExpnId {
195195
#[instrument(level = "trace", skip(ctx), ret)]
196196
pub fn create_untracked_expansion(
197197
mut expn_data: ExpnData,
198+
dep_node: FakeDepNode,
198199
ctx: impl HashStableContext,
199200
) -> LocalExpnId {
200201
debug_assert_eq!(expn_data.parent.krate, LOCAL_CRATE);
201-
let expn_hash = update_disambiguator(&mut expn_data, ctx);
202+
let expn_hash = update_disambiguator(&mut expn_data, dep_node, ctx);
202203
HygieneData::with(|data| {
203204
let expn_id = data.local_expn_data.push(Some(expn_data));
204205
let _eid = data.local_expn_hashes.push(expn_hash);
205206
debug_assert_eq!(expn_id, _eid);
206207
let _old_id = data.expn_hash_to_expn_id.insert(expn_hash, expn_id.to_expn_id());
207-
debug_assert!(_old_id.is_none());
208+
if !_old_id.is_none() {
209+
panic!("Hash collision while creating expansion. Cannot continue.");
210+
}
208211
expn_id
209212
})
210213
}
211214

212215
/// Implementation detail of `TyCtxt::finalize_expansion`.
213216
#[inline]
214217
#[instrument(level = "trace", skip(ctx))]
215-
pub fn set_untracked_expn_data(self, mut expn_data: ExpnData, ctx: impl HashStableContext) {
218+
pub fn set_untracked_expn_data(
219+
self,
220+
mut expn_data: ExpnData,
221+
dep_node: FakeDepNode,
222+
ctx: impl HashStableContext,
223+
) {
216224
debug_assert_eq!(expn_data.parent.krate, LOCAL_CRATE);
217-
let expn_hash = update_disambiguator(&mut expn_data, ctx);
225+
let expn_hash = update_disambiguator(&mut expn_data, dep_node, ctx);
218226
HygieneData::with(|data| {
219227
let old_expn_data = &mut data.local_expn_data[self];
220228
assert!(old_expn_data.is_none(), "expansion data is reset for an expansion ID");
221229
*old_expn_data = Some(expn_data);
222230
debug_assert_eq!(data.local_expn_hashes[self].0, Fingerprint::ZERO);
223231
data.local_expn_hashes[self] = expn_hash;
224232
let _old_id = data.expn_hash_to_expn_id.insert(expn_hash, self.to_expn_id());
225-
debug_assert!(_old_id.is_none());
233+
if !_old_id.is_none() {
234+
panic!("Hash collision while creating expansion. Cannot continue.");
235+
}
226236
});
227237
}
228238

@@ -339,6 +349,14 @@ impl ExpnId {
339349
}
340350
}
341351

352+
/// This struct is meant to be a surrogate for the actual `DepNode` in rustc_middle.
353+
/// Both types should be kept in sync.
354+
#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
355+
pub struct FakeDepNode {
356+
pub kind: u16,
357+
pub hash: PackedFingerprint,
358+
}
359+
342360
#[derive(Debug)]
343361
pub struct HygieneData {
344362
/// Each expansion should have an associated expansion data, but sometimes there's a delay
@@ -358,7 +376,7 @@ pub struct HygieneData {
358376
/// would have collisions without a disambiguator.
359377
/// The keys of this map are always computed with `ExpnData.disambiguator`
360378
/// set to 0.
361-
expn_data_disambiguators: FxHashMap<Hash64, u32>,
379+
expn_data_disambiguators: FxHashMap<FakeDepNode, FxHashMap<Hash64, u32>>,
362380
}
363381

364382
impl HygieneData {
@@ -1031,9 +1049,10 @@ impl ExpnData {
10311049
}
10321050

10331051
#[inline]
1034-
fn hash_expn(&self, ctx: &mut impl HashStableContext) -> Hash64 {
1052+
fn hash_expn(&self, dep_node: FakeDepNode, ctx: &mut impl HashStableContext) -> Hash64 {
10351053
let mut hasher = StableHasher::new();
10361054
self.hash_stable(ctx, &mut hasher);
1055+
dep_node.hash(&mut hasher);
10371056
hasher.finish()
10381057
}
10391058
}
@@ -1248,7 +1267,9 @@ pub fn register_local_expn_id(data: ExpnData, hash: ExpnHash) -> ExpnId {
12481267
let expn_id = expn_id.to_expn_id();
12491268

12501269
let _old_id = hygiene_data.expn_hash_to_expn_id.insert(hash, expn_id);
1251-
debug_assert!(_old_id.is_none());
1270+
if !_old_id.is_none() {
1271+
panic!("Hash collision while creating expansion. Cannot continue.");
1272+
}
12521273
expn_id
12531274
})
12541275
}
@@ -1268,7 +1289,9 @@ pub fn register_expn_id(
12681289
let _old_hash = hygiene_data.foreign_expn_hashes.insert(expn_id, hash);
12691290
debug_assert!(_old_hash.is_none());
12701291
let _old_id = hygiene_data.expn_hash_to_expn_id.insert(hash, expn_id);
1271-
debug_assert!(_old_id.is_none());
1292+
if !_old_id.is_none() {
1293+
panic!("Hash collision while creating expansion. Cannot continue.");
1294+
}
12721295
});
12731296
expn_id
12741297
}
@@ -1452,16 +1475,27 @@ impl<D: Decoder> Decodable<D> for SyntaxContext {
14521475
/// `set_expn_data`). It is *not* called for foreign `ExpnId`s deserialized
14531476
/// from another crate's metadata - since `ExpnHash` includes the stable crate id,
14541477
/// collisions are only possible between `ExpnId`s within the same crate.
1455-
fn update_disambiguator(expn_data: &mut ExpnData, mut ctx: impl HashStableContext) -> ExpnHash {
1478+
#[instrument(level = "trace", skip(ctx), ret)]
1479+
fn update_disambiguator(
1480+
expn_data: &mut ExpnData,
1481+
dep_node: FakeDepNode,
1482+
mut ctx: impl HashStableContext,
1483+
) -> ExpnHash {
14561484
// This disambiguator should not have been set yet.
14571485
assert_eq!(expn_data.disambiguator, 0, "Already set disambiguator for ExpnData: {expn_data:?}");
14581486
assert_default_hashing_controls(&ctx, "ExpnData (disambiguator)");
1459-
let mut expn_hash = expn_data.hash_expn(&mut ctx);
1487+
let mut expn_hash = expn_data.hash_expn(dep_node, &mut ctx);
1488+
debug!(?expn_hash);
14601489

14611490
let disambiguator = HygieneData::with(|data| {
14621491
// If this is the first ExpnData with a given hash, then keep our
14631492
// disambiguator at 0 (the default u32 value)
1464-
let disambig = data.expn_data_disambiguators.entry(expn_hash).or_default();
1493+
let disambig = data
1494+
.expn_data_disambiguators
1495+
.entry(dep_node)
1496+
.or_default()
1497+
.entry(expn_hash)
1498+
.or_default();
14651499
let disambiguator = *disambig;
14661500
*disambig += 1;
14671501
disambiguator
@@ -1471,13 +1505,13 @@ fn update_disambiguator(expn_data: &mut ExpnData, mut ctx: impl HashStableContex
14711505
debug!("Set disambiguator for expn_data={:?} expn_hash={:?}", expn_data, expn_hash);
14721506

14731507
expn_data.disambiguator = disambiguator;
1474-
expn_hash = expn_data.hash_expn(&mut ctx);
1508+
expn_hash = expn_data.hash_expn(dep_node, &mut ctx);
14751509

14761510
// Verify that the new disambiguator makes the hash unique
14771511
#[cfg(debug_assertions)]
14781512
HygieneData::with(|data| {
14791513
assert_eq!(
1480-
data.expn_data_disambiguators.get(&expn_hash),
1514+
data.expn_data_disambiguators[&dep_node].get(&expn_hash),
14811515
None,
14821516
"Hash collision after disambiguator update!",
14831517
);

0 commit comments

Comments
 (0)