fix(codegen): #248 ArrayPushSpread + V8 interop codegen arms#251
Merged
Conversation
….366) `arr.push(...src)` was rejected with "expression ArrayPushSpread not yet supported". HIR has lowered the variant for some time; only the codegen arm in crates/perry-codegen/src/expr.rs was missing. Mirrors the existing Expr::ArrayPush arm at line 3016 — same three receiver-storage cases (LocalGet, boxed-captured, plain), same realloc-aware writeback. Reuses the existing js_array_concat runtime helper (already declared in runtime_decls.rs:189; comment-as-spec'd "reserved for the internal push-spread desugaring path"). Set sources work transparently via the SET_REGISTRY check inside js_array_concat. New regression test test_issue_248_array_push_spread.ts covers 10 cases (number/string/object arrays, empty src/dst, array-literal spread, chained push-spread, post-spread .indexOf/.length, push-spread inside a loop that grows past initial 16-cap, mixed push+push-spread) — all byte-for-byte against `node --experimental-strip-types`. Phase 2 of #248 (JsCallFunction/JsCallMethod/JsLoadModule family for the LLVM backend's JS-runtime interop, surfacing when a .ts entry imports from a node_modules/<pkg>/index.js) is intentionally separate.
…ms (v0.5.368)
Adds 8 codegen arms in crates/perry-codegen/src/expr.rs for the Js*
HIR family that perry-hir/src/js_transform.rs::transform_js_imports
produces whenever a `.ts` entry imports from a `.js` module the
resolver classified as JS-runtime-loaded. Pre-fix the LLVM backend
bailed at the catch-all `bail!("Phase 2: expression {} not yet
supported")` for JsLoadModule, JsGetExport, JsCallFunction,
JsCallMethod, JsGetProperty, JsSetProperty, JsNew, JsNewFromHandle.
The new arms call into the existing perry-jsruntime/src/interop.rs
FFI surface; all eight FFIs were already declared in runtime_decls.rs
except js_call_method (added here). New shared helper
lower_js_args_array marshals already-lowered NaN-boxed args into a
stack alloca'd `[N x double]` via the issue-#167 alloca_entry_array
pattern; empty input returns ("null", "0") for the FFI's null fallback.
Module handle representation: V8 module ids are u64; codegen returns
them as f64 via bitcast_i64_to_double to fit lower_expr's return-type
contract, then consumers bitcast back to i64 before the FFI call. JS
value handles are NaN-boxed f64 with V8-handle tag 0x7FFB — handled
internally by perry-jsruntime's v8_to_native / native_to_v8 helpers.
Runtime bootstrap: new `needs_js_runtime: bool` field on
CrossModuleCtx, threaded from CompileOptions, wired into
compile_module_entry so the entry main's prelude calls js_runtime_init()
between js_gc_init and user code. Without this, every js_load_module
site bailed at runtime with `[js_load_module] no JS runtime state!`.
JsCreateCallback deliberately deferred to Phase 2B: the runtime FFI
js_create_callback expects func_ptr to have signature
(closure_env: i64, args_ptr: *const f64, args_len: i64) -> f64, but
Perry closure bodies have (closure_ptr, arg0, arg1, ...) per arity —
no direct call-compatible mapping. Wiring this needs either
codegen-emitted per-arity adapter thunks or a runtime-side
js_closure_call_array dispatcher. For now the arm bails with a clear
message so users see exactly what's blocked.
New regression test test-files/test_issue_248_phase2_js_interop.ts +
fixture test-files/fixtures/issue_248_jsmod.js exercises JsLoadModule
+ JsCallFunction. The user's exact pipeline() repro from
/tmp/issue248/test.ts now compiles + links + runs.
…ray-push-spread # Conflicts: # CLAUDE.md # Cargo.lock # Cargo.toml
This was referenced Apr 28, 2026
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.
Closes #248 — both halves.
Phase 1 (commit 8e512d8):
ArrayPushSpreadarr.push(...src)was rejected withexpression ArrayPushSpread not yet supported. HIR has lowered the variant since v0.5.x (crates/perry-hir/src/lower/expr_call.rs:2077); only the codegen arm incrates/perry-codegen/src/expr.rswas missing. The WASM backend's emit-side handled it (crates/perry-codegen-wasm/src/emit.rs:5441) and analysis helpers all knew about the variant, butexpr.rshad no match arm so codegen fell through to the catch-all bail at line 8434.Single new arm mirroring
Expr::ArrayPushat line 3016 — same three receiver storage cases (LocalGet, boxed-captured, plain), same realloc-aware writeback. No new runtime helper needed:js_array_concat(dst, src)already existed atcrates/perry-runtime/src/array.rs:1011, comment-as-spec'd "reserved for the internal push-spread desugaring path".Phase 2 (commit ae1fd6c): V8 / perry-jsruntime interop codegen arms
Adds 8 codegen arms in
crates/perry-codegen/src/expr.rsfor theJs*HIR family thatperry-hir/src/js_transform.rs::transform_js_importsproduces whenever a.tsentry imports from a.jsmodule the resolver classifies as JS-runtime-loaded (crates/perry/src/commands/compile/collect_modules.rs:73— extension-driven; the user'sbun i @codehz/pipelinerepro pullsindex.jsthrough this path).Pre-fix the LLVM backend bailed for JsLoadModule, JsGetExport, JsCallFunction, JsCallMethod, JsGetProperty, JsSetProperty, JsNew, JsNewFromHandle. Only WASM and JS backends had emit-side handling.
The new arms call into the existing
perry-jsruntime/src/interop.rsFFI surface — all eight FFIs were already declared inruntime_decls.rsexceptjs_call_method(added here). New shared helperlower_js_args_arraymarshals already-lowered NaN-boxed args into a stack alloca'd[N x double]via the issue-#167alloca_entry_arraypattern.Module handle representation: V8 module ids are u64; codegen returns them as f64 via
bitcast_i64_to_doubleto fitlower_expr's return-type contract, then consumers bitcast back to i64 before the FFI call. JS value handles are NaN-boxed f64 with V8-handle tag 0x7FFB — handled internally by perry-jsruntime'sv8_to_native/native_to_v8helpers.Runtime bootstrap: new
needs_js_runtime: boolfield onCrossModuleCtx, threaded fromCompileOptions, wired intocompile_module_entryso the entry main's prelude callsjs_runtime_init()betweenjs_gc_initand user code. Without this, everyjs_load_modulesite bailed at runtime with[js_load_module] no JS runtime state!.JsCreateCallbackdeliberately deferred to Phase 2B follow-up: the runtime FFIjs_create_callbackexpectsfunc_ptrto have signature(closure_env: i64, args_ptr: *const f64, args_len: i64) -> f64(seenative_callback_trampolineatcrates/perry-jsruntime/src/interop.rs:993), but Perry closure bodies have(closure_ptr, arg0, arg1, ...)per arity — no direct call-compatible mapping. Wiring this needs either codegen-emitted per-arity adapter thunks (one_perry_jscb_thunk_<n>per param_count), or a runtime-side closure-array dispatcher. For now the arm bails with a clear message so users see exactly what's blocked.Test plan
Phase 1:
cargo build --release -p perry-runtime -p perry-stdlib -p perry— clean[1,2].push(...[3,4,5])repro: pre-fix bailed; post-fix compiles to 700K binary, prints5matching Nodetest-files/test_issue_248_array_push_spread.ts(10 cases) — byte-for-byte againstnode --experimental-strip-typesPhase 2:
cargo build --release -p perry-runtime -p perry-stdlib -p perry-jsruntime -p perry— cleancargo test --release -p perry-codegen --lib— 22/0pipeline()repro fromnode_modules/@codehz/pipeline/index.jsat/tmp/issue248/test.ts: pre-fix bailed withJsCallFunction not yet supported; post-fix compiles + links + runs, exit 0gp.run({...})) at/tmp/issue248/test_method_no_cb.ts: compiles + links (39MB binary, pulls in V8) + runs, exit 0test-files/test_issue_248_phase2_js_interop.ts+ fixturetest-files/fixtures/issue_248_jsmod.js— verifies compile + link + clean exitCommon:
array_methods/console_methods/typed_arrays— are pre-existing categorical/CI gaps unrelated to this change)Known limitations / follow-ups
JsCreateCallback(Phase 2B): closures passed to JS-imported functions still bail. The user'sgameLoop.use((ctx) => {...})shape requires the closure-marshaling work described above.crates/perry-runtime/src/closure.rs:651-659win the link-order race against perry-jsruntime's strong impls (Mach-O first-wins behavior), so simple tests likepipeline()-then-typeofsee the stub return value (0.0 NaN-boxed →typeof === "number"). Method calls (no stub) pull V8 in correctly. This is a pre-existing tangle in the link orchestration that's separate from Unexpected error: JsCallFunction not yet supported in js source #248's codegen scope.typeofon V8 handles: returns"number"because Perry'stypeofdoesn't recognize the V8-handle tag 0x7FFB. Separate issue.