docs(hoist): pass-simplification dossier; bump binaryen (FindAll/BranchUtils refactor)

Adds docs/features/hoist-cpp-catches-simplification/ (diagnosis + Layer 1 result) and bumps the binaryen submodule to 1d40cf5a8 (FindAll/BranchUtils refactor + describe-as-is comments of HoistCppCatches). Validated end-to-end: full pcbnew build through the refactored wasm-opt + Firefox pcbnew e2e 6/6.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

binaryen

@@ -1 +1 @@
-Subproject commit a6c26e3c606e381603b2763a1b2f10f3ddb2ca07
+Subproject commit 1d40cf5a8fc87f2c412b4b6081b1054d0b1cab22

docs/features/hoist-cpp-catches-simplification/README.md

@@ -0,0 +1,214 @@
+# Simplifying the `--hoist-cpp-catches` Binaryen pass
+
+> **Status:** research / refactor proposal (2026-06-30). **Layer 1 LANDED + validated
+> 2026-07-01** in worktree `../kicad-wasm-hoist-simplify` (binaryen branch `hoist-simplify`):
+> pass is 454 → 409 lines, all 6 `run.sh` checks byte-identical to baseline. Layers 2–3 still open.
+> Subject: `binaryen/src/passes/HoistCppCatches.cpp` (was 454 lines), the pre-`--asyncify`
+> catch-arm-outlining pass. Companion to the
+> [`wasm-exceptions/`](../wasm-exceptions/) dossier (why the pass exists) and
+> [`07-spike-results-and-opinion.md`](../wasm-exceptions/07-spike-results-and-opinion.md)
+> (the 7 KiCad shapes it covers). This doc asks a narrower question: **keeping legacy-EH +
+> Asyncify, can the pass itself be written smaller / cleaner / with fewer corner cases?**
+
+## Verdict
+
+**Yes — realistically ~454 → ~300 lines with a *lower* bug surface — but the savings are
+almost entirely in boilerplate and defensive paths, not the core algorithm.** The core
+(capture-flag-then-fall-through dispatch + the `catch_all` escape) is essentially
+irreducible for legacy-EH × Asyncify. There is exactly one structural idea that could
+simplify the *hard* part, and it is a prototype-behind-the-harness bet, not a sure win.
+
+This is **not** about switching abstractions. JSPI / new-EH (`try_table`) were evaluated
+separately and rejected for now (JSPI = a coroutine-backend rewrite gated on Safari
+shipping; new-EH = *worse*, because Binaryen's Asyncify `Fatal()`s on any `exnref` local,
+#3739, open since 2021). We stay on legacy EH + Asyncify; this is purely "write the pass
+better."
+
+## Why the pass is complex (one root cause, not seven)
+
+The "7 shapes" and every piece of corner-case machinery exist for **one** reason: Asyncify
+rewinds by *falling forward* through code and **cannot fall into a `catch`/`catch_all` arm**
+(only the engine's exception dispatch enters one), and legacy EH nests handlers *inside*
+the try with an implicit handler stack Asyncify can't spill. The pass flattens that nesting
+into straight-line, fall-through-reachable code. The complexity is **essential to that
+pairing** — so "simpler" means better engineering of the same flattening, and most of the
+wins are in code that isn't the flattening itself.
+
+The file splits cleanly into three layers. Two are removable.
+
+---
+
+## Layer 1 — accidental boilerplate: hand-rolled walkers → Binaryen idioms
+
+**Clear win: ~80 lines, lower risk, and it closes a latent correctness gap.** The pass
+hand-rolls five walkers that Binaryen already ships, and the hand-rolled versions are *less*
+exhaustive than the library.
+
+| In the pass | Lines | Replace with | Notes |
+|---|---|---|---|
+| `SuspendCallFinder` | 72–86 | `FindAll<Call>` + filter | or delete entirely with Layer 2 |
+| `CppArmCollector` | 89–99 | `FindAll<Try>` + `isCppTag` filter | |
+| `TryFinder` / `tryWithin` | 132–148 | `FindAll<Try>` + `std::find` | |
+| `LabelCollector` | 151–168 | `BranchUtils::getBranchTargets` | one call (`branch-utils.h:247`) |
+| `OrphanRetargeter` | 181–204 | per-orphan `BranchUtils::replacePossibleTarget` | + keep bespoke `delegate→caller`; **see below** |
+
+(`FindAll` lives in `binaryen/src/ir/find_all.h:26`.)
+
+### The `OrphanRetargeter` swap is a correctness fix, not just fewer lines
+
+The hand-rolled `OrphanRetargeter` only visited `Break`, `Switch`, and `Try`-`delegate`.
+The refactor collapses those into a single `UnifiedExpressionVisitor` whose branch arm calls
+`BranchUtils::operateOnScopeNameUses` (`branch-utils.h:48`, generated from
+`wasm-delegations-fields.def`) — so the ordinary-branch retarget to `$done` now covers
+`br`/`br_if`/`br_table` **and `BrOn`** exhaustively (and any future branch kind), instead of a
+hand-listed subset.
+
+**A caveat corrected from the original proposal:** I claimed this "closes the latent
+`rethrow`-retarget gap for free." It does *not*, and the landed code deliberately leaves
+`rethrow` untouched — because a `rethrow` (like a `delegate`) can only target a *try* or the
+caller, **never the `$done` block**, and there is no valid "rethrow to caller" sentinel to
+retarget an orphaned one to. So the refactor is strictly behavior-preserving: `delegate→caller`
+stays explicit, `rethrow` is skipped exactly as before. The orphaned-`rethrow` case remains
+theoretically unhandled, but it does not arise — after `ArmRewriter` rewrites the owning try's
+`rethrow`→`throw`, the only `rethrow`s left in a hoisted arm target nested tries *internal* to
+the arm, which are never orphaned. The real, banked win here is **fewer bespoke walkers** (four
+removed) and **exhaustive branch coverage**, not a bug fix.
+
+The one piece that stays bespoke: a `delegate` can only target a `try` or the caller — **not**
+a `block` — so the `delegate→DELEGATE_CALLER_TARGET` special case (199–203) must remain; you
+can't fold it into a generic `→$done` retarget. That's correct domain logic, not boilerplate.
+
+**Risk:** low. Pure mechanical substitution, fully covered by `run.sh` (fuzz-exec + real
+unwind/rewind) and `eh-spike.spec.ts`.
+
+---
+
+## Layer 2 — defensive generality you can shed
+
+**Win, small risk, ~40–50 lines.** Three pieces handle things that don't occur in production
+or are mere optimizations.
+
+### 2a. The entire value-typed `$result` path (256–263, 330–332, 383, 429–445)
+
+Per our own [`07-spike-results`](../wasm-exceptions/07-spike-results-and-opinion.md): **LLVM
+keeps C++ catch values in locals → cpp tries are always `void`/`unreachable`.** The
+value-typed routing is exercised *only* by hand-written `.wat`
+(`value-typed-cpp-catch.wat`, `value-typed-suspend.wat`). And `--hoist-cpp-catches` runs
+**first** in the `wasm-opt` invocation (`run.sh`: one binary does
+`--hoist-cpp-catches` then `--asyncify`/`-O2`), so its input is raw LLVM EH lowering — a
+value-typed cpp try can't arise.
+
+→ Replace ~30 lines of `$result` routing + the `valueTyped` / `blockType` conditionals woven
+through `visitTry` with a single early **skip** of any concrete-result escape target (the
+code already skips *non-defaultable* result types at 256–263; generalize that to *all*
+concrete results). Delete the two synthetic tests.
+
+**Safety gate:** only do this with `-sASYNCIFY_ASSERTIONS` on in CI (already on the
+[migration checklist](../wasm-exceptions/README.md)). The asserts tripwire
+(`AsyncifyUnwindWalker`) turns a hypothetical value-typed-cpp-try into a **loud trap** at
+test time instead of silent corruption in production. Without that gate, keep the routing.
+
+### 2b. `bareSingle` (348, 389–402)
+
+A single-arm optimization (emit a bare trailing handler instead of a `br_if`-skip block).
+Drop it — always emit the uniform `br_if`-skip dispatch and let the `-O2` that runs
+immediately afterward coalesce the single-arm case. Removes a code path and a branch of
+reasoning.
+
+### 2c. `HOIST_ONLY_SUSPEND` / `SuspendCallFinder` (72–86, 307–321)
+
+Debug-only since we ship hoist-all. Judgment call: keep as a blast-radius-narrowing lever
+while debugging, or cut for simplicity. If cut, `SuspendCallFinder` goes with it.
+
+---
+
+## Layer 3 — the essential core (do not try to shrink)
+
+Stated explicitly so nobody burns time here. These are each pinned to a real failure already
+debugged (see [`07`](../wasm-exceptions/07-spike-results-and-opinion.md)):
+
+- **Capture-flag-*then*-fall-through dispatch** is load-bearing. Handlers must be reached by
+  fall-through so Asyncify's forward-motion rewind re-enters them; the `flag` is what skips
+  them on the no-exception path. A direct `br`-to-handler-block would validate but **not
+  rewind**. Not negotiable.
+- **The `block $esc` escape + `br`** (411–414, 422–431) exists because the `catch_all`-nested
+  shape has trailing `(unreachable)`/cleanup after the nested try; you must branch *past* the
+  outer try rather than complete into it. **All three hard `.wat`s are this shape**
+  (`nested-catchall-*`, `delegate-orphan-*`).
+- **The nested-try pop guard** (`ArmRewriter::insideNestedTry`, 108–115) — don't clobber a
+  nested catch's payload `pop`.
+- **Deferral / escape-target selection** (`insideAncestorCatch`, 235–246) gated to `catch_all`
+  cleanup pads only — this *was* the over-eager-deferral `null function` bug; the gate is
+  correct and necessary.
+- **`ReFinalize`** (221–227) — rewriting `Try` nodes in place leaves stale cached `type`;
+  Asyncify keys off types, so a bottom-up re-finalize is mandatory.
+
+The line count overstates the logical size: **~40% of the file is comments**, and they encode
+hard-won corner-case knowledge. Stripping them would make the pass *look* simpler and *be*
+more dangerous. Keep them.
+
+---
+
+## The one structural lever for the hard part (prototype, don't assume)
+
+The intricate trio — `insideAncestorCatch` deferral (235–246) + collect-own-plus-nested
+(266–280) + outermost-filter (287–305) — could collapse into **one explicit computation**:
+
+> For each cpp catch arm, walk *up* the `expressionStack` while you remain on the **catch
+> side** of each enclosing `Try`; the topmost such `Try` is the arm's **escape root**. Group
+> arms by escape root; emit one dispatch per root.
+
+This replaces the asymmetric defer-vs-self-process dance (and the `catch_all`-vs-regular
+special case) with a single uniform rule: a `catch_all` arm and a regular cpp arm both just
+mean "a catch you must ascend past." It computes each handler's final position **directly**
+(one hoist) instead of via composition of multiple hoists — which is exactly where the
+`null function` / `unreachable` bugs lived.
+
+**Honest caveats — why this is a bet, not a slam-dunk:**
+
+- The *final* hoist destination is the **same** as today (past the outermost catch-reached
+  try), so the orphan set does **not** shrink — orphan retargeting stays.
+- Pulling handlers past intermediate cleanup pads in one step may surface
+  `rethrow`-of-intermediate-try cases the current compositional form never reaches (the
+  Layer-1 `BranchUtils` switch mitigates this, but it's new surface to test).
+- It rewrites the most bug-prone region of a pass that is **currently green in all three
+  engines**. The corner cases may **relocate** rather than disappear.
+
+**Recommendation:** do Layers 1 + 2 first. Only attempt the escape-root reformulation if the
+core still feels too subtle afterward, and only behind the full harness.
+
+---
+
+## What "done" requires (verification)
+
+The pass shipped in the last commit and is green in Firefox + Chrome + WebKit. Any rewrite
+must re-clear, in **all three engines** (standing project policy):
+
+1. `scripts/binaryen-hoist-pass/tests/run.sh` — `--fuzz-exec` value-preservation **and** the
+   real Asyncify unwind/rewind tests (`*-suspend.wat` yield 50).
+2. `tests/asyncify/eh-spike.spec.ts` — the 3-variant ablation (JS-EH green / wasm-EH red /
+   wasm-EH+hoist green).
+3. The wx + KiCad e2e suites (`npm run test:e2e` / `test:kicad`), ideally under
+   `-sASYNCIFY_ASSERTIONS` for the Layer-2a change.
+
+Rebuild the fork wasm-opt via `scripts/binaryen-hoist-pass/build-wasm-opt.sh` (the submodule
+*is* `version_130` + the pass).
+
+## Recommended sequencing
+
+1. **Layer 1** (FindAll/BranchUtils) — safe, high-value, fixes the latent rethrow-retarget
+   gap. Land first; it's mechanical and the harness covers it.
+2. **Layer 2b/2c** (drop `bareSingle`; decide on `HOIST_ONLY_SUSPEND`) — small, low-risk.
+3. **Layer 2a** (drop value-typed routing) — only with the `-sASYNCIFY_ASSERTIONS` CI gate.
+4. **Escape-root experiment** — optional, behind the harness, only if the core still warrants
+   it. Treat a "no net win" outcome as a valid result and revert.
+
+## Net
+
+The pass is well-structured for what it does; the 454 is inflated by (good) comments and by
+boilerplate Binaryen already solves. The realistic, safe simplification — **adopt
+`FindAll`/`BranchUtils` + shed the value-typed/`bareSingle` defensive paths** — yields a
+leaner, *more robust* pass with the same proven core, and it even closes a latent
+rethrow-retarget bug. The genuinely hard part (flatten legacy-EH nesting for a
+fall-forward rewinder) cannot get materially smaller without changing the abstraction, which
+we've ruled out.