Skip to content

fix: support Exn<String> via two-parameter WASM exception tag (#416)#452

Merged
aallan merged 4 commits into
mainfrom
fix/exn-string-i32pair-416
Apr 9, 2026
Merged

fix: support Exn<String> via two-parameter WASM exception tag (#416)#452
aallan merged 4 commits into
mainfrom
fix/exn-string-i32pair-416

Conversation

@aallan

@aallan aallan commented Apr 9, 2026

Copy link
Copy Markdown
Owner

Summary

  • Exn<String> previously produced invalid WAT: (tag $exn_String (param i32_pair))i32_pair is an internal representation, not a valid WASM value type
  • WASM exception tags natively support multiple parameters, so the fix stores "i32 i32" instead of "i32_pair" in _exn_types, yielding (tag $exn_String (param i32 i32)) — valid WAT
  • _translate_handle_exn detects pair types via _is_pair_type_name, allocates two consecutive locals (following the existing _translate_slot_ref convention of ptr at local_idx, len at local_idx + 1), emits (result i32 i32) for the catch block, and pops both values in LIFO order after catch
  • Also fixes result_spec when the handler body returns a String (_infer_block_result_type returns "i32_pair", which must expand to "i32 i32" in WAT block result annotations)

Files changed

  • vera/codegen/compilability.py — two code paths that populate _exn_types: now use "i32 i32" for i32_pair types
  • vera/wasm/calls.py_translate_handle_exn: pair-type detection, two consecutive locals, two local.set instructions, correct (result i32 i32) annotations
  • tests/conformance/ch07_exn_string.vera — new conformance test (level: run, expected: 10): catches a thrown String exception and verifies both the throw and non-throw paths

Test plan

  • vera run tests/conformance/ch07_exn_string.vera returns 10
  • pytest tests/ -v — all 3223 tests pass
  • python scripts/check_conformance.py — all 73 conformance programs pass
  • Pre-commit hooks pass (mypy, pytest, conformance, doc counts, site assets)

Closes #416

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added Google Gemini 2.5 Pro and DeepSeek V3/R1 as inference providers.
  • Tests

    • Conformance suite expanded from 72 to 73 programs; total tests updated to 3,227.
    • New end-to-end tests for string‑payload exception handling added.
  • Bug Fixes

    • Exception handling corrected to properly encode/decode string (pair‑encoded) payloads.
  • Documentation

    • Docs, FAQ and roadmap updated to reflect new counts and provider additions.

String is a pair type (ptr, len) represented as two consecutive i32
locals in the WASM ABI. Previously, Exn<String> was registered in
_exn_types with wasm type "i32_pair", causing the tag declaration to
emit `(tag $exn_String (param i32_pair))` — invalid WAT that fails
at the assembler stage.

Fix: when a type is i32_pair, store "i32 i32" as the WASM tag
parameter type in both compilability.py code paths (_check_exn_effect
and _scan_body_for_state_handlers). WASM exception tags support
multiple parameters natively, so `(tag $exn_String (param i32 i32))`
is valid and the catch instruction pushes both values onto the stack.

In _translate_handle_exn, detect pair types via _is_pair_type_name:
- Allocate two consecutive i32 locals (ptr + len) following the
  existing convention that _translate_slot_ref uses local_idx and
  local_idx + 1 for pair types
- Emit `(result i32 i32)` for the catch block and try_table
- Pop len first, then ptr (WASM stack is LIFO after catch)

Also fix result_spec: _infer_block_result_type returns "i32_pair" for
String-typed handler bodies, which must expand to "i32 i32" in the
WAT block result annotation.

Adds conformance test ch07_exn_string.vera (level: run, expected: 10):
throws a String exception and catches it, verifying both the throw
path (length 5) and the non-throw path (length 5), main returns 10.

Co-Authored-By: Claude <noreply@anthropic.invalid>
@coderabbitai

coderabbitai Bot commented Apr 9, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c2d7c303-df71-4c3b-baa1-135feb42c486

📥 Commits

Reviewing files that changed from the base of the PR and between 5211fb8 and 26f5636.

📒 Files selected for processing (3)
  • ROADMAP.md
  • TESTING.md
  • tests/test_codegen.py

📝 Walkthrough

Walkthrough

Adds support for pair-encoded exception payloads (e.g., Exn) by normalising WASM exception tags and updating handler translation, adds a new conformance manifest entry, extends end-to-end tests for string exceptions, and updates multiple docs and test counts to reflect 73 conformance programs.

Changes

Cohort / File(s) Summary
Documentation: conformance & test counts
AGENTS.md, CLAUDE.md, FAQ.md, SKILL.md, TESTING.md, ROADMAP.md
Incremented conformance suite size to 73 programs and updated related test counts and matrices; ROADMAP.md also adds DeepSeek V3/R1 and Google Gemini 2.5 Pro entries under inference hardening.
Conformance manifest
tests/conformance/manifest.json
Added ch07_exn_string entry (ch07_exn_string.vera) with level: "run" and features ["exn_handler","string_exception"].
Exception-type registration (codegen)
vera/codegen/compilability.py
Normalised WASM exception tag for Exn<E> when E maps to i32_pair; store (type_name, wasm_tag_t) and adjust duplicate detection to use the normalized tag.
Exception-handler translation (WASM)
vera/wasm/calls.py
_translate_handle_exn detects pair-encoded payloads, allocates two consecutive i32 locals (ptr, len), emits (result i32 i32) in try/catch result annotations, and sets locals in LIFO-consistent order (local.set len then local.set ptr).
Tests: exception handlers
tests/test_codegen.py
Added end-to-end tests for Exn<String> covering thrown string → handler numeric result, non-throwing body path, string-returning handler with WAT ABI assertions and IO.print, and empty-string payload handling.

Sequence Diagram(s)

sequenceDiagram
    participant Frontend as Front-end (parser/typechecker)
    participant Compilability as compilability.py
    participant WasmGen as wasm/calls.py
    participant TestRunner as test runner

    rect rgba(200,220,255,0.5)
    Frontend->>Compilability: identify handler type Exn<String>
    Compilability-->>Frontend: compute wasm_tag_t ("i32 i32"), record (type_name, wasm_tag_t)
    end

    rect rgba(200,255,200,0.5)
    Frontend->>WasmGen: request handler translation for Exn<String>
    WasmGen-->>WasmGen: allocate two i32 locals (ptr, len)
    WasmGen-->>WasmGen: set try_table result to (result i32 i32)
    WasmGen-->>Frontend: emit WAT with local.set order (len then ptr)
    end

    rect rgba(255,230,200,0.5)
    TestRunner->>Frontend: compile ch07_exn_string.vera
    Frontend->>WasmGen: generate module
    TestRunner->>WasmGen: run module, validate caught payload behaviour
    WasmGen-->>TestRunner: return runtime results/assertions
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested labels

compiler, tests, docs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the primary change: fixing support for Exn by using two-parameter WASM exception tags instead of invalid i32_pair.
Linked Issues check ✅ Passed The PR directly addresses issue #416 by implementing the two-scalar representation for pair types in exception tags and handler code generation, enabling Exn to be thrown and caught.
Out of Scope Changes check ✅ Passed All changes are scoped to implementing Exn support or documenting conformance suite updates; no unrelated code changes detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/exn-string-i32pair-416

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Apr 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.32%. Comparing base (fc04d1f) to head (26f5636).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #452      +/-   ##
==========================================
+ Coverage   90.31%   90.32%   +0.01%     
==========================================
  Files          50       50              
  Lines       19406    19419      +13     
  Branches      220      220              
==========================================
+ Hits        17527    17541      +14     
+ Misses       1875     1874       -1     
  Partials        4        4              
Flag Coverage Δ
javascript 50.56% <ø> (ø)
python 95.25% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ROADMAP.md`:
- Line 11: Update the stale test-count mention in ROADMAP.md by replacing the
outdated "3,218 tests" reference with the current "3,223 tests" so the in-file
counts match the project summary; search for the literal "3,218 tests" and
change it to "3,223 tests" to keep the roadmap consistent.

In `@tests/conformance/manifest.json`:
- Around line 592-604: The spec_ref for the manifest entry with id
"ch07_exn_string" is incorrect (it currently reads "§7.2 Exn<E> handler — String
payload"); update the spec_ref field for "ch07_exn_string" to match the existing
mapping for the Exn handler (use "§7.4 Exn<E> handler — String payload") so it
is consistent with "ch07_exn_handler" and other exception-handler entries.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1fa54539-985b-461e-921f-5c353872e015

📥 Commits

Reviewing files that changed from the base of the PR and between fc04d1f and eb12b74.

⛔ Files ignored due to path filters (5)
  • docs/SKILL.md is excluded by !docs/**
  • docs/llms-full.txt is excluded by !docs/**
  • docs/llms.txt is excluded by !docs/**
  • docs/sitemap.xml is excluded by !docs/**
  • tests/conformance/ch07_exn_string.vera is excluded by !**/*.vera
📒 Files selected for processing (9)
  • AGENTS.md
  • CLAUDE.md
  • FAQ.md
  • ROADMAP.md
  • SKILL.md
  • TESTING.md
  • tests/conformance/manifest.json
  • vera/codegen/compilability.py
  • vera/wasm/calls.py

Comment thread ROADMAP.md Outdated
Comment thread tests/conformance/manifest.json
Coverage: the conformance test (ch07_exn_string.vera) exercises the
Exn<String> code path via subprocess, which coverage does not track.
Add three in-process tests to TestExnHandlers that cover the new
is_pair=True branch in _translate_handle_exn and the new wasm_tag_t
lines in compilability.py:
- test_exn_string_throw_caught: throw+catch with Int result
- test_exn_string_no_throw: non-throwing path through String handler
- test_exn_string_handler_returns_string: handler clause returns String
  (covers result_wt == "i32_pair" → result_spec = "(result i32 i32)")

Mark two genuinely defensive guards as pragma: no cover:
- handler_instrs is None (requires unsupported handler body syntax)
- result_spec = "" (result_wt is None; unreachable from valid Vera code)

CodeRabbit fixes:
- ROADMAP.md line 182: update stale "3,218 tests" → "3,223 tests"
  (missed in doc-count sweep; was in mutation-testing description)
- manifest.json ch07_exn_string spec_ref: §7.2 → §7.4 to match
  ch07_exn_handler and reflect the correct spec section

Co-Authored-By: Claude <noreply@anthropic.invalid>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (2)
tests/conformance/manifest.json (1)

598-598: ⚠️ Potential issue | 🟡 Minor

Correct the spec_ref format to match the existing convention.

The spec_ref field uses "§7.4 Exn<E> handler — String payload", but the existing ch07_exn_handler entry at line 585 uses "Section 7.4". The manifest convention throughout uses "Section X.Y" format rather than "§X.Y".

📝 Proposed fix
-    "spec_ref": "§7.4 Exn<E> handler — String payload",
+    "spec_ref": "Section 7.4",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/conformance/manifest.json` at line 598, Update the spec_ref value that
currently reads "§7.4 Exn<E> handler — String payload" in the manifest entry so
it matches the project's convention; replace the leading "§7.4" with "Section
7.4" (i.e., make it "Section 7.4 Exn<E> handler — String payload") to be
consistent with the existing ch07_exn_handler style.
ROADMAP.md (1)

224-224: ⚠️ Potential issue | 🟡 Minor

Synchronise the final test-count summary with the updated totals.

Line 224 still says 3,223 tests, but Line 11 and Line 182 now state 3,226 tests. Please update the closing summary to avoid in-file metric drift.

As per coding guidelines, “**/*.md: Review Markdown files for factual accuracy against the codebase, broken links, and outdated information.”

Suggested patch
-**630+ commits, 108 tagged releases, 3,223 tests, 96% coverage, 73 conformance programs, 30 examples, 13 spec chapters.** See [HISTORY.md](HISTORY.md) for the full narrative of how the compiler was built.
+**630+ commits, 108 tagged releases, 3,226 tests, 96% coverage, 73 conformance programs, 30 examples, 13 spec chapters.** See [HISTORY.md](HISTORY.md) for the full narrative of how the compiler was built.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ROADMAP.md` at line 224, Update the final summary string in ROADMAP.md that
currently reads "**3,223 tests**" (the instance in the closing summary
paragraph) to the correct updated total "**3,226 tests**" so it matches the
other occurrences on Lines 11 and 182; locate the exact phrase "**630+ commits,
108 tagged releases, 3,223 tests, 96% coverage, 73 conformance programs, 30
examples, 13 spec chapters.**" and replace only the test-count token to preserve
the rest of the summary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/conformance/manifest.json`:
- Line 603: Remove the unsupported "expected" field from the manifest entry
(delete the "expected": 10 line) so the manifest only contains recognized keys;
if you actually intend to enforce return-value checks, implement that in the
test harness (the conformance checker) rather than adding an "expected" metadata
field.

In `@tests/test_codegen.py`:
- Around line 3106-3160: Add a new unit test in tests/test_codegen.py that
exercises the empty-string Exn<String> case by invoking throw("") and asserting
behavior matches the other Exn<String> tests; mirror the patterns of
test_exn_string_throw_caught, test_exn_string_no_throw or
test_exn_string_handler_returns_string but use throw("") to cover the
zero-length payload branch and verify the runtime handles the ptr/len pair (len
== 0) correctly (assert expected return value or printed string as appropriate).

---

Duplicate comments:
In `@ROADMAP.md`:
- Line 224: Update the final summary string in ROADMAP.md that currently reads
"**3,223 tests**" (the instance in the closing summary paragraph) to the correct
updated total "**3,226 tests**" so it matches the other occurrences on Lines 11
and 182; locate the exact phrase "**630+ commits, 108 tagged releases, 3,223
tests, 96% coverage, 73 conformance programs, 30 examples, 13 spec chapters.**"
and replace only the test-count token to preserve the rest of the summary.

In `@tests/conformance/manifest.json`:
- Line 598: Update the spec_ref value that currently reads "§7.4 Exn<E> handler
— String payload" in the manifest entry so it matches the project's convention;
replace the leading "§7.4" with "Section 7.4" (i.e., make it "Section 7.4 Exn<E>
handler — String payload") to be consistent with the existing ch07_exn_handler
style.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3cb66ba4-b785-4281-8c5c-469b69498d1c

📥 Commits

Reviewing files that changed from the base of the PR and between eb12b74 and 7bdbc34.

📒 Files selected for processing (5)
  • ROADMAP.md
  • TESTING.md
  • tests/conformance/manifest.json
  • tests/test_codegen.py
  • vera/wasm/calls.py

Comment thread tests/conformance/manifest.json Outdated
Comment thread tests/test_codegen.py
CodeRabbit fixes:
- manifest.json ch07_exn_string: remove unsupported "expected" key
  (test harness only checks exit code 0, never reads expected value);
  fix spec_ref from "§7.4 ..." to "Section 7.4 ..." to match project
  convention used by all other manifest entries
- ROADMAP.md line 224: stale "3,223 tests" → "3,226 tests" (the closing
  summary paragraph was missed in the previous count sweep)

New test: test_exn_string_empty_payload — verifies that throw("") passes
a zero-length ptr/len pair correctly through the (param i32 i32) tag and
that string_length of the caught value is 0; confirms the LIFO local.set
sequence works when len == 0.

Co-Authored-By: Claude <noreply@anthropic.invalid>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ROADMAP.md`:
- Line 224: Update the inconsistent test count in the ROADMAP entry that
currently reads "**630+ commits, 108 tagged releases, 3,226 tests, 96% coverage,
73 conformance programs, 30 examples, 13 spec chapters.**" to match the rest of
the file and other occurrences ("3,227 tests"); locate and replace the "3,226
tests" token with "3,227 tests" so all instances are consistent with lines 11
and 182.

In `@tests/test_codegen.py`:
- Around line 3142-3160: Update the test_exn_string_handler_returns_string test
to include an ABI-level WAT assertion that pins the pair-tag encoding: after
compiling the source used in that test (same src and fn="test"), obtain the
emitted WAT (via the existing test helper used in this suite or by adding a
small helper similar to _run_io that returns WAT) and assert the WAT contains
the expected tag/result signature (e.g., the substring "result i32 i32" or the
exact pair-tag encoding your ABI uses). Keep the runtime assertion (assert
_run_io(src, fn="test") == "caught") and add only the additional WAT-level
assert so the test both verifies behavior and pins the codegen shape for
handle[Exn<String>] / result i32 i32.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c73cc79e-4e40-4260-95d8-01af97f22b5e

📥 Commits

Reviewing files that changed from the base of the PR and between 7bdbc34 and 5211fb8.

📒 Files selected for processing (4)
  • ROADMAP.md
  • TESTING.md
  • tests/conformance/manifest.json
  • tests/test_codegen.py

Comment thread ROADMAP.md Outdated
Comment thread tests/test_codegen.py
CodeRabbit fixes:
- ROADMAP.md line 224: "3,226 tests" → "3,227 tests" — the previous sed
  pattern matched "3,226 tests, 73 conformance" but line 224 has
  "3,226 tests, 96% coverage, 73 conformance" so it was not updated
- test_exn_string_handler_returns_string: add WAT-level assertions that
  pin the ABI encoding for Exn<String>:
    assert "(tag $exn_String (param i32 i32))" in result.wat
    assert "result i32 i32" in result.wat
  This guards against future refactors that might silently change the
  tag encoding while still producing the correct runtime result; keeps
  the existing _run_io behavioral assertion alongside the structural check

Co-Authored-By: Claude <noreply@anthropic.invalid>
@aallan aallan merged commit b2cf06e into main Apr 9, 2026
19 checks passed
@aallan aallan deleted the fix/exn-string-i32pair-416 branch April 9, 2026 17:54
aallan added a commit that referenced this pull request Apr 9, 2026
Remove #416 from KNOWN_ISSUES.md (fixed in PR #452).
Add 9 Apr bug-fix row to HISTORY.md Stage 10 table.

Co-Authored-By: Claude <noreply@anthropic.invalid>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Exn<String> handler type tag fails: String maps to i32_pair, not a valid WASM tag parameter

1 participant