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: use ItemInNs::Macros to convert ModuleItem to ItemInNs #17469

Merged
merged 1 commit into from
Jun 21, 2024

Conversation

roife
Copy link
Member

@roife roife commented Jun 21, 2024

fix #17425.

When converting PathResolution to ItemInNs, we should convert ModuleDef::Macro to ItemInNs::Macros to ensure that it can be found in DefMap.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 21, 2024
@roife
Copy link
Member Author

roife commented Jun 21, 2024

I'm not sure whether it's correct since I am not very familiar with Ns.

What's the difference between ItemInNs::Types(ModuleDef::Macro) and ItemInNs::Macros?👀

@lnicola

This comment was marked as outdated.

@Veykril
Copy link
Member

Veykril commented Jun 21, 2024

ItemInNs carries the namespace this resolution is coming from/supposed to be treated as being in. So ItemInNs::Types(ModuleDef::Macro) means a Macro in a type namespace context which is technically non-sensical, as macros can only exist in the macro namespace. The reason for this ItemInNs system is that some things can be in different namespaces, a unit struct for example can be in the type namespace given its a type but also in the value namespace as it can refer to its unit constructor.

@roife
Copy link
Member Author

roife commented Jun 21, 2024

Thanks, I get it. Then this PR should be OK.

@Veykril
Copy link
Member

Veykril commented Jun 21, 2024

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 21, 2024

📌 Commit d5bb2f0 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jun 21, 2024

⌛ Testing commit d5bb2f0 with merge c4681ea...

@bors
Copy link
Collaborator

bors commented Jun 21, 2024

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

@bors bors merged commit c4681ea into rust-lang:master Jun 21, 2024
11 checks passed
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.

Some custom snippet definitions do not work
5 participants