fix(compiler): stability-review fixes — executor convergence and soundness holes

openspec/changes/archive/2026-08-26-compiler-review-stability-fixes/.openspec.yaml

@@ -0,0 +1,2 @@
+schema: spec-driven
+created: 2026-08-26

openspec/changes/archive/2026-08-26-compiler-review-stability-fixes/design.md

@@ -0,0 +1,60 @@
+# Design
+
+## Context
+
+See proposal.md — Why. Ten confirmed review findings across three executor backends (bootstrap interpreter, wasm, native/LLVM), semantic analysis, ownership, and layout plumbing. Line numbers below are from the review at commit 60c01ed and must be re-anchored after merging latest main.
+
+## Goals / Non-Goals
+
+**Goals:** minimal, surgical edits per finding; three-executor convergence; one focused test per fix.
+
+**Non-Goals:** no refactors beyond what a fix requires (no WasmBackend split, no Mir operand-visitor API, no broader dedup of layout math — those stay tech debt); no new diagnostics infrastructure; no changes to the other 30 confirmed review items.
+
+## Decisions
+
+1. **`MIN % -1` traps; checked returns `None`** (findings 1, 6). The existing spec text ("trap on ... invalid division/remainder"), native's current behavior, and the wasm backend's comment (which wrongly assumed `rem_s` traps) all indicate trap was the intended semantics; checked ops return `None` exactly where ordinary ops trap (matching CheckedDivide). Alternative — converge on 0 (wasm `rem_s` spec behavior) — rejected: it contradicts the written spec and the checked/ordinary correspondence.
+   - `NativeScalarOperation.ts` (~552): widen the `CheckedDivide` MIN/-1 guard condition to also cover `CheckedRemainder`, so `invalid` includes overflow and `safeRight` is substituted — result `None`, no `srem MIN,-1` ever emitted.
+   - `WasmBackend.ts` checked path (~6807): widen the `signedOverflow` guard to `CheckedRemainder` the same way.
+   - `WasmBackend.ts` ordinary signed remainder (~141/169 emit sites): emit an explicit `if (left == MIN && right == -1) unreachable` guard before `rem_s`, mirroring how other invalid-arith traps are emitted; fix the false comment at ~134.
+   - `BootstrapArithmetic.ts` (~87/119): make the ordinary remainder path trap on MIN/-1 (the range check currently passes 0); checked path returns `None` for the same condition (~155).
+
+2. **Exact fmod on wasm via emitted helper** (finding 7). Wasm has no `frem` instruction; the div-trunc-mul-sub expansion is numerically wrong. Emit one synthetic module-local helper per float width implementing exact fmod with the standard exponent-aligned iterative algorithm (musl-style, integer bit manipulation on i32/i64 reinterpretations), and route both Remainder emit sites (~6261, ~6900) through calls to it. Alternative — import a host function — rejected: breaks self-contained wasm output. Alternative — keep expansion but clamp — rejected: still inexact on ordinary operands.
+
+3. **Bootstrap rotate count: Euclidean mod** (finding 8). One-line fix at `BootstrapArithmetic.ts:57`: `Number(((right % w) + w) % w)`. No changes to wasm/native (already mask correctly).
+
+4. **Effect-block typing** (finding 2), three edits in `ExpressionAnalysis.ts`:
+   - `collectTerminals` (~5008): add an `UnsafeStatement` arm recursing into its nested statements, mirroring `returnFlowOf` (StatementAnalysis.ts:1066-1071).
+   - Success type (~5024): join every collected return through `Match.join` — the language's one canonical result-join rule (match arms already use it), so effect blocks and match expressions cannot disagree. Joinable-but-different types form the canonical union (the surrounding context then rejects a mismatched use); a join with no representable form emits the new SEM0163 `effectBlockReturnMismatch` at the first disagreeing return. Alternative — first-return-wins with per-site equality checks — rejected in implementation: it would invent a second join rule beside `Match.join`.
+   - Failure filter (~5013): drop the `Type.isNominal` filter entirely — `FailStatement.failure` is only ever set after StatementAnalysis.ts:950-955 validated it (runtime-concrete or value-kind parameter), so re-filtering at collection can only lose information; `Type.effect` already partitions concrete vs symbolic failures.
+
+5. **EnumValue capture** (finding 3): add an `EnumValue` arm to the `effectCaptureFacts` switch (~4768) that visits `fact.argument`. Also add an exhaustiveness guard (`satisfies never` on the fallthrough) so the next missing fact kind is a compile error, not a silent non-capture — this is the root-cause fix for the bug class, at one line of cost.
+
+6. **Ownership scanRunEnds** (finding 4): add `PlaceReplace`, `EffectResult`, and `EffectBindRequirement` arms (~1445) that recurse into their operand facts the same way the existing composite arms do, so nested occurrences reach the Identifier case (1528-1558). Include the same `satisfies never` exhaustiveness guard if the fact union allows it; otherwise mirror the existing default handling.
+
+7. **Native EffectComposite lanes via callingShape** (finding 5): in `NativeType.lanesFor` (~24), resolve a registered `Layout.callingShape` for the composite first and derive lanes from it — exactly the `WasmLanes.laneKindsOf` pattern (WasmLanes.ts:34-36) — falling back to the current computation only when no shape is registered. Audit `NativeEffectOperation.ts` Pack/RunEffectComposite (~105-118, ~388-403): with overlapped MAX-payload lanes their slot-0 placement becomes consistent; adjust lane-type coercion to the unified payload types from the shape.
+
+8. **Coroutine frame keyed by full suspension key** (finding 9): change `CoroutineFrame.stateLayout` (~150) to match entries with `contractRow` included, using the same canonicalization as `pointKey`/`Instances.keyText` rather than a hand-rolled triple — one comparison via the existing key text kills the drift risk.
+
+9. **Environment lanes from Layout only** (finding 10):
+   - `MirVerification.effectFieldLaneCount` (~1108): delete the re-derivation; return `Layout.effectFieldLanes(...).length` (the exact pattern CleanupEmission.ts:126 already uses).
+   - `WasmBackend.hookReleaseInstructions` `environmentOffsets` (~1767): consume `Layout.effectFieldLanes`/`callableFieldLanePlacements` to enumerate field offsets and hook-bearing lanes instead of re-walking representations. If the Layout helpers don't currently expose per-field offset + hook info, add the minimal accessor to `Layout.ts` next to `effectFieldLanes` rather than widening the backend walk.
+
+10. **Discovered during apply — two additional native/runner bugs fixed** (exposed by the new tests, same bug families as findings 3 and 5):
+   - `EntryAssembly.lowerEffectRunner` lowered capture parameter types via `mirType` without the layout, so a captured scalar enum lost its `Enum` representation and every enum operation in a generated runner body silently failed to lower (the runner was then dropped and `Mir.verify` rejected the module). Fix: pass the layout through.
+   - `MirLinearization.opensRuntimeContinuation` omitted `RunEffectComposite` while listing every sibling `Run*` operation, so locals defined after a composite run in the same linear block leaked as raw SSA values from the synthesized `following` block into later blocks — "non-phi forward value reference" at bitcode encoding. Fix: add the missing tag.
+
+## Risks / Trade-offs
+
+- [Wasm MIN/-1 guard adds a branch to every signed `%`] → only signed integer remainder; two extra instructions; acceptable. Constant-fold cases unaffected.
+- [Emitted fmod helper is the largest new code surface] → gate with parity tests against native/bootstrap on a value sweep (subnormals, extreme exponents, NaN/inf propagation, both widths).
+- [New diagnostic on mismatched effect-block returns may flag existing corpus code] → it flags real unsoundness; fix corpus programs if any trip it.
+- [Finding 5's lanesFor change can shift native frame layouts] → run the full native acceptance corpus; the change only affects EffectComposite types with a registered shape, where current behavior is already wrong.
+- [Line numbers drift after merging main] → tasks reference symbols, not lines; re-locate by symbol.
+
+## Deferred to a follow-up PR
+
+Review findings intentionally left out of this change (tracked as a spawned follow-up task): loan-end handling for effect-bind provider references (Ownership.scanRunEnds); joining only Available returns instead of degrading the block on any Unavailable return, and never-aware offender-span selection; SEM0163 test coverage; subword/i64 remainder and rotate corpus cases; EnumValue/EffectResult loan-scan tests; the dedicated hook-release test (task 10.3); corpus program rename, parity-test tag guard, and an operation-based needsFloatRemainder gate.
+
+## Migration Plan
+
+Land as one change; each fix is an independent commit-sized edit; tests run via `node scripts/turbo.mjs run test`. No data or deployment migration (unreleased project). Rollback = revert.

openspec/changes/archive/2026-08-26-compiler-review-stability-fixes/proposal.md

@@ -0,0 +1,43 @@
+# Compiler Review Stability Fixes
+
+## Why
+
+A high-effort compiler review confirmed 10 defects: cross-executor arithmetic divergence (including one LLVM undefined-behavior path), three soundness holes in effect-block result typing, a missed capture, a borrow-checker blind spot, native-only lane/frame-keying bugs, and environment-lane logic re-derived outside its authority that has already desynced once. All are reachable from legal source or from a routine future change; fixing them now converges the three executors and closes the silent-miscompile paths.
+
+## What Changes
+
+Minimal edits per finding — behavior converges on what the specs already require; no new features.
+
+- **Integer remainder MIN/-1** (findings 1, 6): the intended semantics is the spec's "trap on invalid division/remainder" — native already traps and the wasm backend's comment shows trap was assumed. Fix the two executors that silently return 0: wasm emits an explicit MIN/-1 guard → `unreachable` before `rem_s`; bootstrap traps in its remainder range check. `CheckedRemainder(MIN, -1)` returns `None` on all three executors (native's existing CheckedDivide guard extends to CheckedRemainder, eliminating the `srem MIN,-1` UB; wasm and bootstrap add the same overflow condition).
+- **Float remainder on wasm** (finding 7): replace the inexact `left - trunc(left/right) * right` expansion (two sites) with an exact fmod helper function emitted into the module, matching native `frem` and bootstrap `%`.
+- **Bootstrap rotate counts** (finding 8): mask the rotate count unsigned modulo lane width (Euclidean mod) before shifting, matching wasm `rotl`/`rotr` and native `fshl`/`fshr`.
+- **Effect-block result typing** (finding 2): `collectTerminals` descends into `UnsafeStatement`; all collected returns are joined with a diagnostic on incompatible success types instead of last-return-wins; value-kind type-parameter failures survive into the failure row instead of being dropped by the nominal-only filter.
+- **Effect-block captures** (finding 3): `effectCaptureFacts` visits the `EnumValue` fact's argument so `Enum.value(x)` registers `x` as a capture.
+- **Ownership loan ends** (finding 4): `scanRunEnds` gains cases for `PlaceReplace`, `EffectResult`, and `EffectBindRequirement` so identifier/callable occurrences inside their operands extend loan ends and invalidate `callableEnds`.
+- **Native EffectComposite lanes** (finding 5): `NativeType.lanesFor` resolves `EffectComposite` through the registered `Layout.callingShape` (overlapped MAX-payload lanes) the way `WasmLanes.laneKindsOf` does, instead of concatenating alternatives.
+- **Coroutine frame lookup** (finding 9): `CoroutineFrame.stateLayout` matches frame entries by the full suspension key including `contractRow`, aligning with `pointKey`/`Instances.keyText`.
+- **Environment-lane single source of truth** (finding 10): `WasmBackend.hookReleaseInstructions` and `MirVerification.effectFieldLaneCount` consume `Layout.effectFieldLanes` instead of re-deriving the field walk (one prior desync in history; the verifier's copy is count-only and blesses divergence).
+- Fix the false `rem_s` trap comment at `WasmBackend.ts:134`.
+
+## Capabilities
+
+### New Capabilities
+
+None.
+
+### Modified Capabilities
+
+- `bootstrap-integer-scalars`: pin `MIN % -1` (trap) and `CheckedRemainder(MIN, -1)` (`None`) semantics, and rotate-count wrapping modulo lane width, identically across executors.
+- `bootstrap-floating-point-scalars`: pin float remainder to exact IEEE fmod results on every executor.
+- `bootstrap-flow-functions`: effect-block result typing accounts for terminals inside `unsafe` blocks, rejects incompatible success types across return sites with a diagnostic, keeps generic (type-parameter) failures in the failure row, and captures bindings referenced through enum-value construction.
+- `bootstrap-ownership`: loan live-ranges account for uses nested in place-replace, effect-result, and requirement-binding expressions.
+
+### Implementation-only (no spec delta)
+
+Findings 5, 9, 10 make the native backend and verifier conform to already-specified parity/layout behavior; no requirement changes.
+
+## Impact
+
+- `packages/compiler/src`: `NativeScalarOperation.ts`, `NativeArith.ts`, `WasmBackend.ts`, `BootstrapArithmetic.ts`, `ExpressionAnalysis.ts`, `Ownership.ts`, `NativeType.ts` (+ `NativeEffectOperation.ts` if placement follows lanes), `CoroutineFrame.ts`, `MirVerification.ts`, `Layout.ts` (export of the lane-walk helper if not already public).
+- Observable behavior: programs relying on `MIN % -1 == 0` on wasm/bootstrap now trap (they already trapped on native); effect blocks with mismatched branch returns now get a diagnostic. Silk is unreleased — no compatibility concerns.
+- Tests: one focused check per fix, run via `node scripts/turbo.mjs run test`; cross-executor cases land in the existing engine-parity suites.

openspec/changes/archive/2026-08-26-compiler-review-stability-fixes/specs/bootstrap-floating-point-scalars/spec.md

@@ -0,0 +1,13 @@
+## ADDED Requirements
+
+### Requirement: Float remainder is exact IEEE fmod on every executor
+
+Floating-point `%` SHALL produce the exact IEEE-754 remainder (fmod semantics: the result of `x - n*y` where `n` is `x/y` truncated toward zero, computed without intermediate rounding or overflow) for both `f32` and `f64`, identically on the interpreter, the wasm backend, and the native backend.
+
+#### Scenario: Extreme-magnitude operands do not overflow
+- **WHEN** a program evaluates `1e308 % 1e-308` as `f64` on any executor
+- **THEN** the result is the exact finite fmod value in `[0, 1e-308)` — never infinity or NaN — identically on all three executors
+
+#### Scenario: Ordinary operands agree bit-for-bit
+- **WHEN** the same float remainder expression is evaluated on the interpreter, the wasm backend, and the native backend
+- **THEN** all three produce the identical bit pattern

openspec/changes/archive/2026-08-26-compiler-review-stability-fixes/specs/bootstrap-flow-functions/spec.md

@@ -0,0 +1,25 @@
+## ADDED Requirements
+
+### Requirement: Effect-block result typing accounts for every terminal
+
+An effect block's success and failure types SHALL be derived from every `return` and `fail` terminal reachable in the block, including terminals nested inside `unsafe` blocks. Return sites with differing types SHALL combine through the language's canonical result join — never by silently adopting one site's type: joinable types form their union, and a join with no representable form is reported as a diagnostic at the offending return. A `fail` whose failure type is a value-kind type parameter SHALL contribute that parameter to the block's failure row exactly as a nominal failure would.
+
+#### Scenario: Terminals inside unsafe blocks are collected
+- **WHEN** an effect block's only `fail` (or only `return`) sits inside an `unsafe { }` statement
+- **THEN** the block's failure row (or success type) includes it, and running the effect requires handling the failure
+
+#### Scenario: Disagreeing branch returns cannot pass silently
+- **WHEN** an effect block returns `bool` on one branch and `i32` on another inside a context expecting `Effect<i32>`
+- **THEN** the block types as the canonical join (`Effect<bool | i32>`) and the context rejects it with a type-mismatch diagnostic — the block is never typed from the lexically last return alone
+
+#### Scenario: Generic failures survive into the failure row
+- **WHEN** a generic function's effect block fails with a value of type parameter `E`
+- **THEN** the block types as an effect whose failure row contains `E`, and after specialization the concrete failure must be handled at `run`
+
+### Requirement: Effect-block captures include enum-value arguments
+
+Capture analysis for effect blocks SHALL register a capture for every binding referenced anywhere in the block body, including bindings referenced as the argument of an enum value construction.
+
+#### Scenario: Enum.value argument is captured
+- **WHEN** an effect block's body evaluates `Color.value(c)` for an outer binding `c`
+- **THEN** `c` appears in the effect's capture environment and the deferred runner reads the captured value

openspec/changes/archive/2026-08-26-compiler-review-stability-fixes/specs/bootstrap-integer-scalars/spec.md

@@ -0,0 +1,21 @@
+## ADDED Requirements
+
+### Requirement: Signed remainder overflow semantics are identical across executors
+
+Signed integer `%` with operands `MIN` and `-1` SHALL trap on every executor (interpreter, wasm, native), consistent with the existing rule that ordinary arithmetic traps on invalid division/remainder. The checked remainder of `MIN` and `-1` SHALL return `None` on every executor, and no executor SHALL evaluate it through an operation whose result is undefined for those operands.
+
+#### Scenario: Ordinary remainder of MIN by -1 traps everywhere
+- **WHEN** a program evaluates `i32::MIN % -1` (or the equivalent for any signed width) on any executor
+- **THEN** execution traps, and the same program traps identically on the interpreter, the wasm backend, and the native backend
+
+#### Scenario: Checked remainder of MIN by -1 is None everywhere
+- **WHEN** a program evaluates the checked remainder of `i32::MIN` and `-1` on any executor
+- **THEN** the result is `None`, identically on the interpreter, the wasm backend, and the native backend
+
+### Requirement: Rotate counts wrap modulo lane width on every executor
+
+Rotate-left and rotate-right SHALL interpret the count modulo the operand's bit width using an unsigned (Euclidean) reduction, so negative and out-of-range counts wrap instead of degenerating, identically on every executor.
+
+#### Scenario: Rotate by a negative count wraps
+- **WHEN** a program evaluates `rotate_left(x, -1)` on an odd `i32` value on any executor
+- **THEN** the result equals `rotate_left(x, 31)` — the low bit wraps into bit 31 — identically on the interpreter, the wasm backend, and the native backend

openspec/changes/archive/2026-08-26-compiler-review-stability-fixes/specs/bootstrap-ownership/spec.md

@@ -0,0 +1,9 @@
+## ADDED Requirements
+
+### Requirement: Loan live-ranges account for uses nested in place and effect expressions
+
+Loan-end analysis SHALL treat identifier and callable occurrences nested inside place-replace, effect-result, and requirement-binding expressions as uses at that occurrence: they SHALL extend the enclosing loan's live range and SHALL invalidate any earlier record that treated the callable's last invocation as its final use.
+
+#### Scenario: View used inside a place replace keeps its loan live
+- **WHEN** a shared view's last use sits inside a place-replace expression's value operand and the borrowed owner is mutated between the view's direct uses and that nested use
+- **THEN** ownership analysis reports owner access during the loan — the view loan's live range extends through the place-replace use rather than ending at the last direct use