Repository navigation
[ty] Prefer innermost inline suppressions - #26940
Conversation
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 96.83%. The percentage of expected errors that received a diagnostic held steady at 91.74%. The number of fully passing files held steady at 99/133. |
Memory usage reportMemory usage unchanged ✅ |
|
b1afe5b to
9b56eb2
Compare
cc68889 to
50892ef
Compare
50892ef to
7577dbd
Compare
9b56eb2 to
6012ceb
Compare
|
Would you mind updating your summary with today's behavior and what behavior this PR is exactly changing? Or is all of this behavior new? |
6012ceb to
34b87c8
Compare
7577dbd to
e6efa92
Compare
|
Lol, I only read the first two paragraphs and was like, uff, I don't know how this differs from main. I'm sorry |
MichaReiser
left a comment
There was a problem hiding this comment.
The new datastructure feels somewhat over-engineered to me. I'm to hear your thinking of why you chose it.
MichaReiser
left a comment
There was a problem hiding this comment.
I didn't expect that there's so much room for making our suppression checking faster. Nice!
There was a problem hiding this comment.
This change is from your other PR, right?
There was a problem hiding this comment.
Yes sorry, it just needed to be rebased.
| /// closing line while a separate earlier candidate covers its opening line. The closing-line | ||
| /// candidate wins only when its suppression range is nested within the opening-line candidate's | ||
| /// range; otherwise the opening-line candidate retains precedence. | ||
| fn select_preferred_suppression<'a>( |
| SuppressionTarget::Lint(id) => { | ||
| let mut hasher = FxHasher::default(); | ||
| id.hash(&mut hasher); | ||
| 1 << (1 + hasher.finish() % 63) |
There was a problem hiding this comment.
Clever. I was a bit concerned after making the hashing suggestion that this would actually lead to a pretty bad filter, because & ing hashes are likely to have some bit in common. But the bucketing solves this. I suggest adding a comment somewhere (if there's not already one that I missed) saying that we bucket rules into 64 buckets and only traverse into sub-trees that overlap.
cb4e4f9 to
da3cc62
Compare

Summary
When multiple same-line or own-line suppressions apply to the same diagnostic, prefer the innermost suppression when their ranges are nested. This keeps the closest directive marked as used and lets an outer directive continue to suppress diagnostics elsewhere in the logical statement.
For a diagnostic spanning multiple lines, a suppression on its opening line takes precedence over a separate suppression on its closing line. This is not an innermost/outermost case because neither suppression contains the other. Keeping opening-line precedence also prevents a previously used opening-line suppression from becoming unused.
The lookup is indexed by suppression target so innermost precedence does not require scanning unrelated nested suppressions for every diagnostic. The same precedence is used when checking
unused-ignore-commentandblanket-ignore-comment.Rules
Nested suppression ranges
On main, applicable suppressions are checked in source order. The outer suppression appears first, so it suppresses both diagnostics and the inner suppression is reported as unused:
With this PR, the innermost suppression wins for
missing. The outer suppression remains responsible forabsent, so both suppressions are used:Separate opening-line and closing-line suppressions
For a diagnostic that spans multiple lines, suppressions on its opening and closing lines can both apply even though neither suppression contains the other:
On main, the opening-line suppression wins because it appears first. This PR deliberately preserves that behavior: innermost precedence applies only when the suppression ranges are nested. The closing-line suppression still handles
unresolved-reference, but itsmissing-argumentcode is unused.