Move lint validation to attribute parsing by JonathanBrouwer · Pull Request #164918 · rust-lang/rust

JonathanBrouwer · 2026-10-25T16:08:48Z

This is an alternative approach to #164607

This can be reviewed commit by commit, each commit passes tests.
My goal is for a follow-up to remove lint_name_normalized completely and replace it by a LintId

r? @mejrs

JonathanBrouwer · 2026-10-25T16:10:38Z

-    /// - `custom_tool::blah::bleh` -> `blah::bleh`
-    pub lint_name: Symbol,
+    /// For example, `single_use_lifetime` is changed to `single_use_lifetimes`.
+    pub lint_name_normalized: Symbol,


My goal for a follow-up PR is to change this field from a Symbol that needs to be looked up in LintStore to a LintId. I think this should be possible, but encoding and decoding will be a challenge. I think this PR is already a good step forwards regardless.

View changes since the review

JonathanBrouwer · 2026-10-25T16:11:27Z

 pub use attributes::util::{is_builtin_attr, parse_version};
 pub use context::ShouldEmit;
-pub use diagnostics::ParsedDescription;
+pub use diagnostics::{ParsedDescription, RenamedLintSuggestion, UnknownLintSuggestion};


I'm not sure if people find it cursed to export diagnostic structs such that they can be re-used. I like it

View changes since the review

I don't, it can hide unused diagnostics 😅

While unused diagnostics are bad, the alternative is duplicating the suggestion structs which risks them diverging when someone edits one but not the other. I think that's worse

JonathanBrouwer · 2026-10-25T16:17:06Z

@bors try @rust-timer queue

Move lint validation to attribute parsing

rustbot · 2026-10-25T16:17:15Z

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

Some changes occurred in compiler/rustc_attr_ir

cc @jdonszelmann

clippy is developed in its own repository. If possible, consider making this change to rust-lang/rust-clippy instead.

cc @rust-lang/clippy

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

cc @jdonszelmann

rust-bors · 2026-10-25T17:16:11Z

☀️ Try build successful (CI)
Build commit: 00e2c28 (00e2c28e2c046bbcd149ca7f1bf88c8cfc9e8875)
Base parent: dc44e7f (dc44e7f2c436b6041742e138b9c2176f093196d5)

rust-timer · 2026-10-25T18:05:43Z

Finished benchmarking commit (00e2c28): comparison URL.

Overall result: ✅ improvements - no action needed

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

@bors rollup=iffy rustc-perf
@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.3%	[-0.3%, -0.1%]	9
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.3%	[-0.3%, -0.1%]	9

Max RSS (memory usage)

Results (primary -0.5%, secondary -0.7%)

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.5%, 0.9%]	5
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.4%	[-3.1%, -0.5%]	6
Improvements ✅ (secondary)	-0.7%	[-0.7%, -0.7%]	1
All ❌✅ (primary)	-0.5%	[-3.1%, 0.9%]	11

Cycles

Results (primary 0.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)	0.9%	[0.5%, 2.1%]	9
Regressions ❌ (secondary)	1.3%	[0.5%, 3.4%]	5
Improvements ✅ (primary)	-1.0%	[-1.7%, -0.7%]	3
Improvements ✅ (secondary)	-1.3%	[-2.1%, -0.5%]	2
All ❌✅ (primary)	0.4%	[-1.7%, 2.1%]	12

Binary size

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

Bootstrap: 487.351s -> 486.11s (-0.25%)
Artifact size: 405.87 MiB -> 406.01 MiB (0.03%)

mejrs

I don't think this change is correct, it means lowering ast to hir depends on what lints have been registered. Does this mean cargo clippy and cargo check could produce different artefacts? What happens when rust-analyzer decides to implement its own lints? (it's possible, it's a pregistered tool).

Can you talk to people more knowledgeable about this and get them to sign off on this? The people that are working on sharing cargo build caches probably have opinions about this.

@rustbot author

View changes since this review

mejrs

Can you summarize the zulip discussion in a comment somewhere?

View changes since this review

mejrs · 2026-10-27T17:09:21Z

 pub use attributes::util::{is_builtin_attr, parse_version};
 pub use context::ShouldEmit;
