Skip to content

Commit dbfb925

Browse files
committed
gale: distill the review response
Claim (a) has one exception. `g4::parse` accepts a grammar whose action body the translator cannot render; that failure is at codegen, so the carve-out named a second exception the parser does not have, and pointed at a file that documented neither. `README.md` and `AGENTS.md` follow. The channel remap says what makes it correct rather than what its fallback branch does: a `channels { }` block is the grammar's own numbering, so inside its range an id and the name declared at that index are one channel. `arms_are_one_level` replaces three copies of the same fold. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q2Kw2ucQ6fpgwK1KH8CzLC
1 parent 84f57f4 commit dbfb925

7 files changed

Lines changed: 54 additions & 59 deletions

File tree

package-gale/AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ Files headed `// Do not edit by hand` are generated. To change one, edit its sou
2323

2424
## Compatibility principle
2525

26-
Gale targets full compatibility with the ANTLR4 `.g4` syntax. The g4 parser must accept any well-formed grammar upstream `antlr4` accepts; a real-world `.g4` that ANTLR4 accepts but Gale rejects is a Gale bug. The exceptions are enumerated under claim (a) in [`antlr4-compatibility.md`](./antlr4-compatibility.md) — today an action body Gale's translator cannot render, and `import Foo = Bar;`.
26+
Gale targets full compatibility with the ANTLR4 `.g4` syntax. The g4 parser must accept any well-formed grammar upstream `antlr4` accepts; a real-world `.g4` that ANTLR4 accepts but Gale rejects is a Gale bug. The one exception is `import Foo = Bar;`; claim (a) in [`antlr4-compatibility.md`](./antlr4-compatibility.md) carves it out.
2727

2828
- Compatibility is a capability contract, not byte-for-byte output. Parse trees, tokens, and semantics must match; incidental rendering differences that carry no structure may diverge (e.g. the `<EOF>` marker in `toStringTree()`).
2929
- Gale is a superset: it may accept grammars ANTLR4 rejects only when the meaning is uniquely determined by Gale's language model — never an invented behavior. When accepting would require guessing, reject loudly. Examples: `.`/`~X`-led left-recursive suffixes, and a lexer `mode` inside a combined `grammar` (ANTLR4 allows modes only in a `lexer grammar`; a combined grammar already bundles a lexer, so it desugars unambiguously — still rejected in a `parser grammar`).

package-gale/README.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ with no runtime to install and no version to keep in sync.
77

