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

[GuillaumeGomez]

View all comments

Part of #153099.

This PR sets up the base implementations needed to remove AttributeLintKind entirely and migrate two variants as examples.

r? @JonathanBrouwer

[rustbot]

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

[rustbot]

JonathanBrouwer is currently at their maximum review capacity.
They may take a while to respond.

[GuillaumeGomez]

Added the missing file and fixed the typo. Time to run a perf check.

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: 8dcd0b9 (8dcd0b95692c85cfd497ecfb5770e55e4796ba80, parent: 23903d01c237d7c7d4fb62b82ca846bc45de4e0c)

[rust-timer]

Finished benchmarking commit (8dcd0b9): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

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

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary 3.6%, secondary 6.4%)

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

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

Cycles

Results (secondary -2.0%)

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
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.0%	[-2.0%, -2.0%]	1
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 494.635s -> 485.284s (-1.89%)
Artifact size: 395.08 MiB -> 397.09 MiB (0.51%)

[GuillaumeGomez]

Nice. =D

[JonathanBrouwer]

Is this correct? If the lint/id/span of the lint stays the same, but something in the labels changes, then the hash of the DynAttribute would remain unchanged

[GuillaumeGomez]

I don't see a way around that, whether it's dyn Diagnostic or a callback. Although, do you see a case where a same AttributeLint is emitted on both the same HirId and span but with a different error?

[JonathanBrouwer]

I do think this is actually possible.
This is relevant for incremental compilation, so if the following happens:

• Lint A is emitted, which has a label that for example refers to something in the item (like a field or whatever)
• The user changes the thing in the item, in such a way that the span is unaffected
• The hash of the lint then stays the same, even though the label is in fact changed and the hash should change with it. Therefore incremental will think the lint stayed the same and show the old rendered lint incorrectly

[GuillaumeGomez]

But then the HirId is different no?

[JonathanBrouwer]

I don't think HirId necessarily changes if the item changes

[rustbot]

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.

[GuillaumeGomez]

Fixed merge conflicts.

[rust-bors]

☔ The latest upstream changes (presumably #152369) made this pull request unmergeable. Please resolve the merge conflicts.