Skip to content

LL(1): match with an explicit stack and a shared-input cursor - #1531

Merged
sergey-shandar merged 4 commits into
mainfrom
claude/epic-fermi-ynvbhs
Aug 13, 2026
Merged

LL(1): match with an explicit stack and a shared-input cursor#1531
sergey-shandar merged 4 commits into
mainfrom
claude/epic-fermi-ynvbhs

Conversation

@sergey-shandar

Copy link
Copy Markdown
Contributor

Fixes fjs/bnf/ll1/todo/stack-recursive-matching.md (deleted here).

Problem

fjs/bnf/ll1's matcher recursed natively: after dispatching on a code point it walked the selected rules chain with one nested call per rule. A right-recursive rule — how repeat0Plus encodes repetition — contributes one such chain per repetition, so match depth grew with input length, not grammar size.

On main, parser(repeat0Plus(set(' \t'))) throws RangeError: Maximum call stack size exceeded on 4,000 spaces (the issue recorded 10,000; the threshold has since drifted down), and deterministic() overflows at 5,000 bracket levels — deep nesting reaching the limit through the same mechanism. The sibling descent backend had the same bug, fixed in #1303; the LL(1) matcher was never touched.

Consuming a code point also re-sliced the remainder (const [, ...restCp] = cp), copying the rest of the input at every step — quadratic time and allocation.

What changed

  • f in parserRuleSet is an explicit-stack machine. Suspended rules-chain frames (tag, rules, ruleIndex, seq) live on an immutable cons-cell stack; each iteration either dispatches the current task's rule or feeds the pending result into the innermost frame. The JS call stack stays O(1). LL(1) never backtracks, so — unlike the descent backend — no frame needs rewind state.
  • Positions are cursors into one shared input array, over 0 .. cp.length + 1, where cp.length + 1 means the synthesized EOF has been consumed: the (idx, eofConsumed) pair written as one number, replacing the remainder slice plus separately threaded flag. The public MatchResult remainder is materialized once, when the match returns, and stays physical exactly as before (null still means the match ran out of input).
  • Types: MatchRule and _MatchResultEof described the recursive matcher's signature and are removed — nothing outside the module used them. _DispatchRuleCollection.rules becomes readonly string[]: dispatchMap only ever appends rule names, so the _DispatchRule | string union was dead, and its unreachable branch was this module's one uncovered branch.
  • README.md documents the complete cursor and why matching cannot use the JS call stack here.

Verification

  • New longInput proof group mirroring fjs/bnf/descent/proof.f.mjs: 10,000-code-point repeat0Plus repetition and 5,000-level bracket nesting via deterministic(). Both overflow on main and pass here (34 ms and 37 ms).
  • Behavioral equivalence was checked differentially against the old matcher: 17 grammars (including eof shapes, deterministic(), and the cyclic-nullability grammar) × ~400 pseudo-random inputs plus fixed cases — 6,640 comparisons of JSON.stringify(MatchResult), all identical, plus 15 longer inputs (100–900 code points) where the recursive version still ran.
  • npx tsc clean; node ./fjs/module.mjs t — 2539 pass, 0 fail; bun run of the fjs/bnf subtree passes.
  • Coverage for fjs/bnf/ll1/module.f.mjs is now 100% line / branch / function (98.44% branch on main).

Not addressed: dispatchMap's build-time recursion (dispatchRule) is bounded by grammar size, not input size, and needs no change — as the issue noted.

🤖 Generated with Claude Code

https://claude.ai/code/session_014tCLFXucKxrz85w3D8MjQW


Generated by Claude Code

The LL(1) matcher recursed once per rule in a dispatched chain, and a
right-recursive rule — how `repeat0Plus` encodes repetition — puts one
such chain per repetition, so match depth grew with input length rather
than grammar size. `parser(repeat0Plus(set(' \t')))` overflowed the JS
call stack on a few thousand spaces, and deep bracket nesting hit the
same limit the same way. The sibling descent backend was fixed in #1303;
this ports that fix.

`f` in `parserRuleSet` is now an explicit-stack machine: suspended
rules-chain frames live on an immutable cons-cell stack, and each
iteration either dispatches the current task's rule or feeds the pending
result into the innermost frame.

