Port lint attributes to attribute parser by Bryntet · Pull Request #152369 · rust-lang/rust

Bryntet · 2026-02-09T01:32:19Z

View all comments

Tracking issue: #131229

Ports #[allow], #[deny], #[expect], #[forbid], and #[warn] to being parsed attrs

I tried my best to make this PR as small as possible, it was difficult. I hope it isn't too difficult to review

r? @JonathanBrouwer
r? @jdonszelmann

rustbot · 2026-02-22T18:57:56Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

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

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

JonathanBrouwer · 2026-02-22T19:22:58Z

@bors try @rust-timer queue

Port lint attributes to attribute parser

JonathanBrouwer · 2026-04-03T09:12:47Z

@bors r=JonathanBrouwer,jdonszelmann

rust-bors · 2026-04-03T09:12:51Z

📌 Commit ce7c492 has been approved by JonathanBrouwer,jdonszelmann

It is now in the queue for this repository.

rust-bors · 2026-04-03T17:29:09Z

☀️ Test successful - CI
Approved by: JonathanBrouwer,jdonszelmann
Duration: 3h 37m 9s
Pushing 2972b5e to main...

github-actions · 2026-04-03T17:32:08Z

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing f908263 (parent) -> 2972b5e (this PR)

Test differences

Show 108 test diffs

Stage 1

[ui] tests/rustdoc-ui/lints/renamed-lint-still-applies-2.rs: [missing] -> pass (J1)
[ui] tests/ui/error-codes/E0452.rs: pass -> [missing] (J1)

Stage 2

[ui] tests/ui/error-codes/E0452.rs: pass -> [missing] (J0)
[ui] tests/rustdoc-ui/lints/renamed-lint-still-applies-2.rs: [missing] -> pass (J2)

Additionally, 104 doctest diffs were found. These are ignored, as they are noisy.

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 2972b5e59f1c5529b6ba770437812fd83ab4ebd4 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

aarch64-apple: 2h 45m -> 3h 30m (+27.3%)
dist-aarch64-llvm-mingw: 1h 28m -> 1h 48m (+22.9%)
dist-apple-various: 1h 29m -> 1h 41m (+13.3%)
pr-check-1: 30m 15s -> 33m 52s (+12.0%)
x86_64-gnu-llvm-22-3: 1h 47m -> 1h 59m (+11.8%)
dist-x86_64-apple: 1h 58m -> 2h 12m (+11.5%)
dist-aarch64-apple: 1h 54m -> 1h 42m (-10.9%)
x86_64-gnu-llvm-21-2: 1h 30m -> 1h 39m (+10.0%)
dist-x86_64-llvm-mingw: 1h 54m -> 2h 3m (+8.6%)
dist-i686-msvc: 2h 10m -> 2h 21m (+7.8%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

rust-timer · 2026-04-03T18:11:00Z

Finished benchmarking commit (2972b5e): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

If the regression was expected or you think it can be justified,
please write a comment with sufficient written justification, and add
@rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
If you think that you know of a way to resolve the regression, try to create
a new PR with a fix for the regression.
If you do not understand the regression or you think that it is just noise,
you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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.2%	[0.2%, 0.2%]	1
Regressions ❌ (secondary)	0.3%	[0.2%, 0.5%]	7
Improvements ✅ (primary)	-0.3%	[-0.6%, -0.2%]	20
Improvements ✅ (secondary)	-0.4%	[-0.6%, -0.0%]	37
All ❌✅ (primary)	-0.3%	[-0.6%, 0.2%]	21

Max RSS (memory usage)

Results (primary -1.6%, secondary 2.9%)

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)	4.7%	[1.8%, 10.0%]	13
Improvements ✅ (primary)	-1.6%	[-2.3%, -1.0%]	2
Improvements ✅ (secondary)	-3.0%	[-8.3%, -1.1%]	4
All ❌✅ (primary)	-1.6%	[-2.3%, -1.0%]	2

