Skip to content

Resolver: Batched Import Resolution#145108

Open
LorrensP-2158466 wants to merge 3 commits into
rust-lang:mainfrom
LorrensP-2158466:batched-import-resolution
Open

Resolver: Batched Import Resolution#145108
LorrensP-2158466 wants to merge 3 commits into
rust-lang:mainfrom
LorrensP-2158466:batched-import-resolution

Conversation

@LorrensP-2158466
Copy link
Copy Markdown
Contributor

@LorrensP-2158466 LorrensP-2158466 commented Aug 8, 2025 •
edited by rustbot
Loading

View all comments

Transforms the current algorithm for resolving imports to a batched algorithm. Every import in the indeterminate_imports set is resolved in isolation. This is the only real difference from the current algorithm.

r? petrochenkov

@rustbot 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 Aug 8, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
LorrensP-2158466
LorrensP-2158466 commented Aug 8, 2025
View reviewed changes
Comment thread compiler/rustc_resolve/src/lib.rs Outdated
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@LorrensP-2158466 LorrensP-2158466 marked this pull request as ready for review August 11, 2025 08:37
@rustbot 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 Aug 11, 2025
LorrensP-2158466
LorrensP-2158466 commented Aug 11, 2025
View reviewed changes
Comment thread compiler/rustc_resolve/src/imports.rs Outdated
@rust-log-analyzer

This comment has been minimized.

Sign in to view
petrochenkov
petrochenkov reviewed Aug 11, 2025
View reviewed changes
Comment thread compiler/rustc_resolve/src/imports.rs Outdated
Comment thread compiler/rustc_resolve/src/imports.rs Outdated
Comment thread compiler/rustc_resolve/src/imports.rs Outdated
Comment thread compiler/rustc_resolve/src/imports.rs Outdated
@petrochenkov 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 Aug 11, 2025
@LorrensP-2158466
Copy link
Copy Markdown
Contributor Author

LorrensP-2158466 commented Aug 11, 2025 •
edited
Loading

Code looks a little better now and should be more correct than the previous commits, but I have yet to find a way to resolve the current test failures.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@LorrensP-2158466
Copy link
Copy Markdown
Contributor Author

LorrensP-2158466 commented Aug 11, 2025

Okey, I did fix core not being built, but those other errors happened again. We now resolve the prelude import path when we create such an import at build_reduced_graph phase.

@LorrensP-2158466 LorrensP-2158466 force-pushed the batched-import-resolution branch from a3f8ae2 to 4a2a0dc Compare August 11, 2025 20:37
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@petrochenkov
Copy link
Copy Markdown
Contributor

petrochenkov commented Aug 11, 2025

Could you update the tests to make CI green, so I can see the difference?
(Even if the changes do not seem correct.)

@petrochenkov petrochenkov 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 Aug 12, 2025
LorrensP-2158466
LorrensP-2158466 commented Aug 12, 2025
View reviewed changes
Comment thread tests/ui/imports/ambiguous-9.rs
LorrensP-2158466
LorrensP-2158466 commented Aug 12, 2025
View reviewed changes
Comment thread tests/ui/imports/glob-conflict-cross-crate-1.rs
LorrensP-2158466
LorrensP-2158466 commented Aug 12, 2025
View reviewed changes
Comment thread tests/ui/proc-macro/derive-helper-legacy-spurious.rs
LorrensP-2158466
LorrensP-2158466 commented Aug 12, 2025
View reviewed changes
Comment thread tests/ui/proc-macro/derive-helper-legacy-spurious.rs
@petrochenkov
Copy link
Copy Markdown
Contributor

petrochenkov commented Aug 12, 2025

Moving prelude_import processing to build_reduced_graph may also be necessary for #139493.

@LorrensP-2158466
Copy link
Copy Markdown
Contributor Author

LorrensP-2158466 commented Aug 12, 2025

I'll create a pr for it.

petrochenkov
petrochenkov reviewed Aug 12, 2025
View reviewed changes
Comment thread compiler/rustc_resolve/src/imports.rs Outdated
Comment thread compiler/rustc_resolve/src/imports.rs Outdated
Comment thread compiler/rustc_resolve/src/imports.rs Outdated
@LorrensP-2158466
Copy link
Copy Markdown
Contributor Author

LorrensP-2158466 commented Jun 5, 2026

I am thinking of asking the maintainers of that crate/generator to include those imports, as they know the correct trait to use. Can I ask in t-libs/stdarch?

