fix: #835/#846 followup — feed codegen FFI provenance into auto-optimize stdlib features#884
Merged
Merged
Conversation
…optimize stdlib feature set
Effect's `Stream` lowering and similar compiled-package code emits
`js_readable_stream_*` FFI calls without any `import "streams"` in the
user TS. The codegen-side FFI registry (`ext_registry.rs`) tagged those
as `OwnerKind::Stdlib` and the drain flipped `ctx.needs_stdlib`, but
the auto-optimize layer (`build_optimized_libs`) rebuilds perry-stdlib
with only the Cargo features `compute_required_features` derived from
`ctx.native_module_imports` — which is empty when there's no explicit
`import "streams"`. So `bundled-streams` was dropped, `pub mod streams`
was `#[cfg]`-gated out of the .a, and the link failed with
"Undefined symbols: _js_readable_stream_new" etc. — even though the CLI
said "Linking (with stdlib)...".
Fix: extend `OwnerKind::Stdlib` to carry `feature: Option<&'static str>`
(the perry-stdlib Cargo feature gate for the FFI's providing module),
add `CompilationContext::extra_stdlib_features: BTreeSet<&'static str>`
that the registry drain populates from each Stdlib entry, and union
that set into the feature list `build_optimized_libs` passes to cargo.
Every existing stream FFI in the registry now declares
`feature: Some("bundled-streams")`.
The http path (`OwnerKind::WellKnown("http")`) already worked: the
drain inserts "http" into `native_module_imports`, the well-known
table routes to perry-ext-http, and the link resolves. No change there.
Smoke test `test-files/test_issue_835_846_b_stream_ffi_link.ts` exercises
`new ReadableStream(...)` (without `import "streams"`) AND
`createServer(handler)` from `node:http`. The Effect repro now links
and advances past the symbol-resolution stage to a runtime TypeError
from a different code path (separate, pre-existing bug).
proggeramlug
force-pushed
the
fix/835-846-followup-stdlib-feature-gate
branch
from
1434e8b to
9444781
Compare
4 tasks
proggeramlug
added a commit
that referenced
this pull request
May 17, 2026
…sruntime arm of build_and_run_link (#887) Express (perry.compilePackages: ["express"]) link-failed on _js_node_http_create_server / _js_node_http_res_end because build_and_run_link's `if let Some(ref jsruntime) = jsruntime_lib {...}` arm appended only jsruntime + runtime — the parallel `else if ctx.needs_stdlib` arm that emits well_known_libs + stdlib_lib was unreachable. The codegen-side FFI provenance registry (#884) correctly drained OwnerKind::WellKnown("http") into ctx.native_module_imports, and the auto-optimize layer built libperry_ext_http.a + pushed it onto well_known_libs — but the link command then silently dropped it. Fix mirrors the well-known emission into the jsruntime arm: after jsruntime + runtime, when ctx.needs_stdlib is true, append stdlib_lib (if Some) + every well_known_libs entry. Mach-O / ELF archive scan is first-definition-wins, so jsruntime's bundled-stdlib copies still win for duplicate js_* symbols (the documented duplicate-symbol warnings); the perry-ext-* libs cover symbols neither jsruntime nor its bundled stdlib carries. Validation: express repro links cleanly (advances to a separate runtime TypeError from the Proxy/callable shape — out of scope per the issue). New regression test exercises both halves standalone: static-import a .js fixture (forces ctx.needs_js_runtime) + http.createServer (forces well_known_libs to contain perry-ext-http). cargo test --release --workspace clean (1098 passed, 0 failed).
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
Stdlib-resident symbol needs. The compile driver drains those intoctx.extra_stdlib_features, andbuild_optimized_libsunions them into the feature set BEFORE rebuildinglibperry_stdlib.a. Closes the Link failure:needs_stdlibnot set when compiled-package code (Effect) references ReadableStream FFIs #835/express link-fails:js_node_http_create_servercodegen'd butperry-ext-http-servernot linked #846 follow-up where Effect'sStreamlowering link-failed withUndefined symbols: _js_readable_stream_newbecause the auto-optimize stdlib was built withoutbundled-streams.OwnerKind::StdlibbecomesOwnerKind::Stdlib { feature: Option<&'static str> }. All 35 stream entries in the registry now declarefeature: Some("bundled-streams"). The http path (OwnerKind::WellKnown("http")) already worked via the existing well-known flow and is unchanged.test-files/test_issue_835_846_b_stream_ffi_link.tsexercisesnew ReadableStream(...)withoutimport "streams"pluscreateServer(...)fromnode:http, guarding the regression where the auto-optimize stdlib drops these features.Test plan
cargo build --release -p perry-runtime -p perry-stdlib -p perrycargo test --release --workspace --exclude perry-ui-* --exclude perry-jsruntimeclean (no failures)cargo test --release -p perry-codegenupdated registry test passesstream r1 done: false,stream r1 value: hello,server created: true_js_readable_stream_*undefinedCloses the #835/#846 follow-up. Refs #835, #846.