Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix name resolution when an import is resolved to some namespace and then later in the algorithm another namespace is added #18160

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

ChayimFriedman2
Copy link
Contributor

@ChayimFriedman2 ChayimFriedman2 commented Sep 22, 2024

The import is flagged as "indeterminate", and previously it was re-resolved, but only at the end of name resolution, when it's already too late for anything that depends on it.

This issue was tried to fix in #2466, but it was not fixed fully.

That PR is also why IDE features did work: the import at the end was resolved correctly, so IDE features that re-resolved the macro path resolved it correctly.

I was concerned about the performance of this, but this doesn't seem to regress analysis-stats ., so I guess it's fine to land this. I have no idea about the incremental perf however and I don't know how to measure that, although when typing in zbus (including creating a new function, which should recompute the def map) completion was fast enough.

I didn't check what rustc does, so maybe it does something more performant, like keeping track of only possibly problematic imports.

Fixes #18138.
Probably fixes #17630.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 22, 2024
@ChayimFriedman2 ChayimFriedman2 changed the title Fix name resolution when an import is resolved to some namespace and then later in the algorithm another namespace is added fix: Fix name resolution when an import is resolved to some namespace and then later in the algorithm another namespace is added Sep 22, 2024
…then later in the algorithm another namespace is added

The import is flagged as "indeterminate", and previously it was re-resolved, but only at the end of name resolution, when it's already too late for anything that depends on it.

This issue was tried to fix in rust-lang#2466, but it was not fixed fully.
@Veykril
Copy link
Member

Veykril commented Sep 24, 2024

Thanks!
@bors r+

@bors
Copy link
Contributor

bors commented Sep 24, 2024

📌 Commit 432b255 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Sep 24, 2024

⌛ Testing commit 432b255 with merge a159b37...

@bors
Copy link
Contributor

bors commented Sep 24, 2024

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing a159b37 to master...

@bors bors merged commit a159b37 into rust-lang:master Sep 24, 2024
11 checks passed
@ChayimFriedman2 ChayimFriedman2 deleted the fix-18138 branch October 6, 2024 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rust-analyzer unable to see some types in zbus::fdo False positive diagnostic in proc-macro
4 participants