Skip to content

Simplify compiler internals; fix event leakage in typed parsers - #103

Merged
STRd6 merged 1 commit into
mainfrom
compiler-dedup
Sep 5, 2026
Merged

STRd6 merged 1 commit into
mainfrom
compiler-dedup

Conversation

@STRd6

@STRd6 STRd6 commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

Two commits.

1. Pure refactor, byte-identical output

  • Merge the three terminal-interning functions (defineTerminal, defineRe, defineDynamicRe) into one intern helper.
  • Drop the types parameter threaded through compileOp; it was never read.
  • Write tsHead as a literal instead of deriving it from mjsHead via regex replace and a match on the import block.
  • Share the indented rule-name list between the grammar object and the ESM export block.
  • Collapse prettyPrint to one expression.

2. Unify the JS and TS parser tails, fixing a bug

  • The typed tail hoisted Validator() and ctx out of parse into an IIFE, so Object.assign(ctx, options.events) from one call persisted into later calls. The untyped tail created them per call. Confirmed: after parse("a", { events: { enter } }) then parse("a"), the typed parser fired enter twice, the untyped one once.
  • Both outputs now use one per-call template with type annotations slotted in when types is set.
  • Regression test covers both JS and typed (via transpileTsToJs) parsers.

Verification

Snapshotted compiler output for source/hera.hera, every sample, and the inference fixture across ten option combinations (js/ts, cjs/esm, civet, tokenize, custom libPath, inline map), 180 files total.

  • After commit 1: all 180 identical.

  • After commit 2: untyped outputs identical. Typed outputs differ only in the parser tail, as intended.

  • c8 mocha — 210 passing, 100% coverage

  • pnpm test:typed-parser-samples

  • pnpm test:esbuild-plugin

  • pnpm test:loaders

🤖 Generated with Claude Code

https://claude.ai/code/session_01P6RAoyCaAv1Xe7FjMYwD3b

@greptile-apps

greptile-apps Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR simplifies compiler source generation while fixing typed parsers so runtime event hooks cannot persist across parse() calls.

  • Consolidates literal, regular-expression, and dynamic-regular-expression interning.
  • Removes an unused argument from recursive operator compilation.
  • Shares rule-list and parser-tail generation across output modes.
  • Creates validator and event context state independently for every parse invocation.
  • Adds a regression test covering typed and untyped generated parsers.

Confidence Score: 5/5

The PR appears safe to merge with no actionable correctness, security, or repository-rule issues identified.

The refactors preserve prior generation semantics, while the parser-tail change correctly scopes validator and event state to individual calls and is covered by a focused typed and untyped regression test.

Important Files Changed

Filename Overview
source/compiler.civet Consolidates compiler helpers and generated parser tails while moving validator and event context creation into each parse invocation.
test/main.civet Adds a runtime regression test proving event callbacks do not carry into subsequent typed or untyped parser calls.
CHANGELOG.md Documents the corrected event isolation behavior for typed generated parsers.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Hera grammar] --> B[Compiler]
  B --> C[Generated JS or TS parser]
  C --> D[parse call]
  D --> E[Fresh Validator]
  D --> F[Fresh event context]
  E --> G[Rule execution and validation]
  F --> G
  G --> H[Parse result]
Loading

Reviews (1): Last reviewed commit: "Unify parser tails; fix event leakage in..." | Re-trigger Greptile

@codecov

codecov Bot commented Sep 4, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (ba1d299) to head (15d0e73).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #103   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            9         9           
  Lines         1903      1886   -17     
  Branches       311       307    -4     
=========================================
- Hits          1903      1886   -17     

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

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

Comment thread source/compiler.civet Outdated
ruleNames.map((name) -> ` ${name}`).join(",\n")

return `{\n${meat}}`
compileRulesObject := (ruleNames: string[]) ->

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Perhaps add an optional "newline before close brace" argument so you can use it in export below?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done in b1bd9cf: ruleNameBlock(names, closeOnOwnLine) now emits the braces, and both the grammar object and export use it. Output verified unchanged against the snapshot.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'd probably inline compileRulesObject now i.e. just use ruleNameBlock

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Inlined; compileRulesObject is gone and the call site uses ruleNameBlock directly. Branch squashed to one commit and rebased on main.

Refactor (byte-identical output):
- Merge the three terminal-interning functions into one `intern` helper.
- Drop the `types` parameter threaded through `compileOp`; it was never read.
- Write `tsHead` as a literal instead of deriving it from `mjsHead` with a
  regex replace and a match on the import block.
- Emit the braced rule-name block from one `ruleNameBlock` helper, used by
  both the grammar object and the ESM export block.
- Collapse `prettyPrint` to a single expression.

Fix:
- The typed parser tail hoisted the Validator and context out of `parse`
  into an IIFE, so `Object.assign(ctx, options.events)` from one call
  persisted into the next. The untyped tail created them per call. Emit
  one tail for both, with type annotations slotted in when `types` is set,
  using the per-call structure. Typed output changes only in the tail.
  Adds a regression test for both JS and typed parsers.

Verified against a snapshot of compiler output for source/hera.hera, every
sample, and the inference fixture across ten option combinations.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_01P6RAoyCaAv1Xe7FjMYwD3b
@STRd6
STRd6 merged commit ea2aa3b into main Sep 5, 2026
6 checks passed
@STRd6
STRd6 deleted the compiler-dedup branch September 5, 2026 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants