feat(codegen): #248 Phase 2B — JsCreateCallback closure marshaling#254
Merged
Conversation
…0.5.369)
Wires Perry closures passed to JS-imported functions end-to-end so
`arr.forEach(cb)` / `gameLoop.use(handler)` actually fire. Phase 2A
(v0.5.368) bailed at codegen for JsCreateCallback because the runtime
FFI `js_create_callback(func_ptr, closure_env, param_count)` expects
`func_ptr` to have signature `(closure_env: i64, args_ptr: *const f64,
args_len: i64) -> f64` (the V8 native_callback_trampoline contract),
but Perry closure bodies have `(closure_ptr, arg0, arg1, ...)` per
arity — no direct call-compatible mapping.
Three pieces:
(1) New runtime helper `js_closure_call_array` in
crates/perry-runtime/src/closure.rs that takes (closure_env: i64,
args_ptr: *const f64, args_len: i64) and dispatches to the right
js_closure_callN per args_len. Mirrors the existing js_native_call_value
but with i64 first-arg so SysV-x64 / Win64 register lands correctly.
Body switches on args_len (0..16) and unboxes any INT32_TAG-tagged
f64 to a plain double before passing — V8 NaN-boxes JS integers with
INT32_TAG=0x7FFE, but Perry's closure-body arithmetic uses raw
fadd/fsub/fmul on f64s and assumes plain doubles.
(2) JsCreateCallback codegen arm in crates/perry-codegen/src/expr.rs
now lowers to: lower closure expr, unbox to i64 closure pointer,
ptrtoint @js_closure_call_array to i64, call js_create_callback with
(func_addr, closure_ptr, param_count). Returns NaN-boxed JS handle.
(3) Closure-collector in crates/perry-codegen/src/collectors.rs adds
8 new arms for the Js* HIR family so closures nested inside JS-interop
expressions don't fall through `_ => {}` and end up referenced via
js_closure_alloc with their bodies never defined.
New regression test test_issue_248_phase2b_js_callback.ts + fixture
issue_248_phase2b_jsmod.js exercises 0/1/2/3-arg callbacks with mutable
captures plus same-callback-fired-twice. Uses a method-call dispatcher
so js_call_method forces perry-jsruntime to win Mach-O link resolution
and V8 actually runs end-to-end.
Known follow-up: callbacks reading fields from JS-supplied object args
trigger a pre-existing perry-jsruntime re-entrancy panic ('RefCell
already borrowed') — V8 holds the JsRuntimeState borrow while
invoking the Perry callback, and the callback's js_get_property
re-enters with_runtime on the same borrow. Orthogonal to #248's
codegen scope.
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 last gap in #248: Perry closures passed to JS-imported functions now actually fire end-to-end. Phase 2A (PR #251) bailed at codegen for `JsCreateCallback` with a clear error pointing here.
The mismatch: the runtime FFI `js_create_callback(func_ptr, closure_env, param_count)` expects `func_ptr` to have signature `(closure_env: i64, args_ptr: *const f64, args_len: i64) -> f64` (perry-jsruntime's `native_callback_trampoline` at `crates/perry-jsruntime/src/interop.rs:993`), but Perry closure bodies have `(closure_ptr, arg0, arg1, ...)` per arity — no direct call-compatible mapping.
Three pieces
1. Runtime bridge `js_closure_call_array` (
crates/perry-runtime/src/closure.rs): takes `(closure_env: i64, args_ptr: *const f64, args_len: i64)` and dispatches to the right `js_closure_callN` per `args_len`. Mirrors the existing `js_native_call_value` but with i64 first-arg so SysV-x64 / Win64 register lands in the integer slot (rdi/rcx) rather than xmm0. Body switches on `args_len` (0..16) and unboxes any `INT32_TAG=0x7FFE`-tagged f64 to a plain double before passing.2. JsCreateCallback codegen (`crates/perry-codegen/src/expr.rs`): lowers the closure expression, unboxes to i64 closure pointer, `ptrtoint @js_closure_call_array to i64`, calls `js_create_callback(func_addr, closure_ptr, param_count)`. Returns a NaN-boxed JS handle (V8-handle tag 0x7FFB) that JS code can call like any other JS function.
3. Closure-collector (`crates/perry-codegen/src/collectors.rs`): 8 new arms for the `Js*` HIR family so closures nested inside JS-interop expressions don't fall through the `_ => {}` catch-all and end up referenced via `js_closure_alloc(@perry_closure_*)` with their bodies never defined (clang error: "use of undefined value" — same class as Tier 1.1 in v0.5.329).
Int32 unbox trap
Pre-fix, `cb(10, 20)` from JS into a Perry closure `(a, b) => a + b` returned 10 instead of 30. V8 NaN-boxes JS integers with `INT32_TAG=0x7FFE` (perry-jsruntime's `bridge.rs:215`), but Perry's closure-body arithmetic uses raw `fadd` / `fsub` / `fmul` on f64 inputs and assumes plain doubles — `fadd(10.0, NaN-boxed-int32-20)` produces a NaN whose payload `console.log`'s tag-aware unbox decoded as 10. Unbox at the dispatch boundary fixes it without touching shared bridge code.
Build dependency note
Rebuilding perry-jsruntime is necessary after a perry-runtime change — Cargo bundles the perry-runtime CGU inside `libperry_jsruntime.a` for link-time symbol resolution. Without the rebuild, Mach-O first-wins picks the bundled (stale) CGU and the runtime change doesn't take effect.
Test plan
Known follow-up
Callbacks reading fields from JS-supplied object args (e.g. `cb((ctx) => log(ctx.deltaTime))`) trigger a pre-existing perry-jsruntime re-entrancy panic (`RefCell already borrowed` at `crates/perry-jsruntime/src/lib.rs:123`) — V8's trampoline holds the `JsRuntimeState` borrow while invoking the Perry callback, and the callback's `js_get_property` re-enters `with_runtime` on the same borrow. Orthogonal to #248's codegen scope.