Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion crates/perry-codegen/src/expr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,8 @@ pub(crate) use write_barrier::{
emit_array_numeric_write_note_on_block, emit_jsvalue_slot_store_on_block,
emit_jsvalue_slot_store_scalar_aware_on_block, emit_layout_note_slot_on_block,
emit_root_heap_word_store_on_block, emit_root_nanbox_store_on_block, emit_write_barrier,
emit_write_barrier_slot_on_block, lower_node_stream_super_init, lower_stream_super_init,
emit_write_barrier_slot_on_block, lower_event_emitter_subclass_init,
lower_node_stream_super_init, lower_stream_super_init,
};

/// One in-flight inline-constructor return target. See
Expand Down
37 changes: 30 additions & 7 deletions crates/perry-codegen/src/expr/this_super_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ use super::{
emit_write_barrier_slot_on_block, expr_is_known_non_pointer_shadow_value,
extract_array_of_object_shape, i32_bool_to_nanbox, import_origin_suffix,
is_global_this_builtin_function_name, is_global_this_builtin_name, is_known_finite,
lower_array_literal, lower_channel_reduction, lower_expr, lower_expr_as_i32,
lower_index_set_fast, lower_js_args_array, lower_node_stream_super_init, lower_object_literal,
lower_stream_super_init, lower_url_string_getter, nanbox_bigint_inline, nanbox_pointer_inline,
nanbox_pointer_inline_pub, nanbox_string_inline, proxy_build_args_array, try_flat_const_2d_int,
try_lower_flat_const_index_get, try_match_channel_reduction, try_static_class_name,
unbox_str_handle, unbox_to_i64, variant_name, ChannelReduction, FlatConstInfo, FnCtx,
I18nLowerCtx,
lower_array_literal, lower_channel_reduction, lower_event_emitter_subclass_init, lower_expr,
lower_expr_as_i32, lower_index_set_fast, lower_js_args_array, lower_node_stream_super_init,
lower_object_literal, lower_stream_super_init, lower_url_string_getter, nanbox_bigint_inline,
nanbox_pointer_inline, nanbox_pointer_inline_pub, nanbox_string_inline, proxy_build_args_array,
try_flat_const_2d_int, try_lower_flat_const_index_get, try_match_channel_reduction,
try_static_class_name, unbox_str_handle, unbox_to_i64, variant_name, ChannelReduction,
FlatConstInfo, FnCtx, I18nLowerCtx,
};

/// Built-in constructor names (beyond Error/stream/fetch, which have their own
Expand Down Expand Up @@ -443,6 +443,29 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
)?;
return Ok(result);
}
// #5137: `class X extends EventEmitter` (node:events) —
// `super()` installs the bare EventEmitter listener/emit
// surface onto `this` (see `lower_event_emitter_subclass_init`).
// `super(opts)` takes an optional options bag in Node; we lower
// the args for side effects but the bare emitter seeds no state.
if parent_name.as_str() == "EventEmitter" {
for a in super_args {
let _ = lower_expr(ctx, a)?;
}
let this_box = match ctx.this_stack.last().cloned() {
Some(slot) => ctx.block().load(DOUBLE, &slot),
None => double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)),
};
lower_event_emitter_subclass_init(ctx, &this_box);
let current_class_name =
ctx.class_stack.last().cloned().unwrap_or_default();
crate::lower_call::apply_field_initializers_recursive(
ctx,
&current_class_name,
crate::lower_call::FieldInitMode::SelfOnly,
)?;
return Ok(double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)));
}
Comment on lines +451 to +468

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Handle EventEmitter init for inherited chains, not only direct parents.

At Line 458, the init is gated to parent_name == "EventEmitter" only in the parent_class == None path. This misses cases like class B extends A { constructor(){ super(); } } where A extends EventEmitter and A has no own constructor: parent_class is present (A), so no EventEmitter init runs and .on/.emit stay absent on this.

