P6.x: resolve Ok/Err/Some/None's IrExprKind identity gap (#1225)

[accuser]

Summary

Closes #1225 — the Ok/Err/Some/None IrExprKind identity gap, open since P6.2/P6.3 (#1143/#1145).

The gap. IrExprKind::Variant { sum: Arc<TypeDecl>, tag: String, payload: Vec<IrExpr> } carried a declaration-identity field mirroring Record::def/GlobalRef::sum. A real user-declared sum's constructor (Circle(n), driven by Callee::Ctor { sum: Arc<TypeDecl>, tag }) has one to give it. Ok/Err/Some/None never do — Option/Result are dedicated Ty variants (Ty::Option/Ty::Result), never Ty::Named, never backed by a TypeDecl. Worse: they aren't even calls — dedicated ExprKind::Ok/Err/Some/None AST variants, checked through check_ok/check_err/check_some/check_none with no Callee ever recorded at all.

The ADR (design/pending/p6-option-result-variant-identity.md): drop sum rather than widen it. IrExpr::ty — already present on every node (R6.1) — already carries the identical identity as a TyId: a constructor call's own checked type is the sum it constructs, and lower_call_ir already computes ty as its first line, before ever branching into the Callee::Ctor arm. This mirrors IrPat::Variant's own scrutinee_ty: TyId precedent exactly — the one place this module already solved the identical problem, resolved via bynk_check::checker::variants_of (already pub for this), proven to cover a user sum, Result, Option, ActorSum, and HttpResult uniformly with no built-in special-casing gap.

Implementation. Variant becomes { tag: String, payload: Vec<IrExpr> }. Ok/Err/Some/None now lower to it directly (tag the constructor name, payload the lowered inner expression, empty for None), same as a real Callee::Ctor construction. Closes the todo!() ir/lower.rs has carried since P6.2/P6.3.

Verified against the real regression this closes: 223_store_cell_agent's own store paymentRef: Cell[Option[AuthId]] = None no longer panics through the full, init-lowering lower_store_field_ir — previously worked around, not fixed, by lower_store_field_shape_ir's own never-lower-init posture (#1187's Agent state-field slice 2a, #1206).

No shipped emitter consumer of this construction path exists yet — ir::lower's whole IrItem/IrExpr output remains dormant, exercised only by its own test suite — so this lands with zero behavioural change to any emitted TypeScript, confirmed by a zero-diff bless run.

Question's own separate todo!() (its three-way ?-desugar fork) is unaffected and named again as still open — a distinct design question this ADR does not settle, per #1225's own scope note.

Test plan

• cargo build --workspace
• cargo clippy --workspace --all-targets
• cargo fmt --all -- --check
• cargo test -p bynk-emit --lib — all 220 tests pass, including new ok_err_some_none_all_lower_to_variant_by_tag and store_field_cell_option_init_none_lowers_without_panicking, and the updated bare_and_qualified_variant_construction_both_lower_to_variant
• BYNK_BLESS=1 cargo test -p bynkc --test e2e bless_positive_fixtures — zero fixture diff (no shipped consumer of this path exists yet)
• cargo test -p bynkc --test tsc_verify — real tsc --strict, all 6 pass
• cargo test --workspace — full suite

🤖 Generated with Claude Code

[claude]

Review

The core reasoning holds up under checking. variants_of (bynk-check/src/checker.rs:4326) resolves Ty::Result to tags Ok/Err and Ty::Option to Some/None — byte-identical to the tags this lowering now emits — so dropping sum in favour of the wrapping IrExpr::ty genuinely loses no information, and the IrPat::Variant / scrutinee_ty precedent is a fair one. ty is computed in lower_expr_ir (bynk-emit/src/ir/lower.rs:1912) before the match, ahead of every new arm, and check_ok/check_err/check_some/check_none each return a concrete Option/Result/HttpResult or push a diagnostic, so in a certified unit the identity is always there to read. The Callee::Ctor destructure with .. is safe — that arm returns immediately, nothing else reads callee.

Release discipline: exactly one design/pending/ increment, with level and changelog. patch is right for a pub(crate) enum with no shipped consumer.

Three findings, none blocking. (The inline-comment API is not writable from this environment, so they are collected here with file:line.)

---

1. bynk-emit/src/ir/lower.rs:3691-3704 — the new test does not exercise the claim the ADR rests on.

The whole resolution is that Variant needs no sum field because the wrapping IrExpr::ty already carries the constructed sum identity. ok_err_some_none_all_lower_to_variant_by_tag asserts tag and payload.len() only — it would pass unchanged if tail.ty came back as Ty::Unit, or as the inner type rather than the Option/Result it must be. The sibling bare_and_qualified_variant_construction_both_lower_to_variant was correctly updated to assert tail.ty resolves to Shape; this one should match it:

matches!(&*program.program().ty_intern.get(tail.ty), Ty::Result(..) | Ty::Option(_))

ideally per case (Result for ok_case/err_case, Option for some_case/none_case). As written, the half that was never in doubt is pinned and the half the ADR depends on is not.

Related, same test: Ok is overloaded and the overload is uncovered. check_ok (bynk-check/src/checker/expressions.rs:1654-1690) resolves Ok to either Result.Ok or HttpResult.Ok, returning Ty::HttpResult(t) when the surrounding return peels to one — so ExprKind::Ok in an http handler reaches the new arm with ty = Ty::HttpResult(T), a third sum the identity claim has to carry, named neither in the tests nor in the new doc comments. It does hold: variants_of has a Ty::HttpResult arm whose HTTP_VARIANTS list (bynk-syntax/src/ast.rs:1319) carries its own Ok variant, so tag plus ty still resolve consistently. That is the argument for pinning it rather than leaving it implicit — a fifth case, a handler returning HttpResult[Int] with an Ok(1) tail, asserting tag Ok and ty is Ty::HttpResult, plus a line in the IrExprKind::Variant doc comment noting the overload.

2. bynk-emit/src/ir/lower.rs:1062-1076 — doc comment left stale by this change.

lower_store_field_shape_ir still documents itself as never lowering init because a Cell initialiser can be ExprKind::None or an is-expression, both of which hit the built-in-constructor gap, naming 223_store_cell_agent and 1029_agent_static_init_hoist as the two fixtures that do. The None half is now false — this PR adds a test proving lower_store_field_ir lowers exactly the 223_store_cell_agent shape without panicking. Only the is half survives (ExprKind::Is, still todo!() at lower.rs:2255; 1029 is store active: Cell[Bool] = if true ... 5 is PositiveInt). This matters more than ordinary doc drift because the ADR Consequences section points readers at this very comment as the record of the workaround.

3. bynk-emit/src/ir/lower.rs:480-497 — second stale rationale.

The service-handler signature-only helper justifies not building a real IrHandler on the grounds that an ordinary from http body routinely constructs Ok/Err/Some/None, still todo!() in lower_expr_ir. That grounding is now gone. The conclusion very likely still stands via the remaining gaps — ExprKind::Question (lower.rs:2195) is ubiquitous in http handlers, and ExprKind::Is (lower.rs:2255) — but as written a future reader will conclude the helper is now removable and be wrong. Worth re-pointing at whichever todo!() still blocks it.

---

Otherwise this reads clean: no remaining IrExprKind::Variant consumer carries sum, the None payload is empty exactly as variants_of reports it, and the zero-diff bless run is consistent with the path being dormant.