Resolve: refactor `define` into `define_local` and `define_extern` by LorrensP-2158466 · Pull Request #143884 · rust-lang/rust

LorrensP-2158466 · 2025-07-13T11:08:38Z

Follow up on #143550 and part of #gsoc > Project: Parallel Macro Expansion.

Split up define into define_local and define_extern. Refactor usages of define into either one where it's "correct" (idk if everything is correct atm). Big part of this is that resolution can now take a &Resolver instead of a mutable one.

r? @petrochenkov

compiler/rustc_resolve/src/build_reduced_graph.rs

compiler/rustc_resolve/src/imports.rs

compiler/rustc_resolve/src/lib.rs

compiler/rustc_resolve/src/imports.rs

LorrensP-2158466 · 2025-07-13T20:35:22Z

Conflicts must be resolved in Petrochenkov's PRs, I think. I can then rebase off that.

petrochenkov · 2025-07-14T13:21:34Z

Blocked on #143550.
@rustbot blocked

compiler/rustc_resolve/src/build_reduced_graph.rs

resolve: Use interior mutability for extern module map Module map for extern modules is a lazily populated cache, it's not *significantly* mutable. If some logic in name resolver is parallelized, then this cache can be populated from any thread, and without affecting results of any speculative resolution. Unblocks rust-lang#143884. This is a part of [#gsoc > Project: Parallel Macro Expansion](https://rust-lang.zulipchat.com/#narrow/channel/421156-gsoc/topic/Project.3A.20Parallel.20Macro.20Expansion/with/527348747). cc `@LorrensP-2158466`

Rollup merge of #143550 - petrochenkov:lessmutres, r=lcnr resolve: Use interior mutability for extern module map Module map for extern modules is a lazily populated cache, it's not *significantly* mutable. If some logic in name resolver is parallelized, then this cache can be populated from any thread, and without affecting results of any speculative resolution. Unblocks #143884. This is a part of [#gsoc > Project: Parallel Macro Expansion](https://rust-lang.zulipchat.com/#narrow/channel/421156-gsoc/topic/Project.3A.20Parallel.20Macro.20Expansion/with/527348747). cc `@LorrensP-2158466`

LorrensP-2158466 · 2025-07-17T18:49:56Z

I couldn't find any more &mut Resolver that I could turn into &Resolver. Everything now either has finalize codepaths and/or lints/errors getting added to their respective collections.

resolve: Make disambiguators for underscore bindings module-local Disambiguators attached to underscore name bindings (like `const _: u8 = something;`) do not need to be globally unique, they just need to be unique inside the module in which they live, because the bindings in a module are basically kept as `Map<BindingKey, SomeData>`. Also, the specific values of the disambiguators are not important, so a glob import of `const _` may have a different disambiguator than the original `const _` itself. So in this PR the disambiguator is just set to the current number of bindings in the module. This removes one more piece of global mutable state from resolver and unblocks rust-lang#143884.

resolve: Use interior mutability for extern module map Module map for extern modules is a lazily populated cache, it's not *significantly* mutable. If some logic in name resolver is parallelized, then this cache can be populated from any thread, and without affecting results of any speculative resolution. Unblocks rust-lang/rust#143884. This is a part of [#gsoc > Project: Parallel Macro Expansion](https://rust-lang.zulipchat.com/#narrow/channel/421156-gsoc/topic/Project.3A.20Parallel.20Macro.20Expansion/with/527348747). cc `@LorrensP-2158466`

resolve: Make disambiguators for underscore bindings module-local Disambiguators attached to underscore name bindings (like `const _: u8 = something;`) do not need to be globally unique, they just need to be unique inside the module in which they live, because the bindings in a module are basically kept as `Map<BindingKey, SomeData>`. Also, the specific values of the disambiguators are not important, so a glob import of `const _` may have a different disambiguator than the original `const _` itself. So in this PR the disambiguator is just set to the current number of bindings in the module. This removes one more piece of global mutable state from resolver and unblocks rust-lang#143884.

