Define left recursion by the left-corner relation; reject indirect/hidden cycles at build time#29
Merged
Conversation
The parser guaranteed termination via a local syntactic check - does an alternative's items[0] reference the rule itself? That under-approximates left recursion: it misses INDIRECT cycles (A -> B -> A) and recursion HIDDEN behind a nullable prefix (A -> opt(x) A ...). Both re-enter the rule at the same input position, so recursive descent overflowed the stack with an uncaught RangeError at PARSE time, with no diagnostic at grammar-build time. Make the left-corner relation the single source of truth: a rule is left-recursive iff it can reach itself through the transitive closure of the left-corner edge map (nullable-aware, reusing nullableRules; op/prefix/postfix are left-edge barriers). items[0]===self is demoted to which alternatives the local atom/continuation transform can peel. A left-recursive rule is handleable iff peeling its direct self-alts breaks every cycle (residual graph acyclic) -> classifyLeftRec/Pratt handle it unchanged; otherwise it is rejected at build time with a diagnostic naming the cycle. Behavior-identical for all 7 grammars (they regenerate byte-for-byte; sanity suite unchanged). Adds test/left-recursion.ts.
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.
Problem
The parser's termination guarantee rested on a local syntactic check —
isLeftRecursive, which asked of each rule in isolation: does some alternative'sitems[0]reference the rule itself? That is an under-approximation of left recursion. It recognizes only the length-1, non-nullable witness and misses:A → B | …,B → A | …A → opt(x) A …(the first item isopt(x), not a bare self-ref)Both still re-enter the rule at the same input position, so recursive descent recurses without consuming and overflows the stack. The failure mode was the worst kind: nothing flagged the grammar at build time (
defineGrammar/createParserboth succeed), and parsing certain inputs threw an uncaughtRangeError: Maximum call stack size exceededfrom deep inside the engine, naming no offending rule.Minimal repro (a single-token input is enough):
Both
RangeErroron input"a", while the equivalent direct formA → A W | Wparses fine — proving the gap is specifically the syntactic detector, not left recursion as such.Root cause
Termination depended on a single static, local, syntactic classifier that under-approximates a global, nullable-aware property — whether a rule lies on a cycle in the left-corner relation — with no runtime backstop. The same engine already computes FIRST and nullability as global fixpoints over the rule graph (cycle-safe by construction); left-recursion detection alone reached for a syntactic shortcut, which is exact only for the direct operator/postfix recursion the shipping grammars happen to use.
Fix
Make the left-corner relation the single source of truth:
isLeftRecursive(rule)is now "the rule reaches itself through the transitive closure of the left-corner edge map" — nullable-aware (reusesnullableRules/exprNullable);op/prefix/postfixare left-edge barriers (they consume an operator token). This catches direct, indirect, and nullable-hidden recursion uniformly.items[0] === selfis demoted from "the definition of left recursion" to merely which alternatives the local atom/continuation (and Pratt NUD/LED) transform can peel into an iterative loop (peelsDirect).classifyLeftRec/ Pratt handle it unchanged. Otherwise it is rejected at build time with a diagnostic naming the residual cycle (A → B → A). The engine deliberately does not parse indirect/hidden left recursion — for a grammar→highlighter generator that is an authoring error, and rejecting it early (like tree-sitter/yacc reporting conflicts) is the correct behavior.prattRules/hasMarker,classifyAlts's peeling, and the entire Pratt parse path are untouched.Behavior identity
This is a structural unification, not a capability change:
*.tmLanguage.json/*.monarch.json/*.cst-types.ts/tree-sitter/**unchanged.npm test(sanity) unchanged: 15/15.test/left-recursion.ts(npm run test:left-recursion): direct LR parses; indirect and nullable-hidden LR are rejected at build time (the test asserts rejected, not overflow, so it guards the regression); the diagnostic names the rule and cycle. 4/4.