Skip to content

Recognize an all-different check, and say what replacing it costs - #42

Merged
zmaril merged 6 commits into
mainfrom
pr2-idiom-all-different
Aug 25, 2026
Merged

Recognize an all-different check, and say what replacing it costs#42
zmaril merged 6 commits into
mainfrom
pr2-idiom-all-different

Conversation

@zmaril

@zmaril zmaril commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Stacked on #41.

Recognizes a hand-rolled all-different check and recommends
itertools::Itertools::all_unique, with the conditions under which that swap is
sound.

Why this needs a recognizer rather than a derived behavior

No library checks distinctness by comparing every pair. 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.

It is matched by the same unification everything else goes through. Bindings
already works out what every hole stood for and used to discard it at the door;
Form::resolve_all hands it back as Resolved, so the conditions are stated
about 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:

  • reaching the answer through a hash set needs Eq + Hash where the loop needs
    only PartialEq — not pedantry, since f64 is PartialEq and not Eq, and
    two NaNs are unequal to each other, so the loop calls them distinct and the
    set cannot be built at all
  • it allocates where the loop does not
  • it need not run the comparison at all where the loop runs it quadratically
  • at four elements the loop wins

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 the
reaction (println!("no"); return; and ans = false, not return 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.

continue is the commonest thing a pairwise equality test does — six times more
common 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

  • 322 tests, clippy clean.
  • Coverage on Pairwise has two inhabitants and both are measured; a walk over
    adjacent pairs is deliberately NOT one of them here (it arrives in Catalog the standard library, and recognize a sortedness check #43).
  • Idiom is compiled in rather than pack data. Now that the shape is a Form
    it 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.

Base automatically changed from pr1-normalize-index-span to main August 25, 2026 18:50
zmaril and others added 6 commits August 25, 2026 20:50
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
zmaril force-pushed the pr2-idiom-all-different branch from 20bfb13 to 83b770f Compare August 25, 2026 18:50
@zmaril
zmaril merged commit dd2d08c into main Aug 25, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant