Skip to content

Remove attribute parsing Stage#155775

Open
JonathanBrouwer wants to merge 5 commits intorust-lang:mainfrom
JonathanBrouwer:cleanup_stage
Open

Remove attribute parsing Stage#155775
JonathanBrouwer wants to merge 5 commits intorust-lang:mainfrom
JonathanBrouwer:cleanup_stage

Conversation

@JonathanBrouwer
Copy link
Copy Markdown
Contributor

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

View all comments

I discovered that it is surprisingly easy to remove the concept of Stage in the attribute parsers, imo this makes the code a lot cleaner.

r? @jdonszelmann
cc @mejrs @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 Apr 25, 2026
@JonathanBrouwer
Copy link
Copy Markdown
Contributor Author

JonathanBrouwer commented Apr 25, 2026

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

Sign in to view
@rust-bors

This comment has been minimized.

Sign in to view
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 25, 2026
rust-bors Bot pushed a commit that referenced this pull request Apr 25, 2026
@bors
Auto merge of #155775 - JonathanBrouwer:cleanup_stage, r=<try>
0f16d2c 
Cleanup `Stage`
@JonathanBrouwer JonathanBrouwer changed the title Cleanup Stage Cleanup attribute parsing Stage Apr 25, 2026
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Apr 25, 2026

☀️ Try build successful (CI)
Build commit: 0f16d2c (0f16d2c2b65c0f611f58ef7ec91415060e26d515, parent: fb76025f2fe6a1c8ce03fe0931f941c8077a2c34)

@rust-timer

This comment has been minimized.

Sign in to view
@JonathanBrouwer JonathanBrouwer changed the title Cleanup attribute parsing Stage Remove attribute parsing Stage Apr 25, 2026
@rust-timer
Copy link
Copy Markdown
Collaborator

rust-timer commented Apr 25, 2026

Finished benchmarking commit (0f16d2c): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (secondary 6.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 6.1% [6.0%, 6.3%] 2
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) - - 0

Cycles

