fix(perry-ext-events): #1274 sync emit signature with perry-stdlib (variadic args) - #1289
Merged
Merged
Conversation
…ariadic args) `import 'events'` and `import 'node:events'` route to perry-ext-events through `well_known_bindings.toml`, so listeners installed via the `node:events` shim ended up dispatching against perry-ext-events' `js_event_emitter_emit` — not perry-stdlib's. PR #1186 changed the perry-stdlib signature from fn js_event_emitter_emit(handle, name, arg: f64) -> bool to fn js_event_emitter_emit(handle, name, args_ptr: *mut ArrayHeader) -> f64 so the codegen native_table now lowers `emit` calls with `NA_VARARGS` (single `*mut ArrayHeader` slot) and `NR_F64` return. perry-ext-events was never updated to match — when the linker resolves to its old single-`arg: f64` symbol, the args-array POINTER value gets reinterpreted as `arg: f64`. High-bit heap pointers happen to satisfy the IEEE 754 NaN tag pattern, so every listener observes `NaN` instead of the real payload — `test_express_mount` saw `hitCount: NaN` / `parentName: undefined`, `test_issue_850_eventemitter` saw `once values: [ NaN ]` / `addListener/removeListener: [ NaN ]`. Mirrors perry-stdlib::events exactly: - `js_event_emitter_emit`: takes `args_ptr: *mut ArrayHeader`, returns `f64` (NaN-boxed `TAG_TRUE` / `TAG_FALSE`). Extracts `args_ptr[0]` via `js_array_get` for the closure-call-1 dispatch. - `js_event_emitter_emit0`: same return-type swap (bool → f64) so both emit variants round-trip through codegen's f64 ABI. - `drain_pending_once_promises`: takes the full args array instead of synthesizing a 1-element `[arg]` wrap — matches Node's `events.once` semantics where the resolution value is the full args tuple, and matches perry-stdlib's behaviour. Local repro on macOS arm64 after rebuild: Before fix: `inside listener, x: NaN` After fix: `inside listener, x: 7` Events parity suite (`test-parity/node-suite/events/**`): Before fix: 18 passed, 19 failed After fix: 26 passed, 11 failed (-8 regressions cleared) The remaining 11 failures are pre-existing perry-ext-events behavior gaps (setMaxListeners ignored, newListener/removeListener meta-events not emitted, `error`-without-listener doesn't throw, etc.) — unrelated to this signature fix. Closes #1274.
5 tasks
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
import 'events'/import 'node:events'routes to perry-ext-events viawell_known_bindings.toml. Codegen'snative_tablelowersemitwithNA_VARARGS+NR_F64(post-PR Add granular node events parity suite #1186), but perry-ext-events still had the legacy(handle, name, arg: f64) -> boolABI. The args-array pointer thus got reinterpreted asarg: f64; high heap-pointer bits satisfy the IEEE 754 NaN tag, so every listener observedNaNinstead of the real payload.js_event_emitter_emit+_emit0to perry-stdlib's signatures (args_ptr ArrayHeader; f64 NaN-boxed bool return); updateddrain_pending_once_promisesto pass the full args array soevents.onceresolves with the full tuple per Node.test_express_mount+test_issue_850_eventemitterparity blockers from v0.5.1019).Local validation (macOS arm64)
The remaining 11 events failures are pre-existing perry-ext-events behavior gaps (setMaxListeners ignored, newListener/removeListener meta-events not emitted,
error-without-listener doesn't throw) — unrelated to this signature fix.Test plan
cargo fmt --all -- --checkcleancargo build --release -p perry-ext-eventscleantest_express_mountandtest_issue_850_eventemitterpass locally