Port the proc macro attributes to the new attribute parsing infrastructure #143607
Conversation
rustbot
added
A-attributes
|
|
||
| // Not a built-in macro | ||
| None => (None, helper_attrs), | ||
| }; |
There was a problem hiding this comment.
The logic here should be equivalent to the logic above, other than that this is a bit more readable and that we use the already parsed attribute rather than parsing it from scratch
| @@ -57,7 +57,7 @@ | |||
| // see gated-link-args.rs | |||
| // see issue-43106-gating-of-macro_escape.rs for crate-level; but non crate-level is below at "2700" | |||
| // (cannot easily test gating of crate-level #[no_std]; but non crate-level is below at "2600") | |||
| #![proc_macro_derive()] //~ WARN `#[proc_macro_derive]` only has an effect | |||
| #![proc_macro_derive(Test)] //~ WARN `#[proc_macro_derive]` only has an effect | |||
There was a problem hiding this comment.
Note that a breaking change happened here! The code before this change would now produce an error.
Previously a proc_macro_derive applied to the crate could have any arguments it wants, it was not checked. This PR fixes this bug, and this now errors.
I discussed this privately with @jdonszelmann, and she said this is fine to change, though we can consider doing a crater run for this just to make sure if you wish.
rustbot
added
the
S-waiting-on-review
|
Some changes occurred in compiler/rustc_attr_parsing Some changes occurred in compiler/rustc_passes/src/check_attr.rs Some changes occurred in compiler/rustc_attr_data_structures |
This comment has been minimized.
This comment has been minimized.
|
Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
|
This could use a little squash I think |
|
Not everything, but some commits are a bit small |
JonathanBrouwer
force-pushed
the
proc_macro_attrs
branch
from
a6b7f55 to
432cde9
Compare
|
@jdonszelmann I've squashed things down a bit. I'm experimenting a bit with what commit size is best for reviewability, this was clearly too much of a good thing :P |
JonathanBrouwer
force-pushed
the
proc_macro_attrs
branch
from
432cde9 to
7a60c84
Compare
|
r? @jdonszelmann |
|
|
|
☔ The latest upstream changes (presumably #143645) made this pull request unmergeable. Please resolve the merge conflicts. |
JonathanBrouwer
force-pushed
the
proc_macro_attrs
branch
3 times, most recently
from
2b42dae to
f8c757f
Compare
|
^ Rebased |
Port the proc macro attributes to the new attribute parsing infrastructure Ports `#[proc_macro]`, `#[proc_macro_attribute]`, `#[proc_macro_derive]` and `#[rustc_builtin_macro]` to the new attribute parsing infrastructure for #131229 (comment) I've split this PR into commits for reviewability, and left some comments to clarify things I did 4 related attributes in one PR because they share a lot of their code and logic, and doing them separately is kind of annoying as I need to leave both the old and new parsing in place then. r? `@oli-obk` cc `@jdonszelmann`
|
@traviscross what's the proper way to notify T-lang on this? What label, and could you add that? |
|
Crater results, 3 reported regressions:
So no crates are affected by this change |
JonathanBrouwer
force-pushed
the
proc_macro_attrs
branch
from
f8c757f to
3b773a0
Compare
|
Rebased |
JonathanBrouwer
force-pushed
the
proc_macro_attrs
branch
from
3b773a0 to
e0a495b
Compare
|
☔ The latest upstream changes (presumably #143958) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Perfect, @rust-lang/lang crater seems clear, r=me with your approval |
Signed-off-by: Jonathan Brouwer <[email protected]>
Signed-off-by: Jonathan Brouwer <[email protected]>
Signed-off-by: Jonathan Brouwer <[email protected]>
Signed-off-by: Jonathan Brouwer <[email protected]>
Signed-off-by: Jonathan Brouwer <[email protected]>
|
^ Rebased |
JonathanBrouwer
force-pushed
the
proc_macro_attrs
branch
from
e0a495b to
8166ced
Compare
|
It looks like the breaking change here is that when applied in the wrong place, this was previously a warning and bailed out before it also checked the arguments of the attribute, and now it'll also produce an error if the attribute arguments aren't valid. @rfcbot merge |
|
Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. |
rfcbot
added
proposed-final-comment-period
|
@rfcbot reviewed |
1 similar comment
|
@rfcbot reviewed |
rfcbot
added
final-comment-period
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
traviscross
added
I-lang-radar
|
☔ The latest upstream changes (presumably #144109) made this pull request unmergeable. Please resolve the merge conflicts. |
Ports
#[proc_macro],#[proc_macro_attribute],#[proc_macro_derive]and#[rustc_builtin_macro]to the new attribute parsing infrastructure for #131229 (comment)I've split this PR into commits for reviewability, and left some comments to clarify things
I did 4 related attributes in one PR because they share a lot of their code and logic, and doing them separately is kind of annoying as I need to leave both the old and new parsing in place then.
r? @oli-obk
cc @jdonszelmann