-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Replace in_macro
usage with from_expansion
#7897
Conversation
r? @flip1995 (rust-highfive has picked a reviewer for you, use r? to override) |
c1787f5
to
a7fdeab
Compare
Eh let's wait for the sync |
Ah nice, so the for-loop lowering PR got merged and this will get through without test regressions? Nice! I'll make sure to include this PR in the sync-back to rustc 👍 |
To be clear there are a few changes beyond simply replacing the function, but it's all fairly straightforward. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now also reviewed the other small changes.
Could you also update the Dealing with macros documentation in accordance to this change? It would be good to mention that it's generally better to use |
☔ The latest upstream changes (presumably #7929) made this pull request unmergeable. Please resolve the merge conflicts. |
Since this is a NFC, I decided to rather get the sync through as fast as possible, instead of trying to get this included in the sync. |
9b4d239
to
0c0416a
Compare
Rebased. Removed some redundant |
Thanks! This looks good now. About removing @bors r+ p=1 (codebase wide change) |
📌 Commit 5b1b6a2 has been approved by |
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
changelog: none
Generally replace
in_macro(span)
withspan.from_expansion()
. If we're just trying to avoid expanded code, this seems more appropriate because any kind of expanded code is prone to false positives. One place I did not touch ismacro_use.rs
. I think this lint could use a rewrite so I movedin_macro
there, the only place it is still used.