Recognize an all-different check, and say what replacing it costs - #42
Merged
Conversation
No library checks distinctness by comparing every pair. `itertools::all_unique` reaches the answer through a hash set, so derivation produces a hash-set form and the quadratic loop it exists to replace matches nothing. The shape has to be named directly, the way the strum recognizers name a shape a derive macro would have produced — but over the normalized form rather than the syntax tree, which is what lets one recognizer cover spellings that share no syntax. The shape: a walk over each pair of one sequence that leaves with a constant on finding an equal pair, in a body yielding the opposite constant otherwise. It refuses a relation other than equality (`a < b` over every pair is a sortedness check), a walk that escapes with a computed value, and two arms naming one constant. `Condition` joins the match fact. A match says the code computes what the API computes; it does not say the two are interchangeable, and here they are not. Reaching the answer through a hash set needs `Eq + Hash` where the loop needs only `PartialEq` — not pedantry: `f64` is `PartialEq` and not `Eq`, two `NaN`s are unequal to each other, so the loop calls them distinct and the set cannot be built. It allocates where the loop does not, it need not run the comparison at all where the loop runs it quadratically, and at four elements the loop wins. Where the gap is visible instead of merely possible, the recognizer refuses: a `const fn` has no allocator, and a reader should not be handed a finding they must then reject. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ghYbUJFcVfkKYacbTVJsR
Nine findings across 127,210 Rust files, every one verified by hand as a
hand-rolled distinctness check. Zero across 3,650 files of production Rust,
where the idiom is written with a HashSet already. The first run found zero, and
the two things that were wrong are both in the code now.
The bound. `whole_index_span` demanded `0..v.len()`, and pairwise loops in the
corpus are bound by a bare variable six to one. The sequence is therefore read
off the body — whatever it indexes at both positions, and it must be exactly one
thing — and the bound decides the extent: `0..v.len()` walks `v`, and `0..n`
walks `v[..n]`, which is the form the frontend already produces for that slice
written out. The prefix is recorded rather than glossed, because a walk bounded
by something other than the length does not cover the sequence, and saying it
did would recommend an API over elements that were never read. An outer loop
that stops one short of the inner one is the same walk and is now a row in the
table.
The reaction. The recognizer wanted `return false` inside a body yielding
`true`; what the corpus writes is `println!("no"); return;` and `ans = false`.
So the test is no longer the shape of the escape but what it depends on: a
reaction that does not read either element records only that a duplicate
exists, which is exactly what the recommended API answers. What the code then
does with that answer is the caller's business.
This is what keeps the refusals honest rather than loosening them. `continue` is
the commonest thing a pairwise equality test does — six times more common than
every accepted spelling combined — and it skips duplicate pairs inside a larger
computation. `count += 1` counts them. Neither records only existence, and
neither is accepted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ghYbUJFcVfkKYacbTVJsR
The recognizer named `Eq + Hash` as what `all_unique` requires. That was right, and it was a claim about a version of a library that nothing had checked — which is the claim a reader is least placed to verify and exactly what the catalog exists to settle. It is now read from the shipped signature, where itertools 0.15.0 states `Self::Item: Eq + Hash` itself. The same signature now gates the match. AGENTS.md requires a matcher to verify the callable it relies on, and the idiom recognizer named a path without ever looking at what stood behind it. A path is not a promise: a catalog is generated data and an API can change between versions. So the recommendation is made only when the callable still takes a receiver and still returns `bool` — still answers the question the shape decides — and a catalog with no signature at all cannot be checked and is not used. `NORMALIZED_FORM_SCHEMA` goes to 2. Behavior packs are gated by their own envelope version and still deserialize, and both the round-trip test and a scan of the shipped programs agree none of them contain the constructs the new laws touch, so nothing needs regenerating. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ghYbUJFcVfkKYacbTVJsR
CodeNet groups submissions by problem, so a problem that is ABOUT distinctness
labels its submissions for free: every accepted one contains the check, and
whatever is not flagged is a spelling not handled. Three problems answered to
the nine findings — 330 Rust submissions between them.
Most of them are not the target. 181 use a set and 44 sort and dedup: already
idiomatic, and correctly silent. Eighteen hand-roll a nested loop, which is the
population that should be flagged. Eleven now are, up from nine.
Two spellings the label turned up. A reaction written as several steps ending in
`break` rather than `return` — `ans = "No"; break;` — records that a duplicate
exists just as much as returning does, so the test is now that one step records
and no step does anything unaccounted for, rather than that the last step is a
particular thing.
And the square loop. `for i in 0..n { for j in 0..n { if i != j { .. } } }` was
refused with the reasoning that the guard is written over indices the rewrite
forgets. That was the wrong conclusion from a right observation: the guard has to
be CONSUMED, not carried, and removing it is sound because what it excludes is
what the resulting form excludes anyway. So `Coverage` arrives after all, with
the two inhabitants that earned it — each pair once, or each pair both ways
round. The distinction is real and kept: a decision that does not care how often
it sees a pair gets the same answer from either, and a count gets double. Every
reaction this recognizer accepts is idempotent, which is why it may take both,
and it says so where it takes them.
A span may now start anywhere, so `1..k` walks `v[1..k]` exactly as `0..n` walks
`v[..n]`. This gained nothing measured — the submissions it was aimed at turn
out to compare `S[i]` against `S[j].to_string()`, and equating `a` with `f(b)`
is not something to do — but the asymmetry it removes was arbitrary, and the
lower triangle now has to start where its outer loop did.
Still zero false positives: 127,210 CodeNet files and 3,650 of production Rust,
every finding read by hand.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ghYbUJFcVfkKYacbTVJsR
`Pairwise` appeared nowhere in matching.rs. Adding a variant without an arm does not fail to compile — the fallthrough answers `false` — so the form silently matched nothing, and any behavior ever written over it would have gone quiet with no error to explain why. The arm compares coverage as well, because seeing each pair once and seeing it both ways round reach the same pairs a different number of times, and a pattern that counts would get double from the wrong one. The body admits fusion the way a traversal's does. Nothing observable changes today: no derived library behavior contains a walk over pairs, because no library writes one. The hole was in the layer that will be asked first when one does. Measured while establishing this: across the three labelled problems, 14 functions reduce to a `Pairwise` and 11 are reported. The three that are not are the same submission thrice, comparing `S[i]` against `S[j].to_string()`, which is a correct refusal. So the recognizer turns away nothing it should take, and every remaining miss is upstream, in whether the form becomes a `Pairwise` at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ghYbUJFcVfkKYacbTVJsR
The recognizer walked the form by hand: find a Pairwise, confirm its body is a Branch, pull out the condition, pull out the consequence. All four are things unification already does, and doing them again beside it is how a matcher comes to be reimplemented next to itself — with its own idea of what counts as the same shape, free to drift. What was missing was not a way to match but a way to keep what matching found. `Bindings` already works out what every hole and every name stood for, because a hole has to mean the same thing each time it appears, and then threw it all away at the door. `Form::resolve_all` hands it back as `Resolved`, so a recognizer with something to say about a PART of what matched can say it about the piece the matcher found rather than one a second traversal went looking for. The all-different shape is now a `Form` — a pairwise walk whose body is a branch with no else — with the test and the reaction left as holes. The holes are wide on purpose: every judgement about them stays in the recognizer, which is what lets it say WHICH one was wrong, where a pattern narrow enough to reject on its own could only ever answer "no". Every refusal reason survives. Three things the idiom path had copied now come from where they belong. Evidence and span construction were written out once per emitter until there were three. And placement: findings pointed at whole functions because the idiom path never called `locate_all`. It does now, and four of the eleven findings narrowed from the enclosing function to the loop itself — s117010675 from lines 3-22 to 13-20, which is exactly the nested loop. Same eleven findings, better aimed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ghYbUJFcVfkKYacbTVJsR
zmaril
force-pushed
the
pr2-idiom-all-different
branch
from
August 25, 2026 18:50
20bfb13 to
83b770f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #41.
Recognizes a hand-rolled all-different check and recommends
itertools::Itertools::all_unique, with the conditions under which that swap issound.
Why this needs a recognizer rather than a derived behavior
No library checks distinctness by comparing every pair.
all_uniquereaches theanswer through a hash set, so derivation produces a hash-set form and the
quadratic loop it exists to replace matches nothing. The shape has to be named
directly — the way the strum recognizers name a shape a derive macro would have
produced, but over the normalized form rather than the syntax tree, which is
what lets one recognizer cover spellings that share no syntax.
It is matched by the same unification everything else goes through.
Bindingsalready works out what every hole stood for and used to discard it at the door;
Form::resolve_allhands it back asResolved, so the conditions are statedabout the pieces the matcher found rather than ones a second traversal went
looking for. Evidence, span placement and target naming come from the shared
helpers.
Conditions
A match says the code computes what the API computes. It does not say the two
are interchangeable, and here they are not:
Eq + Hashwhere the loop needsonly
PartialEq— not pedantry, sincef64isPartialEqand notEq, andtwo NaNs are unequal to each other, so the loop calls them distinct and the
set cannot be built at all
The bound is read off the catalog, not written from memory. The same signature
gates the match: the callable must still take a receiver and still return
bool, per AGENTS.md's rule that a matcher verify what it relies on.Measured
11 findings across 127,210 CodeNet Rust files, every one read by hand and
confirmed. Zero across 3,650 files of production Rust, where the idiom is
written with a HashSet already.
The first run found zero, and the corpus corrected the shape twice — the bound
(pairwise loops are bound by a bare variable six to one over
len()) and thereaction (
println!("no"); return;andans = false, notreturn false).Recall was then checked against a labelled set: CodeNet groups submissions by
problem, so a problem that is about distinctness labels its submissions for
free. Of 330 submissions to three such problems, 18 hand-roll a nested loop and
11 are found. The 7 misses each have a reason, listed in the commit messages.
continueis the commonest thing a pairwise equality test does — six times morecommon than every accepted spelling combined — and it is skipping duplicate
pairs inside a larger computation. It is refused, and so is
count += 1.Review notes
CoverageonPairwisehas two inhabitants and both are measured; a walk overadjacent pairs is deliberately NOT one of them here (it arrives in Catalog the standard library, and recognize a sortedness check #43).
Idiomis compiled in rather than pack data. Now that the shape is aFormit is nearly authorable as data; what blocks it is the predicate on the holes,
which would need alternation and negative conditions in the pattern language.