fix(wasm): #1049 instances 2+3 — wrapForI64 non-i64 return handling#1063
Merged
Conversation
…n-i64 returns 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)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the remaining two instances of #1049 (PR #1056 closed instance 1):
Cannot convert 0 to a BigIntafter the editor mounts.TreeSitterEngine.parse(text, langId)FFI import throwing the same error from inside the WASM module, with all three args verifiably BigInt at the__classDispatchboundary.Both originated from the same root cause as instance 1 fix but in the opposite direction: PR #1056 made
wrapForI64/wrapFfiForI64coerce small-integerNumberreturns toBigInt. That fix was right for callees declaredi64-return on the WASM side (thehone_editor_createhandle path). But the rt namespace also includes:mem_call(declaredf64-return — the central memory-bridge dispatcher)mem_call_i32(declaredi32-return)t_*_i32signatures (is_truthy,js_strict_eq,class_instanceof,regexp_test,has_exception, …)For those, returning a
BigIntmakes the WASM caller throw the inverse error:TypeError: Cannot convert a BigInt value to a number. The trap fires on the very firstclass_set_methodmem_call inside_start, blowing up boot before any user code runs.Fix
__NON_I64_RETURN_IMPORTS— a hard-coded set of every rt import whose declared WASM return isi32,f64, or void. Built from the type tables incrates/perry-codegen-wasm/src/emit.rs.coerceNumberToBigIntparameter towrapForI64(defaulttrue, set tofalsefor entries in the set).wrapRtNamespaceProxy threads each property name into the wrapper so it can consult the set per-import.wrapFfiForI64is tightened to always routeNumberreturns through__jsValueToBits(NaN-boxes the value so downstream WASM code that reinterprets i64 → f64 sees the original Number — previouslyBigInt(7)returned raw7nwhich Perry mis-read as5e-324).console.log("mem_call: object_*", …)debug statements from the March WASM-target landing that have been spamming stdout on every property access.Test plan
/tmprepro from the task (Cursor class +moveBy(5)+desiredColumnfield) — boots cleanly, printscol=5 desiredColumn=5matchingnode --experimental-strip-types.tests/wasm/run_wasm_tests.sh— was 0/20 (every test crashed with the BigInt error at boot), now 14/20 + the new regression test = 14/21 passing. The 7 still-failing tests are pre-existing output-format issues (Object.keysincluding__class__, partial Map/Set/JSON.stringify, regex capture index, FFI handle encoding) that surface only now that boot succeeds; they were hidden under the boot crash.tests/wasm/21_class_method_i64— exercises class registration + i64 method dispatch + i32-return truthy check + cursor field re-read.test-files/test_issue_1049_class_method_i64.ts— verified byte-for-byte againstnode --experimental-strip-types.cargo fmt --allclean.cargo build --release -p perry-codegen-wasm -p perry-runtime -p perry-stdlib -p perryclean.No version bump or
CHANGELOG.mdentry per agent instructions; maintainer will fold those in at merge time.Closes #1049.