Cycles

Results (primary 2.2%, secondary 3.6%)

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

	mean	range	count
Regressions ❌ (primary)	2.2%	[1.3%, 2.8%]	5
Regressions ❌ (secondary)	4.4%	[3.3%, 5.5%]	8
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.3%	[-3.3%, -3.3%]	1
All ❌✅ (primary)	2.2%	[1.3%, 2.8%]	5

Binary size

Results (primary 0.0%, secondary 0.0%)

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

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

Bootstrap: 491.353s -> 489.759s (-0.32%)
Artifact size: 395.11 MiB -> 395.08 MiB (-0.01%)

JonathanBrouwer · 2026-04-03T18:14:33Z

Perf wins significantly outweigh the regressions
@rustbot label: +perf-regression-triaged

the8472 · 2026-04-06T12:36:17Z

Perf wins significantly outweigh the regressions

Walltime numbers seem to disagree, and eyeballing the summary charts looks like those regressions are real, i.e. the shift persists.

JonathanBrouwer · 2026-04-06T13:08:08Z

Ooh interesting, walltime numbers are usually super noisy so I tend to ignore them, but this does indeed look kinda real
This is the one for eza incr-patched, the worst walltime regression in the primary benchmarks, and this one does seem to go back to normal. Which one were you looking at @the8472 ?

the8472 · 2026-04-06T13:10:08Z

The summary charts: https://perf.rust-lang.org/?start=2026-02-16&kind=percentfromfirst&stat=wall-time

Bryntet · 2026-04-06T13:10:29Z

Perf wins significantly outweigh the regressions

Walltime numbers seem to disagree, and eyeballing the summary charts looks like those regressions are real, i.e. the shift persists.

I think some walltime increase can be expected due to: https://github.com/rust-lang/rust/pull/152369/changes#diff-f108051d119300655064d5988094ba485d04880e8669944bbc484f35c49b6743R126

JonathanBrouwer · 2026-04-06T13:28:05Z

I don't really believe that sort could cause such a major change, given how small the list it is sorting is, though perf is always a surprise so it always could be...

Am I reading the summary charts correctly when I say the walltime regressions only persists in doc builds?

the8472 · 2026-04-06T13:47:13Z

Actually, that's just in the summary charts, where churn from other PRs can easily mask it if they happen to be an improvement.

Going back to individual benchmarks shows that unicode-normalization, tuple-stress and ucd are persistent and not limited to doc benchmarks.

apiraino · 2026-04-09T08:29:36Z

hey @Bryntet @JonathanBrouwer @jdonszelmann 👋

I notice a number of issues popping up linked to this PR. Tomorrow a beta branching is scheduled and a crater run will soon run on it. My vague feeling is that we might get a number of regressions and then we need to fix and backport them.

So, what is your gut feeling about #154906? Would it solve all/most of the issues I see linked to this PR? And what do you think about temporarily reverting this PR to give us a bit more time before the beta cut?

Leaving a note here as today we have the weekly compiler triage (on Zulip), so possibly also include your opinion first (feel free to stop by if you want).

and thanks :)

Bryntet · 2026-04-09T08:34:30Z

I think temporarily reverting this PR may be wise.

I think #154906 fixes most problems from this PR (especially those that would commonly be stumbled upon by end users), but the way it does so is not so idiomatic, and preferably still needs some work

I don't know what the process for reverting/not including a PR in beta, but I'd love to help in anyway I can

JonathanBrouwer · 2026-04-09T09:05:41Z

I'll take a look later today at what the best option is here, this will be a painful revert because of the large amount of changes since this pr, but it might be needed

Bryntet · 2026-04-09T09:07:34Z

Right, didn't think of that. This could indeed be quite painful to revert.

JonathanBrouwer · 2026-04-09T16:05:37Z

To summarize, we currently have the following regressions:

