diff --git a/crates/perry-codegen/src/codegen/method.rs b/crates/perry-codegen/src/codegen/method.rs index 42d8aba4d0..b33cf7c9e7 100644 --- a/crates/perry-codegen/src/codegen/method.rs +++ b/crates/perry-codegen/src/codegen/method.rs @@ -444,12 +444,13 @@ pub(super) fn compile_method( for la in &forwarded { ctor_args.push((DOUBLE, la.as_str())); } - ctx.pending_declares.push(( - ctor_sym.clone(), - crate::types::VOID, - ctor_param_types, - )); - ctx.block().call_void(&ctor_sym, &ctor_args); + // Synthesized default-ctor forwarding to an imported parent + // ctor: discard the return (parent override does not + // replace `this`). Declared DOUBLE to match the symbol's + // real signature (see codegen/mod.rs). + ctx.pending_declares + .push((ctor_sym.clone(), DOUBLE, ctor_param_types)); + let _ = ctx.block().call(DOUBLE, &ctor_sym, &ctor_args); } } } diff --git a/crates/perry-codegen/src/codegen/mod.rs b/crates/perry-codegen/src/codegen/mod.rs index 06366d7f85..f0dc2deef5 100644 --- a/crates/perry-codegen/src/codegen/mod.rs +++ b/crates/perry-codegen/src/codegen/mod.rs @@ -1162,6 +1162,7 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> param_count: ic.constructor_param_count, has_own_constructor: ic.has_own_constructor, has_instance_fields: ic.has_instance_fields, + has_rest: ic.constructor_has_rest, }, ) }) @@ -1902,13 +1903,20 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> } // Constructor: declared as - // `___constructor(i64 this, double arg0, …) → void` + // `___constructor(double this, double arg0, …) → double`. + // The source module's standalone ctor symbol returns DOUBLE — the + // ECMAScript constructor return-override value (an explicit + // `return `) or `undefined` for an ordinary ctor. Declaring it + // VOID discarded a returned object/function, so `new Chalk(opts)` (whose + // ctor `return chalkFactory(opts)`) yielded the empty instance instead of + // the factory. The dispatch in `lower_new` applies `js_ctor_return_override` + // to this value. let ctor_fn = format!("{}__{}_constructor", sanitize(src), sanitize(&ic.name),); let mut ctor_params: Vec = vec![DOUBLE]; for _ in 0..ic.constructor_param_count { ctor_params.push(DOUBLE); } - llmod.declare_function(&ctor_fn, VOID, &ctor_params); + llmod.declare_function(&ctor_fn, DOUBLE, &ctor_params); // Cross-module static methods. Source modules emit these as static // functions with no `this` receiver, normally qualified by the source diff --git a/crates/perry-codegen/src/codegen/opts.rs b/crates/perry-codegen/src/codegen/opts.rs index a2e332fb0a..eb877a91ed 100644 --- a/crates/perry-codegen/src/codegen/opts.rs +++ b/crates/perry-codegen/src/codegen/opts.rs @@ -453,6 +453,14 @@ pub struct ImportedClass { pub constructor_param_count: usize, /// Whether the source class declared its own constructor body. pub has_own_constructor: bool, + /// Whether the source class's constructor's last declared parameter is + /// `...rest`. Symmetric to `method_has_rest` but for the constructor: the + /// source module compiled `_constructor(this, arg0, …)` expecting + /// the rest slot to receive a PACKED ARRAY of the trailing args. Without + /// this flag the cross-module `new C(a, b, c)` dispatch passed the args + /// positionally, so `arg0 = a` (raw) and `b`/`c` were dropped — a + /// `constructor(...args)` saw `args = a`, length 1. + pub constructor_has_rest: bool, /// Whether the source class has instance fields that require initializer replay. pub has_instance_fields: bool, /// Method names defined on this class. @@ -520,6 +528,10 @@ pub(crate) struct ImportedCtor { pub param_count: usize, pub has_own_constructor: bool, pub has_instance_fields: bool, + /// True when the constructor's last declared param is `...rest`. Tells + /// the cross-module `new` dispatch to pack the trailing args into an + /// array for the rest slot rather than passing them positionally. + pub has_rest: bool, } impl ImportedCtor { diff --git a/crates/perry-codegen/src/expr/this_super_call.rs b/crates/perry-codegen/src/expr/this_super_call.rs index ba8f0bd19c..118a42510e 100644 --- a/crates/perry-codegen/src/expr/this_super_call.rs +++ b/crates/perry-codegen/src/expr/this_super_call.rs @@ -875,12 +875,13 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { for la in &lowered_args { ctor_args.push((DOUBLE, la.as_str())); } - ctx.pending_declares.push(( - ctor.symbol.clone(), - crate::types::VOID, - ctor_param_types, - )); - ctx.block().call_void(&ctor.symbol, &ctor_args); + // `super(...)` to an imported parent: the parent ctor's return + // override does not replace the derived `this`, so discard the + // return. Declared DOUBLE to match the symbol's real signature + // (the source standalone ctor returns DOUBLE — see codegen/mod.rs). + ctx.pending_declares + .push((ctor.symbol.clone(), DOUBLE, ctor_param_types)); + let _ = ctx.block().call(DOUBLE, &ctor.symbol, &ctor_args); } // After the parent body has run (which may have set `this.config` diff --git a/crates/perry-codegen/src/lower_call/new.rs b/crates/perry-codegen/src/lower_call/new.rs index f385fcc577..3e8daa4624 100644 --- a/crates/perry-codegen/src/lower_call/new.rs +++ b/crates/perry-codegen/src/lower_call/new.rs @@ -134,6 +134,50 @@ fn pack_lowered_args_array(ctx: &mut FnCtx<'_>, args: &[String]) -> String { nanbox_pointer_inline(ctx.block(), ¤t) } +/// Marshal the lowered `new`-site args into the value list a cross-module +/// imported constructor symbol expects. The source module compiled the +/// standalone `_constructor(this, p0, …)` with `ctor.param_count` +/// explicit slots. When the constructor's last param is `...rest` +/// (`ctor.has_rest`), that final slot must receive a PACKED ARRAY of every +/// trailing arg — not the first trailing arg passed raw. Mirrors the +/// inline-ctor `inline_constructor_param_values` rest packing and the +/// `method_has_rest` path for imported methods (#672). Returns exactly +/// `ctor.param_count` value strings; missing leading args are padded with +/// `undefined`. +fn marshal_imported_ctor_args( + ctx: &mut FnCtx<'_>, + ctor: &crate::codegen::ImportedCtor, + lowered_args: &[String], +) -> Vec { + let undef = double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)); + let param_count = ctor.param_count; + if ctor.has_rest && param_count > 0 { + // The first `param_count - 1` slots are positional; the last slot is + // the rest array packing every remaining arg. + let n_positional = param_count - 1; + let mut out: Vec = Vec::with_capacity(param_count); + for i in 0..n_positional { + out.push( + lowered_args + .get(i) + .cloned() + .unwrap_or_else(|| undef.clone()), + ); + } + let tail: Vec = lowered_args.iter().skip(n_positional).cloned().collect(); + out.push(pack_lowered_args_array(ctx, &tail)); + out + } else { + // No rest: positional, padded to `param_count` with `undefined`. + let mut out: Vec = lowered_args.to_vec(); + while out.len() < param_count { + out.push(undef.clone()); + } + out.truncate(param_count.max(out.len())); + out + } +} + /// The effective constructor arity for `new (...)`: the class's own /// ctor params, else — for a subclass with no own ctor — the closest /// ancestor-with-a-ctor's param count (the synthesized default ctor forwards @@ -951,7 +995,7 @@ pub(crate) fn lower_new(ctx: &mut FnCtx<'_>, class_name: &str, args: &[Expr]) -> let after_idx = ctx.new_block("ctor.return.after"); let after_label = ctx.block_label(after_idx); ctx.inline_ctor_return.push(crate::expr::InlineCtorReturn { - result_slot: ctor_result_slot, + result_slot: ctor_result_slot.clone(), after_label, // A class is "derived" (and thus subject to the stricter // return-override rules) if it has ANY heritage — a named parent, @@ -1377,50 +1421,51 @@ pub(crate) fn lower_new(ctx: &mut FnCtx<'_>, class_name: &str, args: &[Expr]) -> .filter(|_| effective_class_name != lookup_class) { // Walked to an ancestor — call its ctor with this and forwarded args. - let undef_lit = - crate::nanbox::double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)); - while lowered_args.len() < ctor.param_count { - lowered_args.push(undef_lit.clone()); - } + // `...rest` ctors get the trailing args packed into one array + // for the final slot (mirrors method_has_rest, #672). + let marshalled = marshal_imported_ctor_args(ctx, &ctor, &lowered_args); let mut ctor_args: Vec<(crate::types::LlvmType, &str)> = - Vec::with_capacity(1 + lowered_args.len()); + Vec::with_capacity(1 + marshalled.len()); ctor_args.push((DOUBLE, &obj_box)); let ctor_param_types: Vec = std::iter::once(DOUBLE) - .chain(lowered_args.iter().map(|_| DOUBLE)) + .chain(marshalled.iter().map(|_| DOUBLE)) .collect(); - for la in &lowered_args { + for la in &marshalled { ctor_args.push((DOUBLE, la.as_str())); } - ctx.pending_declares.push(( - ctor.symbol.clone(), - crate::types::VOID, - ctor_param_types, - )); - ctx.block().call_void(&ctor.symbol, &ctor_args); + // Walked to an ANCESTOR ctor: its return-override does not replace + // the leaf instance, so discard the return value. Declared DOUBLE + // to match the symbol's real signature (see codegen/mod.rs). + ctx.pending_declares + .push((ctor.symbol.clone(), DOUBLE, ctor_param_types)); + let _ = ctx.block().call(DOUBLE, &ctor.symbol, &ctor_args); } else if let Some(ctor) = ctx.imported_class_ctors.get(class_name).cloned() { // Pad missing optional args with TAG_UNDEFINED so the constructor - // doesn't read garbage from stale registers. - let undef_lit = - crate::nanbox::double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)); - while lowered_args.len() < ctor.param_count { - lowered_args.push(undef_lit.clone()); - } + // doesn't read garbage from stale registers, and pack the rest + // slot into an array when the ctor's last param is `...rest`. + let marshalled = marshal_imported_ctor_args(ctx, &ctor, &lowered_args); // Pass `this` as NaN-boxed double (same as compile_method's this_arg). let mut ctor_args: Vec<(crate::types::LlvmType, &str)> = - Vec::with_capacity(1 + lowered_args.len()); + Vec::with_capacity(1 + marshalled.len()); ctor_args.push((DOUBLE, &obj_box)); let ctor_param_types: Vec = std::iter::once(DOUBLE) - .chain(lowered_args.iter().map(|_| DOUBLE)) + .chain(marshalled.iter().map(|_| DOUBLE)) .collect(); - for la in &lowered_args { + for la in &marshalled { ctor_args.push((DOUBLE, la.as_str())); } - ctx.pending_declares.push(( - ctor.symbol.clone(), - crate::types::VOID, - ctor_param_types, - )); - ctx.block().call_void(&ctor.symbol, &ctor_args); + // The standalone `_constructor` symbol returns DOUBLE: the + // value an explicit `return ` produced (ECMAScript ctor + // return-override) or `undefined` for an ordinary ctor. Capture it + // into `ctor_result_slot` so the return-override applied at the end + // of `lower_new` honors it — chalk's `class Chalk { constructor(o){ + // return chalkFactory(o); } }` returns a FUNCTION, so `new Chalk(o)` + // must yield that function, not the empty allocated instance + // ("value is not a function" on `new Chalk(...).red(...)`). + ctx.pending_declares + .push((ctor.symbol.clone(), DOUBLE, ctor_param_types)); + let ctor_ret = ctx.block().call(DOUBLE, &ctor.symbol, &ctor_args); + ctx.block().store(DOUBLE, &ctor_ret, &ctor_result_slot); } } // end !found_inherited_ctor } diff --git a/crates/perry-hir/src/lower/lower_expr.rs b/crates/perry-hir/src/lower/lower_expr.rs index 3fbf540bf4..777f57f4ff 100644 --- a/crates/perry-hir/src/lower/lower_expr.rs +++ b/crates/perry-hir/src/lower/lower_expr.rs @@ -235,6 +235,64 @@ fn anonymous_class_has_static_name_member(class: &ast::Class) -> bool { }) } +/// True when an `Expr` is cheap to evaluate more than once with no observable +/// side effects — safe to duplicate into an optional-call guard condition. +/// Conservative: only the obvious read-only leaf/access shapes qualify. +fn opt_call_receiver_repeatable(expr: &Expr) -> bool { + match expr { + Expr::LocalGet(_) + | Expr::GlobalGet(_) + | Expr::This + | Expr::Undefined + | Expr::Null + | Expr::Number(_) + | Expr::String(_) + | Expr::Bool(_) => true, + // `a.b` / `a[const]` chains over repeatable receivers stay repeatable + // (property reads are not side-effecting in this codebase's model). + Expr::PropertyGet { object, .. } => opt_call_receiver_repeatable(object), + Expr::IndexGet { object, index } => { + opt_call_receiver_repeatable(object) && opt_call_receiver_repeatable(index) + } + _ => false, + } +} + +/// Build the condition under which `obj.method?.(args)` short-circuits to +/// `undefined`: the resolved function value is nullish. The naive check +/// `obj.method == null` is WRONG when `obj` is a primitive string, because +/// `PropertyGet{string, method}` reads back `undefined` even though the +/// builtin (`split`/`replace`/…) is perfectly callable through the call path +/// — so the guard wrongly short-circuited (`mime`'s +/// `type?.split?.(';')[0]` returned `undefined`). Per spec, a string DOES have +/// the method, so we must NOT short-circuit. When the receiver is repeatable +/// we widen the guard to `func_value == null && typeof receiver !== "string"`: +/// for a real string the typeof clause is false (never short-circuit → the +/// call dispatches the builtin), while a user object missing the method still +/// short-circuits (#830 preserved). Non-repeatable receivers keep the plain +/// function-value check to avoid double-evaluating side effects. +fn opt_call_func_nullish_guard(receiver: &Expr, func_value: Expr) -> Expr { + let func_nullish = Expr::Compare { + op: CompareOp::LooseEq, + left: Box::new(func_value), + right: Box::new(Expr::Null), + }; + if opt_call_receiver_repeatable(receiver) { + let not_string = Expr::Compare { + op: CompareOp::Ne, + left: Box::new(Expr::TypeOf(Box::new(receiver.clone()))), + right: Box::new(Expr::String("string".to_string())), + }; + Expr::Logical { + op: LogicalOp::And, + left: Box::new(func_nullish), + right: Box::new(not_string), + } + } else { + func_nullish + } +} + pub(crate) fn lower_expr_assignment( ctx: &mut LoweringContext, expr: &ast::Expr, @@ -832,28 +890,23 @@ fn lower_expr_impl(ctx: &mut LoweringContext, expr: &ast::Expr) -> Result None } } - ast::Expr::Member(member) => { - let native_module = if let ast::Expr::Ident(obj_ident) = member.obj.as_ref() - { - let obj_name = obj_ident.sym.as_ref(); - // `Temporal.` constructors dispatch via brand arms, - // not a class chain, so route them through the runtime - // dynamic path (`js_instanceof_dynamic` → - // `temporal_ctor_kind`) by lowering the constructor to - // its closure value here. - obj_name == "Temporal" - || ctx.lookup_builtin_module_alias(obj_name).is_some() - || matches!(ctx.lookup_native_module(obj_name), Some((_, None))) - } else { - false - }; - if native_module { - match lower_expr(ctx, &bin.right) { - Ok(e) => Some(Box::new(e)), - Err(_) => None, - } - } else { - None + ast::Expr::Member(_member) => { + // Lower the member RHS to its value and route through + // `js_instanceof_dynamic`. The pre-fix code only did this + // for native modules (`Temporal.X`, builtin aliases) and + // otherwise left codegen with the static `ty = "obj.prop"` + // string, which it can't resolve to a class id for a + // user-module member (`x instanceof sv.SemVer` where `sv` + // is a default/namespace import) → class_id 0 → instanceof + // always false (semver's `new SemVer(semVerObj)` clone path + // hit this: `version instanceof SemVer` was false, so the + // ctor mis-parsed the object as a string). `sv.SemVer` + // lowers to the same class-ref value `const C = sv.SemVer` + // produces, which the dynamic path resolves correctly; for + // native modules it still derives the brand/synthetic id. + match lower_expr(ctx, &bin.right) { + Ok(e) => Some(Box::new(e)), + Err(_) => None, } } // Any other right-hand side (a primitive literal like @@ -1872,6 +1925,11 @@ fn lower_expr_impl(ctx: &mut LoweringContext, expr: &ast::Expr) -> Result // "X is not a function" (issue #4699: zod `safeParse`'s // `iss.inst?._zod.def?.error?.(iss)` error-map probe). let mut callee_from_chain = false; + // Receiver of an `obj.method?.(args)` callee, captured so the + // function-value nullish guard can avoid false-short-circuiting + // on string builtins (`type?.split?.(...)`) — see + // `opt_call_func_nullish_guard`. `None` for non-member callees. + let mut opt_call_member_receiver: Option = None; let (check_expr, callee_expr) = { let mut lower_member_flat = |member: &ast::MemberExpr| -> Result<(Expr, Expr)> { @@ -1909,7 +1967,8 @@ fn lower_expr_impl(ctx: &mut LoweringContext, expr: &ast::Expr) -> Result // (prop), call the function (prop) — codegen still sees // a PropertyGet callee so `this` binds to obj. ast::Expr::Member(m) => { - let (_obj, prop) = lower_member_flat(m)?; + let (obj, prop) = lower_member_flat(m)?; + opt_call_member_receiver = Some(obj); (prop.clone(), prop) } ast::Expr::OptChain(inner) => match &*inner.base { @@ -1939,7 +1998,9 @@ fn lower_expr_impl(ctx: &mut LoweringContext, expr: &ast::Expr) -> Result // callee + dispatches the builtin normally. ast::OptChainBase::Member(m) => { callee_from_chain = opt_chain.optional; - lower_member_flat(m)? + let (obj, prop) = lower_member_flat(m)?; + opt_call_member_receiver = Some(obj.clone()); + (obj, prop) } _ => { let ce = lower_expr(ctx, callee)?; @@ -1985,12 +2046,19 @@ fn lower_expr_impl(ctx: &mut LoweringContext, expr: &ast::Expr) -> Result // before calling — otherwise an `undefined` property is // invoked and throws "X is not a function" (#4699). let else_expr: Box = if callee_from_chain { - Box::new(Expr::Conditional { - condition: Box::new(Expr::Compare { + // String-builtin-safe nullish guard: a real string + // receiver never short-circuits even though + // `string.method` reads as undefined. + let guard_cond = match &opt_call_member_receiver { + Some(recv) => opt_call_func_nullish_guard(recv, fixed_callee), + None => Expr::Compare { op: CompareOp::LooseEq, left: Box::new(fixed_callee), right: Box::new(Expr::Null), - }), + }, + }; + Box::new(Expr::Conditional { + condition: Box::new(guard_cond), then_expr: Box::new(Expr::Undefined), else_expr: Box::new(outer_call), }) @@ -2050,15 +2118,24 @@ fn lower_expr_impl(ctx: &mut LoweringContext, expr: &ast::Expr) -> Result // before the call — otherwise an `undefined` property is // invoked and throws "X is not a function" (#4699). let else_expr: Box = match func_value_for_guard { - Some(func_value) => Box::new(Expr::Conditional { - condition: Box::new(Expr::Compare { - op: CompareOp::LooseEq, - left: Box::new(func_value), - right: Box::new(Expr::Null), - }), - then_expr: Box::new(Expr::Undefined), - else_expr: Box::new(call_expr), - }), + Some(func_value) => { + // String-builtin-safe: do not short-circuit when the + // receiver is a primitive string whose builtin method + // reads back as `undefined` (`type?.split?.(...)`). + let guard_cond = match &opt_call_member_receiver { + Some(recv) => opt_call_func_nullish_guard(recv, func_value), + None => Expr::Compare { + op: CompareOp::LooseEq, + left: Box::new(func_value), + right: Box::new(Expr::Null), + }, + }; + Box::new(Expr::Conditional { + condition: Box::new(guard_cond), + then_expr: Box::new(Expr::Undefined), + else_expr: Box::new(call_expr), + }) + } None => Box::new(call_expr), }; @@ -2069,12 +2146,27 @@ fn lower_expr_impl(ctx: &mut LoweringContext, expr: &ast::Expr) -> Result // undefined to fall through and produce // `[object Object]` (or worse) when the receiver // is `Map.get(missing)` etc. - Ok(Expr::Conditional { - condition: Box::new(Expr::Compare { + // + // For the simple `obj.method?.(args)` shape (`callee_from_chain` + // is false and we captured a member receiver), `check_expr` is + // the FUNCTION VALUE `obj.method`. Reading `string.method` as a + // property yields `undefined` for builtins even though they're + // callable, so use the string-builtin-safe guard to avoid a + // false short-circuit (`"a/b".split?.(...)`). Otherwise + // (`check_expr` is a receiver, or callee is not a member) the + // plain nullish check is correct. + let condition = if !callee_from_chain && opt_call_member_receiver.is_some() { + let recv = opt_call_member_receiver.unwrap(); + opt_call_func_nullish_guard(&recv, check_expr) + } else { + Expr::Compare { op: CompareOp::LooseEq, left: Box::new(check_expr), right: Box::new(Expr::Null), - }), + } + }; + Ok(Expr::Conditional { + condition: Box::new(condition), then_expr: Box::new(Expr::Undefined), else_expr, }) diff --git a/crates/perry-runtime/src/object/object_ops_frozen.rs b/crates/perry-runtime/src/object/object_ops_frozen.rs index 5e4ba82ffe..107da4e5ca 100644 --- a/crates/perry-runtime/src/object/object_ops_frozen.rs +++ b/crates/perry-runtime/src/object/object_ops_frozen.rs @@ -149,6 +149,22 @@ pub extern "C" fn js_object_freeze(obj_value: f64) -> f64 { { return obj_value; } + // Map / Set instances are NOT plain `ObjectHeader`s: their entries + // live in a dedicated backing registered in the map/set tables, and + // the `keys_array` slot the walk below reads is not a real key array. + // `mark_all_keys` would dereference that garbage and corrupt the + // backing — after `Object.freeze(map)`, a later `map.get(k)` / + // `map.values()` then read a bad pointer and SIGSEGV (mime's + // `_freeze()` froze its `#typeToExtensions` Map-of-Sets). Per spec, + // freezing a Map/Set only makes the object non-extensible; its + // entries stay mutable through the collection methods. So set the + // integrity GC flags (already done above) and stop — same shape as + // the TypedArray arm. + if crate::map::is_registered_map(obj as usize) + || crate::set::is_registered_set(obj as usize) + { + return obj_value; + } // Closures: own props are `name`/`length` + dynamic props — the // keys_array walk below would read garbage off the ClosureHeader. // Record explicit non-writable/non-configurable attrs. @@ -246,6 +262,13 @@ pub extern "C" fn js_object_seal(obj_value: f64) -> f64 { { return obj_value; } + // Map / Set: GC integrity flags only — the `keys_array` walk would + // corrupt their backing. See the matching arm in `js_object_freeze`. + if crate::map::is_registered_map(obj as usize) + || crate::set::is_registered_set(obj as usize) + { + return obj_value; + } // Closures: seal via the side tables (drop configurable only) — // see the matching arm in `js_object_freeze`. if crate::closure::is_closure_ptr(obj as usize) { diff --git a/crates/perry/src/commands/compile.rs b/crates/perry/src/commands/compile.rs index 1c1f44918d..1f7a9014f2 100644 --- a/crates/perry/src/commands/compile.rs +++ b/crates/perry/src/commands/compile.rs @@ -2569,6 +2569,11 @@ pub fn run_with_parse_cache( .map(|c| c.params.len()) .unwrap_or(0), has_own_constructor: class.constructor.is_some(), + constructor_has_rest: class + .constructor + .as_ref() + .map(|c| c.params.iter().any(|p| p.is_rest)) + .unwrap_or(false), has_instance_fields: !class.fields.is_empty(), method_names: class .methods @@ -2765,6 +2770,11 @@ pub fn run_with_parse_cache( .map(|c| c.params.len()) .unwrap_or(0), has_own_constructor: class.constructor.is_some(), + constructor_has_rest: class + .constructor + .as_ref() + .map(|c| c.params.iter().any(|p| p.is_rest)) + .unwrap_or(false), has_instance_fields: !class.fields.is_empty(), method_names: class .methods @@ -3013,6 +3023,11 @@ pub fn run_with_parse_cache( .map(|c| c.params.len()) .unwrap_or(0), has_own_constructor: class.constructor.is_some(), + constructor_has_rest: class + .constructor + .as_ref() + .map(|c| c.params.iter().any(|p| p.is_rest)) + .unwrap_or(false), has_instance_fields: !class.fields.is_empty(), method_names: class .methods @@ -3079,6 +3094,11 @@ pub fn run_with_parse_cache( .map(|c| c.params.len()) .unwrap_or(0), has_own_constructor: class.constructor.is_some(), + constructor_has_rest: class + .constructor + .as_ref() + .map(|c| c.params.iter().any(|p| p.is_rest)) + .unwrap_or(false), has_instance_fields: !class.fields.is_empty(), method_names: class.methods.iter().map(|m| m.name.clone()).collect(), method_param_counts: class @@ -3232,6 +3252,11 @@ pub fn run_with_parse_cache( .map(|c| c.params.len()) .unwrap_or(0), has_own_constructor: class.constructor.is_some(), + constructor_has_rest: class + .constructor + .as_ref() + .map(|c| c.params.iter().any(|p| p.is_rest)) + .unwrap_or(false), has_instance_fields: !class.fields.is_empty(), method_names: class.methods.iter().map(|m| m.name.clone()).collect(), method_param_counts: class @@ -3692,6 +3717,11 @@ pub fn run_with_parse_cache( .map(|c| c.params.len()) .unwrap_or(0), has_own_constructor: class.constructor.is_some(), + constructor_has_rest: class + .constructor + .as_ref() + .map(|c| c.params.iter().any(|p| p.is_rest)) + .unwrap_or(false), has_instance_fields: !class.fields.is_empty(), method_names: class.methods.iter().map(|m| m.name.clone()).collect(), method_param_counts: class @@ -3886,6 +3916,11 @@ pub fn run_with_parse_cache( .map(|c| c.params.len()) .unwrap_or(0), has_own_constructor: class.constructor.is_some(), + constructor_has_rest: class + .constructor + .as_ref() + .map(|c| c.params.iter().any(|p| p.is_rest)) + .unwrap_or(false), has_instance_fields: !class.fields.is_empty(), method_names: class.methods.iter().map(|m| m.name.clone()).collect(), method_param_counts: class diff --git a/crates/perry/src/commands/compile/object_cache.rs b/crates/perry/src/commands/compile/object_cache.rs index 3578064ea2..90de977623 100644 --- a/crates/perry/src/commands/compile/object_cache.rs +++ b/crates/perry/src/commands/compile/object_cache.rs @@ -1244,6 +1244,7 @@ mod object_cache_tests { source_prefix: "feature_ts".into(), constructor_param_count: 0, has_own_constructor: false, + constructor_has_rest: false, has_instance_fields: true, method_names: vec![], method_param_counts: vec![], @@ -1278,6 +1279,7 @@ mod object_cache_tests { source_prefix: "src".into(), constructor_param_count: 1, has_own_constructor: true, + constructor_has_rest: false, has_instance_fields: true, method_names: vec!["bar".into()], method_param_counts: vec![0], @@ -1297,6 +1299,7 @@ mod object_cache_tests { source_prefix: "src".into(), constructor_param_count: 2, // different arity has_own_constructor: true, + constructor_has_rest: false, has_instance_fields: true, method_names: vec!["bar".into()], method_param_counts: vec![0], @@ -1324,6 +1327,7 @@ mod object_cache_tests { source_prefix: "src".into(), constructor_param_count: 1, has_own_constructor: true, + constructor_has_rest: false, has_instance_fields: true, method_names: vec!["bar".into()], method_param_counts: vec![1], diff --git a/crates/perry/tests/functional_batch2_regressions.rs b/crates/perry/tests/functional_batch2_regressions.rs new file mode 100644 index 0000000000..bcee1e9cb6 --- /dev/null +++ b/crates/perry/tests/functional_batch2_regressions.rs @@ -0,0 +1,200 @@ +//! Regression tests for the functional-correctness batch-2 fixes. +//! +//! Each fix was localized from a real npm package in the differential +//! functional corpus and reduced to a minimal multi-module / single-module +//! fixture here so the behavior is pinned without depending on the corpus. +//! +//! Fixes covered: +//! 1. Cross-module constructor with a `...rest` param dropped all but the +//! first argument (mime: `new Mime(standardTypes, otherTypes)`). +//! 2. `Object.freeze` on a Map/Set corrupted its backing so later +//! `get`/`values` over object-valued entries faulted (mime `_freeze`). +//! 3. Optional-call `obj.method?.(args)` on a string builtin short-circuited +//! to `undefined` (mime `type?.split?.(';')`). +//! 4. `new ImportedClass()` discarded an ECMAScript constructor +//! return-override (chalk `class Chalk { constructor(){ return factory; } }`). +//! 5. `instanceof` with a namespace/import member RHS (`x instanceof ns.C`) +//! always returned false (semver's SemVer-clone guard). + +use std::path::Path; +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +/// Write `files` (relative path -> contents) into `dir`, compile `entry` +/// with `--no-cache`, run it, and return stdout. Asserts compile + run succeed. +fn compile_and_run(dir: &Path, files: &[(&str, &str)], entry: &str) -> String { + for (rel, contents) in files { + let path = dir.join(rel); + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).expect("mkdir"); + } + std::fs::write(&path, contents).expect("write fixture"); + } + let entry_path = dir.join(entry); + let output = dir.join("main_bin"); + + let compile = Command::new(perry_bin()) + .current_dir(dir) + .arg("compile") + .arg(&entry_path) + .arg("--no-cache") + .arg("-o") + .arg(&output) + .env("PERRY_NO_AUTO_OPTIMIZE", "1") + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + + let run = Command::new(&output) + .current_dir(dir) + .output() + .expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary failed\nstatus: {:?}\nstdout:\n{}\nstderr:\n{}", + run.status, + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + String::from_utf8_lossy(&run.stdout).into_owned() +} + +#[test] +fn cross_module_constructor_rest_param_keeps_all_args() { + // Pre-fix: `new C("x","y","z")` on an imported class with a `...args` + // ctor only captured "x" (rest slot got the first arg raw). Node: all 3. + let dir = tempfile::tempdir().expect("tempdir"); + let stdout = compile_and_run( + dir.path(), + &[ + ( + "cls.ts", + r#"export class C { constructor(...args: any[]){ (this as any).n = args.length; (this as any).a = args; } }"#, + ), + ( + "main.ts", + r#"import { C } from "./cls.ts"; +const c: any = new C("x", "y", "z"); +console.log(c.n, JSON.stringify(c.a));"#, + ), + ], + "main.ts", + ); + assert_eq!(stdout, "3 [\"x\",\"y\",\"z\"]\n"); +} + +#[test] +fn object_freeze_on_map_of_sets_preserves_entries() { + // Pre-fix: Object.freeze(map) ran the keys-array walk over the Map's + // backing and corrupted it; a later get/values then SIGSEGV'd. + let dir = tempfile::tempdir().expect("tempdir"); + let stdout = compile_and_run( + dir.path(), + &[( + "main.ts", + r#"const m = new Map>(); +m.set("a", new Set(["x", "y"])); +m.set("b", new Set(["z"])); +Object.freeze(m); +let total = 0; +for (const v of m.values()) { total += v.size; Object.freeze(v); } +console.log("get a:", (m.get("a") as Set).size); +console.log("total:", total);"#, + )], + "main.ts", + ); + assert_eq!(stdout, "get a: 2\ntotal: 3\n"); +} + +#[test] +fn optional_call_on_string_builtin_invokes_method() { + // Pre-fix: `s.split?.(...)` / `s?.split?.(...)` returned undefined because + // the function-value guard saw `string.split` read back as undefined. + let dir = tempfile::tempdir().expect("tempdir"); + let stdout = compile_and_run( + dir.path(), + &[( + "main.ts", + r#"const s = "a/b"; +console.log("A:", JSON.stringify(s.split?.("/"))); +console.log("B:", JSON.stringify(s?.split?.("/"))); +const t: string | undefined = "x;y"; +console.log("C:", JSON.stringify(t?.split?.(";")[0])); +// A user object missing the method still short-circuits. +const o: any = {}; +console.log("D:", o.missing?.());"#, + )], + "main.ts", + ); + assert_eq!( + stdout, + "A: [\"a\",\"b\"]\nB: [\"a\",\"b\"]\nC: \"x\"\nD: undefined\n" + ); +} + +#[test] +fn imported_class_constructor_return_override_honored() { + // Pre-fix: `new ImportedClass()` whose ctor `return ` yielded the + // empty allocated instance instead of the returned function. + let dir = tempfile::tempdir().expect("tempdir"); + let stdout = compile_and_run( + dir.path(), + &[ + ( + "factory.ts", + r#"function make(tag: string) { return (s: string) => tag + ":" + s; } +export class Wrapper { + constructor(tag: string) { + // eslint-disable-next-line no-constructor-return + return make(tag) as any; + } +}"#, + ), + ( + "main.ts", + r#"import { Wrapper } from "./factory.ts"; +const w: any = new Wrapper("hi"); +console.log("typeof:", typeof w); +console.log("call:", w("there"));"#, + ), + ], + "main.ts", + ); + assert_eq!(stdout, "typeof: function\ncall: hi:there\n"); +} + +#[test] +fn instanceof_namespace_member_rhs() { + // Pre-fix: `x instanceof ns.Class` (member RHS over a default/namespace + // import) always returned false (only native modules took the dynamic path). + let dir = tempfile::tempdir().expect("tempdir"); + let stdout = compile_and_run( + dir.path(), + &[ + ( + "ns.ts", + r#"export class Thing { constructor(public v: number) {} }"#, + ), + ( + "main.ts", + r#"import * as ns from "./ns.ts"; +const a = new ns.Thing(5); +console.log("member:", a instanceof ns.Thing); +const C = ns.Thing; +console.log("local:", a instanceof C); +console.log("neg:", ({} as any) instanceof ns.Thing);"#, + ), + ], + "main.ts", + ); + assert_eq!(stdout, "member: true\nlocal: true\nneg: false\n"); +}