diff --git a/docs/issue-151-mutual-left-recursion-plan.md b/docs/issue-151-mutual-left-recursion-plan.md new file mode 100644 index 00000000..aa4b7e9b --- /dev/null +++ b/docs/issue-151-mutual-left-recursion-plan.md @@ -0,0 +1,396 @@ +# Issue #151: mutual (indirect) left-recursion support + +Status: design accepted, implementation in progress +Prepared: 2026-07-26 +Repository baseline: `7eb93072c4fd551db8ce7550e43c35f080f197c8` +Issue: +Validation target: `dotnet/roslyn` `CSharp.Generated.g4` + +## 1. Executive decision + +Accept the class of mutually-left-recursive grammars that ANTLR 4.13.2 rejects +with `error(119)` by **rewriting them, before ATN construction, into an +equivalent grammar that uses only direct left-recursion** — the form our +existing precedence machinery (`rewrite_immediate_left_recursion`) and ANTLR +itself already handle. This is approach (1) from the issue ("transform to an +equivalent accepted form"), chosen over approach (2) ("handle the cycle directly +in prediction") because it is provably correct by construction and requires zero +new runtime surface. + +The transform is **left-corner substitution** (informally, "hub inlining"): for +each left-recursive cycle, one rule is designated the *hub*; every other cycle +member (a *satellite*) has its body substituted into the hub's left-corner +positions until the hub is directly left-recursive, after which the satellite +either disappears (if it was referenced only from within the cycle) or is +retained as a non-recursive copy (if referenced externally). + +Two hard sub-cases are handled explicitly rather than guessed: + +- **Leading-optional recursion** (`hub? rest`) is expanded to `hub rest | rest` + before substitution — a union-preserving rewrite that ANTLR accepts. +- **Anything the direct-LR rewriter still cannot classify** after substitution + (recursive calls carrying arguments, mixed labeled/unlabeled alternatives, + ambiguous cycles with no non-recursive exit) is **rejected with a precise + diagnostic** naming the exact rule and reason — strictly better than ANTLR's + generic `error(119)`. This is staged acceptance criterion 1, and it ships + even for grammars we decline to transform. + +The correctness oracle is ANTLR itself: because the transform emits precisely +the direct-LR grammar we would otherwise hand to ANTLR, and our direct-LR +handling is already conformance-verified byte-for-byte against ANTLR's runtime, +tree-equality on the transformed grammar is both provable and differentially +testable. + +This does **not** touch direct-left-recursion handling, the runtime, prediction, +or the ATN interpreter. It is a model-level grammar transform confined to +`src/bin_support/grammar/`. + +## 2. Grounded current state + +The generator already has every piece this builds on: + +- **A mutual-left-recursion *detector*.** `indirect_left_recursive_components` + (`src/bin_support/grammar/atn/analysis.rs:250`) builds a left-corner graph + over the finalized ATN, runs Tarjan SCC, and emits `G4A005` + ("mutually left-recursive rules: [...]"). This is ATN-level and fires *after* + ATN construction. It stays as the backstop for cycles the new pass declines. +- **A direct-left-recursion *rewriter*.** `rewrite_immediate_left_recursion` + (`src/bin_support/grammar/left_recursion.rs:11`) classifies each alternative + of a directly-LR rule as Primary/Prefix/Binary/Suffix, rebuilds the rule into + the precedence-climbing `primary (operator)*` shape with precedence + predicates, and records `LeftRecursionInfo` for downstream codegen. It runs in + `analyze` (`src/bin_support/grammar/semantics.rs:90`), on the model, before + semantic analysis and ATN build. +- **A model-level transform boundary.** `TransformRegistry` + (`src/bin_support/grammar/transform.rs:88`) runs ordered `GrammarTransform` + passes over the integrated model with analysis invalidation + (`AnalysisInvalidation::{NAMES,CALLS,NULLABILITY,...}`) and `validate_model` + after each mutating pass. The default registry is currently empty. +- **Model-level reachability analysis.** `TransformAnalysis` + (`src/bin_support/grammar/transform_analysis.rs:29`) already computes + `call_graph`, `nullable`, and `recursive_components` over the model + (not the ATN). This is exactly the input the new pass needs, at exactly the + right phase. + +The new pass slots between "integrate/split" and +`rewrite_immediate_left_recursion`: it turns indirect LR into direct LR, then +the existing rewriter does the rest. + +```text +integrate + combined split + -> [NEW] mutual-left-recursion elimination (this pass) + -> rewrite_immediate_left_recursion (unchanged) + -> semantic analysis, numbering + -> ATN construction + G4A005 detector (unchanged backstop) + -> Rust emission +``` + +## 3. The tractable subclass, stated precisely + +Let the **left-corner relation** be: rule `A` left-calls `B` if some alternative +of `A` can reach a reference to `B` through only nullable/epsilon elements +(actions, predicates, optional/star quantifiers, and nullable rule calls) before +consuming any token. A **left-recursion cycle** is a strongly-connected +component (size > 1, or a self-loop) of this relation. This is the same relation +both existing analyses use. + +We **accept** a cycle when, after choosing a hub and performing left-corner +substitution, every resulting alternative of the hub is one the direct-LR +rewriter classifies as Primary, Prefix, Binary, or Suffix — i.e. its left corner +is either non-recursive (Primary/Prefix) or a single non-optional reference to +the hub (Binary/Suffix). Empirically (§4) this covers: + +- **Hub-and-spoke cycles** — one hub lists satellites as plain alternatives; + each satellite's left corner is the hub. All four Roslyn cycles are this + shape. Satellites collapse into the hub directly. +- **Chained/indirect cycles** — a satellite's left corner is *another* + satellite, not the hub (`a : b ... ; b : a ...`). Resolved by transitive + substitution: substitute along the left-corner chain until the reference + reaches the hub. Genuinely mutual `a <-> b` collapses this way too. +- **Multi-alternative satellites** — a satellite with several alternatives + contributes each alternative to the hub. + +We **decline** — leaving the grammar bit-for-bit unchanged, so the existing +ATN-level `G4A005` cycle diagnostic reports it exactly as it does today — when +any of the following holds. Each is a precondition checked *before* the model is +touched, and each has a dedicated decline test: + +- The grammar is a **lexer grammar**. Precedence rewriting is a parser-rule + construct; routing lexer rules through it produced an "unsupported embedded + lexer action" naming an action the grammar never declared. (Left-recursive + lexer rules are invalid in ANTLR regardless — `error(119)` — and diagnosing + them properly is tracked as issue #236.) +- The cycle has **no token-consuming base alternative** — the language is + ill-founded. Mirrors ANTLR's `error(169)` for the immediate case. +- A corner to be substituted is **not bare**: quantified (`b*`, `b+`), + **labelled** (`x=b`), **argument-bearing** (`b[3]`), or carrying element + options. Splicing one satellite body in place of `b*` would drop the closure + and change the accepted language; removing a labelled corner would leave `$x` + dangling in surviving actions. One shared predicate defines "bare" for every + corner derivation, so the checks cannot drift apart. +- The corner is a **nongreedy optional** (`b??`). The split emits the present + branch first — the greedy preference — which would invert the authored + nongreedy ordering, so only greedy optionals are split. +- A surviving **action or predicate references the removed rule by name** + (`$b.text` after the `b` corner is spliced away, on either the splice or the + optional-split path); the reference would dangle. +- The **hub declares arguments** (`e[int x]`). Every in-cycle corner is bare, + so it omits the required arguments; rewriting would delete the invalid call + before semantic call validation could reject it. Declining lets the real + missing-argument diagnostic surface. +- The satellite carries **any embedded action or predicate**. Semantic bodies + are owned by their rule and alternative: `$ctx` (and the semantic-context + parameter itself) means the satellite's context, and the embedded-action + pipeline resolves `$`-references against the enclosing alternative's source + span — neither survives transplantation into the hub. Pattern-matching the + body for the references that happen to break would be target-language + -specific and incomplete, so any inline semantics decline. Actions in the + *hub's* alternatives are unaffected — a rebuilt caller alternative keeps its + own span identity. +- After substitution, a recursive alternative's **tail is nullable** + (`e : e n | ID` with `n : ;`): the direct rewriter rejects a left-recursive + alternative that can be followed by the empty string, and it would do so + against the transformed rule. Declining keeps the diagnostic on the + authored cycle. +- Merging would **capture an implicit reference**: an action on one side of + the splice names a token, rule or label the other side introduces (`$ID` + binds by occurrence within its alternative, so a satellite body arriving + ahead of a caller action with another `ID` would capture the caller's + `$ID`). Inert duplicate occurrences without action references are fine. +- The corner sits **behind a nullable prefix** (`a : n b`, `n :`), so which rule + the author meant as the left corner is genuinely ambiguous. We decline rather + than guess. +- The plan would make **no substitution step at all** (the cycle runs through a + position this pass cannot rewrite, e.g. a corner inside a nested block). A + zero-step plan would otherwise be re-selected verbatim forever. +- A **satellite carries rule-level state** inlining would silently drop: + `arguments`, `returns`, `locals`, `throws`, `@init`/`@after`, `catch`, + `finally`, rule options, or `#`-labelled alternatives. The last also avoids + ANTLR's all-or-none alternative-label rule (`error(122)`); synthesizing labels + for spliced satellites remains deferred (§8). +- Splicing would **merge two overlapping label scopes** (caller and satellite + both bind `x`), which would rebind caller actions to the satellite's element. +- The resulting hub is not **Primary/Prefix/Binary/Suffix** throughout, or + substitution does not terminate within a bound (defensive; see §6). + +Because these are preconditions rather than post-hoc repairs, the pass either +emits a grammar the conformance-verified direct-recursion path accepts, or it +changes nothing — it can never accept-and-miscompile. + +### 3.1 The leading-optional wrinkle + +C#'s range operator is `range_expression : expression? '..' expression?`. Its +**leading** `expression?` is an *optional* left-recursive reference. The direct-LR +rewriter (and ANTLR) require the recursive left corner to be non-optional, so +this alternative alone re-triggers `error(119)` even after the rest of the +`expression` cycle collapses. + +The fix is a standard, union-preserving expansion applied *before* +substitution: an alternative of the form `X? rest` (where `X` is a cycle member) +becomes two alternatives `X rest | rest`. The first is now a well-formed +left-recursive alternative (Binary/Suffix); the second is a Primary. This is +verified to make ANTLR accept the grammar (§4) and does not change the accepted +language. + +## 4. Empirical validation (ANTLR 4.13.2 as oracle) + +Every claim below was checked by generating with ANTLR 4.13.2 and, where noted, +compiling and running the generated Java parser. Artifacts under +`target/roslyn-lab/` and the scratch harness. + +**The Roslyn grammar's actual blockers.** With the six empty rules corrected +(the epsilon `omitted_*` nodes made optional at use sites, the four lexical +stubs pointed at token names — the "minimal lexer adjustment"), ANTLR 4.13.2's +*entire* remaining error output is one line: + +```text +error(119): The following sets of rules are mutually left-recursive + [type, array_type, nullable_type, pointer_type] + and [name, qualified_name] + and [expression, assignment_expression, binary_expression, ...13 rules] + and [pattern, binary_pattern] +``` + +Four cycles. (The issue body also lists a `member_declaration` declaration +cycle; in the corrected staging it is **not** left-recursive — `record_declaration` +reaches `member_declaration` only behind a non-nullable token, so it never +appears in `error(119)`. One fewer cycle to handle.) + +**All four cycles are hub-and-spoke.** Every satellite is referenced only by its +hub, with one exception: `array_type` is also used by +`array_creation_expression`, so it must be retained as a non-recursive copy. + +**Hand-inlining resolves 3 of 4 cycles immediately.** A mechanical inliner that +splices each satellite into its hub and deletes hub-only satellites produces a +grammar ANTLR reduces to a single residual `error(119)` on `[expression]`. + +**The residual is exactly the leading-optional range operator.** Applying the +`X? rest -> X rest | rest` expansion to `expression? '..' expression?` yields a +grammar ANTLR **accepts with zero errors**. + +**The accepted grammar produces a working parser.** Generated as a combined +grammar with a minimal lexer, ANTLR compiles it and the parser cleanly parses +real C# exercising all four cycles — records, `switch` expressions, `is not` +patterns, `and`/`or` patterns, array/nullable types, dotted names, chained +invocation/element access — with no parse errors. + +**Boundary probes** (synthetic grammars, ANTLR verdicts): + +| Shape | ANTLR verdict | Disposition | +|---|---|---| +| `e : e '+' e \| e? '..' e? \| ID` (leading-opt) | `error(119)` | expand optional, then accept | +| `e : e '+' e \| e '..' e? \| '..' e? \| ID` (expanded) | accepted | the fix works | +| hub-and-spoke, satellites inlined | accepted | primary target shape | +| genuinely mutual `a<->b`, substituted along chain | accepted | broader than hubs | +| multi-alt satellite, all alts inlined | accepted | supported | +| recursive call with args `h '+' h[3]` | `error(80)` | reject with diagnostic | +| mixed labeled/unlabeled alts | `error(122)` | reject (label synth deferred) | + +## 5. Semantics and correctness argument + +Roslyn's grammar is machine-generated from the compiler's syntax model. Its +alternatives are ordered **alphabetically**, and every binary operator is lumped +into a single `expression op expression` alternative. The grammar therefore +encodes **no operator precedence or associativity** — real C# precedence lives +in Roslyn's hand-written recursive-descent parser, not in this `.g4`. The tree +it defines is the flat, precedence-agnostic tree ANTLR would build. + +This sharpens the issue's correctness bar. We are **not** claiming to reconstruct +C# precedence (the source grammar makes no such claim). We claim the narrower, +fully-provable property: + +> For any grammar we accept, the parser we generate produces the identical parse +> tree that ANTLR's own runtime produces from the direct-LR grammar our +> transform emits. + +The argument is compositional: + +1. **The transform preserves the language and the intended tree.** Left-corner + substitution is textual inlining of a rule body into a call site in + left-corner position — the classic indirect-to-direct left-recursion + elimination, language-preserving by construction. The optional-expansion step + is a union-preserving alternative split. We prove equivalence *differentially*: + for the accepted grammar, both ANTLR-from-transformed and + ours-from-transformed must agree, and both must agree with + ANTLR-from-original on the *set of accepted strings* (tree shape legitimately + changes when a satellite node is inlined — see §8 fidelity). +2. **The direct-LR rewriter is already correct.** It is exercised across the + full ANTLR runtime testsuite with zero skips; the transform feeds it only + inputs it already accepts (Primary/Prefix/Binary/Suffix alternatives). +3. **The rejection path is sound.** Any cycle we cannot reduce to that shape is + declined with a diagnostic; we never accept-and-miscompile. This is the + issue's hard requirement ("must never mean accepts and silently miscompiles"). + +## 6. Algorithm + +Input: the integrated parser model (`Vec`), `TransformAnalysis`. + +1. **Find cycles.** Compute the left-corner relation over the model (reuse the + nullable set and call graph from `TransformAnalysis`; refine call-graph edges + to left-corner edges by walking each alternative's prefix through nullable + elements). Tarjan SCC → cycles. Non-cyclic grammars are untouched (pass + reports `changed = false`). +2. **Per cycle, expand leading-optionals.** For every alternative in every cycle + member whose left corner is an *optional* reference to a cycle member, + rewrite `X? rest -> X rest | rest`. +3. **Choose the hub.** Prefer the rule that (a) has a non-recursive base + alternative and (b) is referenced from outside the cycle (the cycle's public + entry). For the Roslyn cycles this is unambiguous (`type`, `name`, `pattern`, + `expression`). Ties broken by lowest `RuleId` for determinism. +4. **Substitute to the hub.** Repeatedly: for each hub alternative whose left + corner is a satellite `S`, replace that leading `S` reference by inlining + `S`'s alternatives (one hub alternative per `S` alternative), carrying through + the trailing elements. Continue transitively until every hub alternative's + left corner is either non-recursive or the hub itself. Bound the iteration by + the total alternative count across the cycle; exceeding it is a defensive + rejection (should be unreachable for well-formed finite cycles). +5. **Classify and gate.** Run the direct-LR classifier on the rebuilt hub. If any + alternative is Nonconforming (args, etc.), reject the whole cycle with a + specific diagnostic and leave the model untouched (the ATN-level `G4A005` + backstop will then fire, or the specific new diagnostic supersedes it). +6. **Retire satellites.** Delete a satellite only when nothing that survives + still calls it: removability is computed against the *planned* hub + alternatives (substitution consumes only the corner occurrence — a second + reference in the same alternative, or in an alternative the plan keeps + verbatim, survives) plus every retained rule. A satellite referenced + externally (`array_type`) is kept as a non-recursive copy: its body already + refers to the hub, which is now the precedence rule, so external callers see + an ordinary rule. Symbol-conflict validation reads a snapshot of the units + taken *before* this pass runs, so a conflict involving a deleted satellite's + name (`e returns [i32 s]` vs rule `s`) is still reported against the + authored grammar. +7. **Provenance + labels.** Record substitution provenance so diagnostics and + generated-code comments can trace an inlined alternative back to its original + satellite rule. Attribution is split per planned alternative: the `#label` + and lexer commands come from the *hub* alternative (they name that + alternative position — authored API surface), while alternative options + **accumulate along the splice chain** — the hub alternative's options + unioned with those of every satellite alternative spliced into the + position, because `` rides on the alternative that declares + the operator, which may sit behind an alias-shaped satellite + (`a : b '^' e ; b : e ;`). Two alternatives declaring the + same option with different values along one chain is ambiguous, so the + cycle declines. Element labels and `$`-attribute references from satellite + bodies are preserved. + +The pass then hands off to `rewrite_immediate_left_recursion`, unchanged. + +## 7. Diagnostics (staged acceptance criterion 1) + +Even where we decline to transform, we improve on ANTLR. New codes: + +- `G4R010` — "mutually left-recursive cycle through rules [...] cannot be made + direct: ", with the specific blocker (argument-bearing recursion, + mixed labels, no base case) and related spans on each cycle member. This is + the actionable message ANTLR's generic `error(119)` lacks. +- On success, an *info*-level transform report entry records which satellites + were inlined into which hub, surfaced via the existing `TransformReport`. + +## 8. Fidelity and scope boundaries + +- **Tree shape changes for inlined satellites — by design and by necessity.** + Once `binary_expression` is inlined into `expression`, there is no + `BinaryExpressionContext` node; the operator alternative lives directly under + `expression`. This matches what ANTLR produces from the transformed grammar. + Recovering per-satellite node types would require synthesizing alternative + **labels** (`# BinaryExpression`) so codegen emits typed context subclasses — + but ANTLR forbids mixing labeled and unlabeled alternatives in one rule, so + this is all-or-nothing per hub and is **deferred** (future enhancement, tracked + separately). The initial delivery produces a correct, precedence-climbing hub + with a flat alternative set, which is sufficient to consume the grammar and + parse the language. +- **No runtime change.** If a future grammar needs a cycle that cannot be + reduced to direct LR at all (none known; all probed shapes reduce), that would + motivate approach (2). Out of scope here. +- **The `member_declaration` cycle is a non-issue** in the corrected staging, as + measured. If a future grammar presents a genuinely nullable declaration cycle, + it falls under the same algorithm. + +## 9. Test plan + +- **Unit** (`left_recursion.rs` / new `mutual_recursion.rs` module, insta + snapshots per house style): the boundary matrix from §4 as model-level + fixtures — hub-and-spoke, chained, multi-alt, leading-optional, and each + rejection case with its exact diagnostic. +- **Codegen-direct fixtures** (`tests/codegen-direct/fixtures/`): small + mutually-recursive grammars whose transformed form is snapshotted, plus + parse-and-compare against the existing direct-LR path. +- **Differential Roslyn validation**: generate a recognizer from the corrected + Roslyn grammar through our pipeline; parse a corpus of real C# files; compare + trees against ANTLR's runtime on the identical transformed grammar. This is + the headline acceptance test. +- **Regression**: full ANTLR runtime testsuite, zero skips — the pass must be a + no-op on every grammar that has no mutual-LR cycle (guarded by the + `changed = false` fast path and asserted on the testsuite corpus). + +## 10. Deliverables + +1. `src/bin_support/grammar/mutual_recursion.rs` — the pass (detector + + left-corner substitution + optional-expansion + gating), registered in the + pipeline before `rewrite_immediate_left_recursion`. +2. Diagnostic `G4R010` and transform-report wiring. +3. Unit + codegen-direct fixtures; Roslyn differential test (behind the existing + cleanroom-jar gating, like the Kotlin parity suite). +4. `README`/docs note: which mutual-recursion shapes are supported vs declined + (acceptance criterion 3). +5. This document. diff --git a/docs/mutual-left-recursion-rfc.md b/docs/mutual-left-recursion-rfc.md new file mode 100644 index 00000000..cdfeadbf --- /dev/null +++ b/docs/mutual-left-recursion-rfc.md @@ -0,0 +1,573 @@ +# Eliminating Mutual Left Recursion by Left-Corner Substitution: a Conservative Pre-Pass over ANTLR's Precedence Rewrite + +**Status:** Request for Comments — addressed to ANTLR maintainers and grammar-analysis researchers +**Implementation:** shipped in [`antlr-rust-runtime`](https://github.com/ophi-dev/antlr-rust-runtime) PR [#221](https://github.com/ophi-dev/antlr-rust-runtime/pull/221) (issue [#151](https://github.com/ophi-dev/antlr-rust-runtime/issues/151)) +**Validation oracle:** ANTLR 4.13.2 (Java tool + runtime) +**Date:** 2026-07-26 (rev. 2026-07-27: added the Visual Basic replication, §1.2/§3.1) + +--- + +## Abstract + +ANTLR 4 rewrites *immediate* left recursion into an unambiguous +precedence-climbing form using precedence predicates, but rejects *mutual* +(indirect) left recursion with `error(119)` (`LEFT_RECURSION_CYCLES`), even +when the cycle has a perfectly well-defined meaning under the same +alt-order-precedence convention. We describe a small, conservative grammar +transformation — **left-corner substitution into a designated hub rule** — +that reduces a useful subclass of mutual left recursion to the immediate form +ANTLR already handles, applied before `LeftRecursiveRuleTransformer` would run. +The transform is gated so that it either produces a rule that satisfies the +existing `binaryAlt`/`prefixAlt`/`suffixAlt`/`otherAlt` classification, or +declines and changes nothing, preserving today's diagnostics. Correctness is +established differentially: the transform's output is, by construction, a +grammar the reference tool accepts, so the reference runtime's parse trees are +a machine-checkable oracle. We validate on the grammar that motivated the work +— Roslyn's `CSharp.Generated.g4`, the C# compiler team's own generated +grammar, whose only blocker after trivial repairs is `error(119)` on four rule +cycles — achieving byte-identical parse trees against ANTLR's runtime, with +the full 357-descriptor runtime testsuite unperturbed. A replication on +Roslyn's second generated grammar, `VisualBasic.Grammar.g4` (419 rules, four +cycles including a 32-rule expression cycle), succeeds with the identical +pass, unmodified, supporting the claim that the covered subclass is the +natural shape of syntax-model-generated grammars. We state precisely +which cycle shapes are reduced, which are declined, and why the known-hard +cases (argument-bearing recursion, label mixing, epsilon-only cycles) remain +declined. We invite critique of the subclass boundary, the tree-shape +concession, and the possibility of adopting a similar pre-pass upstream. + +--- + +## 1. Problem statement + +ANTLR 4's celebrated left-recursion support ([Parr, Harwell, Fisher, *Adaptive +LL(\*) Parsing*, OOPSLA 2014]; `doc/left-recursion.md`) is scoped to rules with +**immediate** self-reference: `LeftRecursiveRuleTransformer` selects rules for +which `LeftRecursiveRuleAnalyzer.hasImmediateRecursiveRuleRefs(r.ast, r.name)` +holds, and rewrites them into the `primary (op …)*` loop with `{p >= _p}?` +precedence predicates. A left-recursive **cycle through two or more rules** +never enters that path; it survives to `AnalysisPipeline`, where +`LeftRecursionDetector` computes rule-start SCCs over the ATN and reports: + +```text +error(119): The following sets of rules are mutually left-recursive [a, b] +``` + +The grammar is expressible in ANTLR syntax; the tool declines it. That is a +reasonable engineering boundary — but it bites hardest on grammars nobody can +edit: **machine-generated grammars published by language owners.** + +### 1.1 The motivating instance + +[`dotnet/roslyn`'s `CSharp.Generated.g4`](https://github.com/dotnet/roslyn/blob/main/src/Compilers/CSharp/Portable/Generated/CSharp.Generated.g4) +(≈1 800 lines, ≈340 rules, generated from the compiler's syntax model, +current with C# 12/13) is, to our knowledge, the only complete and maintained +ANTLR-syntax grammar of modern C#; grammars-v4's `csharp` stops around C# 7 +(zero occurrences of `switch_expression`, `record_declaration`, or any pattern +rule). Measured with ANTLR 4.13.2, after repairing six trivially-empty rules +(two `/* epsilon */` bodies and four `/* see lexical specification */` stubs), +the **entire** remaining error output is one `error(119)` naming four cycles: + +| Cycle | Shape | +|---|---| +| `type, array_type, nullable_type, pointer_type` | 1 hub + 3 satellites | +| `name, qualified_name` | 1 hub + 1 satellite | +| `expression, assignment_expression, …` (13 rules) | 1 hub + 12 satellites | +| `pattern, binary_pattern` | 1 hub + 1 satellite | + +Representative excerpts: + +```antlr +name : alias_qualified_name | qualified_name | simple_name ; +qualified_name : name '.' simple_name ; + +pattern : binary_pattern | constant_pattern | … ; +binary_pattern : pattern ('or' | 'and') pattern ; + +expression : … | binary_expression | … ; // 46 alternatives +binary_expression : expression ('+'|'-'|…|'as'|'??') expression ; +range_expression : expression? '..' expression? ; // note the leading '?' +``` + +Each cycle is a **hub** (`name`, `pattern`, `type`, `expression`) whose +left-recursive **satellites** are plain alternatives of the hub, and each +satellite's left corner refers back to the hub. Every satellite is referenced +*only* by its hub, with a single exception (`array_type`, also used by +`array_creation_expression`). This is not a coincidence of C#: it is the +natural shape a syntax-model-driven generator produces, because each syntax +node class becomes a rule and the abstract base (`ExpressionSyntax`) becomes +the hub. We conjecture this hub-and-spoke shape is the dominant shape of +mutual left recursion in machine-generated grammars generally. + +### 1.2 A replication: Roslyn's Visual Basic grammar + +The conjecture invites an obvious test: Roslyn ships a *second* +syntax-model-generated grammar, +[`VisualBasic.Grammar.g4`](https://github.com/dotnet/roslyn/blob/main/src/Compilers/VisualBasic/Portable/Generated/VisualBasic.Grammar.g4) +(2 040 lines, 419 rules), produced by the analogous VB syntax generator and, +as far as we can tell, never before run through the ANTLR tool in anger. It is +a strictly harsher specimen. Reaching the left-recursion question required +repairing, in order: an unescaped `'\='` literal (VB's integer-divide-assign; +`error(156)`); **three duplicate rule definitions** (`error(51)`: +`resume_statement`, `case_block`, `if_directive_trivia`, each emitted once as +a union and once concrete); fourteen empty lexical stubs (C# had four); three +outright generator bugs — the multi-line lambda rules are published *without +their introducing header and with swapped end markers* +(`multi_line_function_lambda_expression : statement* end_sub_statement`), +`array_type : type array_rank_specifier*` (star, not plus — an epsilon +self-loop), and `invocation_expression : expression? argument_list?` (both +sides optional — matches the empty string); and seven intrinsically-nullable +rule bodies (`xml_text : xml_text_token*`, …). The defects we could root-cause +to the VB grammar emitter are reported upstream with fixes identified: +[dotnet/roslyn#84633](https://github.com/dotnet/roslyn/issues/84633) +(duplicate rules from a structure/node-kind name collision), +[#84634](https://github.com/dotnet/roslyn/issues/84634) (lambda header dropped, +and `End` markers swapped by positional kind-pairing), +[#84635](https://github.com/dotnet/roslyn/issues/84635) (the `'\='` escape), +and [#84636](https://github.com/dotnet/roslyn/issues/84636) (required list +children emitted `*` instead of `+`, which subsumes the `array_type` and +several nullable-root repairs). After those repairs — none of which touches +the recursion structure — the **entire** remaining error output is again one +`error(119)`, naming four cycles: + +| Cycle | Shape | +|---|---| +| `expression` + 31 satellites | VB splits *every* binary operator into its own rule (`add_expression : expression '+' expression`, ×25) plus a member-access family using the leading-optional pattern (`expression? '.' identifier_name`) four times | +| `type, array_type, nullable_type` | 1 hub + 2 satellites | +| `name, qualified_name, qualified_cref_operator_reference` | 1 hub + 2 satellites | +| `xml_node, xml_attribute, base_xml_attribute` | VB XML literals: `xml_attribute : xml_node '=' xml_node` | + +All four are hub-and-spoke; the externally-referenced satellites +(`qualified_name` from `implements_clause`, `xml_attribute` from +`xml_declaration_option`, `array_type` again) are exactly the retained-copy +case. The pass of §2, **unmodified**, reduces all four (§3.1). Two grammars +from two independent syntax models is still a small sample, but the +replication is consistent with the conjecture — and the VB expression cycle +(32 rules, 25 of them isomorphic binary-operator satellites) is a usefully +extreme instance of it. + +### 1.3 Why the human fix is unsatisfying + +A human can inline `binary_pattern` into `pattern` by hand — that is exactly +what grammar authors do today to appease `error(119)`. But for a published, +regenerated-on-every-release grammar, hand edits mean a permanently diverging +fork. The question is whether the *tool* can perform that inlining, safely, +with a proof obligation rather than a shrug. + +--- + +## 2. The transformation + +### 2.1 Definitions + +Let *G* be a parser grammar. For rules *A*, *B*, say **A left-calls B** iff +some alternative of *A* can reach a reference to *B* before consuming a token +— i.e. through a (possibly empty) prefix of actions, semantic predicates, +epsilon elements, optional/star-quantified elements, and references to +*nullable* rules. This is the same left-corner relation +`LeftRecursionDetector` computes over ATN epsilon/rule transitions; we compute +it over the grammar model instead, before any ATN exists. A **cycle** is an +SCC of size ≥ 2 of this relation. (Size-1 SCCs are immediate left recursion +and are exactly the existing transformer's territory; we never touch them.) + +### 2.2 Algorithm + +For each cycle *C*: + +1. **Choose the hub** *H* ∈ *C*: prefer a member that (a) has at least one + alternative whose left corner is *not* in *C* (a token-consuming base + case), and (b) is referenced from outside *C* (the cycle's public entry). + Ties break deterministically. If no member satisfies (a), the cycle is + ill-founded (its language is empty); **decline**. + +2. **Expand leading optionals.** In every alternative of every member of *C* + whose left corner is an *optional* reference `X?` with X ∈ *C*, rewrite + + ```text + α X? β → α X β | α β (α epsilon-only) + ``` + + This is the standard union-preserving expansion of a regular operator; it + is required because the immediate-recursion pattern (both ANTLR's and ours) + demands a non-optional recursive left corner. C#'s + `range_expression : expression? '..' expression?` is the live instance. + Only *greedy* optionals are expanded — the split's present-branch-first + order is the greedy preference, so a nongreedy `X??` corner **declines** — + and the expansion declines if a surviving action still references `X`. + +3. **Substitute to the hub (left-corner inlining).** Maintain a worklist of + *H*'s alternatives. For each alternative whose left corner is a satellite + *S* ∈ *C* \ {*H*}: replace the leading reference to *S* by each of *S*'s + alternatives in turn (one output alternative per alternative of *S*), + concatenating the remainder. Repeat until every alternative's left corner + is either outside *C* or is *H* itself. This terminates on every cycle + whose members' left corners eventually reach *H* (a budget guards the + pathological case; exceeding it **declines**). + +4. **Gate on the immediate-form classification.** Classify the rebuilt hub + exactly as `LeftRecursiveRuleAnalyzer` would: every alternative must be + *primary*, *prefix*, *binary*, or *suffix*; at least one primary and one + recursive alternative must exist; no recursive reference may carry + arguments; a bare `H : H | …` self-loop (the image of an epsilon-only + cycle) is nonconforming. Recursion is keyed on the **literal first + element** — the classifier's reading — not on the first + token-consuming one, and every alternative filed as primary is + additionally checked to have a left-corner closure disjoint from *C*, so + nothing still left-recursive can slip through as a "primary". If the gate + fails, **decline: the grammar is left bit-for-bit unchanged**, and the + existing SCC detector reports `error(119)` exactly as today. + +5. **Commit.** Install the rebuilt hub. Delete satellites no retained rule + references — where "retained" includes the **rebuilt hub body itself**: + substitution consumes only the corner occurrence of a satellite, so a + second reference in the same alternative (`e : s s | ID`) or in an + alternative left verbatim (`t : arr | t '?' arr | ID`) keeps that + satellite alive. A satellite referenced from outside + the cycle (`array_type`) is retained verbatim: its body references *H*, + which is now an ordinary immediate-left-recursive rule, so the external + caller is unaffected. Then hand the grammar to the *unchanged* immediate + left-recursion rewrite. + +### 2.3 What the transform deliberately does not do + +Every item below is a **precondition tested before the model is touched**, not a +repair attempted afterwards. That ordering is load-bearing: an earlier draft +checked admissibility only after splicing and consequently dropped +``, element labels, rule arguments and `*`-quantified corners, +in one case emitting a left-associative tree where the reference tool emits a +right-associative one. Deciding first turns each of those into a decline. + +- It does **not** handle cycles where no member has a token-consuming base + alternative (`a: b; b: c; c: a | X` reduces to `a : a | X`, whose recursive + alternative consumes nothing — the same shape ANTLR rejects as + `error(169)`/`NONCONFORMING_LR_RULE` in the immediate case). +- It requires every substituted corner to be **bare**. A quantified corner + (`a : b* 'x'`) is not one satellite occurrence, so splicing a single body in + its place would silently drop the closure; a **labelled** corner (`x=b`) would + leave `$x` dangling in surviving actions; an **argument-bearing** corner + (`b[3]`) has nowhere to put its arguments once the callee is gone; an + **option-bearing** corner would have its options silently discarded. One + shared predicate defines "bare" for every corner derivation. +- It declines a corner that a surviving **action still references by rule + name** (`$b.text` after the `b` element is spliced or split away) — the + reference would dangle — and it declines **nongreedy** optional corners + (§2.2 step 2). +- It declines a **parameterized hub** (`e[int x]`): in-cycle corners are bare, + so they omit the required arguments, and rewriting would delete the invalid + call before argument validation sees it. Declining lets the genuine + missing-argument diagnostic surface instead. +- It declines a satellite carrying **any embedded action or predicate**. + Semantic bodies are owned by their rule and alternative — `$ctx` means the + satellite's context, and `$`-references resolve against the enclosing + alternative's identity — and neither survives transplantation into the hub. + (Filtering bodies for the specific references that break would be + target-language-specific and incomplete.) Actions in the *hub's* own + alternatives are unaffected. It likewise declines any splice where an + action on one side references a token, rule or label name the **other side + introduces** (implicit references bind by occurrence within their + alternative, so the merge would capture them). +- It declines a substitution that leaves a recursive alternative with a + **nullable tail** (`e : e n | ID`, `n : ;`) — the immediate-form rewrite + rejects a left-recursive alternative that can be followed by the empty + string, and the diagnostic must describe the authored cycle, not the + transformed rule. Symbol validation likewise reads the *pre-transform* + grammar, so a name conflict involving a deleted satellite + (`e returns [int s]` vs rule `s`) still surfaces. +- It declines a corner reachable only **past a nullable rule call** + (`a : n b`, `n :`), where which rule the author meant as the left corner is + genuinely ambiguous. +- It declines a cycle for which the planner can make **no substitution step** + (the cycle enters through a position the transform does not rewrite, such + as a corner inside a nested block): a zero-step plan would otherwise be + re-selected forever. +- It declines a satellite carrying **rule-level state** — arguments, returns, + locals, `@init`/`@after`, `catch`/`finally`, rule options — since those attach + to the rule and vanish with it. +- It does **not** synthesize alternative labels, and declines a satellite whose + alternatives are `#`-labelled. Inlined *unlabelled* satellites lose their + per-rule context type (§4.3); transferring an authored `#Add` would require + labelling *all* hub alternatives (`error(122)`: "must label all alternatives + or none"), a mechanical but API-affecting follow-up we chose to defer rather + than bundle. An authored label is API, so silently dropping it is not an + option — hence the decline. +- It does **not** touch **lexer** grammars: precedence rewriting is a + parser-rule construct. (Left recursion in a lexer rule is invalid in ANTLR + either way — `error(119)`.) +- It does **not** modify prediction, the ATN, or any runtime component. It is + a source-model-to-source-model function running where + `SemanticPipeline` invokes `LeftRecursiveRuleTransformer` in the reference + tool — i.e. strictly before ATN construction. + +None of these preconditions excludes any cycle in the two Roslyn grammars (§3), +which is the evidence that the subclass is narrow in the right places. + +### 2.4 Precedence and associativity semantics of the result + +Alt-order precedence composes through substitution in the obvious way: the +inlined alternatives occupy the position of the satellite reference in the +hub's alternative list, so the hub's declared order remains the single source +of precedence truth, and the standard rewrite's left-associativity default +(and `` option) applies unchanged. For Roslyn specifically this +is even simpler than it sounds: both generated grammars are deliberately +**precedence-agnostic** — alternatives are listed alphabetically, with real +precedence living in Roslyn's hand-written parsers. C# lumps *all* binary +operators into one `expression op expression` alternative; VB splits them +into 25 one-per-operator satellite rules, likewise unordered. Either way the +grammar defines a flat operator tree, and the transformed parser reproduces +exactly that tree (§3). A user who wants the language's true precedence must +edit the grammar to order the operator alternatives — in the hub, exactly as +they would today for an immediate-recursive rule. The transform neither helps +nor hinders that. + +--- + +## 3. Correctness argument and validation + +Our correctness claim is deliberately narrow and machine-checkable: + +> **Claim.** For every grammar the pass rewrites, the resulting grammar is +> accepted by ANTLR 4.13.2 without error, and our generated parser and +> ANTLR's runtime produce identical parse trees on identical input. + +The claim's structure removes the need to trust our judgment about language +equivalence: step 3 is textbook indirect→immediate left-recursion elimination +(substitution of nonterminal bodies at left-corner positions), step 2 is a +regular-operator identity, and — decisively — the *output* is itself an +ANTLR-legal grammar, so the reference implementation adjudicates every case. + +Validation performed (all artifacts reproducible; ANTLR 4.13.2 as oracle): + +1. **Acceptance flip.** The repaired Roslyn grammar: reference tool → + `error(119)` (sole error); our pipeline → accepted, parser generated and + compiled. A distilled fixture (`MutualExpr.g4`, all three cycle shapes + including the leading-optional operator) is likewise `error(119)` upstream + and accepted by us — checked in CI both ways. +2. **Tree equality.** For inputs exercising all four Roslyn cycles — dotted + names, array/nullable types, `is` + `and`/`or`/relational patterns, + records, switch expressions, chained calls/indexing, `..` ranges — the + reference runtime (running the *hand-inlined equivalent* grammar) and our + parser (running the *mechanically transformed* original) print + **byte-identical** LISP trees. The same equality holds on the distilled + fixture, asserted in CI (`1+2*3`, `a.b.c`, `x..y`, `f()..g()`). +3. **Non-perturbation.** ANTLR's full runtime-testsuite conformance sweep: + 357/357 descriptors pass, zero skips, before and after. Both pre-existing + mutual-recursion rejection fixtures still produce their diagnostic — + confirming the gate declines them and the legacy path is intact. +4. **Boundary probes.** Each declined shape was probed against the reference + tool to confirm the decline mirrors an upstream refusal (`error(80)`, + `error(122)`, `error(169)`) rather than our own limitation. +5. **Decline is observable.** Thirteen unit tests, one per precondition of + §2.3, each asserting the model is byte-identical to its *pre-pass* + rendering, that no model IDs were consumed and no provenance recorded. A + CLI-level fixture additionally confirms a declined cycle still reports the + pre-existing cycle diagnostic naming the original rules, and emits no + parser artifact. + +### 3.1 Replication on the Visual Basic grammar + +The identical protocol was run on the repaired `VisualBasic.Grammar.g4` +(§1.2), with **no change to the pass**: + +- **Acceptance flip.** Reference tool → `error(119)` (sole error, four + cycles); our pipeline → accepted, parser generated and compiled. +- **Collapse shape.** All hub-only satellites vanish (the 25 binary-operator + rules, `member_access_expression`, `invocation_expression`, + `nullable_type`, `base_xml_attribute`, …); the three externally-referenced + satellites (`qualified_name`, `xml_attribute`, `array_type`) are retained + as non-recursive copies, as specified in §2.2 step 5. +- **Tree equality.** On inputs exercising each cycle — operator chains + (`a + b * c - d / e`), member/call chains (`a.b.c(x).d(y)`), dotted + imports/namespaces, array/nullable/qualified types — the reference runtime + on the hand-inlined equivalent and our parser on the mechanically + transformed original print **byte-identical** trees, all clean parses. +- **Growth.** The 32-rule expression cycle collapses into a hub of 59 + alternatives (from 30): each single-alternative satellite contributes one + alternative, plus one per leading-optional expansion. `type` and `name` + stay at 5 alternatives; `xml_node` grows 15 → 17. Linear in the cycle's own + alternative count, as predicted in §4.5. + +Beyond replication, VB stresses two aspects C# barely exercises: the +leading-optional expansion fires **four times** (the whole +`expression? '.' …` member-access family, vs. C#'s single range operator), +and the XML-literal cycle (`xml_attribute : xml_node '=' xml_node`) shows the +pattern arising outside expression/type/name territory. One honest caveat: +the published VB file needed the §1.2 repairs *before* the recursion question +could even be posed — three of those repairs (the swapped lambda ends, the +`*`-quantified `array_type`, the doubly-optional `invocation_expression`) are +defects in Roslyn's grammar emitter that no parser-side mechanism can absorb, +and in the raw file they entangle `statement` and the lambda rules into the +expression SCC. Mutual-recursion support makes such grammars *consumable*; it +does not make them *correct*. + +What we do **not** claim: that the transform preserves ANTLR's *ambiguity +resolution* on grammars that were ambiguous across the cycle in ways +alt-order does not capture. The gate's requirement that the result fit the +immediate-form pattern — whose semantics ANTLR defines and we inherit — is +precisely what bounds the claim. + +--- + +## 4. Discussion & requested comments + +### 4.1 Is the subclass boundary right? + +Empirically, left-corner substitution reduced every cycle we probed that has +a well-defined base case, *including* genuinely mutual `a ↔ b` cycles that +are not hub-shaped (`a : b '+' a | ID; b : a '.' b | ID` reduces cleanly once +`b` is substituted along the chain). The shapes that remain declined are +exactly the shapes whose *immediate* images ANTLR also rejects. + +We asked this question of our own automated reviewers first, and it was +productive enough to be worth reporting, because it shows how the boundary was +actually located. The exact two hazards anticipated below — substitution order +changing alternative order, and a nullable prefix making the left-corner +relation ambiguous — were both realised, alongside three more: + +| Adversarial shape | Failure it caused | +|---|---| +| `expr : power \| ID; power : expr '^' expr` | `` dropped → **left**-associative tree where the reference tool gives right-associative | +| `a : b* 'x'; b : a 'b'` | `b*` replaced by one satellite body → closure lost, accepted language changed | +| `a : n b; b : a 'b'; n :` | corner *decided* as `b`, *spliced* at `n` | +| `e : s \| e '+' e; s : e '*' e` | spliced alternatives appended rather than positioned → precedence reordered | +| `e : x=s \| ID; s : e '+' ID` (also `s[int x]`, `s @init{}`, `s : … #Add`) | labels, arguments, rule-level actions silently dropped | + +Every one traced to a single architectural error — admissibility was checked +*after* mutation, and the corner's identity was derived twice — and every one is +now a decline or a correct rewrite (§2.3). The lesson generalises beyond this +pass: for a transform whose contract is "provably correct or nothing", the +decision must be a pure function of the untouched input. + +A second review round then probed the rewritten, decide-first implementation +and located a further family, all in the *bookkeeping* that accompanies the +splice rather than in the splice itself: + +| Adversarial shape | Failure it caused | +|---|---| +| `e : (s \| ID) \| e '+' e; s : e '*' e` | a plan that made no substitution step was re-selected verbatim forever (non-termination) | +| `e : s s \| ID; s : e '+' ID` (also `t : arr \| t '?' arr; arr : t '[' ']'`) | removability judged against the *original* hub body → the surviving non-corner reference dangled after the satellite was deleted | +| `e : s #ViaSatellite \| ID #Atom; s : e '+' ID` | spliced alternative took the satellite's (empty) label → the authored context class silently vanished | +| `e : s \| ID; s : {p}? e '+' ID` | admissibility gate skipped the leading predicate while the downstream classifier keys on the literal first element → committed, then failed naming the wrong rule set | +| `r : e?? '..'` | nongreedy optional split with the greedy branch order → authored match preference inverted | +| `e : s {… $s.text …} \| ID; s : e '+' ID` | corner deleted while a surviving action still referenced it by rule name | + +The shared root cause this time: each decision read the original model where it +had to read the *planned* one (or vice versa). Removability is now computed +against the planned alternatives; label and option attribution is split +explicitly (the `#label` names the hub's alternative position, the +`` option describes the satellite's operator); the gate mirrors the +downstream classifier's literal-first reading and backstops every +non-recursive alternative with a left-corner-closure check; zero-step plans, +nongreedy corners and corners still referenced by surviving actions decline. +A follow-up round caught the *chained* form of the associativity hazard — +`e : a | ID; a : b '^' e; b : e;`, where the final alias splice +(`b : e`) overwrote the previously collected `` and flipped +`x^y^z` to left-associative. Options therefore **accumulate across the whole +splice chain** (hub alternative unioned with every satellite alternative +merged into the position) rather than being attributed to any single source; +a chain declaring the same option with two values declines as ambiguous. +One reviewer claim was *refuted* by the reference oracle rather than fixed: +the optional split places the same `#label` on both product alternatives, and +ANTLR accepts that (both map to one context class) — the split is exactly as +label-preserving as the reference tool requires. + +**Question that remains open to reviewers:** with the preconditions of §2.3 in +force, is there a cycle family with well-defined alt-order semantics that the +gate *accepts* but transforms wrongly? A counterexample to that would be the +most valuable outcome of this RFC; the shapes above are now regression tests +rather than open risks. + +### 4.2 Hub choice + +When several cycle members have base alternatives and external callers, hub +choice affects which rule survives as the precedence rule (and therefore tree +labels), not the language. We currently prefer external-referenced-with-base, +tie-broken deterministically; Roslyn's cycles have a unique natural hub. Is +there a principled criterion we're missing — e.g. always the member with the +maximal alternative count, or an explicit grammar option +(`options { lrHub=expression; }`)? + +### 4.3 The tree-shape concession + +Inlined satellites vanish from the tree: there is no `Binary_patternContext`; +the operator alternative lives directly under `pattern`, matching what ANTLR +itself produces for the hand-inlined grammar. For Roslyn this is arguably +*more* faithful to the language (Roslyn's own `BinaryPatternSyntax` is a +child of the pattern hierarchy, not a wrapper rule), but it is a real API +difference from a hypothetical native-mutual-recursion parser. The obvious +remedy — auto-labeling every inlined alternative with its satellite's name, +lifting the all-or-none label restriction tool-side — is mechanical but +changes generated-API surface. Would upstream consider label synthesis +acceptable, or is the flattened tree the honest answer? + +### 4.4 Could ANTLR adopt this? + +The pass is self-contained, language-target-independent, and sits at a point +in the pipeline ANTLR already owns (`SemanticPipeline`, immediately before +`LeftRecursiveRuleTransformer.translateLeftRecursiveRules()`). The gate reuses +the classification `LeftRecursiveRuleAnalyzer` already implements +(`binaryAlt`/`prefixAlt`/`suffixAlt`/`otherAlt`); the SCC computation +duplicates `LeftRecursionDetector` at the AST level. A Java port would be a +few hundred lines plus tests, and `error(119)` would then fire only for +cycles that are declined — with a message that could finally distinguish +"inherently ill-founded" from "well-defined but unsupported". We are glad to +contribute this if there is appetite; we are equally interested in hearing +why it was left out originally — whether as a deliberate scoping decision or +because the generated-grammar use case (§1.1) postdates the design. + +### 4.5 Relation to prior art + +Indirect→direct left-recursion elimination by substitution is classical +(Paull's algorithm; Moore, *Removing Left Recursion from Context-Free +Grammars*, ANLP 2000, discusses the size blow-up that makes the general +algorithm unattractive). The contribution here is not the substitution but +the **scoping and gating**: substituting only within left-corner SCCs, only +into a designated hub, only when the result lands in ANTLR's +precedence-pattern subclass — which keeps the blow-up bounded by the cycle's +own alternative count (C#'s 13-rule expression cycle: 46 → 47 hub +alternatives; VB's 32-rule cycle: 30 → 59; in both, each single-alternative +satellite contributes one alternative and each leading-optional expansion one +more) and inherits, rather than re-derives, the precedence semantics of +[OOPSLA 2014]. Moore-style worst cases are exactly what the budget + gate +decline. + +--- + +## 5. Implementation notes (for the curious; Rust knowledge not required) + +The pass is one file, `src/bin_support/grammar/mutual_recursion.rs` (~600 +lines + ~400 of tests), in a Rust reimplementation of the ANTLR toolchain +that consumes `.g4` source directly. Correspondences to the Java tool: + +| This work | ANTLR 4 (Java) | +|---|---| +| model-level left-corner SCC (Tarjan) | `LeftRecursionDetector` over ATN rule-start states | +| `eliminate_mutual_left_recursion` (the pass) | — (proposed pre-pass) | +| immediate-form gate | `LeftRecursiveRuleAnalyzer` alt classification | +| downstream immediate rewrite | `LeftRecursiveRuleTransformer` + `LeftRecursiveRuleWalker.g` | +| backstop diagnostic `G4A005` | `ErrorType.LEFT_RECURSION_CYCLES` (119) | + +Design doc with the full empirical log: +[`docs/issue-151-mutual-left-recursion-plan.md`](./issue-151-mutual-left-recursion-plan.md). +Repro for the Roslyn measurements is scripted in the PR — for C#, the +six-rule repair, staged error output, and tree-diff harness; for VB, the +§1.2 repair sequence (escape, duplicate rules, lexical stubs, the three +emitter-bug corrections, nullable roots) followed by the same +generate/compile/tree-diff protocol. + +--- + +## 6. Summary of questions for reviewers + +1. Counterexamples: a cycle with well-defined alt-order semantics that the + gate *accepts* but whose transformed parser diverges from intent (§4.1)? +2. Hub selection: is deterministic-with-preference sufficient, or should the + author name the hub (§4.2)? +3. Trees: flattened satellites vs. synthesized labels — which is the right + default for generated APIs (§4.3)? +4. Upstream interest: is a Java port of this pre-pass worth proposing against + `antlr4`, and was mutual recursion originally excluded by design or by + priority (§4.4)? + +Feedback via issues/discussions on +[`ophi-dev/antlr-rust-runtime`](https://github.com/ophi-dev/antlr-rust-runtime) +is very welcome. diff --git a/src/bin_support/grammar/mod.rs b/src/bin_support/grammar/mod.rs index 9b6da366..e0c54a16 100644 --- a/src/bin_support/grammar/mod.rs +++ b/src/bin_support/grammar/mod.rs @@ -13,6 +13,7 @@ mod left_recursion; mod lexer_adaptor; pub(crate) mod loader; pub(crate) mod model; +mod mutual_recursion; pub(crate) mod provenance; mod semantics; pub(crate) mod source; diff --git a/src/bin_support/grammar/mutual_recursion.rs b/src/bin_support/grammar/mutual_recursion.rs new file mode 100644 index 00000000..500d2d2b --- /dev/null +++ b/src/bin_support/grammar/mutual_recursion.rs @@ -0,0 +1,1939 @@ +//! Mutual (indirect) left-recursion elimination — issue #151. +//! +//! ANTLR 4 rewrites *direct* left recursion (`e : e '+' e | INT`) into a +//! precedence-climbing rule, but rejects *mutual* (indirect) left recursion — a +//! left-corner cycle through two or more rules — with `error(119)`. This pass +//! accepts the tractable subclass of those cycles by rewriting them, on the +//! model, into an equivalent grammar that uses only *direct* left recursion, so +//! the existing [`rewrite_immediate_left_recursion`](super::left_recursion) +//! machinery (and, as a differential oracle, ANTLR itself) can handle them. +//! +//! The rewrite is **left-corner substitution** ("hub inlining"): for each +//! left-corner cycle, one member is chosen as the *hub*; every other member (a +//! *satellite*) reachable in left-corner position from the hub has its +//! alternatives spliced into the hub, in place, until the hub is directly +//! left-recursive. Satellites referenced only from within the cycle are then +//! removed; a satellite referenced from outside the cycle is retained unchanged +//! (its body already calls the hub, which is now the precedence rule). +//! +//! # Structure: decide, then act +//! +//! The pass is organised so that **every** decision is made against the +//! untouched model, before anything is mutated: +//! +//! 1. [`plan_cycle`] proves the whole rewrite is admissible and returns a +//! [`CyclePlan`], or `None`. It allocates nothing and mutates nothing. +//! 2. [`apply_plan`] performs the planned splice. It is mechanical and cannot +//! fail. +//! +//! That ordering is what makes the safety claim real: an inadmissible cycle is +//! left **bit-for-bit untouched and silent** — no IDs consumed, no provenance +//! written — so the downstream ATN-level `G4A005` detector reports it exactly as +//! it does today. The alternative (mutate, then check) previously lost +//! ``, element labels, rule arguments and `*`-quantified corners by +//! dropping them mid-splice. +//! +//! # What is required of a cycle +//! +//! [`Requirements`] spells out the preconditions. In brief: the grammar must be +//! a parser grammar; every substituted left corner must be a *bare* call to a +//! cycle member — no quantifier, no label, no arguments, and nothing +//! token-consuming or nullable before it; satellites must carry no rule-level +//! attributes (arguments, returns, locals, `@init`/`@after`, `catch`/`finally`) +//! that inlining would silently drop; and the resulting hub must be a shape the +//! direct-recursion classifier already accepts. Anything else is declined. +//! +//! See `docs/issue-151-mutual-left-recursion-plan.md` for the full design and +//! the empirical validation against `dotnet/roslyn`'s `CSharp.Generated.g4`. + +use std::collections::{BTreeMap, BTreeSet}; + +use petgraph::algo::tarjan_scc; +use petgraph::graph::DiGraph; + +use super::action::{ActionReferenceKind, action_references}; +use super::model::{ + Alternative, AlternativeId, Block, Element, ElementKind, GrammarKind, GrammarUnit, + ModelIdAllocator, ModelNodeId, OptionDecl, Quantifier, Rule, RuleCall, RuleId, RuleKind, + Terminal, +}; +use super::provenance::{Origin, ProvenanceIndex, SyntheticReason}; + +/// Rewrite the tractable mutual-left-recursion cycles in `units` into direct +/// left recursion, in place. Returns `true` when at least one cycle was +/// rewritten (the model changed). Cycles that cannot be reduced are left +/// untouched — the ATN-level `G4A005` detector reports them downstream. +pub(crate) fn eliminate_mutual_left_recursion( + units: &mut [GrammarUnit], + ids: &mut ModelIdAllocator, + provenance: &mut ProvenanceIndex, +) -> bool { + let mut changed = false; + for unit in units.iter_mut() { + // Precedence rewriting is a parser-rule construct: a lexer rule cycle + // must not be routed through it (the injected precedence predicates + // become "unsupported embedded lexer action" much later, far from the + // cause). + if unit.kind == GrammarKind::Parser { + changed |= eliminate_in_unit(unit, ids, provenance); + } + } + changed +} + +/// The rules of one left-corner strongly-connected component, in a +/// deterministic order (ascending `RuleId`). +type Cycle = Vec; + +/// Read-only view of one grammar unit's rule analysis, threaded through the +/// left-corner walks: rule-name lookup and the nullable-rule set. +#[derive(Clone, Copy)] +struct Grammar<'a> { + names: &'a BTreeMap, + nullable: &'a BTreeSet, +} + +impl Grammar<'_> { + fn target(self, call: &RuleCall) -> Option { + self.names.get(&call.name).copied() + } +} + +fn eliminate_in_unit( + unit: &mut GrammarUnit, + ids: &mut ModelIdAllocator, + provenance: &mut ProvenanceIndex, +) -> bool { + let mut changed = false; + // Re-derive the cycle set after each successful rewrite: splicing a + // satellite body into the hub imports the satellite's own left corners, so + // the remaining cycles are a function of the *current* model, not the + // original one. `progress` bounds the loop — one rewrite per pass at most, + // and each rewrite strictly reduces the number of cycle members. + loop { + let names = rule_names(unit); + let nullable = nullable_rules(unit, &names); + let grammar = Grammar { + names: &names, + nullable: &nullable, + }; + let Some(plan) = left_corner_cycles(unit, grammar) + .iter() + .find_map(|cycle| plan_cycle(unit, cycle, grammar)) + else { + return changed; + }; + apply_plan(unit, &plan, ids, provenance); + changed = true; + } +} + +/// A fully-decided rewrite: which hub absorbs which satellite alternatives, at +/// which element index, and which satellites may then be dropped. Produced only +/// from an untouched model, and only when every [`Requirements`] precondition +/// holds, so applying it cannot fail. +#[derive(Debug)] +struct CyclePlan { + hub: RuleId, + /// Rewritten hub alternatives, in final order, as (source alternative that + /// supplied the elements, element list). The source is retained so + /// provenance and alternative options can be attributed correctly. + alternatives: Vec, + /// Satellites safe to delete: in the cycle, not the hub, and unreferenced by + /// every rule that survives. + removable: BTreeSet, +} + +#[derive(Debug)] +struct PlannedAlternative { + /// The original hub alternative this one descends from. Its `#label` and + /// commands are authored API naming this alternative *position* in the hub, + /// so the result inherits them — dropping a caller's `#ViaSatellite` would + /// silently delete its generated context and listener callbacks. + label_from: AlternativeId, + /// Alternative options accumulated along the splice chain: the hub + /// alternative's own options unioned with those of every satellite + /// alternative spliced into this position. `` rides on the + /// alternative that declares the operator, which may sit behind an + /// alias-shaped satellite (`a : b '^' e ; b : e ;`), so no + /// single source alternative can stand in for the set. Conflicting + /// declarations decline the cycle in [`splice_satellite`], keeping the + /// union unambiguous by construction. + options: Vec, + /// Alternative the elements were last taken from, for provenance. + origin: AlternativeId, + elements: Vec, + /// Set when this alternative is a verbatim copy of an original hub + /// alternative, so `apply_plan` can reuse it untouched. + verbatim: bool, +} + +/// The preconditions a cycle must satisfy. Documented as a type so the decline +/// reasons stay enumerated in one place and testable one at a time. +/// +/// * `ParserGrammarOnly` — lexer rules never reach here. +/// * `HasTokenConsumingBase` — some member has an alternative whose left corner +/// leaves the cycle, else the language is empty. +/// * `UnparameterizedHub` — the hub declares no arguments: every in-cycle +/// corner is bare by `BareCorner`, so it necessarily omits a parameterized +/// hub's required arguments, and the splice would launder that +/// missing-argument error into silently default-initialized attributes. +/// * `BareCorner` — every corner we substitute is an unquantified, unlabelled, +/// argument-free call, preceded only by actions/predicates/epsilon. This is +/// the single notion of "the left corner": one index, computed once, used for +/// both the decision and the splice. +/// * `InlinableSatellite` — satellites carry no rule-level attributes that +/// inlining would drop, no alternative labels that would collide, and no +/// embedded actions or predicates at all: semantic bodies are owned by their +/// rule and alternative (`$ctx` means the satellite's context; `$`-references +/// resolve against the enclosing alternative), and that ownership does not +/// survive transplantation into the hub. +/// * `NoRebinding` — merging the caller's and satellite's element lists must +/// not rebind anything: explicit label scopes must not overlap, and no +/// action on either side may reference a token, rule or label name the other +/// side introduces (`$ID` binds by occurrence within its alternative). +/// * `DirectlyRewritable` — the resulting hub is Primary/Prefix/Binary/Suffix +/// throughout, as [`super::left_recursion`] requires, and every recursive +/// alternative's tail can consume input (a nullable tail is a +/// left-recursive alternative followed by the empty string, which the +/// direct rewriter rejects). +/// * `Converges` — substitution terminates within a bound. +struct Requirements; + +/// Position of an alternative's left corner: the index of a bare call to a +/// cycle member, or a reason it is unusable. +enum Corner { + /// A bare call to `target` at `index`, safe to substitute. + Bare { index: usize, target: RuleId }, + /// The left corner does not enter the cycle: the alternative is a base case. + OutsideCycle, + /// The left corner reaches the cycle but is not substitutable (quantified, + /// labelled, argument-bearing, or behind a nullable/optional prefix). + Unusable, +} + +/// Classify `alternative`'s left corner with respect to `cycle_set`. +/// +/// This is the *only* place the corner position is derived. Everything +/// downstream consumes the returned index, which removes the class of bug where +/// the decision walked past a nullable prefix while the splice replaced "the +/// first rule call". +fn classify_corner( + alternative: &Alternative, + cycle_set: &BTreeSet, + grammar: Grammar<'_>, +) -> Corner { + for (index, element) in alternative.elements.iter().enumerate() { + match &element.kind { + // Epsilon-only elements cannot consume input, so keep scanning. + ElementKind::Action { .. } | ElementKind::Predicate { .. } | ElementKind::Epsilon => {} + ElementKind::RuleCall(call) => { + let Some(target) = grammar.target(call) else { + return Corner::OutsideCycle; + }; + if !cycle_set.contains(&target) { + // A nullable non-cycle call could be skipped, leaving a + // cycle member as the real corner. Substituting past it is + // not something we model, so decline rather than guess. + return if grammar.nullable.contains(&target) { + Corner::Unusable + } else { + Corner::OutsideCycle + }; + } + // The corner is a cycle member: it must be bare to be spliced. + return if element.quantifier == Quantifier::One && bare_reference(element, call) { + Corner::Bare { index, target } + } else { + Corner::Unusable + }; + } + // A token, set, range or block consumes input (or is a structure we + // do not descend into): the corner is settled here. + _ => return Corner::OutsideCycle, + } + } + Corner::OutsideCycle +} + +/// Whether `alternative`'s left corner enters the cycle at all (usable or not). +fn corner_enters_cycle( + alternative: &Alternative, + cycle_set: &BTreeSet, + grammar: Grammar<'_>, +) -> bool { + !matches!( + classify_corner(alternative, cycle_set, grammar), + Corner::OutsideCycle + ) +} + +/// The single definition of a "bare" rule reference — the only kind either +/// corner derivation may substitute or split. Shared by [`classify_corner`] and +/// [`planned_corner`] so the two cannot drift apart: a label would dangle in +/// actions once the element is gone, arguments have nowhere to go once the +/// callee is inlined, and element options would be silently discarded. +const fn bare_reference(element: &Element, call: &RuleCall) -> bool { + element.label.is_none() && call.arguments.is_none() && element.options.is_empty() +} + +/// Decide the whole rewrite for one cycle, or decline. Mutates nothing. +fn plan_cycle(unit: &GrammarUnit, cycle: &Cycle, grammar: Grammar<'_>) -> Option { + let rules = rules_by_id(unit); + if cycle.iter().any(|member| !rules.contains_key(member)) { + return None; + } + let cycle_set: BTreeSet = cycle.iter().copied().collect(); + let hub_id = choose_hub(unit, cycle, &cycle_set, grammar)?; + // Note: a *nullable* hub is fine — Roslyn's `pattern` is one + // (`recursive_pattern` is all-optional) and ANTLR accepts the collapsed + // grammar; the ill-founded shapes nullability could smuggle in (an + // epsilon-only alternative, a token-free self-loop) are declined by + // `planned_hub_is_directly_rewritable` on the planned result instead. + // + // Requirements::UnparameterizedHub — semantic call validation runs after + // this pass, so rewriting a parameterized hub would delete the very + // argument-less corner calls that validation should reject. + if rules[&hub_id].arguments.is_some() { + return None; + } + + // Requirements::InlinableSatellite — check before planning any splice, so a + // satellite carrying behaviour we would drop declines the whole cycle. + for member in cycle.iter().filter(|member| **member != hub_id) { + if !satellite_is_inlinable(rules[member]) { + return None; + } + } + + // Expand leading-optional corners to a fixpoint, then splice, preserving + // alternative order throughout (order *is* precedence). + let mut planned: Vec = rules[&hub_id] + .block + .alternatives + .iter() + .map(|alternative| PlannedAlternative { + label_from: alternative.id, + options: alternative.options.clone(), + origin: alternative.id, + elements: alternative.elements.clone(), + verbatim: true, + }) + .collect(); + + let budget = substitution_budget(&cycle_set, &rules); + let mut steps: usize = 0; + while let Some(position) = planned.iter().position(|candidate| { + !matches!( + planned_corner(candidate, hub_id, &cycle_set, grammar), + PlannedCorner::Settled + ) + }) { + steps += 1; + // Requirements::Converges + if steps > budget { + return None; + } + let replacement = match planned_corner(&planned[position], hub_id, &cycle_set, grammar) { + PlannedCorner::Optional { index, target } => { + // The absent branch deletes the element; any surviving action + // that names the corner's rule (`$e.text`) would dangle. + if remaining_actions_reference( + &planned[position].elements, + index, + &rules[&target].name, + ) { + return None; + } + split_optional(&planned[position], index) + } + PlannedCorner::Satellite { index, target } => { + splice_satellite(&planned[position], index, rules[&target])? + } + // The corner enters the cycle but cannot be substituted or split + // (quantified, labelled, argument- or option-bearing): the cycle + // is out of the tractable subclass. + PlannedCorner::Blocked => return None, + PlannedCorner::Settled => unreachable!("position was found to need work"), + }; + // Splice in place: the expansions occupy the slot of the alternative + // they came from, so declared alternative order — and therefore + // precedence — is preserved. + planned.splice(position..=position, replacement); + } + + // A plan that did no work would be reapplied verbatim on every iteration of + // the driver loop — the cycle lives somewhere this pass cannot reach (for + // example inside a nested block), so decline it. + if steps == 0 { + return None; + } + + // Requirements::DirectlyRewritable + if !planned_hub_is_directly_rewritable(&planned, hub_id, &cycle_set, grammar) { + return None; + } + + let removable = removable_satellites(unit, cycle, hub_id, &planned, grammar.names); + Some(CyclePlan { + hub: hub_id, + alternatives: planned, + removable, + }) +} + +/// What still needs doing to a planned alternative before the hub is direct. +enum PlannedCorner { + /// A leading optional call to cycle member `target` at `index`, to be split. + Optional { index: usize, target: RuleId }, + /// A bare call to satellite `target` at `index`, to be spliced. + Satellite { index: usize, target: RuleId }, + /// The corner enters the cycle but is not substitutable: quantified with + /// `*`/`+`, nongreedy-optional, labelled, argument- or option-bearing. + Blocked, + /// Nothing to do: base case, or already a hub self-reference. + Settled, +} + +fn planned_corner( + candidate: &PlannedAlternative, + hub_id: RuleId, + cycle_set: &BTreeSet, + grammar: Grammar<'_>, +) -> PlannedCorner { + for (index, element) in candidate.elements.iter().enumerate() { + match &element.kind { + ElementKind::Action { .. } | ElementKind::Predicate { .. } | ElementKind::Epsilon => {} + ElementKind::RuleCall(call) => { + let Some(target) = grammar.target(call) else { + return PlannedCorner::Settled; + }; + if !cycle_set.contains(&target) { + return PlannedCorner::Settled; + } + // A leading greedy `X?` where X is in the cycle: split it into + // present and absent branches (union-preserving) so the present + // branch becomes a well-formed recursive corner. This applies to + // the hub itself — C#'s `expr? '..' expr?` is exactly that + // shape — so it is checked before the self-reference test below. + // A nongreedy `X??` prefers the absent branch, which the split + // order would invert, so it is Blocked instead; a labelled or + // option-bearing optional would lose those in the absent branch. + if matches!(element.quantifier, Quantifier::Optional { greedy: true }) + && bare_reference(element, call) + { + return PlannedCorner::Optional { index, target }; + } + // A plain hub self-reference is the goal state, not something + // to substitute: recursing on it would never terminate. The + // direct rewriter keys on the call itself (labels included, via + // its deleted-label machinery), so mirror that. + if target == hub_id && element.quantifier == Quantifier::One { + return PlannedCorner::Settled; + } + if element.quantifier == Quantifier::One && bare_reference(element, call) { + return PlannedCorner::Satellite { index, target }; + } + return PlannedCorner::Blocked; + } + _ => return PlannedCorner::Settled, + } + } + PlannedCorner::Settled +} + +/// Whether any action or predicate among `elements` (other than the corner at +/// `skip` itself, and descending into nested blocks) references `rule_name` — +/// e.g. `$s.text` after the `s` element has been spliced away. +fn remaining_actions_reference(elements: &[Element], skip: usize, rule_name: &str) -> bool { + elements.iter().enumerate().any(|(index, element)| { + if index == skip { + return false; + } + element_actions_reference(element, rule_name) + }) +} + +fn element_actions_reference(element: &Element, rule_name: &str) -> bool { + let mut bodies: Vec<&str> = Vec::new(); + match &element.kind { + ElementKind::Action { body, .. } => bodies.push(body), + ElementKind::Predicate { body, fail, .. } => { + bodies.push(body); + if let Some(fail) = fail.as_deref() { + bodies.push(fail); + } + } + ElementKind::Block(block) => { + return block.alternatives.iter().any(|alternative| { + alternative + .elements + .iter() + .any(|nested| element_actions_reference(nested, rule_name)) + }); + } + _ => {} + } + bodies.into_iter().any(|body| { + action_references(body) + .iter() + .any(|reference| match reference.kind { + ActionReferenceKind::Attribute { name, .. } => name == rule_name, + ActionReferenceKind::Qualified { name, .. } => name == rule_name, + ActionReferenceKind::NonLocal { rule, .. } => rule == rule_name, + }) + }) +} + +/// `α X? β` becomes `α X β | α β`, preserving order (present branch first, as +/// the authored greedy `?` prefers matching). Both products keep the caller's +/// `#label` — ANTLR permits the same label on multiple alternatives (they share +/// one context class), which is the faithful reading of a split. +fn split_optional(candidate: &PlannedAlternative, index: usize) -> Vec { + let mut present = candidate.elements.clone(); + if let Some(element) = present.get_mut(index) { + element.quantifier = Quantifier::One; + } + let mut absent = candidate.elements.clone(); + absent.remove(index); + vec![ + PlannedAlternative { + label_from: candidate.label_from, + options: candidate.options.clone(), + origin: candidate.origin, + elements: present, + verbatim: false, + }, + PlannedAlternative { + label_from: candidate.label_from, + options: candidate.options.clone(), + origin: candidate.origin, + elements: absent, + verbatim: false, + }, + ] +} + +/// Replace the bare satellite call at `index` with each satellite alternative, +/// keeping the surrounding prefix and suffix. Returns `None` if any satellite +/// alternative cannot be inlined at this position. +fn splice_satellite( + candidate: &PlannedAlternative, + index: usize, + satellite: &Rule, +) -> Option> { + // Deleting the corner element severs any `$satellite.attr` reference an + // action in the surviving prefix/suffix makes by rule name. + if remaining_actions_reference(&candidate.elements, index, &satellite.name) { + return None; + } + // Requirements::BareCorner already established that `index` holds a bare + // call; the caller's remaining elements are kept verbatim around it. + let prefix = &candidate.elements[..index]; + let suffix = &candidate.elements[index + 1..]; + let mut expansions = Vec::with_capacity(satellite.block.alternatives.len()); + for source in &satellite.block.alternatives { + // Merging two element lists merges their label scopes. If both sides + // bind the same name, caller actions would silently rebind to the + // satellite's element, so decline instead. + if labels_collide(prefix, suffix, &source.elements) { + return None; + } + // Requirements::NoRebinding — implicit references bind by occurrence + // within the alternative: a caller action's `$ID` means the caller's + // own `ID` occurrence, and a satellite body arriving in front of it + // with another `ID` would capture the reference (and vice versa). + // Decline when either side's actions name anything the other side + // introduces. + if implicit_bindings_collide(prefix, suffix, &source.elements) { + return None; + } + // The options of every alternative merged into this position apply to + // the flattened result: an `` declared on an operator + // alternative must survive a later alias splice (`b : e`). Two + // alternatives declaring the same option with different values is + // genuinely ambiguous, so decline rather than pick one. + let mut options = candidate.options.clone(); + for option in &source.options { + match options + .iter() + .find(|existing| existing.name.value == option.name.value) + { + Some(existing) if existing.value.value != option.value.value => return None, + Some(_) => {} + None => options.push(option.clone()), + } + } + let mut elements = Vec::with_capacity(prefix.len() + source.elements.len() + suffix.len()); + elements.extend(prefix.iter().cloned()); + elements.extend(source.elements.iter().cloned()); + elements.extend(suffix.iter().cloned()); + expansions.push(PlannedAlternative { + // The caller's `#label` names this hub position and survives. + label_from: candidate.label_from, + options, + origin: source.id, + elements, + verbatim: false, + }); + } + Some(expansions) +} + +fn labels_collide(prefix: &[Element], suffix: &[Element], spliced: &[Element]) -> bool { + let mut caller = BTreeSet::new(); + collect_labels(prefix, &mut caller); + collect_labels(suffix, &mut caller); + let mut satellite = BTreeSet::new(); + collect_labels(spliced, &mut satellite); + !caller.is_disjoint(&satellite) +} + +/// Every label name bound in `elements`, descending into nested blocks — +/// separately-valid label scopes become one scope after a splice, so collisions +/// anywhere in either tree matter. +fn collect_labels<'a>(elements: &'a [Element], out: &mut BTreeSet<&'a str>) { + for element in elements { + if let Some(label) = &element.label { + out.insert(label.name.as_str()); + } + if let ElementKind::Block(nested) = &element.kind { + for alternative in &nested.alternatives { + collect_labels(&alternative.elements, out); + } + } + } +} + +/// Whether merging the caller's surviving elements with a satellite +/// alternative would capture an *implicit* action reference: an action on one +/// side names a token, rule or label that the other side introduces as an +/// element. Only names actually referenced by an action matter — inert +/// duplicate occurrences (`e : s s`) are fine. +fn implicit_bindings_collide(prefix: &[Element], suffix: &[Element], spliced: &[Element]) -> bool { + let mut caller_refs = BTreeSet::new(); + action_reference_names(prefix, &mut caller_refs); + action_reference_names(suffix, &mut caller_refs); + let mut satellite_intro = BTreeSet::new(); + bindable_names(spliced, &mut satellite_intro); + if !caller_refs.is_disjoint(&satellite_intro) { + return true; + } + let mut satellite_refs = BTreeSet::new(); + action_reference_names(spliced, &mut satellite_refs); + let mut caller_intro = BTreeSet::new(); + bindable_names(prefix, &mut caller_intro); + bindable_names(suffix, &mut caller_intro); + !satellite_refs.is_disjoint(&caller_intro) +} + +/// Every simple `$name` / `$name.attr` reference made by actions and +/// predicates among `elements`, descending nested blocks. +fn action_reference_names<'a>(elements: &'a [Element], out: &mut BTreeSet<&'a str>) { + for element in elements { + let mut bodies: Vec<&str> = Vec::new(); + match &element.kind { + ElementKind::Action { body, .. } => bodies.push(body), + ElementKind::Predicate { body, fail, .. } => { + bodies.push(body); + if let Some(fail) = fail.as_deref() { + bodies.push(fail); + } + } + ElementKind::Block(nested) => { + for alternative in &nested.alternatives { + action_reference_names(&alternative.elements, out); + } + } + _ => {} + } + for body in bodies { + for reference in action_references(body) { + match reference.kind { + ActionReferenceKind::Attribute { name, .. } + | ActionReferenceKind::Qualified { name, .. } => { + out.insert(name); + } + ActionReferenceKind::NonLocal { .. } => {} + } + } + } + } +} + +/// Every name an action can bind by occurrence within an alternative: element +/// labels, token names, and rule-call names, descending nested blocks. +fn bindable_names<'a>(elements: &'a [Element], out: &mut BTreeSet<&'a str>) { + for element in elements { + if let Some(label) = &element.label { + out.insert(label.name.as_str()); + } + match &element.kind { + ElementKind::RuleCall(call) => { + out.insert(call.name.as_str()); + } + ElementKind::Terminal(Terminal::Token(token)) => { + out.insert(token.as_str()); + } + ElementKind::Block(nested) => { + for alternative in &nested.alternatives { + bindable_names(&alternative.elements, out); + } + } + _ => {} + } + } +} + +/// A satellite may only be inlined if nothing rule-level would be lost. Rule +/// arguments/returns/locals, `@init`/`@after` actions, `catch`/`finally` +/// handlers and `#`-labelled alternatives all attach to the *rule*, and vanish +/// when the rule does. +fn satellite_is_inlinable(satellite: &Rule) -> bool { + satellite.kind == RuleKind::Parser + && satellite.arguments.is_none() + && satellite.returns.is_none() + && satellite.locals.is_none() + && satellite.throws.is_empty() + && satellite.actions.is_empty() + && satellite.catches.is_empty() + && satellite.finally_action.is_none() + && satellite.options.is_empty() + && satellite + .block + .alternatives + .iter() + .all(|alternative| alternative.label.is_none()) + && !satellite_has_embedded_semantics(satellite) +} + +/// Whether the satellite carries any embedded action or predicate at all. +/// Semantic bodies are *owned* by their rule and alternative: `$ctx` (and the +/// semantic-context parameter itself) means the satellite's context, and the +/// embedded-action pipeline resolves `$`-references against the enclosing +/// alternative's source span — none of which survives transplantation into the +/// hub. Rather than pattern-match the body for the references that happen to +/// break (a target-language-specific and inherently incomplete test), any +/// satellite with inline semantics declines. +fn satellite_has_embedded_semantics(satellite: &Rule) -> bool { + fn elements_have_semantics(elements: &[Element]) -> bool { + elements.iter().any(|element| match &element.kind { + ElementKind::Action { .. } | ElementKind::Predicate { .. } => true, + ElementKind::Block(nested) => nested + .alternatives + .iter() + .any(|alternative| elements_have_semantics(&alternative.elements)), + _ => false, + }) + } + satellite + .block + .alternatives + .iter() + .any(|alternative| elements_have_semantics(&alternative.elements)) +} + +/// Whether the planned hub is a shape [`super::left_recursion`] accepts. +/// +/// The downstream classifier keys recursion on the **literal first element** +/// (`classify_rule` uses `.first()`), so this gate mirrors that exactly rather +/// than skipping leading actions: an alternative like `{pred} hub '+' ID` is +/// *not* recognisably recursive downstream, would land in the primary block +/// still left-recursive, and must therefore decline here — which the +/// corner-closure check below does uniformly for every non-recursive +/// alternative, covering epsilon prefixes, nested blocks and any leftover +/// cycle-member corner in one place. +fn planned_hub_is_directly_rewritable( + planned: &[PlannedAlternative], + hub_id: RuleId, + cycle_set: &BTreeSet, + grammar: Grammar<'_>, +) -> bool { + let mut has_primary = false; + let mut has_recursive = false; + for candidate in planned { + let elements = &candidate.elements; + // No argument-bearing self-reference anywhere (mirrors G4R001). + if elements.iter().any(|element| { + hub_call(element, hub_id, grammar).is_some_and(|call| call.arguments.is_some()) + }) { + return false; + } + let Some(last_significant) = elements.iter().rposition(|e| !is_epsilon_only(e)) else { + // An epsilon-only alternative would make the precedence hub + // nullable; the original hub was not. + return false; + }; + if elements + .first() + .is_some_and(|element| is_hub_call(element, hub_id, grammar)) + { + // Recursive (Binary/Suffix) form. A bare `hub` with nothing + // significant after it is a nonconforming self-loop, exactly as + // the direct rewriter treats it. + if last_significant == 0 { + return false; + } + // The recursive remainder must consume input: a nullable tail + // (`e : e n | ID` with `n : ;`) would commit a rewrite the direct + // pass then rejects ("can be followed by the empty string"), + // reporting against the transformed rule instead of the authored + // cycle. + if elements[1..] + .iter() + .all(|element| element_nullable(element, grammar.names, grammar.nullable)) + { + return false; + } + has_recursive = true; + } else { + // Primary/Prefix bucket. Its left-corner closure must not re-enter + // the cycle: the downstream rewriter would file it as a primary + // alternative and the committed hub would still be left-recursive, + // failing later with a diagnostic naming the wrong rule set. + let mut corners = BTreeSet::new(); + collect_left_corner_calls(elements, grammar, &mut corners); + if corners.iter().any(|corner| cycle_set.contains(corner)) { + return false; + } + has_primary = true; + } + } + has_primary && has_recursive +} + +const fn is_epsilon_only(element: &Element) -> bool { + matches!( + element.kind, + ElementKind::Action { .. } | ElementKind::Predicate { .. } | ElementKind::Epsilon + ) +} + +fn is_hub_call(element: &Element, hub_id: RuleId, grammar: Grammar<'_>) -> bool { + hub_call(element, hub_id, grammar).is_some() +} + +fn hub_call<'a>( + element: &'a Element, + hub_id: RuleId, + grammar: Grammar<'_>, +) -> Option<&'a RuleCall> { + match &element.kind { + ElementKind::RuleCall(call) + if element.quantifier == Quantifier::One && grammar.target(call) == Some(hub_id) => + { + Some(call) + } + _ => None, + } +} + +/// Perform a planned rewrite. Mechanical: allocates the IDs and provenance the +/// plan implies, installs the hub block, and drops the removable satellites. +fn apply_plan( + unit: &mut GrammarUnit, + plan: &CyclePlan, + ids: &mut ModelIdAllocator, + provenance: &mut ProvenanceIndex, +) { + let hub_index = unit + .rules + .iter() + .position(|rule| rule.id == plan.hub) + .expect("planned hub exists"); + let attributes = collect_alternative_attributes(unit); + let template = unit.rules[hub_index].block.alternatives.first().cloned(); + + let alternatives = plan + .alternatives + .iter() + .map(|planned| { + // The caller's alternative supplies the `#label`, commands and + // position identity; the options were accumulated across the whole + // splice chain by `plan_cycle` (`` describes the + // operator that was inlined, wherever in the chain it was + // declared). + let label_source = attributes + .get(&planned.label_from) + .or(template.as_ref()) + .expect("hub has at least one alternative"); + let id = if planned.verbatim { + // An untouched hub alternative keeps its identity, so unrelated + // provenance and label bindings stay valid. + planned.origin + } else { + let fresh = ids.alternative(); + provenance.record_model( + ModelNodeId::Alternative(fresh), + [Origin::Synthetic { + reason: SyntheticReason::RuleBoundary, + owner: ModelNodeId::Alternative(planned.origin), + }], + ); + fresh + }; + let elements = if planned.verbatim { + planned.elements.clone() + } else { + renumber_elements(planned.elements.clone(), ids, provenance) + }; + Alternative { + id, + elements, + label: label_source.label.clone(), + options: planned.options.clone(), + commands: label_source.commands.clone(), + syntax: label_source.syntax, + span: label_source.span.clone(), + } + }) + .collect::>(); + + let hub = &mut unit.rules[hub_index]; + hub.block = Block { + alternatives, + options: hub.block.options.clone(), + syntax: hub.block.syntax, + span: hub.block.span.clone(), + }; + provenance.record_model( + ModelNodeId::Rule(plan.hub), + [Origin::Synthetic { + reason: SyntheticReason::RuleBoundary, + owner: ModelNodeId::Rule(plan.hub), + }], + ); + unit.rules.retain(|rule| !plan.removable.contains(&rule.id)); +} + +/// Index every alternative in the unit so a plan can recover the options, +/// label and commands of whichever alternative supplied its attributes. +fn collect_alternative_attributes(unit: &GrammarUnit) -> BTreeMap { + let mut index = BTreeMap::new(); + for rule in &unit.rules { + collect_block_alternatives(&rule.block, &mut index); + } + index +} + +fn collect_block_alternatives(block: &Block, index: &mut BTreeMap) { + for alternative in &block.alternatives { + index.insert(alternative.id, alternative.clone()); + for element in &alternative.elements { + if let ElementKind::Block(nested) = &element.kind { + collect_block_alternatives(nested, index); + } + } + } +} + +/// Choose the hub of a cycle: prefer a member with a token-consuming base +/// alternative that is also referenced from outside the cycle (the public +/// entry). Ties break on lowest `RuleId` for determinism. Returns `None` when no +/// member has a base case (ill-founded cycle we must not touch). +fn choose_hub( + unit: &GrammarUnit, + cycle: &Cycle, + cycle_set: &BTreeSet, + grammar: Grammar<'_>, +) -> Option { + let external = externally_referenced(unit, cycle_set, grammar.names); + let rules = rules_by_id(unit); + let candidates = cycle + .iter() + .copied() + .filter(|id| { + rules.get(id).is_some_and(|rule| { + rule.block + .alternatives + .iter() + .any(|alternative| !corner_enters_cycle(alternative, cycle_set, grammar)) + }) + }) + .collect::>(); + candidates + .iter() + .copied() + .filter(|id| external.contains(id)) + .min() + .or_else(|| candidates.into_iter().min()) +} + +/// Set of cycle members referenced by a rule that is *not* in the cycle. +fn externally_referenced( + unit: &GrammarUnit, + cycle_set: &BTreeSet, + names: &BTreeMap, +) -> BTreeSet { + let mut external = BTreeSet::new(); + for rule in &unit.rules { + if cycle_set.contains(&rule.id) { + continue; + } + collect_calls_into(&rule.block, names, &mut |target| { + if cycle_set.contains(&target) { + external.insert(target); + } + }); + } + external +} + +/// Satellites that can be safely removed: no rule that will be *retained* +/// references them. The hub's contribution is taken from the **planned** +/// alternatives, not its current body — verbatim alternatives and spliced +/// suffixes can keep satellite calls alive (`t : arr | t '?' arr`, +/// `e : s s | ID`), and deleting such a satellite would leave a dangling rule +/// reference. Computed to a fixpoint so a retained satellite pulls its own +/// dependencies back in. +fn removable_satellites( + unit: &GrammarUnit, + cycle: &Cycle, + hub_id: RuleId, + planned: &[PlannedAlternative], + names: &BTreeMap, +) -> BTreeSet { + let mut removable = cycle + .iter() + .copied() + .filter(|member| *member != hub_id) + .collect::>(); + loop { + let mut referenced = BTreeSet::new(); + { + let mut sink = |target: RuleId| { + if removable.contains(&target) { + referenced.insert(target); + } + }; + for candidate in planned { + collect_calls_in_elements(&candidate.elements, names, &mut sink); + } + for rule in &unit.rules { + if removable.contains(&rule.id) || rule.id == hub_id { + continue; + } + collect_calls_into(&rule.block, names, &mut sink); + } + } + if referenced.is_empty() { + return removable; + } + for target in referenced { + removable.remove(&target); + } + } +} + +/// A generous upper bound on substitution steps, guarding against +/// non-convergence without rejecting deep legitimate chains. +fn substitution_budget(cycle_set: &BTreeSet, rules: &BTreeMap) -> usize { + let alternatives: usize = cycle_set + .iter() + .filter_map(|id| rules.get(id)) + .map(|rule| rule.block.alternatives.len()) + .sum(); + alternatives.saturating_mul(alternatives).max(64) +} + +/// Compute left-corner strongly-connected components (size > 1) over the parser +/// rules of one unit, each returned as an ascending list of `RuleId`. Single +/// directly-left-recursive rules are excluded — those belong to +/// [`super::left_recursion`] and must not be touched here. +fn left_corner_cycles(unit: &GrammarUnit, grammar: Grammar<'_>) -> Vec { + let mut graph = DiGraph::::new(); + let nodes = unit + .rules + .iter() + .map(|rule| (rule.id, graph.add_node(rule.id))) + .collect::>(); + for rule in &unit.rules { + let mut corners = BTreeSet::new(); + for alternative in &rule.block.alternatives { + collect_left_corner_calls(&alternative.elements, grammar, &mut corners); + } + for target in corners { + if let (Some(source), Some(target)) = (nodes.get(&rule.id), nodes.get(&target)) { + graph.add_edge(*source, *target, ()); + } + } + } + + let mut cycles = tarjan_scc(&graph) + .into_iter() + .filter_map(|component| { + (component.len() > 1).then(|| { + let mut rules = component + .into_iter() + .map(|node| graph[node]) + .collect::(); + rules.sort_unstable(); + rules + }) + }) + .collect::>(); + cycles.sort(); + cycles +} + +/// Collect every rule reachable in left-corner position from `elements` +/// (through leading epsilon/nullable/optional elements), so the SCC graph +/// captures the full left-corner relation the ATN detector uses. +fn collect_left_corner_calls( + elements: &[Element], + grammar: Grammar<'_>, + result: &mut BTreeSet, +) { + for element in elements { + match &element.kind { + ElementKind::Action { .. } | ElementKind::Predicate { .. } | ElementKind::Epsilon => {} + ElementKind::RuleCall(call) => { + let Some(target) = grammar.target(call) else { + return; + }; + result.insert(target); + let skippable = matches!( + element.quantifier, + Quantifier::Optional { .. } | Quantifier::ZeroOrMore { .. } + ) || grammar.nullable.contains(&target); + if !skippable { + return; + } + } + ElementKind::Block(block) => { + for nested in &block.alternatives { + collect_left_corner_calls(&nested.elements, grammar, result); + } + let skippable = matches!( + element.quantifier, + Quantifier::Optional { .. } | Quantifier::ZeroOrMore { .. } + ) || block_is_nullable(block, grammar.names, grammar.nullable); + if !skippable { + return; + } + } + _ => return, + } + } +} + +fn collect_calls_into( + block: &Block, + names: &BTreeMap, + sink: &mut impl FnMut(RuleId), +) { + for alternative in &block.alternatives { + collect_calls_in_elements(&alternative.elements, names, sink); + } +} + +fn collect_calls_in_elements( + elements: &[Element], + names: &BTreeMap, + sink: &mut impl FnMut(RuleId), +) { + for element in elements { + match &element.kind { + ElementKind::RuleCall(call) => { + if let Some(target) = names.get(&call.name) { + sink(*target); + } + } + ElementKind::Block(nested) => collect_calls_into(nested, names, sink), + _ => {} + } + } +} + +fn rules_by_id(unit: &GrammarUnit) -> BTreeMap { + unit.rules.iter().map(|rule| (rule.id, rule)).collect() +} + +fn rule_names(unit: &GrammarUnit) -> BTreeMap { + unit.rules + .iter() + .map(|rule| (rule.name.clone(), rule.id)) + .collect() +} + +/// Rules that can derive the empty string (model-level, matching +/// `transform_analysis::compute_nullable`). +fn nullable_rules(unit: &GrammarUnit, names: &BTreeMap) -> BTreeSet { + let rules = rules_by_id(unit); + let mut nullable = BTreeSet::new(); + loop { + let previous = nullable.len(); + for (id, rule) in &rules { + if rule.block.alternatives.iter().any(|alternative| { + alternative + .elements + .iter() + .all(|element| element_nullable(element, names, &nullable)) + }) { + nullable.insert(*id); + } + } + if nullable.len() == previous { + return nullable; + } + } +} + +fn element_nullable( + element: &Element, + names: &BTreeMap, + nullable: &BTreeSet, +) -> bool { + if matches!( + element.quantifier, + Quantifier::Optional { .. } | Quantifier::ZeroOrMore { .. } + ) { + return true; + } + match &element.kind { + ElementKind::Epsilon | ElementKind::Action { .. } | ElementKind::Predicate { .. } => true, + ElementKind::RuleCall(call) => names + .get(&call.name) + .is_some_and(|target| nullable.contains(target)), + ElementKind::Block(block) => block_is_nullable(block, names, nullable), + _ => false, + } +} + +fn block_is_nullable( + block: &Block, + names: &BTreeMap, + nullable: &BTreeSet, +) -> bool { + block.alternatives.iter().any(|alternative| { + alternative + .elements + .iter() + .all(|element| element_nullable(element, names, nullable)) + }) +} + +/// Assign fresh IDs to a spliced element list, recursing through nested blocks, +/// labels, actions and predicates so no ID is shared between two live nodes. +fn renumber_elements( + elements: Vec, + ids: &mut ModelIdAllocator, + provenance: &mut ProvenanceIndex, +) -> Vec { + elements + .into_iter() + .map(|element| renumber_element(element, ids, provenance)) + .collect() +} + +fn renumber_element( + mut element: Element, + ids: &mut ModelIdAllocator, + provenance: &mut ProvenanceIndex, +) -> Element { + let original = element.id; + element.id = ids.element(); + record_clone( + provenance, + ModelNodeId::Element(element.id), + ModelNodeId::Element(original), + ); + + if let Some(label) = element.label.as_mut() { + let previous = label.id; + label.id = ids.label(); + record_clone( + provenance, + ModelNodeId::Label(label.id), + ModelNodeId::Label(previous), + ); + } + + element.kind = match element.kind { + ElementKind::Block(block) => ElementKind::Block(Block { + alternatives: block + .alternatives + .into_iter() + .map(|mut alternative| { + let previous = alternative.id; + alternative.id = ids.alternative(); + record_clone( + provenance, + ModelNodeId::Alternative(alternative.id), + ModelNodeId::Alternative(previous), + ); + alternative.elements = renumber_elements(alternative.elements, ids, provenance); + alternative + }) + .collect(), + options: block.options, + syntax: block.syntax, + span: block.span, + }), + ElementKind::Action { id, body } => { + let fresh = ids.action(); + record_clone( + provenance, + ModelNodeId::Action(fresh), + ModelNodeId::Action(id), + ); + ElementKind::Action { id: fresh, body } + } + ElementKind::Predicate { + id, + body, + fail, + precedence, + } => { + let fresh = ids.predicate(); + record_clone( + provenance, + ModelNodeId::Predicate(fresh), + ModelNodeId::Predicate(id), + ); + ElementKind::Predicate { + id: fresh, + body, + fail, + precedence, + } + } + kind => kind, + }; + element +} + +fn record_clone(provenance: &mut ProvenanceIndex, fresh: ModelNodeId, original: ModelNodeId) { + let mut origins = provenance.origins(original).to_vec(); + origins.push(Origin::Synthetic { + reason: SyntheticReason::BlockBoundary, + owner: original, + }); + provenance.record_model(fresh, origins); +} + +#[cfg(test)] +#[allow(clippy::disallowed_methods)] // insta assertion macros unwrap internal I/O. +mod tests { + use super::*; + use crate::grammar::frontend::{SourceId, parse_source}; + use crate::grammar::left_recursion::rewrite_immediate_left_recursion; + use crate::grammar::model::{GrammarId, Terminal}; + use crate::grammar::syntax::parse_grammar_unit; + + struct Fixture { + unit: GrammarUnit, + ids: ModelIdAllocator, + provenance: ProvenanceIndex, + } + + fn parse(text: &str) -> Fixture { + let file = parse_source(SourceId::new(0), "P.g4", text).expect("valid grammar"); + let mut ids = ModelIdAllocator::after_loaded_grammars(1); + let mut provenance = ProvenanceIndex::default(); + let unit = parse_grammar_unit(&file, GrammarId::new(0), &mut ids, &mut provenance); + Fixture { + unit, + ids, + provenance, + } + } + + /// Run the pass, returning the rendered model *before* and *after* plus + /// whether it reported a change. Rendering before invoking the pass is what + /// makes the "model untouched" assertions meaningful. + fn run(text: &str) -> (String, String, bool) { + let mut fixture = parse(text); + let before = render(&fixture.unit); + let changed = eliminate_mutual_left_recursion( + std::slice::from_mut(&mut fixture.unit), + &mut fixture.ids, + &mut fixture.provenance, + ); + let after = render(&fixture.unit); + (before, after, changed) + } + + fn rewritten(text: &str) -> GrammarUnit { + let mut fixture = parse(text); + assert!( + eliminate_mutual_left_recursion( + std::slice::from_mut(&mut fixture.unit), + &mut fixture.ids, + &mut fixture.provenance, + ), + "expected the cycle to be rewritten" + ); + fixture.unit + } + + /// Render a unit's rules as `rule: alt | alt ;` lines with a compact + /// per-element notation, for observable snapshots. + fn render(unit: &GrammarUnit) -> String { + let mut out = String::new(); + for rule in &unit.rules { + out.push_str(&rule.name); + out.push_str(":\n"); + for alternative in &rule.block.alternatives { + out.push_str(" | "); + if let Some(assoc) = alternative + .options + .iter() + .find(|option| option.name.value == "assoc") + { + use std::fmt::Write as _; + let _ = write!(out, " ", assoc.value.value); + } + out.push_str(&render_elements(&alternative.elements)); + if let Some(label) = &alternative.label { + use std::fmt::Write as _; + let _ = write!(out, " #{}", label.value); + } + out.push('\n'); + } + } + out + } + + fn render_elements(elements: &[Element]) -> String { + elements + .iter() + .map(render_element) + .collect::>() + .join(" ") + } + + fn render_element(element: &Element) -> String { + let quantifier = match element.quantifier { + Quantifier::One => "", + Quantifier::Optional { .. } => "?", + Quantifier::ZeroOrMore { .. } => "*", + Quantifier::OneOrMore { .. } => "+", + }; + let body = match &element.kind { + ElementKind::RuleCall(call) => call.name.clone(), + ElementKind::Terminal(Terminal::Literal(text)) => format!("'{text}'"), + ElementKind::Terminal(Terminal::Token(name)) => name.clone(), + ElementKind::Terminal(_) => "".to_owned(), + ElementKind::Set { .. } => "".to_owned(), + ElementKind::Block(_) => "".to_owned(), + ElementKind::Range(..) => "".to_owned(), + ElementKind::Action { .. } => "".to_owned(), + ElementKind::Predicate { .. } => "".to_owned(), + ElementKind::Epsilon => "".to_owned(), + }; + let label = element + .label + .as_ref() + .map(|label| format!("{}=", label.name)) + .unwrap_or_default(); + format!("{label}{body}{quantifier}") + } + + fn rule<'a>(unit: &'a GrammarUnit, name: &str) -> &'a Rule { + unit.rules + .iter() + .find(|rule| rule.name == name) + .unwrap_or_else(|| panic!("rule {name} exists")) + } + + /// Assert the pass declined: it reported no change *and* left the model + /// byte-identical to the pre-pass rendering. + fn assert_declined(text: &str) { + let (before, after, changed) = run(text); + assert!( + !changed, + "expected a decline, but the pass reported a change" + ); + assert_eq!( + before, after, + "a declined cycle must leave the model untouched" + ); + } + + #[test] + fn collapses_two_rule_name_cycle_into_the_hub() { + let unit = rewritten( + "parser grammar P; \ + name : qualified_name | simple_name ; \ + qualified_name : name '.' simple_name ; \ + simple_name : ID ;", + ); + assert!( + unit.rules.iter().all(|rule| rule.name != "qualified_name"), + "hub-only satellite is removed" + ); + insta::assert_snapshot!("name_cycle_collapsed", render(&unit)); + } + + #[test] + fn collapsed_hub_is_then_rewritten_by_the_direct_pass() { + let mut fixture = parse( + "parser grammar P; \ + name : qualified_name | simple_name ; \ + qualified_name : name '.' simple_name ; \ + simple_name : ID ;", + ); + assert!(eliminate_mutual_left_recursion( + std::slice::from_mut(&mut fixture.unit), + &mut fixture.ids, + &mut fixture.provenance, + )); + let diagnostics = rewrite_immediate_left_recursion( + std::slice::from_mut(&mut fixture.unit), + &mut fixture.ids, + &mut fixture.provenance, + ); + assert!(diagnostics.is_empty(), "{diagnostics:?}"); + assert!( + rule(&fixture.unit, "name").left_recursion.is_some(), + "collapsed hub is now a direct-left-recursion precedence rule" + ); + } + + #[test] + fn splits_optional_from_inlined_satellite() { + // C#'s range-operator shape: the leading-optional recursion lives in the + // satellite body (`r : e? '..' e?`). After inlining `r` into `e`, the + // leading `e?` splits into `e '..' e?` (left-recursive) and `'..' e?` + // (primary), yielding a well-formed direct-recursion hub. + let unit = rewritten( + "parser grammar P; \ + e : e '+' e | r | ID ; \ + r : e? '..' e? ;", + ); + insta::assert_snapshot!("optional_from_satellite", render(&unit)); + } + + #[test] + fn expands_consecutive_leading_optionals_to_a_fixpoint() { + // Two leading optional corners in one alternative: splitting once would + // leave the absent branch still starting with an optional corner, which + // substitution would then treat as mandatory. + let unit = rewritten( + "parser grammar P; \ + e : e '+' e | r | ID ; \ + r : e? e? '..' ;", + ); + insta::assert_snapshot!("consecutive_optionals", render(&unit)); + } + + #[test] + fn range_operator_hub_is_accepted_by_the_direct_pass() { + let mut fixture = parse( + "parser grammar P; \ + e : e '+' e | r | ID ; \ + r : e? '..' e? ;", + ); + assert!(eliminate_mutual_left_recursion( + std::slice::from_mut(&mut fixture.unit), + &mut fixture.ids, + &mut fixture.provenance, + )); + let diagnostics = rewrite_immediate_left_recursion( + std::slice::from_mut(&mut fixture.unit), + &mut fixture.ids, + &mut fixture.provenance, + ); + assert!(diagnostics.is_empty(), "{diagnostics:?}"); + assert!(rule(&fixture.unit, "e").left_recursion.is_some()); + } + + #[test] + fn retains_satellite_referenced_from_outside_the_cycle() { + // array_type-style: the satellite is left-recursive through the hub but + // also called by a non-cycle rule, so it is kept (not removed). + let unit = rewritten( + "parser grammar P; \ + t : arr | t '?' | ID ; \ + arr : t '[' ']' ; \ + new_arr : 'new' arr ;", + ); + assert!( + unit.rules.iter().any(|rule| rule.name == "arr"), + "externally-referenced satellite is retained" + ); + insta::assert_snapshot!("external_satellite_retained", render(&unit)); + } + + #[test] + fn preserves_satellite_alternative_associativity() { + // `` is declared on the satellite's alternative and drives + // the direct rewriter's associativity, so the spliced alternative must + // inherit the satellite's options, not the hub call site's. + let unit = rewritten( + "parser grammar P; \ + expr : power | ID ; \ + power : expr '^' expr ;", + ); + insta::assert_snapshot!("assoc_right_preserved", render(&unit)); + } + + #[test] + fn chained_alias_splice_carries_operator_options() { + // `` is declared on `a`'s operator alternative, but the + // final splice in the chain is the alias `b : e`. Options accumulate + // across the whole chain, so the option must survive to the collapsed + // alternative — a later alias splice must not overwrite it. + let unit = rewritten( + "parser grammar P; \ + e : a | ID ; \ + a : b '^' e ; \ + b : e ;", + ); + insta::assert_snapshot!("chained_assoc_carried", render(&unit)); + } + + #[test] + fn declines_conflicting_options_along_a_splice_chain() { + // Two alternatives in one chain declare the same option with different + // values; flattening them into one alternative would have to pick a + // winner, so the cycle declines instead. + assert_declined( + "parser grammar P; \ + e : a | ID ; \ + a : b '^' e ; \ + b : e ;", + ); + } + + #[test] + fn preserves_declared_alternative_order() { + // Alternative order is precedence. A satellite spliced from the *first* + // hub alternative must land first, ahead of the surviving originals. + let unit = rewritten( + "parser grammar P; \ + e : s | e '+' e | ID ; \ + s : e '*' e ;", + ); + insta::assert_snapshot!("declared_order_preserved", render(&unit)); + } + + #[test] + fn ignores_grammar_without_mutual_recursion() { + let (_, _, changed) = run("parser grammar P; \ + e : e '+' t | t ; \ + t : ID ;"); + assert!( + !changed, + "direct-only left recursion is left for the direct pass" + ); + } + + #[test] + fn declines_cycle_without_a_token_consuming_operator() { + // a:b; b:c; c:a|X reduces to `a : a | X` — a bare self-loop the direct + // rewriter rejects. Declining leaves G4A005 to report it downstream. + assert_declined("parser grammar P; a : b ; b : c ; c : a | X ;"); + } + + #[test] + fn declines_argument_bearing_recursion() { + assert_declined( + "parser grammar P; \ + e : s | ID ; \ + s : e '+' e[3] ;", + ); + } + + #[test] + fn declines_argument_bearing_satellite_call() { + // The corner itself carries arguments: removing it would drop them, and + // the satellite's parameter scope cannot travel with the body. + assert_declined( + "parser grammar P; \ + e : s[3] | ID ; \ + s[int x] : e '+' ID ;", + ); + } + + #[test] + fn declines_quantified_corner() { + // `b*` is not one satellite occurrence: splicing a single body in its + // place would silently drop the closure. + assert_declined( + "parser grammar P; \ + a : b* 'x' | 'a' ; \ + b : a 'b' ;", + ); + } + + #[test] + fn declines_corner_behind_a_nullable_prefix() { + // `n` is nullable, so the real left corner is ambiguous between `n` and + // `b`; substituting either is a guess. + assert_declined( + "parser grammar P; \ + a : n b | 'a' ; \ + b : a 'b' ; \ + n : ;", + ); + } + + #[test] + fn declines_labelled_corner() { + // Removing a labelled corner leaves `$x` dangling in caller actions. + assert_declined( + "parser grammar P; \ + e : x=s | ID ; \ + s : e '+' ID ;", + ); + } + + #[test] + fn declines_satellite_with_rule_level_action() { + // `@init` attaches to the rule; inlining the body would discard it. + assert_declined( + "parser grammar P; \ + e : s | ID ; \ + s @init { let _x = 1; } : e '+' ID ;", + ); + } + + #[test] + fn declines_satellite_with_labelled_alternatives() { + // `#`-labels generate context types keyed by the satellite rule; the hub + // cannot host them without colliding with its own labelling scheme. + assert_declined( + "parser grammar P; \ + e : s | ID ; \ + s : e '+' ID # Add ;", + ); + } + + #[test] + fn declines_when_caller_and_satellite_labels_collide() { + // Both sides bind `x`; merging the element lists would rebind the + // caller's action to the satellite's element. + assert_declined( + "parser grammar P; \ + e : s x=ID | ID ; \ + s : e '+' x=ID ;", + ); + } + + #[test] + fn terminates_and_declines_when_no_corner_is_reducible() { + // The only cycle-entering corner is a block, which is never spliced. A + // plan that makes no step must decline rather than spin: this test + // hangs (and times out) if the zero-step guard regresses. + assert_declined( + "parser grammar P; \ + e : (s | ID) | e '+' e ; \ + s : e '*' e ;", + ); + } + + #[test] + fn retains_satellite_still_referenced_by_the_planned_hub() { + // Only the *corner* occurrence of `s` is consumed by the splice; the + // second `s` survives in the planned suffix, so `s` must be retained + // even though no rule outside the cycle references it. + let unit = rewritten( + "parser grammar P; \ + e : s s | ID ; \ + s : e '+' ID ;", + ); + assert!( + unit.rules.iter().any(|rule| rule.name == "s"), + "satellite referenced by the planned hub body is retained" + ); + insta::assert_snapshot!("suffix_satellite_retained", render(&unit)); + } + + #[test] + fn retains_satellite_referenced_from_an_unspliced_alternative() { + // The hub's second alternative keeps its `arr` reference verbatim (it + // is not a left corner), so deleting `arr` would leave a dangling call. + let unit = rewritten( + "parser grammar P; \ + t : arr | t '?' arr | ID ; \ + arr : t '[' ']' ;", + ); + assert!( + unit.rules.iter().any(|rule| rule.name == "arr"), + "satellite referenced by an unspliced alternative is retained" + ); + insta::assert_snapshot!("verbatim_alt_satellite_retained", render(&unit)); + } + + #[test] + fn preserves_the_caller_alternative_label() { + // The `#ViaSatellite` label names the *hub's* alternative — authored + // API surface that must survive the splice (the satellite has no say). + let unit = rewritten( + "parser grammar P; \ + e : s # ViaSatellite | ID # Atom ; \ + s : e '+' ID ;", + ); + insta::assert_snapshot!("caller_alt_label_preserved", render(&unit)); + } + + #[test] + fn splitting_an_optional_keeps_the_label_on_both_products() { + // ANTLR accepts the same `#label` on multiple alternatives (they share + // one context class), so both split products keep the caller's label. + let unit = rewritten( + "parser grammar P; \ + e : e '+' e # Add | r # Range | ID # Atom ; \ + r : e? '..' ;", + ); + insta::assert_snapshot!("split_label_on_both_products", render(&unit)); + } + + #[test] + fn declines_predicate_prefixed_satellite_alternative() { + // Splicing would give the hub `{pred}? e '+' ID`, whose literal first + // element is a predicate — the direct rewriter files that under + // *primary*, leaving the recursion undetected. The gate must mirror + // that reading and decline before anything is touched. + assert_declined( + "parser grammar P; \ + e : s | ID ; \ + s : {true}? e '+' ID ;", + ); + } + + #[test] + fn declines_nongreedy_optional_corner() { + // `e??` prefers the absent branch; the greedy split `e rest | rest` + // would invert that preference, so only greedy optionals are split. + assert_declined( + "parser grammar P; \ + e : r | ID ; \ + r : e?? '..' ;", + ); + } + + #[test] + fn declines_when_a_surviving_action_references_the_satellite() { + // `$s.text` resolves against the corner element by rule name; deleting + // the corner would leave the reference dangling. + assert_declined( + "parser grammar P; \ + e : s { let _x = $s.text; } | ID ; \ + s : e '+' ID ;", + ); + } + + #[test] + fn declines_parameterized_hub() { + // Every in-cycle corner is bare, so it omits the hub's required + // arguments; rewriting would delete the argument-less call before + // semantic call validation could reject it, leaving the parameter + // silently default-initialized. + assert_declined( + "parser grammar P; \ + e[i32 x] : s | ID ; \ + s : e '+' ID ;", + ); + } + + #[test] + fn declines_satellite_action_bound_to_its_rule_context() { + // `$ctx` inside the satellite means the satellite's context; spliced + // into the hub it would silently mean the hub's instead. + assert_declined( + "parser grammar P; \ + e : s | ID ; \ + s : e '+' ID { let _r = $ctx; } ;", + ); + } + + #[test] + fn declines_when_a_splice_would_capture_an_implicit_reference() { + // The caller's `$ID` names its own trailing `ID` occurrence; the + // satellite body arriving in front of the action introduces another + // `ID` that would capture the reference. + assert_declined( + "parser grammar P; \ + e : s { let _t = $ID.text; } ID | INT ; \ + s : e '+' ID ;", + ); + } + + #[test] + fn declines_any_satellite_with_embedded_semantics() { + // Even an action bound only to the satellite's own labelled element + // does not survive transplantation: the embedded-action pipeline + // resolves `$i` against the enclosing alternative's source span, and + // the spliced alternative carries the hub's. Semantic bodies are owned + // by their rule, so any satellite with inline semantics declines. + assert_declined( + "parser grammar P; \ + e : s | ID ; \ + s : e '+' i=ID { let _t = $i.text; } ;", + ); + } + + #[test] + fn declines_nullable_recursive_tail() { + // After splicing, the hub alternative would be `e n` with nullable + // `n` — a left-recursive alternative that can be followed by the + // empty string, which the direct rewriter rejects. Declining keeps + // the diagnostic on the authored cycle instead of the rewritten rule. + assert_declined( + "parser grammar P; \ + e : s | ID ; \ + s : e n ; \ + n : ;", + ); + } + + #[test] + fn declines_when_a_split_absent_branch_is_a_bare_self_loop() { + // `s : e?` splits into `e` (a token-free self-loop) and epsilon; the + // direct rewriter accepts neither, so the plan declines up front. + assert_declined( + "parser grammar P; \ + e : s | ID ; \ + s : e? ;", + ); + } + + #[test] + fn leaves_lexer_grammars_untouched() { + // Precedence rewriting is a parser-rule construct. Routing a lexer SCC + // through it produced an "unsupported embedded lexer action" naming an + // action the grammar never declared. Left-recursive lexer rules are + // invalid in ANTLR regardless (error(119)); diagnosing them properly is + // tracked separately as issue #236. + let mut fixture = parse( + "lexer grammar L; \ + A : B 'a' | 'x' ; \ + B : A 'b' ;", + ); + let before = render(&fixture.unit); + let changed = eliminate_mutual_left_recursion( + std::slice::from_mut(&mut fixture.unit), + &mut fixture.ids, + &mut fixture.provenance, + ); + assert!(!changed, "lexer grammars must not be rewritten"); + assert_eq!(before, render(&fixture.unit)); + } + + #[test] + fn declining_consumes_no_ids_and_writes_no_provenance() { + // The safety claim is that a decline is invisible downstream, so the + // allocator and provenance index must be untouched too. + let mut fixture = parse("parser grammar P; a : b ; b : c ; c : a | X ;"); + let ids_before = format!("{:?}", fixture.ids); + let provenance_before = format!("{:?}", fixture.provenance); + assert!(!eliminate_mutual_left_recursion( + std::slice::from_mut(&mut fixture.unit), + &mut fixture.ids, + &mut fixture.provenance, + )); + assert_eq!( + ids_before, + format!("{:?}", fixture.ids), + "a declined cycle must not consume model IDs" + ); + assert_eq!( + provenance_before, + format!("{:?}", fixture.provenance), + "a declined cycle must not record provenance" + ); + } +} diff --git a/src/bin_support/grammar/semantics.rs b/src/bin_support/grammar/semantics.rs index 0226d876..61bdf567 100644 --- a/src/bin_support/grammar/semantics.rs +++ b/src/bin_support/grammar/semantics.rs @@ -14,6 +14,7 @@ use super::model::{ SemanticBindings, SemanticGrammar, SetElement, Terminal, TerminalBinding, TokenDeclaration, TokenSymbol, TokenSymbolId, Vocabulary, }; +use super::mutual_recursion::eliminate_mutual_left_recursion; use super::provenance::ProvenanceIndex; use super::source::SourceSet; use super::transform::{ @@ -81,12 +82,27 @@ pub(crate) fn analyze( return Err(CompilationError::new(diagnostics)); } + // Snapshot the authored units for symbol validation *before* any + // left-recursion rewriting: the mutual-recursion pass may delete satellite + // rules, and a symbol conflict involving a deleted rule's name (a return + // value named like a rule, say) must still be reported against what the + // author wrote. let symbol_units = integrated .grammar .units .iter() .map(|unit| (unit.id, unit.clone())) .collect::>(); + + // Reduce tractable mutual (indirect) left recursion to direct left + // recursion before the direct-recursion rewrite runs (issue #151). This is + // a no-op on grammars with no reducible left-corner cycle; anything it + // declines is reported later by the ATN-level G4A005 detector. + eliminate_mutual_left_recursion( + &mut integrated.grammar.units, + &mut integrated.ids, + &mut integrated.grammar.provenance, + ); diagnostics.extend(rewrite_immediate_left_recursion( &mut integrated.grammar.units, &mut integrated.ids, diff --git a/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__assoc_right_preserved.snap b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__assoc_right_preserved.snap new file mode 100644 index 00000000..61eaf8f3 --- /dev/null +++ b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__assoc_right_preserved.snap @@ -0,0 +1,7 @@ +--- +source: src/bin_support/grammar/mutual_recursion.rs +expression: render(&unit) +--- +expr: + | expr ''^'' expr + | ID diff --git a/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__caller_alt_label_preserved.snap b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__caller_alt_label_preserved.snap new file mode 100644 index 00000000..cb1d506d --- /dev/null +++ b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__caller_alt_label_preserved.snap @@ -0,0 +1,7 @@ +--- +source: src/bin_support/grammar/mutual_recursion.rs +expression: render(&unit) +--- +e: + | e ''+'' ID #ViaSatellite + | ID #Atom diff --git a/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__chained_assoc_carried.snap b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__chained_assoc_carried.snap new file mode 100644 index 00000000..94c86df1 --- /dev/null +++ b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__chained_assoc_carried.snap @@ -0,0 +1,7 @@ +--- +source: src/bin_support/grammar/mutual_recursion.rs +expression: render(&unit) +--- +e: + | e ''^'' e + | ID diff --git a/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__consecutive_optionals.snap b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__consecutive_optionals.snap new file mode 100644 index 00000000..fcdd0b9b --- /dev/null +++ b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__consecutive_optionals.snap @@ -0,0 +1,10 @@ +--- +source: src/bin_support/grammar/mutual_recursion.rs +expression: render(&unit) +--- +e: + | e ''+'' e + | e e? ''..'' + | e ''..'' + | ''..'' + | ID diff --git a/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__declared_order_preserved.snap b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__declared_order_preserved.snap new file mode 100644 index 00000000..63e0520c --- /dev/null +++ b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__declared_order_preserved.snap @@ -0,0 +1,8 @@ +--- +source: src/bin_support/grammar/mutual_recursion.rs +expression: render(&unit) +--- +e: + | e ''*'' e + | e ''+'' e + | ID diff --git a/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__external_satellite_retained.snap b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__external_satellite_retained.snap new file mode 100644 index 00000000..725103f1 --- /dev/null +++ b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__external_satellite_retained.snap @@ -0,0 +1,12 @@ +--- +source: src/bin_support/grammar/mutual_recursion.rs +expression: render(&unit) +--- +t: + | t ''['' '']'' + | t ''?'' + | ID +arr: + | t ''['' '']'' +new_arr: + | ''new'' arr diff --git a/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__name_cycle_collapsed.snap b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__name_cycle_collapsed.snap new file mode 100644 index 00000000..db4c9356 --- /dev/null +++ b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__name_cycle_collapsed.snap @@ -0,0 +1,9 @@ +--- +source: src/bin_support/grammar/mutual_recursion.rs +expression: render(&unit) +--- +name: + | name ''.'' simple_name + | simple_name +simple_name: + | ID diff --git a/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__optional_from_satellite.snap b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__optional_from_satellite.snap new file mode 100644 index 00000000..69a8d2d0 --- /dev/null +++ b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__optional_from_satellite.snap @@ -0,0 +1,9 @@ +--- +source: src/bin_support/grammar/mutual_recursion.rs +expression: render(&unit) +--- +e: + | e ''+'' e + | e ''..'' e? + | ''..'' e? + | ID diff --git a/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__split_label_on_both_products.snap b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__split_label_on_both_products.snap new file mode 100644 index 00000000..ca8cd698 --- /dev/null +++ b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__split_label_on_both_products.snap @@ -0,0 +1,9 @@ +--- +source: src/bin_support/grammar/mutual_recursion.rs +expression: render(&unit) +--- +e: + | e ''+'' e #Add + | e ''..'' #Range + | ''..'' #Range + | ID #Atom diff --git a/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__suffix_satellite_retained.snap b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__suffix_satellite_retained.snap new file mode 100644 index 00000000..57c0fca7 --- /dev/null +++ b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__suffix_satellite_retained.snap @@ -0,0 +1,9 @@ +--- +source: src/bin_support/grammar/mutual_recursion.rs +expression: render(&unit) +--- +e: + | e ''+'' ID s + | ID +s: + | e ''+'' ID diff --git a/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__verbatim_alt_satellite_retained.snap b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__verbatim_alt_satellite_retained.snap new file mode 100644 index 00000000..a9f428c6 --- /dev/null +++ b/src/bin_support/grammar/snapshots/antlr4_rust_gen__grammar__mutual_recursion__tests__verbatim_alt_satellite_retained.snap @@ -0,0 +1,10 @@ +--- +source: src/bin_support/grammar/mutual_recursion.rs +expression: render(&unit) +--- +t: + | t ''['' '']'' + | t ''?'' arr + | ID +arr: + | t ''['' '']'' diff --git a/tests/antlr4_rust_gen_cli.rs b/tests/antlr4_rust_gen_cli.rs index ab36289a..fd1f0f53 100644 --- a/tests/antlr4_rust_gen_cli.rs +++ b/tests/antlr4_rust_gen_cli.rs @@ -81,6 +81,20 @@ fn utf8(bytes: &[u8]) -> &str { std::str::from_utf8(bytes).expect("process output should be UTF-8") } +/// Lines of `haystack` containing `needle`, numbered, capped so a failure +/// message stays readable when the subject is a large generated file. +fn matching_lines(haystack: &str, needle: &str) -> String { + const LIMIT: usize = 20; + let hits = haystack + .lines() + .enumerate() + .filter(|(_, line)| line.contains(needle)) + .map(|(index, line)| format!(" {}: {}", index + 1, line.trim())) + .take(LIMIT) + .collect::>(); + hits.join("\n") +} + fn temporary_directory(label: &str) -> TempDirectory { let nonce = SystemTime::now() .duration_since(UNIX_EPOCH) @@ -3086,3 +3100,184 @@ mod parser_member_initializer_tests { assert_generated_project(temp.path(), &["p_lexer.rs", "p_parser.rs"], test_source); } + +/// Issue #151: a grammar with mutual (indirect) left recursion — which ANTLR +/// 4.13.2 rejects with error(119) — is reduced to direct left recursion and +/// generates a working precedence-climbing parser. The fixture distills the +/// tractable Roslyn cycle shapes: a hub-and-spoke expression cycle (including +/// the leading-optional range operator) and a two-rule `name` cycle. The +/// asserted trees are byte-identical to what ANTLR's own runtime produces from +/// the equivalent hand-inlined grammar. +#[test] +fn mutual_left_recursion_is_reduced_to_a_working_precedence_parser() { + let temp = temporary_directory("mutual-left-recursion"); + let grammar = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures/antlr4-rust-gen/mutual-left-recursion/MutualExpr.g4"); + let out = temp.path().join("generated"); + + let output = run_antlr4_rust_gen(&[ + grammar.as_os_str(), + OsStr::new("--out-dir"), + out.as_os_str(), + ]); + assert!( + output.status.success(), + "mutual left recursion should now compile, not error(119)\nstdout: {}\nstderr: {}", + utf8(&output.stdout), + utf8(&output.stderr) + ); + + let parser = + fs::read_to_string(out.join("mutual_expr_parser.rs")).expect("parser should be emitted"); + // Hub-only satellites collapse into their hub; the hub becomes a rule method. + // The generated parser is tens of thousands of lines, so failures report the + // matching lines rather than the whole file. + for collapsed in [ + "add_expr", + "mul_expr", + "call_expr", + "range_expr", + "qualified_name", + ] { + let needle = format!("fn {collapsed}("); + let offenders = matching_lines(&parser, &needle); + assert!( + offenders.is_empty(), + "hub-only satellite {collapsed:?} should be inlined away, found:\n{offenders}" + ); + } + for hub in ["fn expr(", "fn name(", "fn primary("] { + assert!( + parser.contains(hub), + "hub {hub:?} should survive; emitted rule methods:\n{}", + matching_lines(&parser, " pub fn ") + ); + } + + assert_generated_project( + temp.path(), + &["mutual_expr_lexer.rs", "mutual_expr_parser.rs"], + r#" +#[cfg(test)] +mod mutual_left_recursion_tests { + use super::mutual_expr_lexer::MutualExprLexer; + use super::mutual_expr_parser::{parse, rule_names}; + use antlr4_runtime::tree::{Node, NodeKind}; + + fn lisp(node: Node<'_>, names: &[&str], out: &mut String) { + match node.kind() { + NodeKind::Rule => { + let rule = node.as_rule().expect("rule node"); + out.push('('); + out.push_str(names.get(rule.rule_index()).copied().unwrap_or("?")); + for child in rule.children() { + out.push(' '); + lisp(child, names, out); + } + out.push(')'); + } + NodeKind::Terminal => out.push_str(&node.as_terminal().expect("terminal").text()), + NodeKind::Error => out.push_str(""), + } + } + + fn tree_of(src: &str) -> String { + let parsed = parse(src, MutualExprLexer::new, |p| p.expr()) + .unwrap_or_else(|error| panic!("{src:?} should parse: {error}")); + let mut out = String::new(); + lisp(parsed.tree(), rule_names(), &mut out); + out + } + + #[test] + fn collapsed_cycles_match_antlr_trees() { + // Precedence-climbing over the collapsed hub (default alt-order + // precedence: `+` binds looser than `*`), left-associative. + assert_eq!( + tree_of("1+2*3"), + "(expr (expr (expr (primary 1)) + (expr (primary 2))) * (expr (primary 3)))" + ); + // Two-rule name cycle collapsed to left-recursive `name`. + assert_eq!(tree_of("a.b.c"), "(expr (primary (name (name (name a) . b) . c)))"); + // Leading-optional range operator split into `expr '..' expr?` + primary. + assert_eq!( + tree_of("x..y"), + "(expr (expr (primary (name x))) .. (expr (primary (name y))))" + ); + assert_eq!( + tree_of("f()..g()"), + "(expr (expr (expr (primary (name f))) ( )) .. (expr (expr (primary (name g))) ( )))" + ); + } +} +"#, + ); +} + +/// Issue #151, decline path: a cycle the transform must *not* rewrite still +/// reports the pre-existing `G4A005` mutual-left-recursion diagnostic, naming +/// both original rules. This is the guard that the transform is additive — it +/// either produces a grammar the verified direct-recursion path accepts, or it +/// changes nothing observable. +#[test] +fn undecidable_mutual_left_recursion_still_reports_the_cycle() { + let temp = temporary_directory("mutual-left-recursion-declined"); + let grammar = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures/antlr4-rust-gen/mutual-left-recursion/DeclinedCycle.g4"); + let out = temp.path().join("generated"); + + let output = run_antlr4_rust_gen(&[ + grammar.as_os_str(), + OsStr::new("--out-dir"), + out.as_os_str(), + ]); + assert!( + !output.status.success(), + "a declined cycle must not generate a parser\nstdout: {}", + utf8(&output.stdout) + ); + let stderr = utf8(&output.stderr); + assert!( + stderr.contains("G4A005"), + "declining must fall through to the cycle detector: {stderr}" + ); + // Both cycle members are still present and named, i.e. nothing was inlined + // or deleted on the way to the diagnostic. + assert!( + stderr.contains("mutually left-recursive rules: [a, b]"), + "the diagnostic must name the original rule set: {stderr}" + ); + assert!( + !out.join("declined_cycle_parser.rs").exists(), + "no parser artifact should be emitted for a declined cycle" + ); +} + +#[test] +fn symbol_conflicts_are_reported_against_the_authored_grammar() { + // The mutual-left-recursion rewrite deletes hub-only satellites, and a + // return value named after one (`e returns [i32 s]` vs rule `s`) must + // still be reported: symbol validation reads a snapshot taken before the + // rewrite runs. + let temp = temporary_directory("mutual-left-recursion-symbol-clash"); + let grammar = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures/antlr4-rust-gen/mutual-left-recursion/ReturnsClash.g4"); + let out = temp.path().join("generated"); + + let output = run_antlr4_rust_gen(&[ + grammar.as_os_str(), + OsStr::new("--out-dir"), + out.as_os_str(), + ]); + assert!( + !output.status.success(), + "a symbol conflict must fail generation even when the conflicting rule \ + is a deletable cycle satellite\nstdout: {}", + utf8(&output.stdout) + ); + let stderr = utf8(&output.stderr); + assert!( + stderr.contains("G4S057"), + "the return-value/rule-name conflict must be diagnosed: {stderr}" + ); +} diff --git a/tests/fixtures/antlr4-rust-gen/mutual-left-recursion/DeclinedCycle.g4 b/tests/fixtures/antlr4-rust-gen/mutual-left-recursion/DeclinedCycle.g4 new file mode 100644 index 00000000..ae36d88f --- /dev/null +++ b/tests/fixtures/antlr4-rust-gen/mutual-left-recursion/DeclinedCycle.g4 @@ -0,0 +1,11 @@ +// A mutual-left-recursion cycle the pass must DECLINE, not rewrite: the +// satellite's left corner is `b*`, so splicing a single satellite body in its +// place would silently drop the closure and change the accepted language. +// Declining leaves the grammar untouched, so the ATN-level G4A005 detector +// reports the cycle exactly as it does without the pass (issue #151). +grammar DeclinedCycle; + +a : b* 'x' | 'a' ; +b : a 'b' ; + +WS : [ \t\r\n]+ -> skip ; diff --git a/tests/fixtures/antlr4-rust-gen/mutual-left-recursion/MutualExpr.g4 b/tests/fixtures/antlr4-rust-gen/mutual-left-recursion/MutualExpr.g4 new file mode 100644 index 00000000..69fd03b8 --- /dev/null +++ b/tests/fixtures/antlr4-rust-gen/mutual-left-recursion/MutualExpr.g4 @@ -0,0 +1,31 @@ +// Mutual (indirect) left recursion — the tractable shapes from issue #151, +// distilled from dotnet/roslyn's CSharp.Generated.g4 cycles. ANTLR 4.13.2 +// rejects this grammar with error(119); our generator reduces each cycle to +// direct left recursion (see src/bin_support/grammar/mutual_recursion.rs) and +// produces a working precedence-climbing parser. +grammar MutualExpr; + +// Hub-and-spoke expression cycle: every binary/postfix satellite's left corner +// is `expr`; each is referenced only by the hub, so all collapse into it. +expr + : add_expr + | mul_expr + | call_expr + | range_expr // leading-optional recursion (C#'s `a? '..' b?`) + | primary + ; + +add_expr : expr '+' expr ; +mul_expr : expr '*' expr ; +call_expr : expr '(' ')' ; +range_expr : expr? '..' expr? ; +primary : INT | name ; + +// Two-rule name cycle: `name`/`qualified_name`, exactly the Roslyn `name` +// shape. qualified_name is hub-only and collapses away. +name : qualified_name | ID ; +qualified_name : name '.' ID ; + +INT : [0-9]+ ; +ID : [a-zA-Z_] [a-zA-Z0-9_]* ; +WS : [ \t\r\n]+ -> skip ; diff --git a/tests/fixtures/antlr4-rust-gen/mutual-left-recursion/ReturnsClash.g4 b/tests/fixtures/antlr4-rust-gen/mutual-left-recursion/ReturnsClash.g4 new file mode 100644 index 00000000..d559d55d --- /dev/null +++ b/tests/fixtures/antlr4-rust-gen/mutual-left-recursion/ReturnsClash.g4 @@ -0,0 +1,8 @@ +// A return value named after a cycle satellite: the mutual-left-recursion +// pass would delete rule `s`, hiding the symbol conflict from validation. +// Symbol checks must therefore run against the authored grammar. +grammar ReturnsClash; +e returns [i32 s] : s | ID ; +s : e '+' ID ; +ID : [a-z]+ ; +WS : [ \t]+ -> skip ;