Skip to content

fix(wasm): #1049 instance 1 — wrapForI64 / wrapFfiForI64 must return BigInt#1056

Merged
proggeramlug merged 1 commit into
mainfrom
worktree-fix-1049-wrap-ffi-i64-bigint
May 19, 2026
Merged

fix(wasm): #1049 instance 1 — wrapForI64 / wrapFfiForI64 must return BigInt#1056
proggeramlug merged 1 commit into
mainfrom
worktree-fix-1049-wrap-ffi-i64-bigint

Conversation

@proggeramlug
Copy link
Copy Markdown
Contributor

Summary

Closes the first known instance from the #1049 tracking issue. Both wrapForI64 and wrapFfiForI64 special-cased small integer Number results and returned them unchanged. The wrappers exist precisely to bridge WASM i64 imports, where modern engines reject a plain Number with TypeError: Cannot convert X to a BigInt.

User-visible symptom in @honeide/editor: hone_editor_create returned handle 1 as Number; the WASM caller threw on the i64 boundary and the editor never mounted. Workaround was return BigInt(h) as any at every FFI consumer.

Fix: coerce integer Numbers via BigInt(n) in both wrappers. Non-integer Numbers (NaN-boxed f64 payloads) still bit-reinterpret to BigInt via the existing _u64[0] round-trip. The block comment on wrapForI64 already promised this behavior ("Number results are coerced back to BigInt for WASM i64 return") — code now agrees.

Test plan

  • cargo build --release -p perry-codegen-wasm — clean
  • cargo test --release -p perry-codegen-wasm — no regressions (no JS-side tests exist for the runtime; verified by static review and tracking-issue's described symptom)

Instances 2 and 3 from #1049 (internal class-method dispatch + TreeSitter parse arg shuffle) are separate codegen sites and remain open.

…BigInt

Both wrappers special-cased small integer Number results and returned
them unchanged. The wrappers exist precisely because the WASM import
sites declare `i64` returns — and modern engines reject a plain Number
return for an `i64` import with `TypeError: Cannot convert X to a BigInt`.

The user-visible symptom in @honeide/editor's boot path: `hone_editor_create`
returned handle 1 as Number 1; the WASM caller threw on the i64 boundary
and the editor never mounted. Workaround was per-FFI `return BigInt(h) as any`,
which forced every consumer to know the per-function ABI.

Coerce integer Numbers via `BigInt(n)` in both wrappers. Non-integer
Numbers (NaN-boxed f64 payloads) still bit-reinterpret to BigInt as before.
The comment block on `wrapForI64` already promised this behavior ("Number
results are coerced back to BigInt for WASM i64 return") — code now agrees.

#1049 is a tracking issue; this closes instance 1. Instances 2 and 3
(internal class-method dispatch + TreeSitter parse arg shuffle) remain
open as separate sites.
@proggeramlug proggeramlug merged commit d4d5e4e into main May 19, 2026
9 checks passed
@proggeramlug proggeramlug deleted the worktree-fix-1049-wrap-ffi-i64-bigint branch May 19, 2026 03:08
proggeramlug added a commit that referenced this pull request May 19, 2026
…n-i64 returns (#1063)

PR #1056 closed instance 1 by coercing small-integer Number returns to
BigInt in both `wrapForI64` and `wrapFfiForI64`. The fix was correct for
imports declared `i64`-return on the WASM side, but the wrapper applies
to EVERY function in the `rt` namespace — including `mem_call` (declared
`f64`-return), `mem_call_i32` (`i32`-return), and the 24+ predicate
imports declared `t_*_i32` (`is_truthy`, `js_strict_eq`, `class_instanceof`,
`regexp_test`, ...). For all of those, returning a BigInt from JS makes
WASM throw the inverse error at the import boundary:

  TypeError: Cannot convert a BigInt value to a number

Boot blew up on the very first `class_set_method` mem_call inside `_start`
— surfacing as #1049 instance 2's class-method dispatch and instance 3's
TreeSitter `parse(text, langId)` call, both of which trace back to the
same wrapper.

Fix: introduce `__NON_I64_RETURN_IMPORTS` listing every rt import whose
declared WASM return is `i32`, `f64`, or void, and thread a
`coerceNumberToBigInt` flag through `wrapForI64` via a new
`wrapRtNamespace` Proxy that consults the set per property. The wrapper
now skips the BigInt coercion (and the f64 bit-reinterpret) for those
imports, returning the raw Number which WASM accepts via `ToInt32` /
direct f64 passthrough.

`wrapFfiForI64` (FFI namespace) gets the same NaN-boxing tightened: it
now always routes Number returns through `__jsValueToBits`, which encodes
them as proper NaN-boxed f64 bits. The historic `BigInt(result)` path
returned raw `1n` for a handle, which Perry-internal code then mis-read
as `f64::from_bits(1)` = 5e-324 (instance 3's `add_numbers` → 7 surfacing
as 3.5e-323 in the FFI smoke test).

Drive-by cleanup: remove two debug `console.log("mem_call: object_*", ...)`
statements that have been leaking into stdout since v0.5 March (test
output and demo dev tools were full of `mem_call: object_get_dynamic ...`
spam on every property access). They were leftover from the
Firefox NaN-canonicalization investigation.

Regression coverage:
- tests/wasm/21_class_method_i64 — new entry in the WASM target suite;
  exercises class registration + i64 method dispatch + i32-return truthy
  check + cursor field re-read.
- test-files/test_issue_1049_class_method_i64.ts — same fixture under the
  native parity tree so the CLI and `--target web` outputs are pinned to
  match `node --experimental-strip-types` byte-for-byte.

Verification:
- tests/wasm suite goes from 0/20 PASSING (every test crashed with the
  BigInt/Number error at boot) to 14/20 PASSING. The 7 remaining
  failures are pre-existing output-format issues (Object.keys returning
  `__class__`, partial Map/Set/JSON.stringify support, regex .exec
  capture-index, etc.) that surface now that boot succeeds; they were
  hidden under the boot crash before.
- /tmp test repro (Cursor class + moveBy + desiredColumn) outputs
  `col=5 desiredColumn=5` matching Node.

Files touched:
  crates/perry-codegen-wasm/src/wasm_runtime.js   (wrapper + dispatch)
  tests/wasm/21_class_method_i64.ts               (new)
  tests/wasm/21_class_method_i64.expected         (new)
  test-files/test_issue_1049_class_method_i64.ts  (new)
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.

1 participant