Check arguments of attributes where no arguments are expected

[JonathanBrouwer]

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]

⚠️ #[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

[JonathanBrouwer]

@bors try

[JonathanBrouwer]

@bors try cancel
@bors try

[rust-bors]

Try build cancelled. Cancelled workflows:

• https://github.com/rust-lang/rust/actions/runs/24305234582

[rust-bors]

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

[JonathanBrouwer]

@craterbot check

[craterbot]

👌 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

[JonathanBrouwer]

@rustbot ready

[traviscross]

Thanks @JonathanBrouwer. Looks good to me. I propose we do it, accepting the breakage.

@rfcbot fcp merge lang

[rust-rfcbot]

Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

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.
See this document for info about what commands tagged team members can give me.

[traviscross]

The Reference already looks correct as it relates to this change.

cc @rust-lang/lang-docs @rust-lang/fls

[joshtriplett]

Crater results seem reasonable. The one regression has no reverse-deps. And the author already sent a PR to that crate. ❤️

[tmandry]

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

To clarify the process (this may be what you meant), we should have lang approval for any breaking change, even if there are no crater regressions.

Thank you for the cleanup and work here!

@rfcbot reviewed

[rust-rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