@petrochenkov
Copy link
Copy Markdown
Contributor

petrochenkov commented Jun 5, 2026

I am thinking of asking the maintainers of that crate/generator to include those imports, as they know the correct trait to use. Can I ask in t-libs/stdarch?

Yes, of course

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Jun 5, 2026

stdarch is developed in its own repository. If possible, consider making this change to rust-lang/stdarch instead.

cc @Amanieu, @folkertdev, @sayantn

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Jun 5, 2026

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.

@LorrensP-2158466
Copy link
Copy Markdown
Contributor Author

LorrensP-2158466 commented Jun 5, 2026

@rustbot ready

Rebased to keep up to date and added the generator fix to see it on CI.

petrochenkov
petrochenkov reviewed Jun 5, 2026
View reviewed changes
Comment thread library/stdarch/crates/stdarch-gen-arm/src/main.rs Outdated
Comment thread tests/ui/imports/duplicate.rs
@petrochenkov
Copy link
Copy Markdown
Contributor

petrochenkov commented Jun 5, 2026

r=me after addressing the remaining comments.
@rustbot author

@LorrensP-2158466
Resolver: Batched import resolution.
c4d84db 
Import resolution now happens in 2 phases:
1. We resolve all undetermined and collect their resolutions
2. Write all resolutions to the Resolver state.

Repeat this untill we reach a fix point.

+ Bless tests
@LorrensP-2158466
hack for RustEmbed with updated test
b09dbec
@LorrensP-2158466
Add extra line in generator script for core_arch that unambiguously…
fe7c7a1 
… imports the correct traits for `aarch64`.

+ run the generator to apply change.
@LorrensP-2158466
Copy link
Copy Markdown
Contributor Author

LorrensP-2158466 commented Jun 6, 2026

@bors r=petrochenkov

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Jun 6, 2026

@LorrensP-2158466: 🔑 Insufficient privileges: not in review users

@petrochenkov
Copy link
Copy Markdown
Contributor

petrochenkov commented Jun 6, 2026

@bors r+

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Jun 6, 2026

📌 Commit fe7c7a1 has been approved by petrochenkov

It is now in the queue for this repository.

@traviscross
Copy link
Copy Markdown
Contributor

traviscross commented Jun 6, 2026

cc @rust-lang/lang-docs @rust-lang/fls

@jhpratt
Copy link
Copy Markdown
Member

jhpratt commented Jun 7, 2026

Moving ahead of a prioritized but rollup-able PR

@bors p=2

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Jun 7, 2026 •
edited
Loading

⌛ Testing commit fe7c7a1 with merge 2a0f475...

Workflow: https://github.com/rust-lang/rust/actions/runs/27078725841

@jhpratt
Copy link
Copy Markdown
Member

jhpratt commented Jun 7, 2026

yet another PR will msvc stalled

@bors yield

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Jun 7, 2026

Auto build was cancelled. Cancelled workflows:

The next pull request likely to be tested is #157558.

@jhpratt
Copy link
Copy Markdown
Member

jhpratt commented Jun 7, 2026

@bors try jobs=dist-i686-msvc

@rust-bors

This comment has been minimized.

Sign in to view
@jhpratt
Copy link
Copy Markdown
Member

jhpratt commented Jun 7, 2026

rolling back priority

@bors p=0

@JonathanBrouwer
Copy link
Copy Markdown
Contributor

JonathanBrouwer commented Jun 7, 2026

@bors rollup=iffy
The queue is starting to get very large and this is only not rollupable because of insignificant perf effects

This was referenced Jun 7, 2026
Closed
Closed
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Jun 7, 2026

💔 Test for 3eeb2b3 failed: CI

@JonathanBrouwer JonathanBrouwer mentioned this pull request Jun 7, 2026
Closed
@JonathanBrouwer
Copy link
Copy Markdown
Contributor

JonathanBrouwer commented Jun 7, 2026

(spurious)

@JonathanBrouwer JonathanBrouwer mentioned this pull request Jun 7, 2026
Open
@jhpratt jhpratt mentioned this pull request Jun 7, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@petrochenkov petrochenkov petrochenkov left review comments

+1 more reviewer

@jplatte jplatte jplatte left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

@petrochenkov petrochenkov

Labels

disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. I-lang-radar Items that are on lang's radar and will need eventual work or consideration. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue. to-announce Announce this issue on triage meeting waived-reference-pr This language change does not need a Reference PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.