Please add an ancestor-chain termination check for EventEmitter in this super-call flow, and mirror the same logic in crates/perry-codegen/src/lower_call/new.rs (the no-own-ctor fallback path).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-codegen/src/expr/this_super_call.rs` around lines 458 - 479, The
EventEmitter initialization in the super-call handler is only triggered when the
direct parent_class is "EventEmitter" (i.e., parent_class == None), which misses
inheritance chains like B extends A extends EventEmitter where A has no own
constructor. Modify the condition around the parent_name == "EventEmitter" check
in the this_super_call.rs file to walk up the class hierarchy and detect if
EventEmitter is an ancestor anywhere in the chain, not just as the direct
parent. Then apply the same ancestor-chain detection logic to the no-own-ctor
fallback path in crates/perry-codegen/src/lower_call/new.rs to ensure
EventEmitter initialization runs consistently across both code paths when
EventEmitter appears anywhere in the inheritance hierarchy.

// `class X extends Request` / `extends Response`:
// `super(input, init)` allocates the underlying native
// Web-Fetch handle and stashes its id on `this` under
Expand Down
18 changes: 18 additions & 0 deletions crates/perry-codegen/src/expr/write_barrier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,3 +391,21 @@ pub(crate) fn lower_node_stream_super_init(

Ok(undef_lit)
}

/// #5137: install the bare EventEmitter listener/emit surface onto `this_box`
/// for a source-compiled `class X extends EventEmitter` (node:events). Shared
/// by the explicit-`super()` arm (`expr/this_super_call.rs`) and the
/// no-own-constructor `new` path (`lower_call/new.rs`). The runtime helper
/// reuses the generic `ns_*` emitter closures (they key all state off the
/// receiver), so a plain object that never went through a stream constructor
/// gets working `.on`/`.emit`/`.once`/…. Reached when an EventEmitter
/// subclass's real npm source is compiled — e.g. commander's `Command` under
/// `perry.compilePackages`, where the `new Command()` → `js_commander_*`
/// native-shim path is intentionally off.
pub(crate) fn lower_event_emitter_subclass_init(ctx: &mut FnCtx<'_>, this_box: &str) {
ctx.block().call(
DOUBLE,
"js_event_emitter_subclass_init",
&[(DOUBLE, this_box)],
);
}
12 changes: 12 additions & 0 deletions crates/perry-codegen/src/lower_call/new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1337,6 +1337,18 @@ pub(crate) fn lower_new(ctx: &mut FnCtx<'_>, class_name: &str, args: &[Expr]) ->
found_inherited_ctor = true;
}
}
// #5137: implicit-ctor `class X extends EventEmitter {}` — install the
// emitter surface (the explicit-`super()` arm does this when a ctor is
// written). Gated `!has_imported_ctor` so an imported class whose real
// ctor lives in another module (commander's `Command`) still reaches
// the imported-ctor fallback below and runs its real `super()`.
if !found_inherited_ctor
&& !has_imported_ctor
&& class.extends_name.as_deref() == Some("EventEmitter")
{
crate::expr::lower_event_emitter_subclass_init(ctx, &obj_box);
found_inherited_ctor = true;
}
// Issue #573: if the parent walk reached an Error-like built-in
// without finding any user-class constructor, synthesize the JS
// spec default ctor `constructor(...args) { super(...args); }` —
Expand Down
1 change: 1 addition & 0 deletions crates/perry-codegen/src/runtime_decls/stdlib_ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1253,6 +1253,7 @@ pub fn declare_stdlib_ffi(module: &mut LlModule) {
module.declare_function("js_lru_cache_size", DOUBLE, &[I64]);

// ========== node:stream stubs (issue #631) ==========
module.declare_function("js_event_emitter_subclass_init", DOUBLE, &[DOUBLE]); // #5137 EE subclass init
module.declare_function("js_node_stream_readable_new", DOUBLE, &[DOUBLE]);
module.declare_function(
"js_node_stream_readable_subclass_init",
Expand Down
12 changes: 12 additions & 0 deletions crates/perry-hir/src/ir/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,18 @@ pub fn clear_compile_packages_override() {
COMPILE_PACKAGES_OVERRIDE.with(|cell| cell.borrow_mut().clear());
}

/// Refs #5137: true when the user explicitly opted `pkg` into
/// `perry.compilePackages` (the package's real npm source is being
/// compiled). Native-instance registration and native-shim method
/// lowering must back off for such packages even when a class name
/// like `Command` / `Big` would otherwise hit a hardcoded
/// library-name fallback — otherwise `new Command()` from commander's
/// own source is still routed to the `js_commander_*` shim instead of
/// the compiled-from-source class.
pub fn is_compile_package_override(pkg: &str) -> bool {
COMPILE_PACKAGES_OVERRIDE.with(|cell| cell.borrow().contains(pkg))
}

// ---- #5009 build-time `process.env` define substitution ----

/// #5009: a build-time `process.env.<NAME>` substitution value, esbuild
Expand Down
11 changes: 6 additions & 5 deletions crates/perry-hir/src/ir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,12 @@ pub use constants::{
clear_allow_dynamic_stdlib_packages, clear_compile_packages_override,
clear_current_module_source, clear_env_defines, clear_precompile_state,
current_module_has_allow_dynamic_at, current_module_line_at, current_module_source_slice,
determine_module_kind, dynamic_stdlib_allowed_for_package, env_define_lookup, is_native_module,
is_native_module_with_externals, is_node_builtin_module, package_name_for_source_path,
precompile_capture_enabled, precompile_result_at, refuse_dynamic_stdlib_dispatch_enabled,
requires_stdlib, set_allow_dynamic_stdlib_packages, set_compile_packages_override,
set_current_module_source, set_env_defines, set_precompile_capture, set_precompile_results,
determine_module_kind, dynamic_stdlib_allowed_for_package, env_define_lookup,
is_compile_package_override, is_native_module, is_native_module_with_externals,
is_node_builtin_module, package_name_for_source_path, precompile_capture_enabled,
precompile_result_at, refuse_dynamic_stdlib_dispatch_enabled, requires_stdlib,
set_allow_dynamic_stdlib_packages, set_compile_packages_override, set_current_module_source,
set_env_defines, set_precompile_capture, set_precompile_results,
set_refuse_dynamic_stdlib_dispatch, typed_array_kind_for_name, ClassId, EnumId, EnvDefine,
InterfaceId, ModuleInitKind, ModuleKind, PosixCredentialKind, TypeAliasId, NATIVE_MODULES,
TYPED_ARRAY_KIND_BIGINT64, TYPED_ARRAY_KIND_BIGUINT64, TYPED_ARRAY_KIND_FLOAT16,
Expand Down
13 changes: 13 additions & 0 deletions crates/perry-hir/src/lower/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1079,6 +1079,19 @@ impl LoweringContext {
module_name: String,
class_name: String,
) {
// #5137: if the user opted this package into `perry.compilePackages`,
// its real npm source is being compiled and the binding resolves to
// the compiled-from-source class. Registering a native instance here
// would re-route the instance's fluent methods (`new Command()` →
// `.name()`/`.option()`/`.parse()`) to the `js_commander_*` native
// shim that was deliberately kept off the import path — so the call
// emits an FFI reference the source-compile build never links (or, in
// a shimless build, returns `undefined`). Back off so the source class
// is used. `is_native_module` already makes the same back-off for the
// import-resolution side (#665).
if is_compile_package_override(&module_name) {
return;
}
self.native_instances
.push((local_name, module_name, class_name));
}
Expand Down
26 changes: 26 additions & 0 deletions crates/perry-runtime/src/node_stream_constructors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,32 @@ pub extern "C" fn js_node_stream_readable_subclass_init(this: f64, opts: f64) ->
this
}

/// #5137: `super()` for a source-compiled `class X extends EventEmitter`
/// (from `node:events`). Installs the bare EventEmitter listener/emit
/// methods directly onto `this` — the same generic `ns_*` closures the
/// stream subclasses use — so `.on`/`.emit`/`.once`/… resolve as the
/// instance's own bound methods. This is the EventEmitter analog of
/// `js_node_stream_readable_subclass_init`; commander's `Command extends
/// EventEmitter` reaches it when its real npm source is compiled (the
/// package is in `perry.compilePackages`, so the `new Command()` → native
/// `js_commander_*` shim path is deliberately off). Unlike the stream
/// inits there is no option-driven state to seed — a plain EventEmitter
/// has no `_read`/`highWaterMark`/etc.
#[no_mangle]
pub extern "C" fn js_event_emitter_subclass_init(this: f64) -> f64 {
let raw = raw_ptr_from_value(this);
if raw == 0 {
return this;
}
if unsafe { gc_type_for_ptr(raw) } != Some(crate::gc::GC_TYPE_OBJECT) {
return this;
}
let obj = raw as *mut ObjectHeader;
let methods = emitter_methods();
install_methods_on_existing_object(obj, this, &methods, &[]);
this
}