Rollup merge of #144013 - petrochenkov:disambunder, r=oli-obk resolve: Make disambiguators for underscore bindings module-local Disambiguators attached to underscore name bindings (like `const _: u8 = something;`) do not need to be globally unique, they just need to be unique inside the module in which they live, because the bindings in a module are basically kept as `Map<BindingKey, SomeData>`. Also, the specific values of the disambiguators are not important, so a glob import of `const _` may have a different disambiguator than the original `const _` itself. So in this PR the disambiguator is just set to the current number of bindings in the module. This removes one more piece of global mutable state from resolver and unblocks #143884.

LorrensP-2158466 · 2025-07-27T10:18:32Z

@rustbot ready

petrochenkov · 2025-07-27T11:12:45Z

@bors r+

bors · 2025-07-27T11:12:47Z

📌 Commit 451a93e has been approved by petrochenkov

It is now in the queue for this repository.

bors · 2025-07-27T12:40:49Z

⌛ Testing commit 451a93e with merge edc3841...

bors · 2025-07-27T15:42:44Z

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing edc3841 to master...

github-actions · 2025-07-27T15:45:36Z

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing eed187c (parent) -> edc3841 (this PR)

Test differences

No test diffs found

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard edc3841c5d28e0f54c6d3c7e906ad083129f3903 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

