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: derive source scope from syntax node to be transformed #14989

Merged
merged 2 commits into from Jun 11, 2023

Conversation

lowr
Copy link
Contributor

@lowr lowr commented Jun 5, 2023

Fixes #14534

When we use PathTransform for associated items of a trait, we have been feeding SemanticsScope for the trait definition to it as source scope. PathTransform uses the source scope to resolve paths in associated items to find which path to transform. In the course of path resolution, the scope is responsible for lowering ast::MacroTypes (because they can be written within a path) using AstIdMap for the scope's HirFileId.

The problem here is that when an associated item is generated by a macro, the scope for the trait is different from the scope for that associated item. The former can only resolve the top-level macros within the trait definition but not the macro calls generated by those top-level macros. We need the latter to resolve such nested macros.

This PR makes sure that we pass SemanticsScope for each associated item we're applying path transformation to.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 5, 2023
@lowr lowr force-pushed the fix/nested-macro-in-assoc-item-panic branch from ea7bc2d to 7df5768 Compare June 6, 2023 10:06
@Veykril
Copy link
Member

Veykril commented Jun 9, 2023

So that was the problem
@bors r+

@bors
Copy link
Collaborator

bors commented Jun 9, 2023

📌 Commit 7df5768 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jun 9, 2023

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

  1. git checkout fix/nested-macro-in-assoc-item-panic (switch to your branch)
  2. git fetch upstream master (retrieve the latest master)
  3. git rebase upstream/master -p (rebase on top of it)
  4. Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
  5. git push self fix/nested-macro-in-assoc-item-panic --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message
Auto-merging crates/ide-assists/src/utils.rs
CONFLICT (content): Merge conflict in crates/ide-assists/src/utils.rs
Auto-merging crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs
CONFLICT (content): Merge conflict in crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs
Auto-merging crates/ide-assists/src/handlers/add_missing_impl_members.rs
CONFLICT (content): Merge conflict in crates/ide-assists/src/handlers/add_missing_impl_members.rs
Automatic merge failed; fix conflicts and then commit the result.

@lowr lowr force-pushed the fix/nested-macro-in-assoc-item-panic branch from 7df5768 to d091991 Compare June 11, 2023 06:25
@lnicola
Copy link
Member

lnicola commented Jun 11, 2023

@bors r=Veykril

@bors
Copy link
Collaborator

bors commented Jun 11, 2023

📌 Commit d091991 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jun 11, 2023

⌛ Testing commit d091991 with merge c3bab96...

@bors
Copy link
Collaborator

bors commented Jun 11, 2023

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

@bors bors merged commit c3bab96 into rust-lang:master Jun 11, 2023
10 checks passed
bors added a commit that referenced this pull request Jun 11, 2023
…nicola

minor: remove commented out conflicts

Follow-up to #14989 🤦‍♂️
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.

Panic at 'Can't find MACRO_CALL@[..] in AstIdMap'
5 participants