Skip to content

Move feature gating to the new attr parsing infrastructure#157195

Open
JonathanBrouwer wants to merge 4 commits into
rust-lang:mainfrom
JonathanBrouwer:gating-port
Open

Move feature gating to the new attr parsing infrastructure#157195
JonathanBrouwer wants to merge 4 commits into
rust-lang:mainfrom
JonathanBrouwer:gating-port

Conversation

@JonathanBrouwer
Copy link
Copy Markdown
Contributor

@JonathanBrouwer JonathanBrouwer commented May 31, 2026

Does the feature gate checking for attributes during attribute parsing, rather than during expansion.
This is one more step towards removing BUILTIN_ATTRIBUTES.

My goal was to keep the diagnostics mostly the same during this PR, and do some improvements and cleanups later, to reduce the diff. Some diagnostics still had minor changes to keep the implementation simpler.

This PR was fully manually written, I triple-checked that I didn't accidentally stabilize or change the gate of an attribute, but I'm terrified of a mistake still slipping through so please check this again.

r? @mejrs @jdonszelmann
cc @nnethercote @scrabsha @Bryntet

@rustbot rustbot added 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. labels May 31, 2026
const ATTRIBUTES: AcceptMapping<Self> = &[(
&[sym::diagnostic, sym::on_const],
template!(List: &[r#"/*opt*/ message = "...", /*opt*/ label = "...", /*opt*/ note = "...""#]),
AttributeStability::Stable, // Unstable, stability checked manually in the parser
Copy link
Copy Markdown
Contributor Author

@JonathanBrouwer JonathanBrouwer May 31, 2026

Choose a reason for hiding this comment

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

I'm considering adding special logic for this, but I'd prefer to do that in a separate PR as this one is already big enough

View changes since the review

cx: &mut AcceptContext<'_, '_>,
args: &ArgParser,
) -> impl IntoIterator<Item = Self::Item> {
if !cx.features().staged_api() {
Copy link
Copy Markdown
Contributor Author

@JonathanBrouwer JonathanBrouwer May 31, 2026

Choose a reason for hiding this comment

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

Not sure why specifically staged_api had a manual feature gate (and is marked as stable in builtin_attr), from looking at the git history this has always been the case?

View changes since the review

sym::lang_items => ("lang items are subject to change".to_string(), &[]),
sym::prelude_import => ("`#[prelude_import]` is for use by rustc only".to_string(), &[]),
sym::profiler_runtime => ("the `#[profiler_runtime]` attribute is used to identify the `profiler_builtins` crate which contains the profiler runtime and will never be stable".to_string(), &[]),
sym::thread_local => ("`#[thread_local]` is an experimental feature, and does not currently handle destructors".to_string(), &[]),
Copy link
Copy Markdown
Contributor Author

@JonathanBrouwer JonathanBrouwer May 31, 2026

Choose a reason for hiding this comment

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

I would like to change and fine-tune some of these messages, but I have kept them the same as much as easily possible in this PR and will send a follow-up PR to clean this up

View changes since the review


/// Attributes that have a special meaning to rustc or rustdoc.
#[rustfmt::skip]
pub static BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
Copy link
Copy Markdown
Contributor Author

@JonathanBrouwer JonathanBrouwer May 31, 2026

Choose a reason for hiding this comment

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

Goal is to make a follow-up PR to remove all of BUILTIN_ATTRIBUTES, we'll see how realistic that is when I get to it :P

View changes since the review

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this would possibly be a bit nicer if this was in a block that did use sym::*; so you could not have all the sym:: prefixes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't really like that because that would indent everything more, but also don't have strong opinions about it. If everything goes well this code will be gone soon anyways so 🤷

| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: add `#![feature(rustc_attrs)]` to the crate attributes to enable
= note: the `#[rustc_diagnostic_item]` attribute is an internal implementation detail that will never be stable
Copy link
Copy Markdown
Contributor Author

@JonathanBrouwer JonathanBrouwer May 31, 2026

Choose a reason for hiding this comment

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

rustc_diagnostic_item was the only attribute with the rustc_attrs feature gate that was excluded from this message, idk why I so I added it for consistency (and to not make the PR more complex)

View changes since the review

@JonathanBrouwer JonathanBrouwer marked this pull request as ready for review May 31, 2026 13:38
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 31, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 31, 2026

Some changes occurred to diagnostic attributes.

cc @mejrs

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

Development

Successfully merging this pull request may close these issues.

4 participants