Repository navigation
feat(grammar): one normative grammar, with a conformance test (E7); 2 review fixes for #431 - #432
Merged
Merged
Conversation
spec/ephapax-v2-grammar.ebnf (150 productions) and ephapax.pest (125
rules) had only 68 names in common, and no test compared them. Owner
decision 2026-11-10: the parser grammar is the source.
The specification file now has 2 parts.
- Part 1 is the parser grammar. A test of ephapax-parser
(`grammar_conformance`) makes it from ephapax.pest, with the comments
of that file, and fails when the 2 files are not the same grammar.
`UPDATE_GRAMMAR=1 cargo test -p ephapax-parser grammar_conformance`
writes it again.
- Part 2 keeps each of the 82 design productions that are not rules of
the parser, as the design wrote them. Each one has a mark:
`[ALIAS rule]` (25: the parser has the construct as `rule`) or
`[UNIMPLEMENTED]` (57) with a fixture. A "DESIGN FORM" comment keeps
the design form of a production that part 1 has in a smaller form.
The specification does not shrink.
The test proves each production:
- each rule of the parser is in the parse tree of a fixture in
tests/v2-grammar/productions (5 rules can not be in a tree; the test
names them and the reason);
- the 2 parsers refuse each fixture in tests/v2-grammar/unimplemented;
- each alias names a rule of the parser;
- rules + aliases + refused = all productions: 0 unaccounted for.
Import and extern syntax: part 1 has the forms of the parser
(`import a/b(x, y)`, `extern "abi" { ... }`); the design form
`import X from "f"` is in part 2 with the mark UNIMPLEMENTED.
Not in this change: an unimplemented production is refused with a
syntax error, not with an error that names the production.
pest_meta is a dev-dependency of ephapax-parser. It was in the lock
file already, as a dependency of pest_derive.
Tests: 614 all-target.
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0136P7ZmKrYLDC4WHi2yt11u
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 37 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
2 review findings on #431, each checked against the code. - A guard could consume a linear value and return false. The next arm is checked from the state after the scrutinee, so it used the value a second time: `check` accepted the program, and the interpreter stopped with "Linear value used after consumption". The checker now refuses a guard that consumes a binding that can be used one time, from outside the match or from the pattern of the arm (`GuardConsumesLinear`). A guard can read or borrow such a binding. The surface syntax reaches this case since the guard lowering of #431. - A type alias is visible in each module, and the alias registered last had the name. So `pub data Tally = T(Count)` of a module with `type Count = I32` got the `Count` of an importer that declares `type Count = Bool`. A module now gets its own alias first, and the body of a data type expands in the scope of its module. Tests: 2 refused programs and 1 alias case in the gate tests of run_wasm.rs. 614 all-target. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136P7ZmKrYLDC4WHi2yt11u
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
hyperpolymath
changed the title
|
hyperpolymath
marked this pull request as ready for review
November 10, 2026 16:47
hyperpolymath
enabled auto-merge (squash)
November 10, 2026 16:47
Codex Review Summary: ❌ FailedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
This security review couldn’t start because the paying account has exhausted its Codex allowance and has insufficient available credits. The payer can check their balance and add credits in the Codex usage dashboard, then try again. If you do not manage the paying account, contact this repository's admins. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5775cc35bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
hyperpolymath
pushed a commit
that referenced
this pull request
Nov 10, 2026
The Hypatia code scan reported `code_safety/panic_macro` (error) and `code_safety/expect_in_hot_path` (warning) in grammar_conformance.rs, on #432. The file is a test-only module (`#[cfg(test)] mod grammar_conformance` in lib.rs), but it is a file of its own below src/, so the scanner reads it as library code. The file now has the 2 `hypatia: allow` directives, with the reason, in the form that other files of the repository use. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136P7ZmKrYLDC4WHi2yt11u
hyperpolymath
pushed a commit
that referenced
this pull request
Nov 10, 2026
Review finding on #432. With `UPDATE_GRAMMAR=1`, one test wrote spec/ephapax-v2-grammar.ebnf again while another test read the same file on a second thread. The reader could get the old text or a part of the new text, so the update command could fail or check stale content. The tests now take the text from one function with a `OnceLock`. The first call reads the file and, in update mode, writes part 1 again; each other test waits for that call and gets the same text. Checked: a file with a damaged part 1 fails in normal mode; in update mode, 5 runs in sequence pass and give back the committed text. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136P7ZmKrYLDC4WHi2yt11u
hyperpolymath
added a commit
that referenced
this pull request
Nov 10, 2026
…r test (#433) ## Summary 2 small follow-ups to #432, which merged with one red check and one open review finding. Both are in `src/ephapax-parser/src/grammar_conformance.rs`. ## 1. Hypatia alerts The Hypatia code scan reported 2 new alerts in that file on #432: - `code_safety/panic_macro` (error, 4 places) - `code_safety/expect_in_hot_path` (warning, 11 places) The file is a test-only module: `lib.rs` declares it as `#[cfg(test)] mod grammar_conformance`. But it is a file of its own below `src/`, so the scanner reads it as library code. In a test, a panic with a message is how the test fails, and there is no hot path. The file now has the 2 `hypatia: allow` directives, each with its reason. This is the form that other files of the repository use (for example `src/ephapax-vram-cache/benches/cache_bench.rs`). ## 2. Update mode (Codex finding on #432) With `UPDATE_GRAMMAR=1`, one test wrote `spec/ephapax-v2-grammar.ebnf` again while another test read the same file on a second thread. The reader could get the old text or a part of the new text. The tests now take the text from one function with a `OnceLock`. The first call reads the file and, in update mode, writes part 1 again. Each other test waits for that call and gets the same text. ## Verification - `cargo fmt --all --check` and `cargo clippy -p ephapax-parser --all-targets`: clean. - `cargo test -p ephapax-parser grammar_conformance`: 3 passed. - A file with a damaged part 1 fails in normal mode. In update mode, 5 runs in sequence pass and give back the committed text. - I can not run the Hypatia scan in this session. The check on this pull request shows if the 2 alerts close. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_0136P7ZmKrYLDC4WHi2yt11u --------- Co-authored-by: Claude <noreply@anthropic.com>
hyperpolymath
added a commit
that referenced
this pull request
Nov 10, 2026
## Summary
The last open item of the E7 gate in the v2 plan: "productions that are
not implemented must fail with a named error". Before, the parser
refused such syntax with a plain syntax error.
```
$ ephapax check trait.eph
Parse error: `trait_decl` is syntax of the v2 design that is not implemented
(see the mark [UNIMPLEMENTED] in spec/ephapax-v2-grammar.ebnf, part 2)
```
## The change
- **Grammar.** `ephapax.pest` has 25 rules with the name
`unimplemented_<production>`, one for each form of the design that the
parser does not have: traits, `impl`, effect and handler declarations,
comptime, contracts, `where` clauses, effect rows, `import X from "f"`,
capabilities, annotation arguments, `++`, deref, tuple and borrow
patterns, and the handler forms. A rule finds the syntax; it accepts
nothing.
- **Parsers.** The surface parser and the core parser walk the parse
tree before they build a module. The first such rule gives
`ParseError::Unimplemented { production }`.
- **No change for valid programs.** Each rule stands only where no valid
program can have its first token. A new test has 9 valid programs that
are near these rules (for example `handle shallow(1) with ...`, where
`shallow` is a function).
## The grammar test is stronger
This also answers the review finding on #432 that one fixture stood for
a group of productions, with no check of the reason.
- Each `[UNIMPLEMENTED]` mark in part 2 now says `refused: <fixture> as
<production>`. The 2 parsers must refuse the fixture with an error that
names that production.
- Each rule `unimplemented_<production>` must have its production in
part 2, refused under its own name.
- A production that is only inside another one (for example
`comptime_if` inside `comptime_decl`) has the name of the outer
production in its mark.
Part 1 of `spec/ephapax-v2-grammar.ebnf` is written again from the
grammar, and the 57 marks have the new `as <production>` part.
## Verification
- `cargo test --workspace`: 618 passed, 0 failed.
- `cargo fmt --all --check` and `cargo clippy --workspace
--all-targets`: clean.
- `scripts/status-gate.sh`: 615 all-target tests.
- `scripts/probe-battery.sh`: 51 SAME, 7 SKIP (the SKIP rows need the
wasmtime CLI).
- Each of the 26 fixtures in `tests/v2-grammar/unimplemented` gives the
named error on `ephapax check`.
---
_Generated by [Claude
Code](https://claude.ai/code/session_0136P7ZmKrYLDC4WHi2yt11u)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
2 commits.
1. One normative grammar
Before:
spec/ephapax-v2-grammar.ebnf(150 productions) andephapax.pest(125 rules) had only 68 names in common, and no test compared them.The specification file now has 2 parts.
Part 1: the parser grammar. A test of
ephapax-parser(grammar_conformance) makes it fromephapax.pest, with the comments of that file. The test fails when the 2 files are not the same grammar. To write part 1 again after a change ofephapax.pest:UPDATE_GRAMMAR=1 cargo test -p ephapax-parser grammar_conformancePart 2: the v2 design that the parser does not have by that name. Each of the 82 old productions that are not rules of the parser stays, as the design wrote it. Each one has a mark on the line before it:
[ALIAS rule](25): the parser has the construct asruleof part 1.[UNIMPLEMENTED](57): the parser does not have it. The mark names a fixture that the parser refuses.A "DESIGN FORM" comment keeps the design form of a production that part 1 has in a smaller form (for example
fn_declwithwhere, contracts and effects). The specification does not shrink.What the test proves
ephapax.pest, production for rule.tests/v2-grammar/productions. 5 rules can not be in a parse tree; the test names them and the reason.tests/v2-grammar/unimplemented(26 files).Import and extern syntax
Part 1 has the forms of the parser:
import a/b,import a/b(x, y),extern "abi" { ... }. The design formimport X from "f"is in part 2 with the mark[UNIMPLEMENTED]. Which form wins later is an owner decision for each construct.Not in this change
Other files
pest_metais a new dev-dependency ofephapax-parser. It was inCargo.lockalready, as a dependency ofpest_derive(same version).EXPLAINME.adoc: the entry of the grammar file says the new structure.2. Review fixes for #431
checkaccepted the program, and the interpreter stopped with "Linear value used after consumption". The checker now refuses such a guard:TypeError::GuardConsumesLinear. A guard can read or borrow the binding.type Count = Boolchanged the field ofpub data Tally = T(Count)of a module withtype Count = I32. The registry now keeps the alias of each module.Verification
cargo test --workspace: 617 passed, 0 failed.cargo fmt --all --checkandcargo clippy --workspace --all-targets: clean.scripts/status-gate.sh: 614 all-target tests.scripts/probe-battery.sh: 51 SAME, 7 SKIP (the SKIP rows need the wasmtime CLI).🤖 Generated with Claude Code
https://claude.ai/code/session_0136P7ZmKrYLDC4WHi2yt11u