Results (secondary 2.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 2.6% [2.6%, 2.6%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) - - 0

Binary size

Results (secondary 0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 0.1% [0.1%, 0.1%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) - - 0

Bootstrap: 487.131s -> 486.905s (-0.05%)
Artifact size: 394.07 MiB -> 394.02 MiB (-0.01%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 25, 2026
@JonathanBrouwer
Copy link
Copy Markdown
Contributor Author

JonathanBrouwer commented Apr 25, 2026

@bors try @rust-timer queue
Made some more changes, so lets rerun to be sure

@rust-timer

This comment has been minimized.

Sign in to view
@JonathanBrouwer JonathanBrouwer marked this pull request as ready for review April 25, 2026 17:09
@rust-bors

This comment has been minimized.

Sign in to view
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 25, 2026

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

cc @jdonszelmann

Some changes occurred to diagnostic attributes.

cc @mejrs

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

@rustbot rustbot added S-waiting-on-perf Status: Waiting on a perf run to be completed. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 25, 2026
rust-bors Bot pushed a commit that referenced this pull request Apr 25, 2026
@bors
Auto merge of #155775 - JonathanBrouwer:cleanup_stage, r=<try>
1605ec0 
Remove attribute parsing `Stage`
@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 Apr 25, 2026
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Bryntet
Copy link
Copy Markdown
Contributor

Bryntet commented Apr 25, 2026

Wow this is a very nice change!!

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Apr 25, 2026

☀️ Try build successful (CI)
Build commit: 1605ec0 (1605ec0500eb4d5265572f443e427b9ceb0d6307, parent: fb76025f2fe6a1c8ce03fe0931f941c8077a2c34)

@rust-timer

This comment has been minimized.

Sign in to view
@rust-timer
Copy link
Copy Markdown
Collaborator

rust-timer commented Apr 25, 2026

Finished benchmarking commit (1605ec0): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -0.2% [-0.3%, -0.2%] 2
Improvements ✅
(secondary)		 -0.5% [-0.6%, -0.3%] 8
All ❌✅ (primary) -0.2% [-0.3%, -0.2%] 2

Max RSS (memory usage)

Results (primary -0.7%, secondary 1.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 0.6% [0.6%, 0.6%] 1
Regressions ❌
(secondary)		 6.2% [5.9%, 6.5%] 2
Improvements ✅
(primary)		 -2.1% [-2.1%, -2.1%] 1
Improvements ✅
(secondary)		 -2.3% [-2.5%, -2.1%] 3
All ❌✅ (primary) -0.7% [-2.1%, 0.6%] 2

Cycles

Results (primary 2.4%, secondary 0.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 2.4% [2.4%, 2.4%] 1
Regressions ❌
(secondary)		 2.5% [2.0%, 3.0%] 2
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -3.1% [-3.1%, -3.1%] 1
All ❌✅ (primary) 2.4% [2.4%, 2.4%] 1

Binary size

Results (secondary 0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 0.1% [0.1%, 0.1%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) - - 0

Bootstrap: 487.131s -> 488.391s (0.26%)
Artifact size: 394.07 MiB -> 393.58 MiB (-0.12%)

@rustbot

This comment has been minimized.

Sign in to view
jdonszelmann
jdonszelmann reviewed Apr 28, 2026
View reviewed changes
Comment thread compiler/rustc_attr_parsing/src/attributes/diagnostic/on_unknown.rs
@jdonszelmann
Copy link
Copy Markdown
Contributor

jdonszelmann commented Apr 28, 2026

This seems like a very clear improvement, even a small perf win. Could you make sure anywhere we still mention "early parsing" gets proper docs, or links to maybe even the crate docs to explain about where we parse attributes?

r=me otherwise

@jdonszelmann
Copy link
Copy Markdown
Contributor

jdonszelmann commented Apr 28, 2026

@rustbot author

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

rustbot commented Apr 28, 2026

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

@rustbot

This comment has been minimized.

Sign in to view
@JonathanBrouwer
Copy link
Copy Markdown
Contributor Author

JonathanBrouwer commented Apr 28, 2026

This PR now removes all mentions of "late" attribute parsing in the crate (only one). The term of "early" attribute parsing is used a lot throughout the crate, so I've documented it in the crate root.

@bors r=jdonszelmann,petrochenkov rollup=never p=1
Rollup=never for perf
p=1 because this PR is quite bitrotty, I've had to rebase it daily

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Apr 28, 2026

📌 Commit 89714d4 has been approved by jdonszelmann,petrochenkov

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened.

@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. 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 28, 2026
@rust-bors

This comment has been minimized.

Sign in to view
@rust-bors rust-bors Bot removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Apr 28, 2026
@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

@bors r=jdonszelmann,petrochenkov p=6
Scheduling this before the next rollup, very bitrotty

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Apr 29, 2026

📌 Commit 08babc8 has been approved by jdonszelmann,petrochenkov

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 Apr 29, 2026
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Apr 29, 2026 •
edited
Loading

⌛ Testing commit 08babc8 with merge 0424cc1...

Workflow: https://github.com/rust-lang/rust/actions/runs/25096754807

rust-bors Bot pushed a commit that referenced this pull request Apr 29, 2026
@bors
Auto merge of #155775 - JonathanBrouwer:cleanup_stage, r=jdonszelmann…
0424cc1 
…,petrochenkov

Remove attribute parsing `Stage`



I discovered that it is surprisingly easy to remove the concept of `Stage` in the attribute parsers, imo this makes the code a lot cleaner.

r? @jdonszelmann 
cc @mejrs @Bryntet
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jdonszelmann jdonszelmann jdonszelmann left review comments

@petrochenkov petrochenkov petrochenkov approved these changes

Assignees

@jdonszelmann jdonszelmann

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. S-waiting-on-perf Status: Waiting on a perf run to be completed. 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.

7 participants

@JonathanBrouwer @rust-timer @rustbot @rust-log-analyzer @Bryntet @jdonszelmann @petrochenkov