Skip to content

fix(semantics): resolve #' function designators to their local binding - #101

Merged
takeokunn merged 2 commits into
mainfrom
fix/binding-table-function-designators
Aug 3, 2026
Merged

fix(semantics): resolve #' function designators to their local binding#101
takeokunn merged 2 commits into
mainfrom
fix/binding-table-function-designators

Conversation

@takeokunn

Copy link
Copy Markdown
Collaborator

references.rs returned early on a Function reader prefix, so #'g was dropped entirely:

(flet ((g () 1)) (mapcar #'g list))   ; -> zero references to g

Any rule reading BindingTable sees a live, called function as unused — and would advise deleting working code.

(function g) long-hand had the same symptom by a different path: reader_form matched the shape and returned true without ever visiting the name. Two drops, one bug.

The early return was about namespace, not about #'car

Deleting it makes (let ((g 1)) #'g) resolve to the variable g#' on a variable designates nothing in CL, and the value layer would then propagate a variable's value into a function designator.

So the fix overrides the namespace instead of cancelling the occurrence:

let namespace = if view.reader_prefixes.contains(&ReaderPrefix::Function) {
    Namespace::Function
} else {
    namespace
};

Namespace::admits keeps a Lisp-2 honest: the variable case still misses (verified — (let ((g 1)) (mapcar #'g list)) records zero references post-change), and #'car resolves to nothing unless the file really did flet a local car, in which case it genuinely is a reference. The quasiquote check moved above the prefix check — behaviour-preserving, both returned before — so '#'g and `(a #'g) stay inert.

No shipped rule or report changes — and I verified why

corpus inspect lint pre post gained lost
SBCL (1428 files) 23309 23309 0 0
Quicklisp (623 files) 5302 5302 0 0

Byte-identical JSON, and the same for every semantic report. The brief's premise that "four shipped rules consume this table, so fixing it will change their findings" does not hold, and the reason matters: the only consumer that reads Binding::references() emits reference_count solely for propagated bindings, and blocked_reason returns NotAVariable first — so a Function binding never reaches that field.

The zero is not a dead harness. Because the old walk resolved exactly zero designators by construction, the post-change resolved count is the delta:

corpus designator refs gained local callables 0 → ≥1 references
SBCL 507 279
Quicklisp 94 46

325 bindings across ~2000 files stop looking unused — the same order as the 250 that motivated this. Four adjudicated in source, all reachable only through #': sb-concurrency's enc-1 under dynamic-extent, uiop's separatorp via position-if, full-eval's to-native-funs via mapcar, win32-sockets' make-definition via mapcar.

Two claims in the brief were wrong

Neither let_report nor unused_local_callable_report touches BindingTable — they use lexical_scope and callable_scope. The latter's callable_reference_target already handled #' correctly, which is why inspect unused-local-callables never had this bug.

Triage of the five other reported gaps

  • loop is not a modelled binder — confirmed, and worse than reported. It is in head_has_registered_semantics, so the scope is marked transparent while binding nothing. In (let ((j 1)) (loop for j from 1 to 10 do (print j))) the outer j absorbs both the binder occurrence and the body's use — the false-positive direction, and a rename driven by this table would rewrite the loop variable. Deferred (LOOP's clause grammar is its own sub-language); a one-line mitigation is to mark loop scopes opaque.
  • Template splice in operator position — REFUTED. `(,fn 1 2) resolves correctly. The cited SBCL instance (target-sxhash.lisp:410-437) fails for the next reason instead — both uses sit under #+64-bit.
  • Reader conditionals inside a quasiquote — confirmed. Outside one the dispatch is marked opaque; inside one it is not, because atom_in returns on quasiquote depth before reaching is_reader_dispatch. The scope claims transparency while a live reference is hidden. Not bundled: opacity feeds BlockedReason::OpaqueScope and value propagation, so it needs its own differential. Recommended as the next change.
  • declare (ignore …) — confirmed, well-scoped, deferred. Needs a new public field on Binding, which moves the semantic-report envelopes and pinned counts. The machinery exists as a template in special_names.rs.
  • Cross-file specials — not a bug at this layer. BindingTable is one file's lexical context by contract, and inferring specialness from earmuffs is the trap that already killed two rules in this repo. The right home is the project layer.

A hole in the existing oracle, documented rather than papered over

The differential partition oracle cannot catch this class: its bound set is derived by filtering the live occurrences, and #' atoms were excluded from live on both sides — so the bug was, and would again be, invisible to it. That's now noted in collect_live.

Verification

cargo build --workspace, cargo test --workspace (semantics 453 passed, up from 448), cargo test --test cli (3083 passed), cargo fmt --all --check, cargo clippy --all-targets --all-features -- -D warnings — all exit 0. No golden or pinned count moved. packages/core/syntax untouched.

`references.rs` returned early on a `Function` reader prefix, so `#'g`
was dropped entirely: `(flet ((g () 1)) (mapcar #'g list))` recorded
**zero** references to `g`. Any rule reading the table sees a live,
called function as unused -- and would advise deleting working code.

`(function g)` long-hand had the same symptom by a **different** path:
`reader_form` matched the shape and returned true without ever visiting
the name. Two drops, one bug.

**The early return was about namespace, not about `#'car`.** Deleting it
makes `(let ((g 1)) #'g)` resolve to the *variable* `g`, and the value
layer would then propagate a variable's value into a function
designator. So the fix overrides the namespace rather than cancelling
the occurrence, and `Namespace::admits` keeps a Lisp-2 honest: the
variable case still misses, verified. The quasiquote check moved above
the prefix check -- behaviour-preserving, both returned before -- so
`'#'g` and `` `(a #'g) `` stay inert.

**No shipped rule or report changes.** 23309 findings on SBCL and 5302
on Quicklisp, byte-identical JSON before and after, and the same for
every semantic report. I verified *why* rather than trusting the zero:
the only consumer reading `Binding::references()` emits `reference_count`
solely for propagated bindings, and `blocked_reason` returns
`NotAVariable` first, so a `Function` binding never reaches that field.

The zero is not a dead harness. Because the old walk resolved exactly
zero designators by construction, the new resolved count *is* the delta:
**507 designator references gained on SBCL and 94 on Quicklisp, taking
325 local callables from 0 to at least 1 reference** -- the same order as
the 250 that motivated this. Four adjudicated in source, all reachable
*only* through `#'`: sb-concurrency's `enc-1` under `dynamic-extent`,
uiop's `separatorp` via `position-if`, full-eval's `to-native-funs` via
`mapcar`, and win32-sockets' `make-definition` via `mapcar`.

Two claims in the brief were wrong: neither `let_report` nor
`unused_local_callable_report` touches `BindingTable`. The latter uses
`callable_scope`, whose `callable_reference_target` already handled `#'`
correctly -- which is why `inspect unused-local-callables` never had
this bug.

Triage of the five other reported gaps:

- **`loop` is not a modelled binder -- confirmed, and worse than
  reported.** It is in `head_has_registered_semantics`, so the scope is
  marked *transparent* while binding nothing: in
  `(let ((j 1)) (loop for j from 1 to 10 do (print j)))` the outer `j`
  absorbs both the binder occurrence and the body's use. That is the
  false-positive direction -- an outer binding reads as used when it is
  not, and a rename driven by this table would rewrite the loop
  variable. Deferred; one-line mitigation is to mark `loop` opaque.
- **Template-splice-in-operator-position -- refuted.** `,x` resolves
  correctly; the cited SBCL instance fails for the *next* reason.
- **Reader conditionals inside a quasiquote -- confirmed.** Outside one
  the dispatch is marked opaque; inside one it is not, because `atom_in`
  returns on quasiquote depth before reaching `is_reader_dispatch`. The
  scope claims transparency while a live reference is hidden. Not
  bundled: opacity feeds `BlockedReason::OpaqueScope` and value
  propagation, so it needs its own differential.
- **`declare (ignore ...)` -- confirmed, well-scoped, deferred.** Needs a
  new public field on `Binding`, which moves the semantic-report
  envelopes and pinned counts.
- **Cross-file specials -- not a bug at this layer.** `BindingTable` is
  one file's *lexical* context by contract, and inferring specialness
  from earmuffs is the trap that already killed two rules.

The differential test file gained a note rather than a paper-over: the
existing partition oracle cannot catch this class, because its `bound`
set is derived by filtering the live occurrences and `#'` atoms were
excluded from `live` on both sides -- so the bug was, and would again
be, invisible to it.
`lint-binding-analysis` merged in #99, after this fix was measured, and
the two interact: its corpus exercised `UnexplainedOccurrence` through a
`#'`-referenced local function, which was a *workaround for this very
bug*. With designators resolving the workaround is no longer reached,
and `the_correct_corpus_exercises_the_guards` failed -- correctly, since
it exists to catch a guard that has stopped earning its place.

The guard is not dead; `#'` was only one of the ways the table can lose
a reference. Replaced the corpus case with one that exercises it through
a gap that is still genuinely open: a reference buried in a reader
conditional in a *sibling* form.

Two things have to hold at once for this guard rather than `OpaqueScope`
to fire, and getting it wrong is instructive -- my first attempt put the
blob inside the binder and `OpaqueScope` claimed it. The parse folds
`#+64-bit (...)` into one opaque atom, so neither the binding table nor
a symbol-level scan sees the reference; and the blob sits *outside* the
binder, so the binder's own scope is not opaque and the cheaper guard
declines. `every_occurrence_is_explained` then reaches past the form to
the enclosing top-level one, finds an occurrence it cannot account for,
and refuses to conclude anything.

The two designator cases stay in the corpus, re-labelled as
*resolution* cases rather than *suppression* ones: if a future change
re-breaks designator resolution they go back to reporting, which is
exactly what they should do.
@takeokunn
takeokunn force-pushed the fix/binding-table-function-designators branch from 1f2d2d1 to 33abb2f Compare August 3, 2026 12:57
@takeokunn
takeokunn merged commit 1138691 into main Aug 3, 2026
10 checks passed
@takeokunn
takeokunn deleted the fix/binding-table-function-designators branch August 3, 2026 13:23
takeokunn added a commit that referenced this pull request Aug 3, 2026
RULE_COUNT 334 -> 345. Four standalone commands (the Clojure rules);
Racket and Emacs Lisp are registry-only.

lint-racket-depth (5): racket-match-unreachable-clause (Error),
racket-for-comprehension-value-discarded, racket-begin0-single-form
(fixable), racket-case-lambda-single-clause (fixable),
racket-parameterize-empty-bindings.

lint-elisp-depth (2): elisp-process-filter-assumes-whole-output,
elisp-repeating-timer-handle-discarded.

lint-clojure-depth (4, all Error): go-block-blocking-channel-op,
parking-op-outside-go-machinery, contains-on-non-associative,
reference-type-operator-mismatch.

All three batches dropped most of what they proposed, and the
refutations are the useful part.

**Racket** got both a corpus and a toolchain -- 4492 .rkt files and
Racket v9.2 -- so every premise was executed. Typed Racket's `Any`
does the **opposite** of what was proposed: `(needs-number (takes-any
5))` is a type error, `expected: Number given: Any`, so `Any`
maximally *constrains* consumers and `(-> _ Any)` is idiomatic for
side-effecting functions. That is the second time a Typed Racket
shape-match premise has been refuted here. `module+` reads a *later*
`define` fine, because it is `module*` with `#f`, declared after the
body. `define/contract` on a non-exported function still guarantees
every internal caller -- the boundary is definition-vs-rest-of-module,
verified by a same-module call raising while the recursive call did
not.

`racket-contract-out-arity-mismatch` was built, tested, corpus-audited
and then **deleted on measurement**: 1.25ms for one realistic file,
~40000x the control, super-linear -- and zero findings over 214
`contract-out` occurrences, so the cost bought nothing.

**Emacs Lisp shipped 2 of 8, and that is the honest number.** A bulk
writer proved the process-filter bug: 10000 lines in one write reached
the filter as 8 invocations with **7 mid-line splits**, because
`read-process-output-max` is 65536 -- so the defect passes every
small-output test and corrupts data past 64 KiB. Refuted: `goto-char`
*clamps* rather than signalling; `advice-remove` compares with `equal`
so a lambda **is** removable; and `setq` on a buffer-local variable
creates a buffer-local binding, which had already been refuted once in
this project and was re-proposed anyway.

Two more elisp rules were built, audited and deleted rather than
shipped at 11% and 20% fire rates on GNU Emacs's own tree: narrowing
(112/1044 -- rmail, gnus and ediff narrow as their *display model*) and
display properties (500/2679 -- `font-lock-ensure` leaves
`buffer-modified-p` nil, so `'face` writes during fontification are
correct).

**Clojure**'s `go`-block rule survived a check that could have killed
it: core.async *does* ship a blocking-op detector, but
`check-blocking-in-dispatch` fires only under a system property read
once at namespace load, which its own docs call development-only and
"covers only part of the problem". `go-impl` does no static inspection.
Conversely `defrecord-implements-protocol-method-not-in-protocol` **is**
a compile error -- `Compiler.java:9128` throws "Can't define method not
in interfaces" -- so that rule died. And a no-init `reduce` rule died on
measurement: 35 of 172 corpus occurrences pass a literal fn with no
0-arity, all working code.

`dynamic-scope-returns-lazy-seq` was built, corpus-run at 978
candidates / 1 finding / **0 true positives**, and withdrawn.

Wiring notes:

- **`RULE_DOCS` had to become `static`.** The 345th rule crossed
  clippy's default 16 KiB `array-size-threshold` (345 x 48 = 16560 B;
  at 334 it was 16032, just under). Took clippy's own suggestion rather
  than an `#[allow]`: nothing reads it in a const context, so `static`
  is also the substantive fix, since a `const` is substituted into every
  use site. That threshold is now permanently crossed.
- The two `Fixable` Racket rules needed a **Racket** fixture in
  `fixable_rules_match_the_fix_engine` -- the Scheme fixture does not
  reach them, because `lint-scheme-idiom` declares `[Scheme, Racket]`
  while these declare `[Racket]` alone. Proved by removing the line and
  watching exactly those two go missing. Third time this test has caught
  a `--fix` that would silently do nothing.
- treefmt reformatted `lint-racket-depth`'s own fixtures: the
  `tests/fixtures/*` exclusion is root-anchored and does not cover
  `packages/feature/*/tests/fixtures/`. Same trap as PR #88.
- `docs/src/reference/architecture.md` had drifted across PRs #97, #99,
  #101 and #102 -- it claimed 320 rules and 66 packages. Rewritten with
  measured values.

Two parser gaps found and reported, not fixed. **Racket: 1777 of 4492
files (39.6%) fail to parse** on `#rx`/`#px`, `#%kernel`, `#'`/`` #` ``,
`#<<` here-strings, `#hash`, `#"..."` -- which caps what any Racket rule
in this workspace can see. **Emacs Lisp: 191 of 1674 (11.4%)**, 153 of
them radix literals (`#x010101`, `#b01111110`, `#o777`, `#24r1k`) in
ordinary files like `bookmark.el`, `ansi-color.el` and `calc.el`.
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