Positions are now cursors into one shared input array over
`0 .. cp.length + 1`, where `cp.length + 1` means the synthesized EOF has
been consumed — the `(idx, eofConsumed)` pair as one number, replacing
the remainder slice plus threaded flag. Consuming a code point no longer
re-slices the rest of the input (which made a match quadratic); the
`MatchResult` remainder is materialized once, when the match returns.

`MatchRule` and `_MatchResultEof` described the recursive matcher's
signature and are removed; nothing outside this module used them.
`_DispatchRuleCollection.rules` becomes `readonly string[]`: the builder
only ever appends rule names, so the `_DispatchRule | string` union was
dead and its unreachable branch was the module's one coverage gap.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014tCLFXucKxrz85w3D8MjQW
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
functionalscript f5c8e3e Commit Preview URL

Branch Preview URL
Aug 13 2026, 07:52 PM

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014tCLFXucKxrz85w3D8MjQW
@sergey-shandar
sergey-shandar marked this pull request as draft August 13, 2026 19:20
Rewrites 669-bnf-data-shared-helpers against today's code and renames it:
the backends moved out of `bnf/data`, both result constructors are records
now, and the duplication has grown past the `mrSuccess`/`mrFail` pair the
issue described. `ll1` and `descent` each carry the complete cursor, the
AST family, and the constructors, and `AstTag` is declared byte-identically
in both `types.ts`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014tCLFXucKxrz85w3D8MjQW

@o2alexanderfedin o2alexanderfedin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. Verified against origin/main at c51e6dfa (merge-base 893b4eeb).

