Skip to content

Check arguments of attributes where no arguments are expected#155193

Open
JonathanBrouwer wants to merge 14 commits intorust-lang:mainfrom
JonathanBrouwer:args_used_check
Open

Check arguments of attributes where no arguments are expected#155193
JonathanBrouwer wants to merge 14 commits intorust-lang:mainfrom
JonathanBrouwer:args_used_check

Conversation

@JonathanBrouwer
Copy link
Copy Markdown
Contributor

@JonathanBrouwer JonathanBrouwer commented Apr 12, 2026 •
edited by rustbot
Loading

View all comments

This PR does the following:

  • Add a debug assertion to rustc_attr_parsing, to ensure we never forget to check the arguments of a meta item again
  • Removes the unused #[derive(Clone)] from ArgParser as that would break this debug assertion
  • [BREAKING] Properly check that #[inline(always(...))] gets no arguments
  • [BREAKING] Properly check that #[instruction_set(arm::a32(...))] gets no arguments
  • [BREAKING] Properly check that #[macro_export(local_inner_macros(...))] gets no arguments.
    Fixes Invalid value accepted for #[macro_export(local_inner_macros)] #154977
  • [BREAKING] Properly check that #[used(compiler(...))] gets no arguments.
  • Properly check that #[optimize(size(...))] gets no arguments.
  • Properly check that #[coverage(on(...))] gets no arguments.
  • Properly check that #[rustc_dump_layout(debug(...))] gets no arguments.
  • Properly check that #[rustc_abi(debug(...))] gets no arguments.
  • Properly check that #![test_runner(arg(...))] gets no arguments.
  • Properly check that #[rustc_must_implement_one_of(arg(...))] gets no arguments.
  • Properly check that #[allow_internal_unstable(arg(...))] gets no arguments.
  • Properly check that #[unstable_feature_bound(arg(...))] gets no arguments.
  • Properly check that #[rustc_allow_const_fn_unstable(arg(...))] gets no arguments.
  • Properly check that #[rustc_if_this_changed(arg(...))] gets no arguments.
  • Properly check that #[rustc_then_this_would_need(arg(...))] gets no arguments.

r? @jdonszelmann

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 12, 2026

⚠️ #[rustc_allow_const_fn_unstable] needs careful audit to avoid accidentally exposing unstable
implementation details on stable.

cc @rust-lang/wg-const-eval

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

@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 Apr 12, 2026
@JonathanBrouwer JonathanBrouwer mentioned this pull request Apr 12, 2026
Closed
@JonathanBrouwer
Copy link
Copy Markdown
Contributor Author

JonathanBrouwer commented Apr 12, 2026

@bors try

@rust-bors

This comment has been minimized.

Sign in to view
rust-bors Bot pushed a commit that referenced this pull request Apr 12, 2026
@bors
Auto merge of #155193 - JonathanBrouwer:args_used_check, r=<try>
d2ef8ec 
Check arguments of attributes where no arguments are expected
@JonathanBrouwer JonathanBrouwer added S-blocked Status: Blocked on something else such as an RFC or other implementation work. needs-crater This change needs a crater run to check for possible breakage in the ecosystem. labels Apr 12, 2026
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@JonathanBrouwer JonathanBrouwer force-pushed the args_used_check branch 2 times, most recently from 3231b2a to 15b2265 Compare April 12, 2026 11:18
@JonathanBrouwer
Copy link
Copy Markdown
Contributor Author

JonathanBrouwer commented Apr 12, 2026

@bors try cancel
@bors try

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Apr 12, 2026

Try build cancelled. Cancelled workflows:

rust-bors Bot pushed a commit that referenced this pull request Apr 12, 2026
@bors
Auto merge of #155193 - JonathanBrouwer:args_used_check, r=<try>
b98202a 
Check arguments of attributes where no arguments are expected
@rust-bors

This comment has been minimized.

Sign in to view
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Apr 12, 2026

☀️ Try build successful (CI)
Build commit: b98202a (b98202ad067d72e45cec8be3d5c15d86ef0fd086, parent: 540f43a224317d894a9a0710a8d67704f179a33c)

@JonathanBrouwer
Copy link
Copy Markdown
Contributor Author

JonathanBrouwer commented Apr 12, 2026

@craterbot check

@craterbot
Copy link
Copy Markdown
Collaborator

craterbot commented Apr 12, 2026

👌 Experiment pr-155193 created and queued.
🤖 Automatically detected try build b98202a
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Apr 12, 2026
jakubadamw
jakubadamw reviewed Apr 12, 2026
View reviewed changes
Comment thread compiler/rustc_attr_parsing/src/interface.rs Outdated
@rust-bors

This comment has been minimized.

Sign in to view
@rustbot

This comment has been minimized.

Sign in to view
@rust-bors

This comment has been minimized.

Sign in to view
@JonathanBrouwer JonathanBrouwer mentioned this pull request Apr 25, 2026
Closed
@craterbot
Copy link
Copy Markdown
Collaborator

craterbot commented Apr 27, 2026

🚧 Experiment pr-155193 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 29, 2026

Some changes occurred to diagnostic attributes.

cc @mejrs

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 29, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@JonathanBrouwer
Copy link
Copy Markdown
Contributor Author

JonathanBrouwer commented Apr 29, 2026 •
edited
Loading

^ Rebased, no other changes
(rustbot only triggered because the "Some changes occurred to diagnostic attributes." rule is new)

Random other question: This PR has a lot of commits, I can squash them but on the other hand all commits are independent changes and I kinda like them being separate, what to do here?

@craterbot
Copy link
Copy Markdown
Collaborator

craterbot commented Apr 29, 2026

🎉 Experiment pr-155193 is completed!
📊 12 regressed and 3 fixed (890207 total)
📊 5467 spurious results on the retry-regressed-list.txt, consider a retry1 if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Footnotes

  1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-155193/retry-regressed-list.txt ↩

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Apr 29, 2026
@JonathanBrouwer
Copy link
Copy Markdown
Contributor Author

JonathanBrouwer commented Apr 29, 2026 •
edited
Loading

11 regressed and 2 fixed are spurious.
1 regression is real, spectest-0.1.2 uses the incorrect #[inline(always = true)] syntax. The bug that allowed this is not recent (i.e. not introduced by the new parsing infrastructure) and has existed for years

Given that there is breakage, let's ask for T-lang approval: @rust-lang/lang

Given that this is quite a niche crate, I propose to make a PR to this crate and accept the breakage.

@JonathanBrouwer JonathanBrouwer added T-lang Relevant to the language team I-lang-nominated Nominated for discussion during a lang team meeting. I-lang-easy-decision Issue: The decision needed by the team is conjectured to be easy; this does not imply nomination labels Apr 29, 2026
@JonathanBrouwer JonathanBrouwer mentioned this pull request Apr 29, 2026
Open
@JonathanBrouwer
Copy link
Copy Markdown
Contributor Author

JonathanBrouwer commented Apr 29, 2026

PR submitted to spectest: aalexandrov/spectest#1

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

Reviewers

1 more reviewer

@jakubadamw jakubadamw jakubadamw left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

@jdonszelmann jdonszelmann

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) I-lang-easy-decision Issue: The decision needed by the team is conjectured to be easy; this does not imply nomination I-lang-nominated Nominated for discussion during a lang team meeting. needs-crater This change needs a crater run to check for possible breakage in the ecosystem. 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. T-lang Relevant to the language team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Invalid value accepted for #[macro_export(local_inner_macros)]

6 participants

@JonathanBrouwer @rustbot @rust-log-analyzer @craterbot @jakubadamw @jdonszelmann