-pub use diagnostics::ParsedDescription;
+pub use diagnostics::{ParsedDescription, RenamedLintSuggestion, UnknownLintSuggestion};


I don't, it can hide unused diagnostics 😅

mejrs · 2026-10-27T18:09:51Z

+        // We only want to emit these lints once, on the HIR.
+        // On the AST `target_span` will be `DUMMY_SP`, which we use to detect which instance we are.
+        // #164898 will fix this, after that PR this check can be removed
+        let should_lint = cx.target_span != DUMMY_SP;


I don't think this is a good idea:

I would not accept this as a standalone PR (instead demanding a more rigorous fix)

Try to keep behavior changes to a minimum during refactors

This needs to be in this PR because the old code also had similar, though broken, logic through selflint_added_lints. It was broken because it duplicated a lot of diagnostics anyways, which this PR fixes. Emulating the old logic in this PR would be more complicated than just fixing it
I found a slightly more rigorous fix, using parse_filter instead of Span.
I think that should be good enough, since this code is temporary anyways

mejrs · 2026-10-27T18:25:58Z

I also don't understand why this ought to block #164607

I still think we should store a LintName rather than a Symbol. (whether or not they are normalized), at least until you figure out your Lintid thing

mejrs · 2026-10-27T22:34:36Z

This PR is a breaking change, it now correctly rejects unknown tool lints

Consider the following code:

macro_rules! wat {
    () => {
        #[allow(unused_attributes)]
        {
            #[allow(unregistered_tool::lint)]
            println!("boo");
        }
    };
}

fn main() {
    wat!();
}

Behavior on current Rust (all channels)

compiles, no diagnostic issued

With this pr:

error[E0710]: unknown tool name `unregistered_tool` found in scoped lint: `unregistered_tool::lint`
  --> $DIR/inner.rs:5:21
   |