The head moved mid-review (1b7a34f25b421413). The new commit touches only
fjs/bnf/todo/*.md; git diff 1b7a34f2 5b421413 -- fjs/bnf/ll1 CHANGELOG.md is empty, so
the code verification below carries over unchanged, and I re-ran the link check at the new
head.

Gates

  • npx tsc --noEmit → exit 0.
  • npm run prepack from a freshly cleaned tree → exit 0 (both passes).
  • npm test2539 pass, 0 fail. Merge-base 893b4eeb is 2537, origin/main is 2538
    (the +1 is #1528, which this branch is behind). 2537 + 2 new longInput cases = 2539. Exact.
  • Link check → broken links 140 → 137, removals only, no additions. The todo/*.md
    reference that pointed at the deleted ll1/todo/stack-recursive-matching.md was
    redirected to the longInput proof groups rather than left stranded, and nothing else in
    the tree still names that file.
  • Dual-axis surface diff (526 → 527 type aliases, 889 → 889 consts, both non-empty):
    added _Cursor, _Position, _Result, _Frame, _Stack, _Task — all _-prefixed
    per §6.2; removed MatchRule, _DispatchRuleOrName, _MatchResultEof. No unprefixed
    public type additions, no signature widened to any. @module survives in the emitted
    module.f.d.mts and types.d.ts; no elided / any in the emitted ll1 declarations.
  • CHANGELOG: present, links only /pull/1531, and correctly carries **BREAKING CHANGES:**
    — removing the exported MatchRule is a real public-surface removal.

Behavioural equivalence with main

The thing I most wanted to rule out is the #1516 hazard: encodeTerminal is not
order-preserving (EOF is semantic -1 but encodes to the top of the encoded space), so a
rewrite of the matching loop is exactly where a comparison could regress onto encoded
values, and an ASCII-only test suite would not notice. It has not regressed here — the
dispatch map still holds decoded terminals (rangeDecode), and symbolAt yields a code
point or eofSymbol (-1), so every comparison is on decoded values.

I checked that behaviourally rather than by reading. Differential harness importing both
matchers side by side, comparing full JSON.stringify(MatchResult) — AST, success flag and
remainder — over 15 grammars × fixed inputs plus 4000 seeded-random inputs drawn from an
alphabet straddling ASCII / non-ASCII / structural characters:

compared=4510 mismatches=0

The grammars deliberately include the cases ASCII tests miss: range([0x80, 0x10FFFF]),
unicodeMax, remove(unicodeMax)(set('"')), a variant whose branches straddle
low / high / eof, and { z: range([0,0]), e: eof } — EOF adjacent to code point 0, the
one place a top-of-space encoded ordering would reorder. Inputs include lone U+10FFFF,
U+10FFFF before and after ASCII, and U+0000.

Negative controls, both fired:

  • symbolAt masked to & 0xFFFF (only astral code points affected) → 3 mismatches.
  • symbolAt returning 0xFFFFFF for EOF, i.e. exactly the "compared the encoded value"
    regression → 1071 mismatches.

So the harness is sensitive to both failure modes and the unmutated PR is clean on all 4510.

Whole-input grammars (the #1516 finish = beforeEOF || afterEOF property): [body, eof],
[eof], [eof, eof], [range('AA'), eof], [repeat0Plus(unicodeMax), eof] are all in the
harness and all agree with main. The full suite passing also covers the djs / js /
media/json tokenizers and parsers that ride on this backend.

Explicit stack, depth and termination. Compared both backends at increasing sizes:

n      flat repeat0Plus        deep [ ... ] nesting
1000   main ok      pr ok      main ok      pr ok
2000   main OVERFLOW pr ok     main OVERFLOW pr ok
50000  main OVERFLOW pr ok     main OVERFLOW pr ok

Identical results everywhere main survives, and main really does die at ~2000 — the PR's
stated motivation is accurate, not a hypothetical. Termination is sound: a frame is only
pushed when a symbol is consumed and pos > cp.length short-circuits dispatch to null,
so total pushes are bounded by cp.length + 1 and heap stack depth is O(n).

I also walked the case table against main's f/items and it lines up on every branch,
including the three-way dr === null split (emptyTag defined → success at pos;
pos >= cp.length → success with null remainder; otherwise fail at pos) and the
pos > cp.length state that replaces the old eofConsumed flag.

Mutation-testing the two new proof cases rather than trusting them: reverting the
remainder slice (remainderAt → return cp) fails both longInput[1] and longInput[2],
and both cases are exactly the shapes and sizes (10000 / 5000) at which main overflows — so
they genuinely pin the new paths. For completeness: they do not catch an AST-sequence
mutation ([...frame.seq, ast]frame.seq), which is fine given their stated purpose;
the 4510-case AST-level differential covers that ground.

rules: readonly string[] narrowing is sound — the builder only ever appends names
(addRuleToDispatch), starts from rules: [], and addTagToDispatch preserves them, so
no _DispatchRule object was ever reachable in that position.

Not verified

  • Coverage numbers. npm run cov is not trustworthy in this environment (Node v23) and
    reports the same vacuous result on main.
  • The "no longer quadratic" half of the CHANGELOG claim I confirmed only directionally
    (n=50000 completes promptly), not by fitting a curve.

Nothing to change.

@sergey-shandar
sergey-shandar marked this pull request as ready for review August 13, 2026 19:52
@sergey-shandar
sergey-shandar added this pull request to the merge queue Aug 13, 2026
Merged via the queue into main with commit 373c332 Aug 13, 2026
19 checks passed
@sergey-shandar
sergey-shandar deleted the claude/epic-fermi-ynvbhs branch August 13, 2026 21:09
@sergey-shandar sergey-shandar mentioned this pull request Aug 14, 2026
sergey-shandar pushed a commit that referenced this pull request Aug 14, 2026
Release under the new directory-per-version changelog workflow:
changelog/unreleased/ is renamed to changelog/0.45.0/ with its entry
files kept as they are (.gitkeep dropped — the next entry PR recreates
unreleased/). Minor bump: the release contains BREAKING CHANGES entries
(#1516, #1520, #1530, #1531, #1547).

Update AGENTS.md §8.3–8.4 and changelog/README.md for the new workflow:
releasing renames the directory instead of concatenating entries, and
future entries carry no PR number or link inside the file — the file
name already has it. Released entries are kept as-is. Extend
todo/changelog-website.md so the future generator reads both release
forms: <version>.md files (through 0.44.0) and <version>/ directories.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M296KXwQHHuUGhryRReKpJ
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.

3 participants