88
The `.g4` format is ANTLR4's; for the full grammar language, see ANTLR4's
99
[documentation](https://github.com/antlr/antlr4/tree/master/doc). Gale accepts
10-
every grammar ANTLR4 accepts bar a short list — and a few it rejects, where the
11-
meaning is unambiguous (see [Design](#design)).
10+
every grammar ANTLR4 accepts bar one — and a few it rejects, where the meaning
11+
is unambiguous (see [Design](#design)).
1212

1313
## Design
1414

@@ -21,9 +21,9 @@ with no remaining choice — e.g. a `.`- or `~X`-led left-recursive suffix like
2121
`e ~';' e`, which ANTLR4 errors on (no operator token to climb); or a lexer
2222
`mode` inside a combined `grammar`, which ANTLR4 restricts to a `lexer grammar`
2323
but which is unambiguous since a combined grammar already bundles a lexer. Where
24-
the meaning is not uniquely determined, Gale rejects loudly rather than guessing
25-
— including a handful of grammars ANTLR4 accepts, listed under claim (a) in
26-
[`antlr4-compatibility.md`](./antlr4-compatibility.md).
24+
the meaning is not uniquely determined, Gale rejects loudly rather than guessing.
25+
That costs it one grammar ANTLR4 accepts, `import Foo = Bar;`, carved out under
26+
claim (a) in [`antlr4-compatibility.md`](./antlr4-compatibility.md).
2727

2828
Self-contained output, no version drift. Gale inlines its entire runtime into
2929
every generated parser. There is no `gale-runtime` package to keep aligned with

package-gale/antlr4-compatibility.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,14 @@ that should be fixed before Stage C lands.
123123
> **Claim:** the generated code satisfies each of these independently:
124124
>
125125
> - **(a)** Every well-formed `.g4` file accepted by upstream `antlr4`
126-
> is accepted by Gale's `g4::parse`, with two exceptions: the
127-
> action-body one documented in [`AGENTS.md`](./AGENTS.md), and
126+
> is accepted by Gale's `g4::parse`, with one exception:
128127
> `import Foo = Bar;`, whose alias names the delegate for qualified
129128
> action references Gale has no counterpart for
130-
> ([`import.md`](./import.md)). A descriptor bringing slave grammars
131-
> claims (a) of the whole composition, through `assemble_grammar`.
129+
> ([`import.md`](./import.md)). An action body the translator cannot
130+
> render parses fine and fails later, at codegen, with a diagnostic
131+
> naming it ([`action.md`](./action.md)), so it is not an exception
132+
> here. A descriptor bringing slave grammars claims (a) of the whole
133+
> composition, through `assemble_grammar`.
132134
> - **(b)** For every descriptor whose upstream `[output]` describes a
133135
> _successful_ parse, the parser generated by Gale accepts the
134136
> descriptor's `[input]` without returning an error.

package-gale/import.md

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,14 @@ it, given four conventions in the delegate:
6969
the delegate's mode, never produces that token. The gate is deliberate: a
7070
parser rule is reached in whatever mode its caller pushed, and nothing records
7171
that, so a literal is read as the mode the parser starts in.
72-
- No token of its own may span the host's terminator. A composite has one lexer,
73-
so inside the mode a delegate rule is matched like any other: a
74-
`JS_COMMENT : '//' ~[\r\n]*` runs straight through `</script>`, the mode never
75-
pops, and the rest of the document is lexed as JavaScript. A mode cannot
76-
express HTML's rule that a closing tag ends raw text whatever the embedded
77-
language thinks — deciding that needs lookahead a mode does not have. So a
78-
scanning rule stops at `<`, and the `<` it then rejects inside a string or a
79-
comment is a stated hole in the delegate rather than a document in the wrong
80-
mode.
72+
- No token of its own may span the host's terminator. Inside the mode a delegate
73+
rule is matched like any other, so a `JS_COMMENT : '//' ~[\r\n]*` runs straight
74+
through `</script>`. The mode then never pops, and the rest of the document is
75+
lexed as JavaScript. A lexer mode cannot express HTML's rule that a closing tag
76+
ends raw text whatever the embedded language thinks, because deciding that
77+
needs lookahead the mode does not have. So a scanning rule stops at `<`, and
78+
the `<` it then rejects inside a string or a comment is a stated hole in the
79+
delegate rather than a document in the wrong mode.
8180

8281
The host declares the same mode name for the token that leaves it
8382
(`STYLE_CLOSE : '</style>' -> popMode ;` under its own `mode CSS;`), and the

package-gale/src/ir.wado

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1539,8 +1539,10 @@ fn rebase_lexer_rules(g: &Grammar, modes: &mut List<String>, channels: &mut List
15391539
if r.set_mode >= 0 {
15401540
r.set_mode = mode_remap[r.set_mode];
15411541
}
1542-
// A numeric `channel(N)` literal is an explicit id with no
1543-
// `channels { }` entry behind it, so it has no remap slot.
1542+
// A `channels { }` block is the grammar's own numbering, so inside its
1543+
// range an id and the name declared at that index are one channel and
1544+
// rebasing carries both. Past the range a numeric `channel(N)` has no
1545+
// entry behind it and stays as written.
15441546
if r.channel >= 2 {
15451547
let src_idx = r.channel - 2;
15461548
if src_idx < chan_remap.len() {

wado-compiler/src/optimize/sroa_variant_return.rs

Lines changed: 26 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -105,11 +105,8 @@ impl Layout {
105105
.map(|i| u32::try_from(i).expect("variant case index overflow"))
106106
}
107107

108-
/// The payload a binding for `case_index` receives, `None` for a unit case,
109-
/// which has no slot to read. The slot's own type can be the nullable
110-
/// widening [`slot_shape`] chose for it, which `wir_build` bridges; a
111-
/// wrapper around the payload it never bridges, so that is what the binding
112-
/// has to be measured against.
108+
/// The payload a binding for `case_index` receives. `None` for a unit case,
109+
/// which has no slot to read.
113110
fn payload_read_type(&self, case_index: u32) -> Option<TypeId> {
114111
self.case_slots[case_index as usize].flat()?;
115112
Some(self.case_payloads[case_index as usize])
@@ -1608,10 +1605,7 @@ fn check_uses(
16081605
let rewritable = via_temp
16091606
.or(via_call)
16101607
.and_then(|f| candidates.get(&f))
1611-
.is_some_and(|c| {
1612-
arms.iter()
1613-
.all(|a| arm_is_one_level(body, a, rebind, &c.layout))
1614-
});
1608+
.is_some_and(|c| arms_are_one_level(body, &arms, rebind, &c.layout));
16151609
if !rewritable {
16161610
invalid.extend(via_temp.into_iter().chain(via_call));
16171611
}
@@ -1699,26 +1693,18 @@ enum Rebound<'a> {
16991693
Boxed { box_type: TypeId, name: &'a str },
17001694
}
17011695

1702-
fn peel_refs(mut ty: TypeId, type_table: &TypeTable) -> TypeId {
1703-
while let ResolvedType::Ref(inner) | ResolvedType::MutRef(inner) = type_table.get(ty) {
1704-
ty = *inner;
1705-
}
1706-
ty
1707-
}
1708-
17091696
/// Per-function facts deciding whether a payload binding can be re-minted, and
17101697
/// in what form. A local a `stores` parameter aliases cannot: the alias is
17111698
/// established outside this body, and the `let` would not re-establish it.
1712-
/// Neither can one whose declared type is neither the pattern's own type nor a
1713-
/// `Box` of it — local indices are pooled, so a declaration that matches
1714-
/// nothing the pattern names belongs to another binding.
1699+
/// Neither can one whose declared type is neither the payload nor a `Box` of it
1700+
/// — local indices are pooled, so a declaration matching neither belongs to
1701+
/// another binding.
17151702
struct Rebind {
17161703
aliased: IndexSet<u32>,
17171704
local_types: Vec<TypeId>,
1718-
/// Each declared local type with its `&` / `&mut` layers stripped. `&T` is
1719-
/// `T` at WIR level (`wir_build::context`), and everything needing a cell
1720-
/// reaches here already rewritten to `Box<T>`, so a reference binding takes
1721-
/// the payload as it stands.
1705+
/// Each declared local type with its `&` / `&mut` layers stripped: `&T` is
1706+
/// `T` at WIR level (`wir_build::context`), what needs a cell arriving as
1707+
/// `Box<T>` instead.
17221708
peeled_types: Vec<TypeId>,
17231709
/// Declared `Box<T>` local type → (`T`, the struct's rendered name).
17241710
boxes: IndexMap<TypeId, (TypeId, String)>,
@@ -1760,10 +1746,9 @@ impl Rebind {
17601746
}
17611747
}
17621748

1763-
/// `payload_type` is what the slot read produces, never what the pattern
1764-
/// spells: `lower::plan::boxing` rewrites an address-taken binding's pattern
1765-
/// type along with its local, so the two agreeing proves nothing about the
1766-
/// value that arrives.
1749+
/// `payload_type` is what the slot read produces. Never pass the pattern's
1750+
/// own type: `lower::plan::boxing` rewrites it along with the local, so the
1751+
/// two agreeing says nothing about the value that arrives.
17671752
fn rebound(&self, local: u32, payload_type: TypeId) -> Option<Rebound<'_>> {
17681753
if self.aliased.contains(&local) {
17691754
return None;
@@ -1782,6 +1767,18 @@ impl Rebind {
17821767
}
17831768
}
17841769

1770+
fn peel_refs(mut ty: TypeId, type_table: &TypeTable) -> TypeId {
1771+
while let ResolvedType::Ref(inner) | ResolvedType::MutRef(inner) = type_table.get(ty) {
1772+
ty = *inner;
1773+
}
1774+
ty
1775+
}
1776+
1777+
fn arms_are_one_level(body: &Body, arms: &[ArmData], rebind: &Rebind, layout: &Layout) -> bool {
1778+
arms.iter()
1779+
.all(|a| arm_is_one_level(body, a, rebind, layout))
1780+
}
1781+
17851782
/// One level deep over the scrutinee's own variant, binding at most one name
17861783
/// the rewrite can re-mint. A nested pattern (`Ok(Some(x))`) is rejected rather
17871784
/// than peeled — peeling is where the WIR pass's complexity lives.
@@ -2313,9 +2310,7 @@ fn collect_call_scrutinees(
23132310
if let NodeRef::Expr(e) = node
23142311
&& let ExprKind::Match { expr: scrut, arms } = &body.exprs[e].kind
23152312
&& let Some(cand) = call_callee(body, *scrut).and_then(|f| candidates.get(&f))
2316-
&& arms
2317-
.iter()
2318-
.all(|a| arm_is_one_level(body, a, rebind, &cand.layout))
2313+
&& arms_are_one_level(body, arms, rebind, &cand.layout)
23192314
{
23202315
out.push(e);
23212316
}
@@ -2458,10 +2453,7 @@ fn check_temp_uses(
24582453
}
24592454
ExprKind::Match { expr: scrut, arms } => {
24602455
if is_local(body, *scrut, local) {
2461-
if !arms
2462-
.iter()
2463-
.all(|a| arm_is_one_level(body, a, rebind, layout))
2464-
{
2456+
if !arms_are_one_level(body, arms, rebind, layout) {
24652457
*ok = false;
24662458
return;
24672459
}

wado-compiler/tests/fixtures/bug_sroa_variant_return_boxed_payload_binding.wado

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@
33
// `make` returns `Result<Big, Failure>`, which variant-return SROA flattens to
44
// `[tag, Option<Big>, Option<Failure>]`. `err` is address-taken by `${err:?}`,
55
// so lowering promotes its local to `Box<Failure>` and rewrites the pattern's
6-
// own type to match. The rewrite asked whether the local's declared type equals
7-
// the pattern's, which both hold after that promotion, and bound the raw slot
8-
// read into a `Box<Failure>` local: "type mismatch: expected (ref null $type),
9-
// found (ref $type)" at codegen. The question is what the slot read produces.
6+
// own type to match. Measuring the local against the pattern rather than
7+
// against the payload the slot yields therefore passed, and the raw read landed
8+
// in a `Box<Failure>` local: "type mismatch: expected (ref null $type), found
9+
// (ref $type)" at codegen.
1010
use { println, Stdout } from "core:cli";
1111

1212
variant Failure {

0 commit comments

Comments
 (0)