[Task 53] Parser: effect declarations + handle expressions#19
Conversation
Stage-6 surface scaffolding for Plan B. Adds parser support for
`effect Name[T] { op: (T) -> R, ... }` (default one-shot) and
`effect Name[T] resumes: many { ... }` (multi-shot opt-in)
declarations, plus `handle <body> with { return(v) => arm,
Effect.op(p, ..., k) => arm, ... }` handler expressions. The
typechecker (Task 54) and CPS transform (Task 55) consume these
shapes; this commit ships the parser surface plus staged
diagnostics so a partial Plan B program cannot reach downstream
passes until the typing rules land.
Lexer:
- New keywords `effect`, `handle`, `with`. The attribute words
`resumes` and `many` stay as plain identifiers and are matched
contextually inside `parse_effect_decl` so user code that
legitimately wants `let resumes = 5` is not regressed.
AST:
- `Item::Effect(Box<EffectDecl>)` with `EffectDecl { name,
generic_params, resumes_many, ops, ... }` and
`EffectOp { name, params: Vec<TypeExpr>, return_type, ... }`.
- `Expr::Handle { body, return_arm, op_arms, ... }` with the
matching `HandleReturnArm` (single value binding + body) and
`HandleOpArm { effect, op, params, k_name, body, ... }`. The
trailing parameter of an op arm is always `k`; arms with empty
parameter lists are rejected at parse time.
Parser:
- Top-level dispatch wires `Effect` into `parse_program`. The
empty-body case (`effect E { }`) is rejected at parse time
because handlers cannot meaningfully discharge no operations.
- `parse_handle_expr` parses the body under `no_record_lits=true`
so `handle Foo with { ... }` doesn't try to read `Foo {` as a
record literal. Arms enforce at-least-one-op-arm,
at-most-one-return-arm, and mandatory trailing `k` rules.
- The arm grammar uses fully-qualified `Effect.op(...)` rather
than the plan body's bare `op(...)` so a single `handle` can
dispatch operations from more than one effect; the qualified
form also mirrors `perform Effect.op(args)`. Documented as
Plan B Task 53 design choice in PLAN_B_DEVIATIONS.md.
Errors:
- Catalog entries `E0133` (effect decl pending Task 54) and
`E0134` (handle expression pending Tasks 54/55) with full
long-form text plus workaround examples.
Downstream stubs (workspace compile-clean):
- typecheck emits `E0133` per effect decl + `E0134` per handle
expression, then walks children so nested type errors still
surface in the same compile pass. `no_user_facing_error_uses_e0001`
sweep extended with two new programs covering the staged shapes.
- elaborate / closure_convert / monomorphize / color all gain
pass-through arms with structural recursion through arm bodies
and arm-binding scope tracking where applicable.
- codegen `lower_expr` and `type_of_expr` use `unreachable!`
(Plan A pattern); the codegen-entry walker hard-rejects any
`Item::Effect` or `Expr::Handle` that slips past typecheck.
Tests:
- 5 lexer tests pin keyword recognition + `resumes`/`many`
remaining-as-Idents.
- 18 parser tests cover happy and error paths (effect decls
with/without generics/resumes, multi-op decls with trailing
comma, empty body / non-`many` rejection, `resumes` outside-
effect-decl regression guard, minimal handle, return-arm,
multi-effect handle, k-binding-as-trailing-param,
missing-with / empty-arms / no-k / duplicate-return error
paths, body-no-record-lit regression).
- 5 typecheck tests pin `E0133` / `E0134` emission and the
cascade-friendly recursion behavior.
Pod-verify clean. CI is authoritative for the full test suite
and multi-host verification.
Review — please address before merge1. Bug — stale doc comment contradicts code
/// At least one operation arm is required. The parser does not
/// reject duplicate `return` arms — the typechecker (Task 54)
/// produces a more specific diagnostic with both spans.
But lines 1125-1135 do reject duplicates with 2. Design — duplicate
|
Review — context-aware passVerdict: merge-with-fixes. Combined with the prior review (#issuecomment-4320067338, items 1–6), the consolidated directive list is below. Mechanical sweep is clean. Per-task
Mechanical (all green)
Must-fix before mergeItems 1, 2, 4, 5 from the prior review stand as written. Plus: 7. Second deviation entry missing. PR documents the qualified-arm shape in 8. Qualified arm syntax confirmed as the v1 surface. After review, qualified Action: add a 9. Adversarial lexer test for Follow-up (Task 54 PR — note in PLAN_B_PROGRESS.md Task-54 prep)10. Typecheck arm-walk does not extend binding scope. 11. Codegen-gate alignment for Task 54. Per item 6 of the prior review. The Deferred (Task 54+ / Stage 6 later / v2+)
RegressionsNone. The discipline sweep Once items 7–9 plus prior-review items 1, 2, 4, 5 land, this is ready to merge. Note items 10 and 11 in |
Addresses PR #19 review items 1, 2, 4, 7, 8, 9; records items 10 + 11 as Task 54 prep. Item 3 skipped per reviewer's "skip if no churn"; item 5 lands as a separate PR-body edit; item 6 forwarded to Task 54's PR description per reviewer direction. **Item 1 — stale doc comment on `parse_handle_expr`.** The pre-fix doc claimed "the parser does not reject duplicate `return` arms" while the body explicitly emitted a duplicate error at lines 1131–1135. Updated the doc to describe the actual behaviour (parser rejects duplicates with first-wins AST semantics; Task 54 can build a cross-span diagnostic from the recorded error and the preserved AST). **Item 2 — duplicate `return` arm: first-wins AST semantics.** Pre-fix the duplicate `return_arm = Some(Box::new(ra))` after the error overwrote the first arm with the second. Now the assignment is gated on `return_arm.is_none()` so the first arm survives in the AST; the duplicate is dropped on the floor and the error span points at the second (offending) arm. New unit test `handle_expr_duplicate_return_arm_first_wins_in_ast` pins the behaviour by parsing a program with `return(first) => first, return(second) => second` and asserting the resulting AST's `return_arm.binding == "first"`. **Item 4 — `expr_uses_generic` Handle walker comment.** Pre-fix the comment implied the walker was load-bearing today. Tightened to call out that the arm is dead code under the current `Item::Effect → return true` short-circuit; kept for the future case where Task 54 lifts the `Item::Effect` gate but `Expr::Handle` still needs a guard during the CPS-transform handoff in Task 55. **Item 7 — second `[DEVIATION Task 53]` entry.** Adds the `resumes` / `many` context-ident deviation to PLAN_B_DEVIATIONS.md with the same four-section structure (plan said / PR did / why / closure point). Documents the engineering choice to keep the words as plain Idents matched contextually inside `parse_effect_decl` rather than reserving them at the lexer level — the cost of reserving common English words like `many` outweighs the surface ambiguity benefit, and the existing `resumes_and_many_remain_idents` + `resumes_outside_effect_decl_remains_ident` tests serve as regression guards. **Item 8 — QUESTIONS.md `[PLAN-B]` handler-arm-surface entry.** Adds an open question on whether Task 54 should keep qualified-only arms or introduce bare-op-as-sugar within unambiguous handler contexts. The first-deviation entry's "Closure point" line now cross-references the new question rather than leaving the closure point dangling. **Item 9 — keyword-side adversarial tests.** Three lexer-keyword adversarials pinning that `let effect: Int = 1`, `let handle: ...`, and `let with: ...` all parse-error because the words are reserved by the lexer as of Task 53. Together with the existing ident-side regression guard `resumes_outside_effect_decl_remains_ident` for the context-keyword half, this closes the keyword matrix. **Items 10 + 11 — recorded as Task 54 prep in PLAN_B_PROGRESS.md.** Item 10: the staged-E0134 arm-walker doesn't extend the local env with op-arm bindings or `k` before walking arm bodies; programs like `E.op(x, k) => x + 1` would emit a spurious E0046 alongside E0134 today. Item 11: the codegen-entry `Item::Effect → return true` short-circuit only catches programs with an effect decl in scope; if Task 54 lifts E0134 before Task 55's CPS expansion lands, codegen will panic on a `handle` over an unhandled effect with no decl in scope. Both items recorded under Task 54's "prep" section so they don't get lost across the PR boundary. Test deltas: +3 parser tests (first-wins AST shape, effect-as-keyword adversarial, handle/with-as-keyword adversarial). Pod-verify clean.
Review fixups pushed (
|
| # | Item | Disposition | Where |
|---|---|---|---|
| 1 | Stale doc comment on parse_handle_expr |
Fixed | compiler/src/parser.rs:1097-1106 |
| 2 | Duplicate return arm clobbers first |
Fixed (first-wins); test added | parser.rs:1136-1141 + new test handle_expr_duplicate_return_arm_first_wins_in_ast |
| 3 | eat_resumes_many_attr Option<bool> → enum |
Skipped (optional, no churn justified) | — |
| 4 | expr_uses_generic Handle walker comment misleading |
Comment tightened | compiler/src/codegen.rs:355-364 |
| 5 | PR body "no new allocation sites" too strong | PR body rewritten | (gh pr edit applied) |
| 6 | Codegen-gate alignment — note in Task 54 PR | Recorded as Task 54 prep item 11 | PLAN_B_PROGRESS.md |
| 7 | Second [DEVIATION Task 53] entry missing |
Added | PLAN_B_DEVIATIONS.md (new section: "resumes and many shipped as context-sensitive idents") |
| 8 | QUESTIONS.md entry + cross-ref from deviation |
Added; first deviation entry's "Closure point" now cross-refs the question | QUESTIONS.md (new entry "Task 54: revisit handler arm surface"); PLAN_B_DEVIATIONS.md first-deviation entry's "Closure point" line |
| 9 | Adversarial lexer test for let effect: Int = 1 |
Added 3 tests covering effect, handle, with |
parser.rs::effect_as_user_ident_in_let_is_rejected, handle_with_keywords_rejected_in_user_ident_position |
| 10 | Typecheck arm-walk binding-scope extension | Recorded as Task 54 prep | PLAN_B_PROGRESS.md Task 54 prep |
| 11 | Codegen-gate alignment for Task 54 | Recorded as Task 54 prep | PLAN_B_PROGRESS.md Task 54 prep |
Test deltas: +3 parser tests (first-wins AST shape; effect-as-keyword adversarial; handle/with-as-keyword adversarials). All pre-existing tests still green; pod-verify clean.
Notes on item 2 (first-wins): the AST now keeps the first return arm; the duplicate is dropped on the floor and the error span points at the second (offending) arm so the user's eye lands on the line they need to remove. New test handle_expr_duplicate_return_arm_first_wins_in_ast parses return(first) => first, return(second) => second and asserts return_arm.binding == "first" to pin the contract.
Notes on items 7/8: the second deviation entry uses the same four-section structure as the qualified-arm one. The "Closure point" line on the qualified-arm deviation now cross-references the new QUESTIONS.md entry. The new question carries forward implications and notes that HandleOpArm::effect: String becomes Option<String> if Task 54 desugars bare-op-as-sugar — purely additive, no breaking change.
Notes on items 10/11: recorded under Task 54's prep: section in PLAN_B_PROGRESS.md. Item 10 calls out that the staged-E0134 arm-walker doesn't extend the local env with op-arm bindings or k before walking arm bodies (so E.op(x, k) => x + 1 would emit a spurious E0046 today; the existing test sidesteps this with body: true). Item 11 calls out that the codegen-entry Item::Effect → return true short-circuit only catches programs with an effect decl in scope, so a handle over an unhandled effect with no decl in scope would skip the gate and reach the unreachable! if Task 54 lifts E0134 before Task 55's CPS expansion lands.
CI polling on the fixup push.
Re-review — mergeSpot-check confirms all directives addressed; CI green on all 4 lanes.
CI: Merge. Squash-merge SHA lands in |
1 participant
Plan B Stage 6 begins. This PR ships the parser surface for
effectdeclarations and
handleexpressions, the foundational scaffoldingfor the algebraic-effects runtime that Tasks 54–61 will build on.
Per the Plan B convention, this is a parser-only commit. The
typechecker emits
E0133for everyeffectdecl andE0134forevery
handleexpression, ensuring no partial Plan B program canreach monomorphization, CPS transform, or codegen until Task 54's
typing rules and Task 55's CPS transform land.
Surface forms shipped
Landed in this PR
effect+handleHighlights
Lexer: new keywords
effect,handle,with. The attributewords
resumesandmanystay as plain identifiers and are matchedcontextually inside
parse_effect_decl, so user code thatlegitimately wants
let resumes = 5is not regressed. Two[DEVIATION Task 53]entries inPLAN_B_DEVIATIONS.mddocument thequalified-arm surface choice and the context-keyword choice with
their respective closure points.
AST:
Item::Effect(Box<EffectDecl>)with supportingEffectDecl/EffectOptypes;Expr::Handle { body, return_arm, op_arms, span }withHandleReturnArmandHandleOpArm.Design choice — qualified arm shape: the plan body writes
handler arms as
op(args, k) => arm(bare op name). This PR shipsthe qualified form
Effect.op(args, k) => armso a singlehandlecan dispatch operations from more than one effect, and so the
surface mirrors
perform Effect.op(args). Documented as[DEVIATION Task 53]inPLAN_B_DEVIATIONS.mdand tracked as[PLAN-B] Task 54: revisit handler arm surface (qualified-only vs bare-op-as-sugar)inQUESTIONS.md— Task 54+ ergonomic data candecide whether to introduce bare-op desugaring within unambiguous
handler contexts. The AST records both
effectandopnames, sobare-op-as-sugar is a strict forward-compatible extension.
Parser-level rules (rejected before typecheck):
returnarm. Duplicates arerejected with first-wins AST semantics so downstream passes see
a stable target.
k.resumes:attribute accepts onlymanyin v1.Errors:
E0133(effect decl pending Task 54) andE0134(handleexpr pending Task 54/55) with full long-form text + workaround
examples. The
no_user_facing_error_uses_e0001discipline sweepgains two new programs covering the staged shapes.
Downstream stubs: every pass that exhaustively matches on
ItemorExprgains a Task-53 arm. Pre-codegen passes preservestructural shape (with arm-body recursion); codegen rejects with
unreachable!since typecheck E0134 must catch any well-formedprogram.
Tests
resumes/many-stay-Idents).keyword-side adversarials added in fixup:
let effect: Int = 1,let handle: Int = 1,let with: Int = 1all parse-errorbecause the words are now reserved; the duplicate-return AST
test pinning first-wins semantics).
E0133/E0134emission.Total: 31 new unit tests. Pod-verify clean. CI is authoritative for
the full test suite + multi-host build.
Plan B context
Stage 5 closed at 2026-04-25 with PR #18 (
6305bcb); Stage 6(Tasks 53–61) is the algebraic-effects runtime. Per the plan body's
explicit instruction, every Stage 6 PR receives human review before
merging. Tasks 54–61 still pending after this lands.
Memory discipline
This commit adds AST surface, parser code, and structural pass-through
arms only. Non-effect programs see an unchanged Cranelift baseline
because typecheck rejects every
effect/handleshape withE0133/E0134before lowering. Programs that do contain effectdeclarations or handler expressions allocate the new AST nodes
(
EffectDecl,HandleOpArm, theVec<HandleArmParam>per arm) atparse time, but those programs abort before codegen so no
allocation-sensitive runtime paths are affected. The Cranelift
per-compile peak should remain at the post-PR-#18 baseline; Task 56's
runtime work is the next inflection point.
Plan B PROGRESS hygiene
Task 53 status
done-pending-ciwith[HEAD]placeholder; flipsto
donewith the squash-merge hash in the next task's PR perestablished convention. Items 10 + 11 from review (typecheck
arm-walk binding-scope extension; codegen-gate alignment) recorded
under Task 54's "prep" entry in
PLAN_B_PROGRESS.md.