[ICE]: cannot decode AttrId with CacheDecoder #154878
- This regression is P-critical imo, it should not get into a release.
- Partially fixed by Change hashing/comparison of LintExpectationId (fix ICE) #154906
- I am not happy with the direction of this fix, and would prefer not to merge this in its current state
[ICE]: malformed lint attributes #154800
- Fixed by Allow compilation recovery after emitting lint attr parsing error #154911
- Does not pass CI yet and has not been reviewed yet
#[expect] doesn't work anymore with pre-expansion early lints #155008
- Fixed by KeywordIdents Lint fix #155038
- Approved
Minor regression in walltime perf, even tho there is an improvement in instruction counts. We can likely just accept this.
Possibly more regressions since this is a relatively recently merged PR

Because of this, I believe the best decision is to revert. Note that I still believe this PR to be awesome and I think we will re-land it with the fixes on top in the near future, but I have limited time and I don't want to cause time pressure on a project that we're all contributing to for fun, because that is a path doomed to burnout <3

Because quite a few PRs already landed on top of this one reverting will be painful. I will see if I can get a revert up before the beta cutoff, but no promises. Otherwise we can beta-nominate the revert.

JonathanBrouwer · 2026-04-09T17:03:22Z

Revert was surprisingly painless, there were quite a few conflicts but they solved relatively easy
#155056

cjgillot · 2026-03-05T01:18:52Z

+            bug!("tools required while parsing attributes");
+        };
+
+        let tools = tools.iter().map(|tool| tool.name).collect::<Vec<_>>();


View changes since the review

Do we really need to collect here, or could we change the tools.contains check below to use cx.tools directly?

cjgillot · 2026-03-05T01:21:39Z

+    if !skip_reason_check && !errored && lint_instances.is_empty() {
+        cx.warn_empty_attribute(cx.attr_span);
+    }
+
+    (!errored).then_some(LintAttribute {
+        reason,
+        lint_instances,
+        attr_span: cx.attr_span,
+        attr_style: cx.attr_style,
+        attr_id: HashIgnoredAttrId { attr_id: cx.attr_id },
+        attr_index,
+    })


View changes since the review

Suggested change

if !skip_reason_check && !errored && lint_instances.is_empty() {

cx.warn_empty_attribute(cx.attr_span);

}

(!errored).then_some(LintAttribute {

reason,

lint_instances,

attr_span: cx.attr_span,

attr_style: cx.attr_style,

attr_id: HashIgnoredAttrId { attr_id: cx.attr_id },

attr_index,

})

if errored {

return None;

}

if !skip_reason_check && lint_instances.is_empty() {

cx.warn_empty_attribute(cx.attr_span);

}

Some(LintAttribute {

reason,

lint_instances,

attr_span: cx.attr_span,

attr_style: cx.attr_style,

attr_id: HashIgnoredAttrId { attr_id: cx.attr_id },

attr_index,

})

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 Feb 9, 2026

This comment has been minimized.

Sign in to view

Bryntet force-pushed the lint_attrs branch from 6d9cad0 to 6568db4 Compare February 9, 2026 05:46

This comment has been minimized.

Sign in to view

Bryntet force-pushed the lint_attrs branch from 6568db4 to 1c130a6 Compare February 9, 2026 18:47

This comment has been minimized.

Sign in to view

JonathanBrouwer mentioned this pull request Feb 9, 2026

Tracking issue: Attribute refactor #131229

Open

9 tasks

Bryntet force-pushed the lint_attrs branch from 1c130a6 to 1139fa4 Compare February 9, 2026 19:45

This comment has been minimized.

Sign in to view

Bryntet force-pushed the lint_attrs branch from 1139fa4 to 3431f73 Compare February 14, 2026 17:02

Bryntet force-pushed the lint_attrs branch from 3431f73 to 4a2a189 Compare February 22, 2026 18:55

rustbot added the T-clippy Relevant to the Clippy team. label Feb 22, 2026

rustbot assigned JonathanBrouwer and jdonszelmann Feb 22, 2026