#[no_mangle]
pub extern "C" fn js_node_stream_writable_new(opts: f64) -> f64 {
let methods = writable_methods();
Expand Down
30 changes: 30 additions & 0 deletions crates/perry-runtime/src/node_stream_readwrite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1817,6 +1817,36 @@ pub(super) fn readable_methods() -> [(&'static str, StubFn); 39] {
]
}

/// #5137: the bare `EventEmitter` surface — the same 15 listener/emit
/// methods that `readable_methods`/`writable_methods` share, minus all the
/// stream-specific entries. Installed onto `this` by
/// `js_event_emitter_subclass_init` so a source-compiled `class X extends
/// EventEmitter` (e.g. commander's `Command`) gets working
/// `.on`/`.emit`/`.once`/… without routing through the handle-based
/// `js_event_emitter_*` shim. The closures are the generic
/// `ns_*` emitter helpers, which key all state off the receiver object, so
/// they work unchanged on a plain object that never went through a stream
/// constructor.
pub(super) fn emitter_methods() -> [(&'static str, StubFn); 15] {
[
("on", cast2(ns_on2)),
("once", cast2(ns_once2)),
("prependListener", cast2(ns_prepend_listener2)),
("prependOnceListener", cast2(ns_prepend_once_listener2)),
("off", cast2(ns_off2)),
("addListener", cast2(ns_on2)),
("removeListener", cast2(ns_remove_listener2)),
("removeAllListeners", cast1(ns_remove_all_listeners1)),
("emit", cast2(ns_emit_rest)),
("setMaxListeners", cast1(ns_set_max_listeners)),
("getMaxListeners", cast0(ns_get_max_listeners)),
("eventNames", cast0(ns_event_names)),
("listenerCount", cast1(ns_listener_count)),
("listeners", cast1(ns_listeners)),
("rawListeners", cast1(ns_raw_listeners)),
]
}

pub(super) fn writable_methods() -> [(&'static str, StubFn); 22] {
[
("on", cast2(ns_on2)),
Expand Down
Loading