x86_64-apple-2: 4320.0s -> 6344.8s (46.9%)
i686-gnu-1: 8734.6s -> 7346.3s (-15.9%)
dist-apple-various: 6381.9s -> 7369.0s (15.5%)
pr-check-2: 2666.3s -> 2274.6s (-14.7%)
x86_64-gnu-llvm-19: 2879.4s -> 2461.1s (-14.5%)
dist-ohos-x86_64: 4113.2s -> 4676.2s (13.7%)
dist-x86_64-apple: 8969.3s -> 10008.7s (11.6%)
i686-gnu-2: 5998.3s -> 5314.7s (-11.4%)
aarch64-gnu-llvm-19-2: 2490.0s -> 2223.6s (-10.7%)
pr-check-1: 1677.6s -> 1511.5s (-9.9%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

rust-timer · 2025-07-27T16:53:23Z

Finished benchmarking commit (edc3841): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

If the regression was expected or you think it can be justified,
please write a comment with sufficient written justification, and add
@rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
If you think that you know of a way to resolve the regression, try to create
a new PR with a fix for the regression.
If you do not understand the regression or you think that it is just noise,
you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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.1%	[0.1%, 0.1%]	3
Regressions ❌ (secondary)	0.5%	[0.3%, 0.7%]	5
Improvements ✅ (primary)	-0.5%	[-0.8%, -0.2%]	15
Improvements ✅ (secondary)	-0.3%	[-0.5%, -0.2%]	27
All ❌✅ (primary)	-0.4%	[-0.8%, 0.1%]	18

Max RSS (memory usage)

Results (primary 0.8%, secondary -3.6%)

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

	mean	range	count
Regressions ❌ (primary)	1.8%	[1.2%, 2.3%]	2
Regressions ❌ (secondary)	2.5%	[2.5%, 2.5%]	1
Improvements ✅ (primary)	-1.2%	[-1.2%, -1.2%]	1
Improvements ✅ (secondary)	-4.9%	[-6.9%, -1.8%]	5
All ❌✅ (primary)	0.8%	[-1.2%, 2.3%]	3

Cycles

Results (primary 2.4%, 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)	2.4%	[2.4%, 2.4%]	1
Regressions ❌ (secondary)	2.7%	[2.7%, 2.7%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.5%	[-2.5%, -2.5%]	1
All ❌✅ (primary)	2.4%	[2.4%, 2.4%]	1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 468.356s -> 469.387s (0.22%)
Artifact size: 376.74 MiB -> 376.71 MiB (-0.01%)

resolve: Make disambiguators for underscore bindings module-local Disambiguators attached to underscore name bindings (like `const _: u8 = something;`) do not need to be globally unique, they just need to be unique inside the module in which they live, because the bindings in a module are basically kept as `Map<BindingKey, SomeData>`. Also, the specific values of the disambiguators are not important, so a glob import of `const _` may have a different disambiguator than the original `const _` itself. So in this PR the disambiguator is just set to the current number of bindings in the module. This removes one more piece of global mutable state from resolver and unblocks rust-lang/rust#143884.

resolve: Make disambiguators for underscore bindings module-local (take 2) The difference with rust-lang/rust#144013 can be seen in the second commit. Now we just keep a separate disambiguator counter in every `Module`, instead of a global counter in `Resolver`. This will be ok for parallel import resolution because we'll need to lock the module anyway when updating `resolutions` and other fields in it. And for external modules the disabmiguator could be just passed as an argument to `define_extern`, without using any cells or locks, once rust-lang/rust#143884 lands. Unblocks rust-lang/rust#143884.

resolve: Make disambiguators for underscore bindings module-local Disambiguators attached to underscore name bindings (like `const _: u8 = something;`) do not need to be globally unique, they just need to be unique inside the module in which they live, because the bindings in a module are basically kept as `Map<BindingKey, SomeData>`. Also, the specific values of the disambiguators are not important, so a glob import of `const _` may have a different disambiguator than the original `const _` itself. So in this PR the disambiguator is just set to the current number of bindings in the module. This removes one more piece of global mutable state from resolver and unblocks rust-lang/rust#143884.

resolve: Make disambiguators for underscore bindings module-local (take 2) The difference with rust-lang/rust#144013 can be seen in the second commit. Now we just keep a separate disambiguator counter in every `Module`, instead of a global counter in `Resolver`. This will be ok for parallel import resolution because we'll need to lock the module anyway when updating `resolutions` and other fields in it. And for external modules the disabmiguator could be just passed as an argument to `define_extern`, without using any cells or locks, once rust-lang/rust#143884 lands. Unblocks rust-lang/rust#143884.

resolve: Make disambiguators for underscore bindings module-local Disambiguators attached to underscore name bindings (like `const _: u8 = something;`) do not need to be globally unique, they just need to be unique inside the module in which they live, because the bindings in a module are basically kept as `Map<BindingKey, SomeData>`. Also, the specific values of the disambiguators are not important, so a glob import of `const _` may have a different disambiguator than the original `const _` itself. So in this PR the disambiguator is just set to the current number of bindings in the module. This removes one more piece of global mutable state from resolver and unblocks rust-lang#143884.

resolve: Make disambiguators for underscore bindings module-local (take 2) The difference with rust-lang#144013 can be seen in the second commit. Now we just keep a separate disambiguator counter in every `Module`, instead of a global counter in `Resolver`. This will be ok for parallel import resolution because we'll need to lock the module anyway when updating `resolutions` and other fields in it. And for external modules the disabmiguator could be just passed as an argument to `define_extern`, without using any cells or locks, once rust-lang#143884 lands. Unblocks rust-lang#143884.

resolve: Make disambiguators for underscore bindings module-local Disambiguators attached to underscore name bindings (like `const _: u8 = something;`) do not need to be globally unique, they just need to be unique inside the module in which they live, because the bindings in a module are basically kept as `Map<BindingKey, SomeData>`. Also, the specific values of the disambiguators are not important, so a glob import of `const _` may have a different disambiguator than the original `const _` itself. So in this PR the disambiguator is just set to the current number of bindings in the module. This removes one more piece of global mutable state from resolver and unblocks rust-lang#143884.

resolve: Make disambiguators for underscore bindings module-local (take 2) The difference with rust-lang#144013 can be seen in the second commit. Now we just keep a separate disambiguator counter in every `Module`, instead of a global counter in `Resolver`. This will be ok for parallel import resolution because we'll need to lock the module anyway when updating `resolutions` and other fields in it. And for external modules the disabmiguator could be just passed as an argument to `define_extern`, without using any cells or locks, once rust-lang#143884 lands. Unblocks rust-lang#143884.

resolve: Make disambiguators for underscore bindings module-local Disambiguators attached to underscore name bindings (like `const _: u8 = something;`) do not need to be globally unique, they just need to be unique inside the module in which they live, because the bindings in a module are basically kept as `Map<BindingKey, SomeData>`. Also, the specific values of the disambiguators are not important, so a glob import of `const _` may have a different disambiguator than the original `const _` itself. So in this PR the disambiguator is just set to the current number of bindings in the module. This removes one more piece of global mutable state from resolver and unblocks rust-lang/rust#143884.

resolve: Make disambiguators for underscore bindings module-local (take 2) The difference with rust-lang/rust#144013 can be seen in the second commit. Now we just keep a separate disambiguator counter in every `Module`, instead of a global counter in `Resolver`. This will be ok for parallel import resolution because we'll need to lock the module anyway when updating `resolutions` and other fields in it. And for external modules the disabmiguator could be just passed as an argument to `define_extern`, without using any cells or locks, once rust-lang/rust#143884 lands. Unblocks rust-lang/rust#143884.

rustbot assigned petrochenkov Jul 13, 2025

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 13, 2025

LorrensP-2158466 commented Jul 13, 2025

View reviewed changes

compiler/rustc_resolve/src/build_reduced_graph.rs Outdated Show resolved Hide resolved

LorrensP-2158466 commented Jul 13, 2025

View reviewed changes

compiler/rustc_resolve/src/imports.rs Show resolved Hide resolved

LorrensP-2158466 commented Jul 13, 2025

View reviewed changes

compiler/rustc_resolve/src/lib.rs Outdated Show resolved Hide resolved

LorrensP-2158466 commented Jul 13, 2025

View reviewed changes

compiler/rustc_resolve/src/imports.rs Outdated Show resolved Hide resolved

This comment was marked as resolved.

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 14, 2025

petrochenkov reviewed Jul 14, 2025

View reviewed changes

compiler/rustc_resolve/src/build_reduced_graph.rs Outdated Show resolved Hide resolved

This was referenced Jul 16, 2025

resolve: Make disambiguators for underscore bindings module-local #144013

Merged

resolve: Use interior mutability for extern module map #143550

Merged

LorrensP-2158466 force-pushed the resolve-split-define branch from 8515b3c to 1296920 Compare July 17, 2025 15:57

This comment has been minimized.

Sign in to view

LorrensP-2158466 force-pushed the resolve-split-define branch from 89cf98b to 4497a13 Compare July 17, 2025 18:47

LorrensP-2158466 marked this pull request as ready for review July 17, 2025 19:57

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 17, 2025

LorrensP-2158466 changed the title ~~[WIP] Resolve: refactor define into define_local and define_extern~~ Resolve: refactor define into define_local and define_extern Jul 17, 2025

This comment was marked as resolved.

Sign in to view

petrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 18, 2025

split up define into define_extern and define_local

451a93e

LorrensP-2158466 force-pushed the resolve-split-define branch from e889081 to 451a93e Compare July 27, 2025 10:18

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 Jul 27, 2025

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 27, 2025

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 27, 2025

bors merged commit edc3841 into rust-lang:master Jul 27, 2025
11 checks passed

rustbot added this to the 1.90.0 milestone Jul 27, 2025

This was referenced Jul 27, 2025

Constify trait aliases #144291

Merged

Introduce ModernIdent type to unify macro 2.0 hygiene handling #144439

Merged

Mark-Simulacrum added the perf-regression-triaged The performance regression has been triaged. label Jul 29, 2025

Uh oh!

Conversation

LorrensP-2158466 commented Jul 13, 2025 • edited by petrochenkov 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!

This comment was marked as resolved.

LorrensP-2158466 commented Jul 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrochenkov commented Jul 14, 2025

Uh oh!

Uh oh!

This comment has been minimized.

LorrensP-2158466 commented Jul 17, 2025

Uh oh!

This comment was marked as resolved.

LorrensP-2158466 commented Jul 27, 2025

Uh oh!

petrochenkov commented Jul 27, 2025

Uh oh!

bors commented Jul 27, 2025

Uh oh!

bors commented Jul 27, 2025

Uh oh!

bors commented Jul 27, 2025

Uh oh!

Uh oh!

github-actions bot commented Jul 27, 2025

Test differences

Job duration changes

Uh oh!

rust-timer commented Jul 27, 2025

Overall result: ❌✅ regressions and improvements - please read the text below

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

LorrensP-2158466 commented Jul 13, 2025 •

edited by petrochenkov

Loading

LorrensP-2158466 commented Jul 13, 2025 •

edited

Loading