Bryntet changed the title ~~lint attrs~~ Port lint attributes to attribute parser Feb 22, 2026

Bryntet marked this pull request as ready for review February 22, 2026 18:57

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 22, 2026

rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Feb 22, 2026

This comment has been minimized.

Sign in to view

rust-bors bot pushed a commit that referenced this pull request Feb 22, 2026

Auto merge of #152369 - Bryntet:lint_attrs, r=<try>

46a7bb3

Port lint attributes to attribute parser

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 22, 2026

Bryntet force-pushed the lint_attrs branch 2 times, most recently from e70b414 to d9434c9 Compare February 22, 2026 20:19

JonathanBrouwer reviewed Feb 22, 2026

View reviewed changes

Comment thread compiler/rustc_passes/src/check_attr.rs Outdated

Bryntet force-pushed the lint_attrs branch from d9434c9 to a0d52ad Compare February 22, 2026 21:05

This comment has been minimized.

Sign in to view

This was referenced Apr 3, 2026

Introduce a #[diagnostic::on_unknown] attribute #152901

Merged

Set up API to make it possible to pass closures instead of AttributeLint #154432

Open

Add gated const item paths support for selected builtin attributes. #154708

Open

This was referenced Apr 4, 2026

[ICE]: malformed lint attributes #154800

Closed

[ICE]: cannot decode AttrId with CacheDecoder #154878

Closed

matthiaskrgr mentioned this pull request Apr 6, 2026

[ICE]: wrong meta list delimiters #154847

Closed

JonathanBrouwer mentioned this pull request Apr 8, 2026

#[expect] doesn't work anymore with pre-expansion early lints #155008

Closed

Bryntet mentioned this pull request Apr 9, 2026

KeywordIdents Lint fix #155038

Closed

JonathanBrouwer mentioned this pull request Apr 9, 2026

Remove AttributeExt #155041

Draft

JonathanBrouwer mentioned this pull request Apr 9, 2026

Revert "Port lint attributes to attribute parser" #155056

Merged

cjgillot reviewed Apr 12, 2026

View reviewed changes

Uh oh!

Conversation

Bryntet commented Feb 9, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rustbot commented Feb 22, 2026

Uh oh!

JonathanBrouwer commented Feb 22, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

JonathanBrouwer commented Apr 3, 2026

Uh oh!

rust-bors bot commented Apr 3, 2026

Uh oh!

This comment has been minimized.

rust-bors bot commented Apr 3, 2026

Uh oh!

github-actions bot commented Apr 3, 2026

Test differences

Stage 1

Stage 2

Job duration changes

Uh oh!

rust-timer commented Apr 3, 2026

Overall result: ❌✅ regressions and improvements - please read the text below

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

JonathanBrouwer commented Apr 3, 2026

Uh oh!

the8472 commented Apr 6, 2026

Uh oh!

JonathanBrouwer commented Apr 6, 2026

Uh oh!

the8472 commented Apr 6, 2026

Uh oh!

Bryntet commented Apr 6, 2026

Uh oh!

JonathanBrouwer commented Apr 6, 2026

Uh oh!

the8472 commented Apr 6, 2026

Uh oh!

apiraino commented Apr 9, 2026

Uh oh!

Bryntet commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JonathanBrouwer commented Apr 9, 2026

Uh oh!

Bryntet commented Apr 9, 2026

Uh oh!

JonathanBrouwer commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JonathanBrouwer commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cjgillot Mar 5, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjgillot Mar 5, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

Bryntet commented Feb 9, 2026 •

edited by rustbot

Loading

Bryntet commented Apr 9, 2026 •

edited

Loading

JonathanBrouwer commented Apr 9, 2026 •

edited

Loading

JonathanBrouwer commented Apr 9, 2026 •

edited

Loading

cjgillot Mar 5, 2026 •

edited by rustbot

Loading

cjgillot Mar 5, 2026 •

edited by rustbot

Loading