LL | macro_rules! wat {
...
LL |             #[allow(unregistered_tool::lint)]
   |                     ^^^^^^^^^^^^^^^^^
...
LL |     wat!();
   |     ------ in this macro invocation
   |
   = help: add `#![register_tool(unregistered_tool)]` to the crate root

error: aborting due to 1 previous error

JonathanBrouwer · 2026-10-28T11:43:54Z

Right, that's a fun one I didn't think about!
I think we can go for merging #164607 first given that this PR needs to go through lang

JonathanBrouwer · 2026-10-30T11:43:04Z

This PR is a breaking change, it now correctly rejects unknown tool lints on macro calls

I've added an extra check to remove the breaking change from this PR, so we can do it in a follow-up PR instead. Added a test for this

I went through all review comments and resolved the ones which were addressed by the latest commit, and left comments on the ones that need further discussion.
@rustbot ready

mejrs

Sorry about all the yapping about attributes on macro calls. It's fine to leave it unfixed for now, I do plan to crater a bunch of macro call things Soon(tm), I'll include this in that.

It looks good, I'll take another pass once it's all rebased and stuff.

View changes since this review

mejrs · 2026-10-30T22:45:14Z

        cx: &mut AcceptContext<'_, 'a>,
    ) -> Option<(&'a [LintId], Symbol)> {
-        let lint_result = cx.sess.lint_store.as_ref().unwrap().check_lint_name(
+        // FIXME For historic reasons these checks are not executed on macro calls


More precisely, it's only ignored on macro calls produced by another macro's expansion. Prior to macro expansion it's still caught by the pre expansion pass collection.

That is:

macro_rules! identity { ($( $tt:tt )*) => { $( $tt )* } } fn main() { #[expect(wat::doesnt_exist)] // ERROR println!(); identity!{ #[expect(wat::doesnt_exist)] // OK println!(); } }

I think that's maintained here, as we pass Crate as the pre-expansion target for that. Can you add a test, I don't believe one exists.

The lint attribute on a plain macro actually doesn't seem to work, because both errors are only produced during late parsing, when cx.target is set accurately.
I tried for a bit to mimic the old behavior, but it doesn't seem to be easy...

We will need the `lint_tools` in the next commits

This is a lot of code being moved around, but not much of the logic changes. I changed some stuff though, so I recommend reviewing the logic side by side.

rustbot · 2026-10-31T18:44:19Z

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 commented Oct 25, 2026

View reviewed changes

rustbot assigned mejrs Oct 25, 2026

This comment has been minimized.

Sign in to view

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

JonathanBrouwer marked this pull request as ready for review October 25, 2026 16:17

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Oct 25, 2026

Auto merge of #164918 - JonathanBrouwer:lint-id, r=<try>

00e2c28

Move lint validation to attribute parsing

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 25, 2026

JonathanBrouwer mentioned this pull request Oct 25, 2026

destringlyfy lint infrastructure #164607

Open

JonathanBrouwer force-pushed the lint-id branch from 1a9e5b3 to cb8338e Compare October 25, 2026 16:51

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 25, 2026

This comment has been minimized.

Sign in to view

JonathanBrouwer force-pushed the lint-id branch from cb8338e to 5a6079c Compare October 26, 2026 07:05

This comment has been minimized.

Sign in to view

JonathanBrouwer force-pushed the lint-id branch from 5a6079c to 62ff266 Compare October 26, 2026 14:33

This comment has been minimized.

Sign in to view

mejrs reviewed Oct 26, 2026

View reviewed changes

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 Oct 26, 2026

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 27, 2026

mejrs requested changes Oct 27, 2026

View reviewed changes

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 Oct 27, 2026

This comment was marked as duplicate.

Sign in to view

This comment has been minimized.

Sign in to view

JonathanBrouwer force-pushed the lint-id branch from 62ff266 to 417deac Compare October 30, 2026 11:27

This comment has been minimized.

Sign in to view

JonathanBrouwer force-pushed the lint-id branch from 417deac to cc723c2 Compare October 30, 2026 11:31

This comment has been minimized.

Sign in to view

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 30, 2026

This comment has been minimized.

Sign in to view

JonathanBrouwer force-pushed the lint-id branch from cc723c2 to bbdf8c5 Compare October 30, 2026 13:06

mejrs reviewed Oct 30, 2026

View reviewed changes

JonathanBrouwer force-pushed the lint-id branch from bbdf8c5 to 60e801c Compare October 31, 2026 16:23

This comment has been minimized.

Sign in to view

JonathanBrouwer added 3 commits October 31, 2026 19:38

Pass both attr_tools and lint_tools into the attribute parser

aa1a450

We will need the `lint_tools` in the next commits

Move lint validation from Levels::add to attribute parsing

5671830

This is a lot of code being moved around, but not much of the logic changes. I changed some stuff though, so I recommend reviewing the logic side by side.

Remove span of tool

6e6df22

JonathanBrouwer force-pushed the lint-id branch from 60e801c to 9d15b14 Compare October 31, 2026 18:44

This comment has been minimized.

Sign in to view

Review

679ce9a

JonathanBrouwer force-pushed the lint-id branch from 9d15b14 to 679ce9a Compare October 31, 2026 19:28

Conversation

JonathanBrouwer commented Oct 25, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JonathanBrouwer Oct 25, 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!

JonathanBrouwer Oct 25, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JonathanBrouwer commented Oct 25, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rustbot commented Oct 25, 2026

Uh oh!

rust-bors Bot commented Oct 25, 2026

Uh oh!

This comment has been minimized.

rust-timer commented Oct 25, 2026

Overall result: ✅ improvements - no action needed

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

mejrs left a comment • 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!

mejrs left a comment • 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as duplicate.

Uh oh!

mejrs commented Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mejrs commented Oct 27, 2026

Uh oh!

JonathanBrouwer commented Oct 28, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

JonathanBrouwer commented Oct 30, 2026

Uh oh!

This comment has been minimized.

mejrs left a comment • 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!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

JonathanBrouwer commented Oct 25, 2026 •

edited by rustbot

Loading

JonathanBrouwer Oct 25, 2026 •

edited by rustbot

Loading

JonathanBrouwer Oct 25, 2026 •

edited by rustbot

Loading

mejrs left a comment •

edited by rustbot

Loading

mejrs left a comment •

edited by rustbot

Loading

mejrs commented Oct 27, 2026 •

edited

Loading

mejrs left a comment •

edited by rustbot

Loading

JonathanBrouwer Oct 31, 2026 •

edited

Loading