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 allow_attributes when expanded from some macros #13599

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

RuairidhWilliamson
Copy link
Contributor

fixes #13349

The issue here was that the start pattern being matched on the original source code was not specific enough. When using derive macros or in the issue case a #[repr(C)] the # would match the start pattern meaning that the expanded macro appeared to be unchanged and clippy would lint it.

The change I made was to make the matching more specific by matching #[ident at the start. We still need the second string to match just the ident on its own because of things like #[cfg_attr(panic = "unwind", allow(unused))].

I also noticed some typos with start and end, these code paths weren't being reached so this doesn't fix anything.

changelog: FP: [allow_attributes]: don't trigger when expanded from some macros

@rustbot
Copy link
Collaborator

rustbot commented Oct 24, 2024

r? @blyxyas

rustbot has assigned @blyxyas.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 24, 2024
Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking lintcheck, seems that this is only affecting to regular macros, and local macros at that point. Could you add an item to the test that checks that this works?

clippy_utils/src/check_proc_macro.rs Outdated Show resolved Hide resolved
@RuairidhWilliamson RuairidhWilliamson force-pushed the proc_macro_attr branch 2 times, most recently from 2fa4dc3 to 4a81320 Compare October 25, 2024 14:18
@RuairidhWilliamson
Copy link
Contributor Author

I've created a macro that recreates what bytemuck was doing and added it to the tests.

On the bright side, given how much effort it takes to create a macro that generates tokens where the spans start with # in the source code, this bug probably isn't that common.

Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! ❤️

@blyxyas
Copy link
Member

blyxyas commented Oct 30, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Oct 30, 2024

📌 Commit 59ecf4d has been approved by blyxyas

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Oct 30, 2024

⌛ Testing commit 59ecf4d with merge 1bdc08a...

@bors
Copy link
Contributor

bors commented Oct 30, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: blyxyas
Pushing 1bdc08a to master...

@bors bors merged commit 1bdc08a into rust-lang:master Oct 30, 2024
8 checks passed
@RuairidhWilliamson RuairidhWilliamson deleted the proc_macro_attr branch October 30, 2024 23:22
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.

allow_attributes lint triggers inside derive macro
4 participants