Inline single EBNF operators in single alternatives

Author: linkrope

.github/copilot-instructions.md

@@ -43,10 +43,11 @@ See [plan-replaceEpsilonAnalyzer.md](../plan-replaceEpsilonAnalyzer.md), [TODO.m
 - Methods and local variables: `camelCase`
 - Private fields: `camelCase` — trailing underscore (`camelCase_`) only when the field name conflicts with a getter or setter of the same name
 - Module names: lowercase dot-separated packages (e.g. `gamma.grammar.Nonterminal`)
-- Do not abbreviate names — write `nonterminal` not `nt`, `alternative` not `alt`; exceptions: well-known domain abbreviations such as `lhs`, `rhs`, `eag`
+- Do not abbreviate names — write `nonterminal` not `nt`, `alternative` not `alt`, `operator` not `op`; exceptions: well-known domain abbreviations such as `lhs`, `rhs`, `eag`
 
 ### Code style
 - Avoid `cast` whenever possible; prefer design choices (templates, interfaces, `std.sumtype`) that eliminate the need for runtime type casting
+- Prefer a well-named helper function over a code block preceded by an explanatory comment
 - Defensive copies in constructors: use `.dup` on arrays
 - D `in`-contracts for preconditions (e.g. `in (lexer.front == Token.string_)`)
 - **No bare `true`/`false` arguments** — use `std.typecons.Flag` instead. Declare parameters as `Flag!"name"` and pass `Yes!"name"` / `No!"name"` at call sites. `Yes!` and `No!` convert implicitly to `bool`, so the function body needs no changes (see `Variable`'s `Unequal` flag for an example).

plan-replaceEpsilonAnalyzer.md

@@ -83,13 +83,13 @@ Checkpoint: manual review + `dub test --build=unittest --config=example`
 
 ---
 
-## Phase 2½: Shrink — merge named nonterminal with its sole EBNF operator in the parser
+## Phase 2½: Inline single EBNF operator — merge named nonterminal with its single EBNF operator in the parser
 
 **Goal**: when a named rule's entire RHS is a single EBNF operator (`()`, `[]`, `{}`) with formal params and nothing else, the parser must NOT create a separate anonymous nonterminal. The named nonterminal *is* the operator. Fixing this at the grammar-model level eliminates all downstream workarounds.
 
 ### Background — what epsilon does
 
-Epsilon's `Shrink` pass (called from `Specification()`) walks every hyper alternative. If it finds an `EAG.Grp` factor that is the sole factor in the alternative, carries no actual params, and whose body alternatives have formal params (i.e. an anonymous nonterminal whose rule has the shape `<formals>: body <formals>.`), it merges: `HNont[namedSym]` takes the signature and the body alternatives of the anonymous nonterminal directly. The anonymous nonterminal entry is then unused.
+Epsilon's `Shrink` pass (called from `Specification()`) walks every hyper alternative. If it finds an `EAG.Grp` factor that is the single factor in the alternative, carries no actual params, and whose body alternatives have formal params (i.e. an anonymous nonterminal whose rule has the shape `<formals>: body <formals>.`), it merges: `HNont[namedSym]` takes the signature and the body alternatives of the anonymous nonterminal directly. The anonymous nonterminal entry is then unused.
 
 ### What gamma currently does instead
 
@@ -99,23 +99,23 @@ The parser creates an anonymous nonterminal via `hyperGrammarBuilder.buildAnonym
 
 ### Plan
 
-- [ ] **Parser — detect the Shrinkable pattern at the point of EBNF operator creation**: after `parseHyperExpr` returns and `rule` is built, check:
+- [x] **Parser — detect the single-operator wrapper pattern at the point of EBNF operator creation**: after `parseHyperExpr` returns and `rule` is built, check:
   1. `hasFormalParams` — the anonymous nonterminal's lhs has a signature
   2. `nodes.empty` — nothing preceded the operator in this alternative (checked *before* `nodes ~= operator`)
   3. `enclosingNonterminal` is not an `AnonymousNonterminal` — we are inside a named rule
   4. Nothing follows: current `lexer.front` is a terminator (`|`, `.`, `)`, `]`, `}`, or empty)
 
   When all four hold: instead of using `identifier` (freshly created anonymous nonterminal), patch the alternatives so their `lhs.nonterminal` is `enclosingNonterminal` (or build them that way from the start by passing `enclosingNonterminal` to `parseHyperExpr`). The `Operator` node inserted into the outer `nodes` then references `enclosingNonterminal` as the operator's rule's lhs — no anonymous nonterminal is created.
 
-- [ ] **Remove signature trick from `parser.d`**: delete the `signatureByNonterminal[enclosingNonterminal]` block (currently lines ~574–587); it is no longer needed because the body alternatives already have the correct lhs nonterminal.
+- [x] **Remove signature trick from `parser.d`**: delete the `signatureByNonterminal[enclosingNonterminal]` block (currently lines ~574–587); it is no longer needed because the body alternatives already have the correct lhs nonterminal.
 
-- [ ] **Remove `CheckRep` imitation from `EAGBuilder.buildAffixes()`**: delete the `repActualIsUndecided` block and the `affix_wellMatchedEmpty` helper; with the grammar model correct, the `repAlt.params` situation that triggered the imitation no longer arises.
+- [x] **Remove `CheckRep` imitation from `EAGBuilder.buildAffixes()`**: delete the `repActualIsUndecided` block and the `affix_wellMatchedEmpty` helper; with the grammar model correct, the `repAlt.params` situation that triggered the imitation no longer arises.
 
-- [ ] **Remove the `open != '('` guard removal** (the last parser.d fix): the Shrinkable-pattern detection now handles all three bracket types uniformly; the explicit guard is moot.
+- [x] **Remove the `open != '('` guard removal** (the last parser.d fix): the single-operator wrapper detection now handles all three bracket types uniformly; the explicit guard is moot.
 
-- [ ] **Simplify `buildHyper` and `compareHyper`** in `EAGBuilder.d`: remove any code paths that special-case named-vs-anonymous nonterminal merging; the grammar model now tells the truth.
+- [x] **Simplify `buildHyper` and `compareHyper`** in `EAGBuilder.d`: remove any code paths that special-case named-vs-anonymous nonterminal merging; the grammar model now tells the truth.
 
-Checkpoint: `dub test --build=unittest --config=example` — all tests pass with the Shrink fix in place and workarounds removed
+Checkpoint: `dub test --build=unittest --config=example` — all tests pass with the single-EBNF-operator inlining in place and workarounds removed
 
 ---
 
@@ -148,7 +148,7 @@ builder.compareMeta();         // logs/asserts diffs — free regression check
 - [x] `compareHyper()`: diff against `EAG.HNont[]`, walking the Alt/Factor chains structurally; translate embedded MNont/MTerm references through the same Id-bijection maps used in `compareMeta()`
 - Reference: epsilon/analyzer.d Specification() for exact buffer/pointer layout
 
-- [ ] **Pending simplification**: once the parser performs Shrink (merging the named nonterminal with its sole-EBNF-operator anonymous nonterminal in the grammar model), the workarounds in `buildHyper` and `buildAffixes` that compensate for the grammar model having two distinct nonterminals where epsilon sees one will need to be removed.
+- [ ] **Pending simplification**: once the parser inlines the single EBNF operator (merging the named nonterminal with its single-EBNF-operator anonymous nonterminal in the grammar model), the workarounds in `buildHyper` and `buildAffixes` that compensate for the grammar model having two distinct nonterminals where epsilon sees one will need to be removed.
 
 ### 3c — Affix forms / parameter model
 - [ ] `buildAffixes()`: walk `Term` trees from Phase 1 (Variable/Composite hierarchy); emit builder's `VarRecord[]` (Def, Neg, Num, Sign), `NodeBuf[]`, `MSymBuf[]`, `ParamRecord[]`, `ScopeDesc[]`; pure format conversion — no re-parsing

src/gamma/grammar/Grammar.d

@@ -4,6 +4,7 @@ import gamma.grammar.Nonterminal;
 import gamma.grammar.Rule;
 import gamma.grammar.Terminal;
 import gamma.grammar.Visitor;
+import std.range;
 
 public class Grammar
 {
@@ -24,9 +25,11 @@ public class Grammar
 
         this.nonterminals_ = nonterminals.dup;
         this.terminals_ = terminals.dup;
-        this.rules_ = rules.dup;
+        this.rules_ = new Rule[nonterminals.length]; // null for abandoned nonterminals
+        foreach (rule; rules)
+            this.rules_[rule.alternatives.front.lhs.nonterminal.index] = rule;
         this.startSymbol_ = startSymbol;
-        this.isPlain_ = this.rules_
+        this.isPlain_ = rules
             .all!(rule => rule.alternatives
                 .all!(alternative => alternative.rhs
                     .all!(node => cast(SymbolNode) node !is null)));
@@ -49,7 +52,10 @@ public class Grammar
 
     public Rule[] rules()
     {
-        return this.rules_;
+        import std.algorithm : filter;
+        import std.array : array;
+
+        return this.rules_.filter!(rule => rule !is null).array;
     }
 
     public Nonterminal startSymbol()

src/gamma/grammar/GrammarBuilder.d

@@ -2,7 +2,6 @@ module gamma.grammar.GrammarBuilder;
 
 import gamma.grammar.Alternative;
 import gamma.grammar.Grammar;
-import gamma.grammar.hyper.AnonymousNonterminal;
 import gamma.grammar.LhsNode;
 import gamma.grammar.Nonterminal;
 import gamma.grammar.Rule;
@@ -16,15 +15,15 @@ version (unittest) import gamma.util.Position;
 
 public struct GrammarBuilder
 {
-    private Nonterminal[string] nonterminalMap;
+    package(gamma.grammar) Nonterminal[string] nonterminalMap;
 
-    private Nonterminal[] nonterminals;
+    package(gamma.grammar) Nonterminal[] nonterminals;
 
     private Terminal[string] terminalMap;
 
     private Terminal[] terminals;
 
-    private Alternative[][] alternativesMap;
+    package(gamma.grammar) Alternative[][] alternativesMap;
 
     private bool[Nonterminal] undefinedNonterminals;
 
@@ -46,26 +45,6 @@ public struct GrammarBuilder
         return nonterminal;
     }
 
-    public AnonymousNonterminal buildAnonymousNonterminal()
-    {
-        import std.exception : enforce;
-        import std.format : format;
-
-        const index = this.nonterminals.length;
-        auto nonterminal = new AnonymousNonterminal(index);
-
-        enforce(nonterminal.toString !in this.nonterminalMap,
-                format!"generated nonterminal name already defined by the user: %s"(nonterminal));
-
-        this.nonterminalMap[nonterminal.toString] = nonterminal;
-        this.nonterminals ~= nonterminal;
-        this.alternativesMap ~= null;
-
-        // anonymous nonterminals for EBNF expressions are never undefined
-
-        return nonterminal;
-    }
-
     public Terminal buildTerminal(string representation)
     {
         Terminal terminal = this.terminalMap.get(representation, null);

src/gamma/grammar/hyper/EBNFConverter.d

@@ -6,6 +6,7 @@ import gamma.grammar.hyper.Group;
 import gamma.grammar.hyper.HyperLhsNode;
 import gamma.grammar.hyper.HyperSymbolNode;
 import gamma.grammar.hyper.HyperVisitor;
+import gamma.grammar.hyper.Operator;
 import gamma.grammar.hyper.Option;
 import gamma.grammar.hyper.Repetition;
 import gamma.grammar.hyper.RepetitionAlternative;
@@ -57,7 +58,8 @@ private class EBNFConverter : HyperVisitor
     {
         this.rhsStack ~= null;
         alternative.rhs.each!(node => node.accept(this));
-        this.alternatives ~= new Alternative(alternative.lhs, this.rhsStack.back, alternative.position);
+        if (!alternative.isInlinedOperator)
+            this.alternatives ~= new Alternative(alternative.lhs, this.rhsStack.back, alternative.position);
         this.rhsStack.popBack;
     }
 
@@ -135,6 +137,17 @@ private class EBNFConverter : HyperVisitor
             if (!alternatives.empty)
                 rules ~= new Rule(alternatives.array);
         }
+
         return new Grammar(this.nonterminals, this.terminals, rules, this.startSymbol);
     }
 }
+
+private bool isInlinedOperator(Alternative alternative)
+{
+    if (alternative.rhs.length != 1)
+        return false;
+
+    auto operator = cast(Operator) alternative.rhs.front;
+
+    return operator !is null && operator.rule.lhs.nonterminal == alternative.lhs.nonterminal;
+}

src/gamma/grammar/hyper/HyperGrammarBuilder.d

@@ -0,0 +1,116 @@
+module gamma.grammar.hyper.HyperGrammarBuilder;
+
+import gamma.grammar.affixes.Signature;
+import gamma.grammar.Alternative;
+import gamma.grammar.GrammarBuilder;
+import gamma.grammar.hyper.AnonymousNonterminal;
+import gamma.grammar.hyper.Group;
+import gamma.grammar.hyper.HyperLhsNode;
+import gamma.grammar.hyper.Operator;
+import gamma.grammar.hyper.Option;
+import gamma.grammar.hyper.Repetition;
+import gamma.grammar.Node;
+import gamma.grammar.Nonterminal;
+import gamma.grammar.Rule;
+import std.range;
+
+public struct HyperGrammarBuilder
+{
+    GrammarBuilder builder;
+
+    alias builder this;
+
+    public AnonymousNonterminal buildAnonymousNonterminal()
+    {
+        import std.exception : enforce;
+        import std.format : format;
+
+        const index = builder.nonterminals.length;
+        auto nonterminal = new AnonymousNonterminal(index);
+
+        enforce(nonterminal.toString !in builder.nonterminalMap,
+                format!"generated nonterminal name already defined by the user: %s"(nonterminal));
+
+        builder.nonterminalMap[nonterminal.toString] = nonterminal;
+        builder.nonterminals ~= nonterminal;
+        builder.alternativesMap ~= null;
+
+        return nonterminal;
+    }
+
+    /**
+     * Replaces each anonymous nonterminal whose EBNF operator appears exclusively on the RHS
+     * of a single alternative with the nonterminal from the LHS.
+     *
+     * Returns: a map of nonterminal → signature for each nonterminal that was inlined
+     * and whose operator carried formal parameters.
+     */
+    public Signature[Nonterminal] inlineSingleOperators()
+    {
+        import std.algorithm : map;
+        import std.array : array;
+
+        Signature[Nonterminal] signatureByNonterminal;
+
+        foreach (nonterminal; builder.nonterminals)
+        {
+            if (cast(AnonymousNonterminal) nonterminal)
+                continue;
+
+            auto alternatives = builder.alternativesMap[nonterminal.index];
+
+            if (alternatives.length != 1)
+                continue;
+
+            auto alternative = alternatives.front;
+
+            if (alternative.rhs.length != 1)
+                continue;
+
+            auto operator = cast(Operator) alternative.rhs.front;
+
+            if (operator is null)
+                continue;
+
+            auto alternativeLhs = cast(HyperLhsNode) alternative.lhs;
+
+            if (alternativeLhs !is null && alternativeLhs.params !is null)
+                continue;
+
+            auto rule = new Rule(operator.rule.alternatives.map!(a => a.rebuild(nonterminal)).array);
+            Node rebuiltOperator = rebuildOperator(operator, rule);
+            auto rebuiltAlternative = new Alternative(alternative.lhs, [rebuiltOperator], alternative.position);
+
+            builder.alternativesMap[nonterminal.index][0] = rebuiltAlternative;
+
+            if (auto lhsNode = cast(HyperLhsNode) rule.lhs)
+                if (lhsNode.signature !is null)
+                    signatureByNonterminal[nonterminal] = lhsNode.signature;
+        }
+        return signatureByNonterminal;
+    }
+}
+
+private Operator rebuildOperator(Operator operator, Rule rule)
+{
+    if (auto group = cast(Group) operator)
+        return new Group(group.params, rule, group.position);
+    if (auto option = cast(Option) operator)
+        return new Option(option.params, rule, option.endParams, option.position);
+    if (auto repetition = cast(Repetition) operator)
+        return new Repetition(repetition.params, rule, repetition.endParams, repetition.position);
+
+    assert(false, "unknown operator type");
+}
+
+private Alternative rebuild(Alternative alternative, Nonterminal nonterminal)
+{
+    import gamma.grammar.hyper.RepetitionAlternative : RepetitionAlternative;
+
+    auto alternativeLhs = cast(HyperLhsNode) alternative.lhs;
+    auto lhs = new HyperLhsNode(nonterminal, alternativeLhs.signature, alternativeLhs.params, alternativeLhs.position);
+
+    if (auto repetitionAlt = cast(RepetitionAlternative) alternative)
+        return new RepetitionAlternative(lhs, alternative.rhs, repetitionAlt.params, alternative.position);
+    return new Alternative(lhs, alternative.rhs, alternative.position);
+}

src/gamma/input/epsilang/analyzer.d

@@ -69,7 +69,7 @@ class Analyzer
             import gamma.grammar.hyper.PrintingHyperVisitor : toPrettyString;
 
             this.plainHyperGrammar_ = new HyperGrammar(convert(hyperEBNFGrammar), hyperEBNFGrammar.terms);
-            log.trace!"converted hyper grammar:\n%s"(this.plainHyperGrammar_.toPrettyString);
+            log.trace!"transformed BNF grammar:\n%s"(this.plainHyperGrammar_.toPrettyString);
         }
 
         this.hyperGrammarProperties =