delegation: fix def path hash collision, add per owner disambiguators#153955

aerooneqq · 2026-03-16T13:46:04Z

View all comments

This PR addresses the following delegation issues:

It fixes [ICE]: found DefPathHash collision between DefPath #153410 when generating new DefIds for generic parameters by ~~saving DisambiguatorStates from resolve stage and using them at AST -> HIR lowering~~ introducing per owner disambiguators and transferring them to AST -> HIR lowering stage
~~Next it fixes the ICE which is connected to using DUMMY_SP in delegation code, which was found during previous fix~~
~~Finally, after those fixes the ICE: delegation: DefId::expect_local DefId(..) isn't local #143498 is also fixed, only bugs with propagating synthetic generic params are left.~~

Fixes #153410. ~~Fixes #143498~~. Part of #118212.

r? @petrochenkov

tests/ui/delegation/generics/trait-impl-wrong-args-count.stderr

tests/ui/delegation/generics/def-id-isnt-local-ice-143498.rs

compiler/rustc_resolve/src/def_collector.rs

petrochenkov · 2026-03-16T16:01:54Z

This is all pretty awful.
A number of other def ids / def paths are also created during AST lowering, but they don't require keeping per-node disambiguator tables around. Why is delegation so special in this regard?

What exactly definitions create collisions like this? Generic parameters?

As I understand the small per-node disambiguator tables partially duplicate content from the big table.
What prevents us from looking up the current disambiguators in the big table instead?

aerooneqq · 2026-03-17T12:34:49Z

A number of other def ids / def paths are also created during AST lowering, but they don't require keeping per-node disambiguator tables around. Why is delegation so special in this regard?

We generate new DefIds for each delegation generic param, as creation of DefId requires a name, we want to use names that are defined in a function being reused, for example for proper error messages. It is possible to create DefIds with, for example, sym::dummy name, but in this case error messages will be terrible. If we prefer to use names of generic params of the delegee, then we use names that are created by user, so they can clash with already used names in delegation.

The difference between delegation and other DefIds creation is that latter don't use user-defined names, as other cases of DefIds creation are DefPathData::DesugaredAnonymousLifetime which uses Some(kw::UnderscoreLifetime) as name and DefPathData::LateAnonConst which uses None as name.

Maybe we can create our own DefPathData variant and use it during creation of DefIds, but it still will need to hold the name of the param, as it is required for example in hir_ty_param_name. I am not sure that this is a correct option.

What exactly definitions create collisions like this? Generic parameters?

From our side yes, when we create generic params, their def path data can be either DefPathData::TypeNs(symbol) for DefKind::TyParam, DefPathData::ValueNs(symbol) for DefKind::ConstParam or DefPathData::LifetimeNs(symbol) for DefKind::LifetimeParam, so this def path data can have a collision with anything that have same def path created in scope of delegation LocalDefId during resolve stage.

What prevents us from looking up the current disambiguators in the big table instead?

Big table which is Resolver::disambiguator is not passed anywhere, as DefIds which are created during AST -> HIR lowering before this PR do not need it. Passing the whole disambiguator to lowering stage seems to be not the best option as it contains many unneeded information and it is not passed from Resolver to ResolverAstLowering now.

Internally DisambiguatorState uses UnordMap<(LocalDefId, DefPathData), u32> which prevents from fast lookups by LocalDefId only and changing it (for example to UnordMap<LocalDefId, UnordMap<DefPathData, u32>>) seemed to be out of scope of this PR.

Given this underlying structure I also didn't want to iterate over all map to find delegation-related key-values, and I also didn't want to expose the possibility of traversal of whole DisambiguatorState.

This left me with an option that I implemented in this PR: for each delegation we create additional disambiguator which follows the main one and then stored in delegation_infos map, for this purpose I exposed next operation on DisambiguatorState, which is also not that good, beacuase it was inlined in create_def function, thus only create_def could update it, but in the given conditions I think it is more-or-less OK solution.

aerooneqq · 2026-03-17T12:47:20Z

@rustbot ready

To get feedback on the comments.

rustbot · 2026-03-19T09:51:24Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

petrochenkov · 2026-03-19T14:05:47Z

This PR may be relevant:

Stop passing resolver disambiguator state to AST lowering. #143361

It solved the collision problem by adding new DefPathData variants.

petrochenkov · 2026-03-19T14:23:32Z

