Skip to content

Fix path resolution for child mods of those expanded by include! #17650

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

Merged
merged 1 commit into from
Jul 21, 2024

Conversation

ObsidianMinor
Copy link
Contributor

Child modules wouldn't use the correct candidate paths due to a branch that doesn't seem to be doing what it's intended to do. Removing the branch fixes the problem and all existing test cases pass.

Having no knowledge of how any of this works, I believe this fixes #17645. Using another test that writes the included mod directly into lib.rs instead, I found the difference can be traced to the candidate files we use to look up mods. A separate branch for if the file comes from an include! macro doesn't take into account the original mod we're contained within:

None if file_id.macro_file().map_or(false, |it| it.is_include_macro(db.upcast())) => {
    candidate_files.push(format!("{}.rs", name.display(db.upcast())));
    candidate_files.push(format!("{}/mod.rs", name.display(db.upcast())));
}

I'm not sure why this branch exists. Tracing the branch back takes us to 3bb9efb but it doesn't say why the branch was added. The test case that was added in this commit passes with the branch removed, so I think it's just superfluous at this point.

Child modules wouldn't use the correct candidate paths due to a branch that doesn't seem to be doing what it's intended to do. Removing the branch fixes the problem and all existing test cases pass.
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 20, 2024
@Veykril
Copy link
Member

Veykril commented Jul 21, 2024

That branch mightve gotten superceded by some recent changes to the include logic

@Veykril
Copy link
Member

Veykril commented Jul 21, 2024

Thanks!
@bors r+

@bors
Copy link
Contributor

bors commented Jul 21, 2024

📌 Commit b9469f5 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jul 21, 2024

⌛ Testing commit b9469f5 with merge 5f26438...

@bors
Copy link
Contributor

bors commented Jul 21, 2024

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

@bors bors merged commit 5f26438 into rust-lang:master Jul 21, 2024
11 checks passed
@ObsidianMinor ObsidianMinor deleted the fix/17645 branch July 21, 2024 06:38
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.

Modules inserted via include can't resolve later files
4 participants