-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix(disallowed_macros): Fix emitting on attr macros #15452
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
base: master
Are you sure you want to change the base?
Conversation
827b00d
to
5b3451a
Compare
fn check_attribute(&mut self, cx: &LateContext<'_>, attr: &Attribute) { | ||
let span = match attr { | ||
Attribute::Unparsed(attr_item) => attr_item.span, | ||
Attribute::Parsed(kind) => match kind { |
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.
I am not particularly happy with this giant match block.
There is a method that is perhaps meant to return the Attribute
's Span
, but it is not complete, and panics for many attributes. It would make sense for this match to move next to that span()
method if this is the right approach at all.
@@ -1,33 +1,11 @@ | |||
error: use of a disallowed macro `serde::Serialize` |
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.
The order here changed. The new order is correct and matches the order that was here prior to a regression that introduced the bug that this PR fixes.
See my first comment on the linked issue for more details.
5b3451a
to
50b3736
Compare
|
||
// When a macro is disallowed in an early pass, it's stored | ||
// and emitted during the late pass. This happens for attributes. | ||
early_macro_cache: AttrStorage, |
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.
It is not clear to me what the plan is for AttrStorage
and its usage in DisallowedMacros
and elsewhere. Since it was introduced early in the attribute refactor, perhaps it was a temporary solution.
@jdonszelmann Could you please take a look?
changelog: [
disallowed_macros
]: Fix emitting on attr macros. Fixes #14017.