fix(codegen): #678 — V8-fallback callsites route through V8 bridge#843
Merged
Conversation
proggeramlug
force-pushed
the
worktree-agent-a34f334834d88bc22
branch
from
911cbcb to
b2be3cc
Compare
Builds on #785's re-export-rename suffix fix with the second half of the linker-error story: when an import resolves to a `ModuleKind::Interpreted` module (V8 fallback — e.g. a `.js` file outside `perry.compilePackages`, or ink-style packages where a transitive dep like yoga-layout couldn't compile natively), no `perry_fn_<src>__<name>` symbol exists for the linker to bind to. The CLI driver was silently dropping V8-routed imports from `import_function_prefixes` (the `if import.module_kind != NativeCompiled { continue }` guard at compile.rs:3409), so codegen emitted bare extern callsites the linker rejected with `Undefined symbols: _perry_fn_..._render` — exactly the failure shape #678 reports for `import { render } from "ink"` in the ink-on-yoga repro. * New runtime bridge `js_call_v8_export(specifier, name, args, argc)` in `perry-jsruntime/src/interop.rs` bundles `js_load_module` + `js_call_function` into a single FFI entry. * New `import_function_v8_specifiers: HashMap<String, String>` on `CompileOptions` parallels `import_function_origin_names` from #785. Propagated through `CrossModuleCtx` and every `FnCtx` construction site (6 in codegen.rs). * New `emit_v8_export_call` helper in `expr.rs` materializes per-call-site rodata constants for specifier + export name, stack-allocates an f64 args buffer, and emits the bridge call. * Codegen dispatch routes V8 imports through the bridge at every extern-construction site: namespace dispatch + direct extern call + fallback path in `lower_call.rs`; StaticMethodCall + ExternFuncRef-as-value + PropertyGet-namespace-value-read in `expr.rs`; FuncRef-as-value wrapper emission in `codegen.rs`. * CLI driver in `compile.rs` adds a V8-imports loop after the existing native-imports loop ends. Populates both `import_function_prefixes` (synthetic `__v8__<sanitized_path>` prefix) and `import_function_v8_specifiers` (real specifier the bridge hands to `js_load_module`). * Object cache key includes `import_function_v8_specifiers` so builds where a package flipped between native and V8 fallback don't share a cached `.o`. Defense-in-depth alongside the existing `transform_js_imports` HIR pass: that pass already rewrites the common shapes (`Call { callee: ExternFuncRef }` / `PropertyGet { ExternFuncRef }` / `ExternFuncRef`-as-value) into `JsCallFunction` / `JsCallMethod` / `JsGetExport` HIR variants and pre-empts the codegen-level bridge for >95% of real-world V8 import sites. The codegen-level path is fallback coverage for shapes the HIR pass misses (`StaticMethodCall`-lifted uppercase namespace members, exotic re-export chains, future demotion paths) and forward-compat with the planned "demote on native compile failure" path described in the issue body. Validation: * New regression fixture `test-files/test_issue_678_v8_fallback.ts` + `test-files/fixtures/issue_678_v8/` byte-for-byte parity with `node --experimental-strip-types`. * `cargo test --release --workspace` (excluding cross-host UI crates per CLAUDE.md) green; `cargo test --release -p perry-codegen` 29/0/0; `cargo test --release -p perry` 223/0/0. * `/tmp/run_gap_tests.sh` 34/36 (same two pre-existing failures as v0.5.913 — `test_gap_console_methods`, `test_gap_regexp_advanced`). * Standalone ink-shape repro at `/tmp/ink_repro/` compiles, links, and runs cleanly. Refs: #678. Builds on #785.
proggeramlug
force-pushed
the
worktree-agent-a34f334834d88bc22
branch
from
b2be3cc to
305356b
Compare
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
1 participant
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 second half of the #678 linker-error story (the first half, re-export-rename suffix, landed as #785). When an import resolves to a
ModuleKind::Interpretedmodule — i.e. a.jsfile outsidecompilePackages, or a package whose transitive dep can't compile natively — noperry_fn_<src>__<name>symbol exists for the linker to bind to. The CLI driver was silently dropping V8-routed imports fromimport_function_prefixes, so codegen emitted bare extern callsites and the linker rejected them withUndefined symbols: _perry_fn_..._render.Three coordinated edits, mirroring #785's diff template:
crates/perry-jsruntime/src/interop.rs: newjs_call_v8_export(specifier, export_name, args, argc) -> f64bundles the existingjs_load_module+js_call_functionpair into a single FFI entry. NaN-boxed in/out, V8 exceptions surfaced through the existing TryCatch path.import_function_v8_specifiers: HashMap<String, String>(consumer-name → module specifier), parallel toimport_function_origin_namesfrom fix(codegen): #678 — re-export rename resolves to origin export name #785. Threaded throughCrossModuleCtxand everyFnCtxconstruction site. Newemit_v8_export_callhelper inexpr.rsmaterializes rodata constants + stack-alloca'd args.lower_call.rs(namespace dispatch, direct extern call, bare-extern fallback), two inexpr.rs(StaticMethodCall on uppercase namespace member, ExternFuncRef-as-value), one incodegen.rs(FuncRef-as-value wrapper emission). Each site probesimport_function_v8_specifiersfirst and short-circuits to the bridge.crates/perry/src/commands/compile.rs: new V8-imports loop after the existing native-imports loop. Registers(consumer_name → synthetic __v8__<sanitized_path> prefix)and(consumer_name → specifier)for Named and Default imports..o.The existing
transform_js_importsHIR pass already covers the most commonCall/PropertyGet/ExternFuncRef-as-value shapes by rewriting them toJsCall*/JsGetExportHIR variants — so the codegen-level bridge added here is defense-in-depth for shapes the HIR pass misses (StaticMethodCall-lifted uppercase namespace members, exotic re-export chains) and forward-compat with a future "demote on native compile failure" path. Both layers coexist; nothing breaks for sites the HIR pass already covers.Test plan
test-files/test_issue_678_v8_fallback.ts+test-files/fixtures/issue_678_v8/mod.js— TS entry imports two named exports (greet,add) from a.jsmodule outsidecompilePackages, byte-for-byte parity withnode --experimental-strip-types./tmp/ink_repro/compiles, links, and runs.cargo build --releaseclean.cargo test --release --workspace(excluding cross-host UI crates per CLAUDE.md) green.cargo test --release -p perry-codegen— 29/0/0.cargo test --release -p perry— 223/0/0./tmp/run_gap_tests.sh— 34/36 (same two pre-existing failures:test_gap_console_methods,test_gap_regexp_advanced).Refs: #678. Builds on #785.