Skip to content

fix(codegen): consume original code units for length-changing case-insensitive folds#662

Open
spokodev wants to merge 1 commit into
peggyjs:mainfrom
spokodev:fix/case-insensitive-length-changing-fold
Open

fix(codegen): consume original code units for length-changing case-insensitive folds#662
spokodev wants to merge 1 commit into
peggyjs:mainfrom
spokodev:fix/case-insensitive-length-changing-fold

Conversation

@spokodev

Copy link
Copy Markdown

Repro

start = "İ"i .

"İ" is U+0130 (LATIN CAPITAL LETTER I WITH DOT ABOVE): one UTF-16 code unit, but "İ".toLowerCase() is "i" + U+0307 (two code units).

grammar input expected actual on main
start = "İ" . (case-sensitive control) "İx" ["İ", "x"] ["İ", "x"]
start = "İ"i . (case-insensitive) "İx" ["İ", "x"] Expected "İ" but "İ" found.

The control parses fine; only the i (case-insensitive) flag breaks. The expected == found error text is the tell of a broken match length. Same-length folds ("ß"i, "K"i, "Σ"i) are unaffected, which isolates the length change as the cause.

Root cause

A case-insensitive string literal is stored already lower-cased, so for a SOMETIMES_MATCH literal the bytecode is:

MATCH_STRING_IC <s>   // literals[s] = node.value.toLowerCase()  -> "i̇", length 2
ACCEPT_N <n>          // n = node.value.length                   -> 1

In generate-js.js, MATCH_STRING_IC used literals[litNum].length (the lower-cased length, 2) as the input chunk length, while the paired ACCEPT_N advances by the original length (1). When the two disagree:

  • the fast-path fusion guard (bc[...] === inputChunkLength, i.e. 1 === 2) fails, so the match falls back to the unfused form;
  • the generated test reads input.substr(peg$currPos, 2).toLowerCase() and compares it to the 2-unit literal, but only 1 code unit is ever consumed.

For "İx", input.substr(0, 2).toLowerCase() is "i̇x" (3 units), which never equals "i̇", so the match fails. The read span, compare span, and accept span must all be equal.

Fix

In the MATCH_STRING_IC case, derive the input chunk length from the paired ACCEPT_N count (the original code-unit length) instead of the lower-cased literal length:

const inputChunkLength = bc[ip + 4] === op.ACCEPT_N
  ? bc[ip + 5]
  : literals[litNum].length;

The generated test now reads and consumes node.value.length code units and lower-cases that span before comparing, so the read, compare, and accept spans agree. MATCH_STRING_IC is only emitted by literal() paired with ACCEPT_N node.value.length, so this is always available; the literals[litNum].length fallback is defensive.

For same-length folds the chunk length is unchanged, the fusion fast path still engages, and the generated output is byte-for-byte identical (verified: the bootstrap lib/parser.js regenerates with no diff).

Tests

Added a behavior test under literal > matching:

it("matches a case-insensitive literal whose lower case changes its length", () => {
  const parser = peg.generate("start = \"İ\"i .", options);
  expect(parser).to.parse("İx", ["İ", "x"]);
});
  • Red on main (fails across all option variants), green with the fix.
  • Full suite green: 29 test suites, 1375 tests passing.
  • npm run parser regenerates lib/parser.js with no diff (bootstrap/self-compile stable), lint clean.

…sensitive folds

A case-insensitive string literal whose `.toLowerCase()` changes the number
of UTF-16 code units (for example `"İ"i`, where U+0130 is one code unit but
lower-cases to `"i" + U+0307`, two units) was mis-compiled.

For `MATCH_STRING_IC`, generate-js used the stored, already-lower-cased
literal's length as the input chunk length, while the paired `ACCEPT_N`
advanced by the original literal's length. When the two lengths disagreed,
the fast-path fusion guard failed and the generated test read more code units
than it consumed, so `start = "İ"i .` failed to parse `"İx"` even though the
case-sensitive `start = "İ" .` parses it to `["İ", "x"]`.

Use the paired `ACCEPT_N` count (the original code-unit length) as the input
chunk length so the read, compare, and accept spans agree. Same-length folds
keep the existing fused fast path and unchanged output.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@spokodev
spokodev force-pushed the fix/case-insensitive-length-changing-fold branch from 8e131ac to d2745b9 Compare June 25, 2026 17:26
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.49%. Comparing base (1f3f6a9) to head (d2745b9).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #662   +/-   ##
=======================================
  Coverage   99.48%   99.49%           
=======================================
  Files          34       34           
  Lines        4313     4314    +1     
=======================================
+ Hits         4291     4292    +1     
  Misses         22       22           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@hildjj

hildjj commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Great catch. If you're into Turkish, how do you handle case-insensitive matching of "I" and "ı" (U+0131: LATIN SMALL LETTER DOTLESS I)? Full Unicode case folding is more complex and locale-specific than toLowerCase does, I assume?

@spokodev

Copy link
Copy Markdown
Author

peggy folds with plain .toLowerCase() on both sides (the MATCH_STRING_IC op emits chunk.toLowerCase() === literal), so matching is locale-independent. For "I"i:

  • input I (U+0049): matches (toLowerCase gives i)
  • input i (U+0069): matches
  • input ı (U+0131, dotless): does not match, since "ı".toLowerCase() is still ı, not i
  • input İ (U+0130, dotted): does not match "I"i, but "İ"i now matches it (and i + U+0307), which is the length-changing case this PR fixes

So I and ı stay distinct, which is the Unicode default (untailored) behavior. The Turkish equivalence (I to ı, İ to i) only appears under toLocaleLowerCase('tr'), which peggy does not use, and reasonably so: a locale-sensitive fold would make the same grammar parse differently depending on the host locale.

One nuance: even full Unicode case folding (CaseFolding.txt) keeps I and ı distinct by default. The I/ı merge lives only in the Turkic-tailored (T) rules, so it is the locale tailoring that creates the equivalence, not the fold-versus-lowercase difference.

This PR does not touch any of that. It keeps the existing toLowerCase semantics and only corrects the consumed length when the fold changes UTF-16 length (İ to i + U+0307), so the set of matched inputs is unchanged; those cases just stop throwing the spurious length error.

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