Skip to content

Port #[used] to new attribute parsing infrastructure #142818

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

Port #[used] to new attribute parsing infrastructure #142818

wants to merge 1 commit into from
+185 −125

Conversation

JonathanBrouwer
Copy link
Contributor

Ports used to the new attribute parsing infrastructure for #131229 (comment)

r? @jdonszelmann

@rustbot
Copy link
Collaborator

rustbot commented Jun 21, 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 21, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 21, 2025

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

Some changes occurred in compiler/rustc_codegen_ssa/src/codegen_attrs.rs

cc @jdonszelmann

Some changes occurred in compiler/rustc_attr_data_structures

cc @jdonszelmann

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann

JonathanBrouwer
JonathanBrouwer commented Jun 21, 2025
View reviewed changes
@jdonszelmann jdonszelmann mentioned this pull request Jun 21, 2025
Open
9 tasks
jdonszelmann
jdonszelmann requested changes Jun 21, 2025
View reviewed changes
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 21, 2025
bjorn3
bjorn3 reviewed Jun 21, 2025
View reviewed changes
@JonathanBrouwer
Copy link
Contributor Author

@rustbot author

@rustbot
Copy link
Collaborator

rustbot commented Jun 21, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@jdonszelmann jdonszelmann mentioned this pull request Jun 21, 2025
Open
@bors
Copy link
Collaborator

bors commented Jun 21, 2025

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

@JonathanBrouwer
Copy link
Contributor Author

@rustbot ready
Implemented ratcheting behaviour & reverted change to the warning

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 22, 2025
@JonathanBrouwer JonathanBrouwer force-pushed the used_new_parser branch 2 times, most recently from e70cbef to 30d851d Compare June 22, 2025 13:02
@JonathanBrouwer
Copy link
Contributor Author

Rebased to solve merge conflicts

@bors
Copy link
Collaborator

bors commented Jun 22, 2025

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

@JonathanBrouwer JonathanBrouwer force-pushed the used_new_parser branch 2 times, most recently from be02b70 to 0031aab Compare June 23, 2025 16:10
@jdonszelmann
Copy link
Contributor

jdonszelmann commented Jun 23, 2025 •
edited
Loading

just to be clear @JonathanBrouwer, is the behavior right now exactly the same as before with regards to errors and which attribute applies in case of duplicates or not?

@JonathanBrouwer
Copy link
Contributor Author

@jdonszelmann
The behaviour regarding errors and duplicates is exactly the same when multiple identical attributes are specified. This provides a warning "unused attribute" without stating it will be removed.

When multiple different attributes are applied the behaviour has changed. This is fine because [used(arg)] with an argument is unstable.

What has changed:

#[used]
#[used(linker)]

Before: Error, "compiler and linker can't be used together". This is a wrong error since #[used] should be equivalent to #[used(linker)]
After: Changed: now a warning, unused attribute

#[used]
#[used(compiler)]

Before: No error, ratcheting behaviour so "linker " is used
After: unchanged

#[used(linker)]
#[used(compiler)]

Before: Error
After: Changed: Now also does ratcheting to linker.

@JonathanBrouwer
Copy link
Contributor Author

^ Rebased

@jdonszelmann
Copy link
Contributor

I'd say we should keep this an error if it is right now, since if we relax the requirement it's very hard to at some point make it an error again since that'd be a breaking change. Until T-lang decided on #142836 I'm not entirely comfortable with removing the error

@JonathanBrouwer
Copy link
Contributor Author

@jdonszelmann
I don't think there is any situation that doesn't require unstable features that is changed by this PR. It only affects cases where a #[used] attribute is used with arguments, which is unstable.

Would you want to keep all behaviour exactly as it was before? I think the old behaviour is really weird, we should either always ratchet or always error with different attrs (which is still my preference) ideally. I'm happy to implement the old behaviour if you really want though :)

@jdonszelmann
Copy link
Contributor

ah right, fair enough

@jdonszelmann
Copy link
Contributor

jdonszelmann commented Jun 25, 2025 •
edited
Loading

ok then I would approve but I'm suspending all attribute PRs for a little bit until #142777 goes through (see #t-compiler > attribute parsing rework @ 💬

@JonathanBrouwer
Copy link
Contributor Author

Sounds good, I'll keep an eye on when it gets through and rebase on it :)

@workingjubilee workingjubilee added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Jun 25, 2025
@jdonszelmann
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jun 26, 2025

📌 Commit 708f45f has been approved by jdonszelmann

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 26, 2025
@JonathanBrouwer
Port #[used] to new attribute parsing infrastructure
9b3f729 
Signed-off-by: Jonathan Brouwer <jonathantbrouwer@gmail.com>
@JonathanBrouwer
Copy link
Contributor Author

@jdonszelmann This one was not rebased on the encode-cross-crate PR yet, did that now ^

@jdonszelmann
Copy link
Contributor

Ah I see

@jdonszelmann
Copy link
Contributor

Whoops

@jdonszelmann
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 26, 2025

📌 Commit 9b3f729 has been approved by jdonszelmann

It is now in the queue for this repository.

compiler-errors added a commit to compiler-errors/rust that referenced this pull request Jun 26, 2025
@compiler-errors
Rollup merge of rust-lang#142818 - JonathanBrouwer:used_new_parser, r…
2dea2cf 
…=jdonszelmann

Port `#[used]` to new attribute parsing infrastructure

Ports `used` to the new attribute parsing infrastructure for rust-lang#131229 (comment)

r? `@jdonszelmann`
@compiler-errors compiler-errors mentioned this pull request Jun 26, 2025
Closed
@jdonszelmann
Copy link
Contributor

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 26, 2025
@bors
Copy link
Collaborator

bors commented Jun 27, 2025

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bjorn3 bjorn3 bjorn3 left review comments

@jdonszelmann jdonszelmann jdonszelmann requested changes

Assignees

@jdonszelmann jdonszelmann

Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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
@JonathanBrouwer @rustbot @bors @jdonszelmann @bjorn3 @workingjubilee