Skip to content

Add target checking to #[register_tool]#157377

Open
JonathanBrouwer wants to merge 1 commit into
rust-lang:mainfrom
JonathanBrouwer:target-register-tool
Open

Add target checking to #[register_tool]#157377
JonathanBrouwer wants to merge 1 commit into
rust-lang:mainfrom
JonathanBrouwer:target-register-tool

Conversation

@JonathanBrouwer
Copy link
Copy Markdown
Contributor

r? @mejrs

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Jun 3, 2026

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 Jun 3, 2026
@rust-bors

This comment has been minimized.

@rustbot

This comment has been minimized.

Copy link
Copy Markdown
Contributor

@mejrs mejrs left a comment

Choose a reason for hiding this comment

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

I'm not sure I understand crate level attributes, actually 🤔

View changes since this review

Comment on lines +107 to 110
#[register_tool(xyz)]
//~^ WARN crate-level attribute should be an inner attribute
unreachable!();
}
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.

Can this be an error? I notice crate level attrs are basically allowed anywhere, I'd like that not to be the case with a new crate level attribute if I can help it.

@rust-bors

This comment has been minimized.

@JonathanBrouwer
Copy link
Copy Markdown
Contributor Author

The reason that this is a warning is because it historically was. When we initially wrote the attribute parsers the first goal was preserving behavior.

I think it makes sense to change this tho, and I think the time has come that for all unstable attributes we should change this. I'll open a PR tomorrow

Copy link
Copy Markdown
Contributor

@mejrs mejrs left a comment

Choose a reason for hiding this comment

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

Sounds good.

@bors r+ rollup

View changes since this review

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Jun 4, 2026

📋 This PR cannot be approved because it has merge conflicts. Please resolve the conflicts and try again.

@mejrs mejrs 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 4, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Jun 4, 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

@bors r=mejrs

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Jun 4, 2026

📌 Commit 957e505 has been approved by mejrs

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 4, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jun 4, 2026
…ol, r=mejrs

Add target checking to `#[register_tool]`

r? @mejrs
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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

3 participants