Skip to content

Attribute rework: a parser for single attributes without arguments #142964

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Attribute rework: a parser for single attributes without arguments #142964

wants to merge 1 commit into from
+100 −114

Conversation

GrigorenkoPV
Copy link
Contributor

@GrigorenkoPV GrigorenkoPV commented Jun 24, 2025 •
edited
Loading

Part of #131229

r? @jdonszelmann

I think code (with comments) speaks for itself.

The only subtlety: now #[cold], #[no_mangle], & #[track_caller] do not get thrown away when malformed (i.e. have arguments). This doesn't matter too much (I think), because an error gets emitted either way, so the compilation will not finish.

@rustbot
Copy link
Collaborator

rustbot commented Jun 24, 2025

jdonszelmann is currently at their maximum review capacity.
They may take a while to respond.

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 24, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 24, 2025

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

JonathanBrouwer
JonathanBrouwer approved these changes Jun 24, 2025
View reviewed changes
Copy link
Contributor

@JonathanBrouwer JonathanBrouwer left a comment

Choose a reason for hiding this comment

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

We're gonna have a lot of these, seems like a good change to me

@GrigorenkoPV
Copy link
Contributor Author

Should I try to include a65ea33 into this PR too?

@bors
Copy link
Collaborator

bors commented Jun 25, 2025

☔ The latest upstream changes (presumably #142997) made this pull request unmergeable. Please resolve the merge conflicts.

@GrigorenkoPV GrigorenkoPV force-pushed the attributes/argless branch 2 times, most recently from 3c71263 to b350089 Compare June 26, 2025 10:58
@bors
Copy link
Collaborator

bors commented Jun 29, 2025

☔ The latest upstream changes (presumably #143173) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Collaborator

bors commented Jun 30, 2025

☔ The latest upstream changes (presumably #143233) made this pull request unmergeable. Please resolve the merge conflicts.

JonathanBrouwer
JonathanBrouwer reviewed Jul 1, 2025
View reviewed changes
compiler/rustc_attr_parsing/src/attributes/mod.rs

impl<T: NoArgsAttributeParser<S>, S: Stage> SingleAttributeParser<S> for WithoutArgs<T, S> {
const PATH: &[Symbol] = T::PATH;
const ATTRIBUTE_ORDER: AttributeOrder = AttributeOrder::KeepLast;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is KeepLast acceptable here? Are there no single attributes which have KeepFirst?

@JonathanBrouwer
Copy link
Contributor

JonathanBrouwer commented Jul 1, 2025 •
edited
Loading

I think my personal preference would be having only the macro and not the extra trait (the macro generating a SingleAttributeParser). I'm not a member of T-compiler though so please wait for their feedback before changing the PR. Regardless of what form ends up being accepted, I think getting this PR through would make converting all the internal rustc internal attributes a lot nicer so should get some priority.

Jana is on holidays so r? @oli-obk

@rustbot rustbot assigned oli-obk and unassigned jdonszelmann Jul 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JonathanBrouwer JonathanBrouwer JonathanBrouwer approved these changes

Assignees

@oli-obk oli-obk

Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@GrigorenkoPV @rustbot @bors @JonathanBrouwer @oli-obk @jdonszelmann