Blocked on #154049.
Can try the approach with new DefPathData variants in the meantime.
@rustbot blocked

rustbot · 2026-03-20T13:17:39Z

Some changes occurred in compiler/rustc_sanitizers

cc @rcvalle

rust-timer · 2026-04-07T17:59:08Z

Finished benchmarking commit (99547e4): comparison URL.

Overall result: ❌ regressions - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.2%	[0.1%, 0.3%]	12
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 1.0%, secondary 0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	1.0%	[0.7%, 1.4%]	3
Regressions ❌ (secondary)	2.8%	[0.9%, 4.8%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.3%	[-1.7%, -1.1%]	4
All ❌✅ (primary)	1.0%	[0.7%, 1.4%]	3

Cycles

Results (primary 2.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.2%	[2.2%, 2.2%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.2%	[2.2%, 2.2%]	1

Binary size

Results (primary 0.1%, secondary 0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.0%, 0.3%]	125
Regressions ❌ (secondary)	0.1%	[0.0%, 0.9%]	63
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[0.0%, 0.3%]	125

Bootstrap: 487.434s -> 490.624s (0.65%)
Artifact size: 395.10 MiB -> 395.15 MiB (0.01%)

petrochenkov · 2026-04-08T10:26:29Z

@bors try @rust-timer queue

delegation: fix def path hash collision, add per owner disambiguators

petrochenkov · 2026-04-09T10:30:01Z

@bors try @rust-timer queue

delegation: fix def path hash collision, add per owner disambiguators

rust-bors · 2026-04-09T12:37:47Z

☀️ Try build successful (CI)
Build commit: 8888ea1 (8888ea18828032d998b603c7ba3b9bce0f9e4615, parent: 1948ee19e95ef7835624c591eef11a8838b66ec7)

rust-timer · 2026-04-09T13:18:20Z

Finished benchmarking commit (8888ea1): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	0.2%	[0.2%, 0.2%]	1
Regressions ❌ (secondary)	0.2%	[0.1%, 0.3%]	7
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.6%	[-3.1%, -0.1%]	2
All ❌✅ (primary)	0.2%	[0.2%, 0.2%]	1

Max RSS (memory usage)

Results (primary -1.1%, secondary 0.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.7%	[2.7%, 2.7%]	1
Improvements ✅ (primary)	-1.1%	[-1.1%, -1.0%]	2
Improvements ✅ (secondary)	-1.1%	[-1.4%, -0.7%]	2
All ❌✅ (primary)	-1.1%	[-1.1%, -1.0%]	2

Cycles

Results (secondary -5.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-5.5%	[-5.5%, -5.5%]	1
All ❌✅ (primary)	-	-	0

Binary size

Results (primary 0.1%, secondary 0.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.0%, 0.3%]	79
Regressions ❌ (secondary)	0.2%	[0.0%, 0.9%]	41
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[0.0%, 0.3%]	79

Bootstrap: 490.984s -> 490.081s (-0.18%)
Artifact size: 395.64 MiB -> 395.67 MiB (0.01%)

petrochenkov · 2026-04-09T13:30:36Z

Perf looks ok.

petrochenkov · 2026-04-09T14:04:07Z

compiler/rustc_hir/src/definitions.rs

+}
+
+#[derive(Debug, Default, Clone)]
+pub struct PerDefDisambiguatorState {


View changes since the review

Suggested change

pub struct PerDefDisambiguatorState {

pub struct PerParentDisambiguatorState {

petrochenkov · 2026-04-09T14:04:56Z

compiler/rustc_ast_lowering/src/lib.rs

    tcx: TyCtxt<'hir>,
    resolver: &'a mut R,
-    disambiguator: DisambiguatorState,
+    owner_disambiguator: PerDefDisambiguatorState,


View changes since the review

Suggested change

owner_disambiguator: PerDefDisambiguatorState,

disambiguator: PerDefDisambiguatorState,

Can keep the old field name here, I think.

petrochenkov · 2026-04-09T14:07:36Z

compiler/rustc_middle/src/ty/mod.rs

    // Information about delegations which is used when handling recursive delegations
    pub delegation_infos: LocalDefIdMap<DelegationInfo>,
+
+    pub per_owner_disambiguators: LocalDefIdMap<PerDefDisambiguatorState>,


View changes since the review

Suggested change

pub per_owner_disambiguators: LocalDefIdMap<PerDefDisambiguatorState>,

pub per_parent_disambiguators: LocalDefIdMap<PerDefDisambiguatorState>,

Still per-parent here.

rustbot assigned petrochenkov Mar 16, 2026

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 16, 2026

bjorn3 reviewed Mar 16, 2026

View reviewed changes

tests/ui/delegation/generics/trait-impl-wrong-args-count.stderr Show resolved Hide resolved

petrochenkov reviewed Mar 16, 2026

View reviewed changes

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 16, 2026

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 17, 2026

aerooneqq mentioned this pull request Mar 17, 2026

Select proper span for delegation, update span of child path segment if needed #154002

Closed

This comment has been minimized.

Sign in to view

aerooneqq force-pushed the def-path-hash-collision branch from fbb4d60 to b9ef605 Compare March 19, 2026 09:51

This comment has been minimized.

Sign in to view

rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 19, 2026

aerooneqq mentioned this pull request Mar 19, 2026

ty-aware delayed AST -> HIR lowering #153489

Merged

aerooneqq force-pushed the def-path-hash-collision branch from 5c0e8e9 to ef583eb Compare March 20, 2026 13:17

rustbot added the PG-exploit-mitigations Project group: Exploit mitigations label Mar 20, 2026

aerooneqq changed the title ~~Fix delegation def path hash collision, fix ICE connected to dummy spans~~ Fix delegation def path hash collision Mar 20, 2026

aerooneqq marked this pull request as draft March 20, 2026 14:31

rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 20, 2026

This comment has been minimized.

Sign in to view

aerooneqq force-pushed the def-path-hash-collision branch from ef583eb to 5b48d28 Compare March 25, 2026 13:57

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Apr 7, 2026

petrochenkov added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 7, 2026

User per definition disambiguators

cd53e9f

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 8, 2026

This comment has been minimized.

Sign in to view

rust-bors bot pushed a commit that referenced this pull request Apr 8, 2026

Auto merge of #153955 - aerooneqq:def-path-hash-collision, r=<try>

3957b97

delegation: fix def path hash collision, add per owner disambiguators

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 8, 2026

petrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 8, 2026

rust-bors bot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 8, 2026

This comment was marked as outdated.

Sign in to view

This comment has been minimized.

Sign in to view

rust-bors bot pushed a commit that referenced this pull request Apr 9, 2026

Auto merge of #153955 - aerooneqq:def-path-hash-collision, r=<try>

8888ea1

delegation: fix def path hash collision, add per owner disambiguators

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 9, 2026

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 9, 2026

petrochenkov reviewed Apr 9, 2026

View reviewed changes

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 9, 2026

	pub struct PerDefDisambiguatorState {
	pub struct PerParentDisambiguatorState {

	owner_disambiguator: PerDefDisambiguatorState,
	disambiguator: PerDefDisambiguatorState,

	pub per_owner_disambiguators: LocalDefIdMap<PerDefDisambiguatorState>,
	pub per_parent_disambiguators: LocalDefIdMap<PerDefDisambiguatorState>,

Uh oh!

Conversation

aerooneqq commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

petrochenkov commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aerooneqq commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aerooneqq commented Mar 17, 2026

Uh oh!

This comment has been minimized.

rustbot commented Mar 19, 2026

Uh oh!

This comment has been minimized.

petrochenkov commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrochenkov commented Mar 19, 2026

Uh oh!

rustbot commented Mar 20, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-timer commented Apr 7, 2026

Overall result: ❌ regressions - please read the text below

Uh oh!

petrochenkov commented Apr 8, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment was marked as outdated.

petrochenkov commented Apr 9, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors bot commented Apr 9, 2026

Uh oh!

This comment has been minimized.

rust-timer commented Apr 9, 2026

Overall result: ❌✅ regressions and improvements - please read:

Uh oh!

petrochenkov commented Apr 9, 2026

Uh oh!

petrochenkov Apr 9, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petrochenkov Apr 9, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petrochenkov Apr 9, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

aerooneqq commented Mar 16, 2026 •

edited

Loading

petrochenkov commented Mar 16, 2026 •

edited

Loading

aerooneqq commented Mar 17, 2026 •

edited

Loading

petrochenkov commented Mar 19, 2026 •

edited

Loading

petrochenkov Apr 9, 2026 •

edited by rustbot

Loading

petrochenkov Apr 9, 2026 •

edited by rustbot

Loading

petrochenkov Apr 9, 2026 •

edited by rustbot

Loading