From c0c55cface318b172dca76d1aee2fedd3aab0287 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 08:17:39 +0000 Subject: [PATCH 1/3] fix(hir,codegen,runtime): make `arguments` in class constructors reflect the call site HIR stops padding a new-site's argument list to the declared arity for a constructor that reads `arguments` (`monomorph/defaults.rs`) -- an appended `undefined` was indistinguishable from one the caller wrote. Runtime: the four dynamic-construct paths (super-apply caps arm, flat-ctor replay, and both class-object/registered-class replay paths) now share `constructor_user_arg_slots`, which packs the synthesized `arguments` slot from every call arg instead of binding it like a user `...rest` (only the args past the declared count) -- construction through a value, an imported class, or a CommonJS class all saw an empty `arguments` before this. Codegen: constructor ABI (`CtorAbi`: param count, has-rest, has-synthetic- arguments) is read from the constructor's fixed/rest/arguments layout instead of inspecting only its last declared parameter, which missed every capturing constructor -- i.e. every CommonJS class, since Perry adds capture params mechanically. The ABI threads through constructor-contract resolution (so a no-own-ctor forwarder inherits its ancestor's full ABI), imported-class metadata, and cross-module `new`-site arg marshaling, which can now pack up to two trailing arrays (a user rest, then `arguments`) instead of assuming at most one. --- .../src/codegen/constructor_contracts.rs | 58 ++-- .../perry-codegen/src/codegen/ctor_arity.rs | 262 +++++++++++++- crates/perry-codegen/src/codegen/mod.rs | 5 +- crates/perry-codegen/src/codegen/opts.rs | 24 +- .../perry-codegen/src/codegen/string_pool.rs | 30 +- .../src/expr/readonly_collection_tests.rs | 1 + crates/perry-codegen/src/lib.rs | 9 +- .../src/lower_call/new_ctor_args.rs | 29 +- .../src/lower_call/typed_shape_bake_tests.rs | 1 + crates/perry-hir/src/monomorph/defaults.rs | 10 + crates/perry-hir/src/monomorph/tests.rs | 115 +++++++ .../src/object/class_constructors.rs | 321 +++++++++++------- .../src/commands/compile/object_cache.rs | 8 + .../object_cache/object_cache_tests.rs | 4 + .../src/commands/compile/run_pipeline.rs | 12 +- .../issue_10484_ctor_arguments/classes.ts | 55 +++ .../issue_10484_ctor_arguments/request.cjs | 46 +++ ...t_gap_10484_class_constructor_arguments.ts | 291 ++++++++++++++++ 18 files changed, 1091 insertions(+), 190 deletions(-) create mode 100644 test-files/fixtures/issue_10484_ctor_arguments/classes.ts create mode 100644 test-files/fixtures/issue_10484_ctor_arguments/request.cjs create mode 100644 test-files/test_gap_10484_class_constructor_arguments.ts diff --git a/crates/perry-codegen/src/codegen/constructor_contracts.rs b/crates/perry-codegen/src/codegen/constructor_contracts.rs index 040df8de36..e8165f46ad 100644 --- a/crates/perry-codegen/src/codegen/constructor_contracts.rs +++ b/crates/perry-codegen/src/codegen/constructor_contracts.rs @@ -4,14 +4,14 @@ use std::collections::{BTreeMap, BTreeSet, HashMap}; -use super::ctor_arity::{context_free_ctor_param_count, UNRESOLVED_PARENT_FWD_ARITY}; +use super::ctor_arity::{context_free_ctor_abi, CtorAbi, UNRESOLVED_PARENT_FWD_ARITY}; use super::opts::{CompileOptions, ImportedClass}; type Symbol = (String, String); enum Contract { - Params(usize), - Parent(Symbol, usize), + Params(CtorAbi), + Parent(Symbol, CtorAbi), } /// A compact graph of constructor edges in each defining module's scope. @@ -23,7 +23,7 @@ pub struct ConstructorContracts { /// The only graph-wide constructor data needed during parallel codegen. pub struct ResolvedConstructorContracts { - arities: BTreeMap, + abis: BTreeMap, } impl ConstructorContracts { @@ -51,26 +51,30 @@ impl ConstructorContracts { imports.entry(imported.effective_name()).or_insert(imported); } for class in &module.classes { - let contract = if let Some(count) = context_free_ctor_param_count(class) { - Contract::Params(count) + let contract = if let Some(abi) = context_free_ctor_abi(class) { + Contract::Params(abi) } else { let mut parent = class.extends_name.as_deref(); let mut visited = BTreeSet::new(); - let mut contract = Contract::Params(UNRESOLVED_PARENT_FWD_ARITY); + let mut contract = + Contract::Params(CtorAbi::positional(UNRESOLVED_PARENT_FWD_ARITY)); while let Some(name) = parent { if !visited.insert(name) { break; } if let Some(local) = locals.get(name) { if let Some(ctor) = &local.constructor { - contract = Contract::Params(ctor.params.len()); + // The forwarder hands every slot to this ctor's + // symbol untouched, so it inherits its ABI — packed + // trailing arrays included (#10484). + contract = Contract::Params(CtorAbi::from_params(&ctor.params)); break; } parent = local.extends_name.as_deref(); } else if let Some(imported) = imports.get(name) { contract = Contract::Parent( (imported.source_prefix.clone(), imported.name.clone()), - imported.constructor_param_count, + imported.ctor_abi(), ); break; } else { @@ -90,26 +94,26 @@ impl ConstructorContracts { for symbol in self.contracts.keys() { resolve(symbol, &self.contracts, &mut resolved, &mut BTreeSet::new()); } - ResolvedConstructorContracts { arities: resolved } + ResolvedConstructorContracts { abis: resolved } } } fn resolve( symbol: &Symbol, contracts: &BTreeMap, - resolved: &mut BTreeMap, + resolved: &mut BTreeMap, visiting: &mut BTreeSet, -) -> usize { - if let Some(count) = resolved.get(symbol) { - return *count; +) -> CtorAbi { + if let Some(abi) = resolved.get(symbol) { + return *abi; } if !visiting.insert(symbol.clone()) { // Cyclic heritage has no constructor-bearing ancestor. Keep the // standalone fallback and, crucially, the same ABI on every edge. - return UNRESOLVED_PARENT_FWD_ARITY; + return CtorAbi::positional(UNRESOLVED_PARENT_FWD_ARITY); } - let count = match &contracts[symbol] { - Contract::Params(count) => *count, + let abi = match &contracts[symbol] { + Contract::Params(abi) => *abi, Contract::Parent(parent, fallback) => { if contracts.contains_key(parent) { resolve(parent, contracts, resolved, visiting) @@ -121,8 +125,8 @@ fn resolve( } }; visiting.remove(symbol); - resolved.insert(symbol.clone(), count); - count + resolved.insert(symbol.clone(), abi); + abi } impl ResolvedConstructorContracts { @@ -132,16 +136,22 @@ impl ResolvedConstructorContracts { .classes .iter() .map(|class| { - let count = self.arities[&(prefix.to_owned(), class.name.clone())]; - (class.name.clone(), count) + let abi = self.abis[&(prefix.to_owned(), class.name.clone())]; + (class.name.clone(), abi.param_count) }) .collect(); for imported in &mut opts.imported_classes { - if let Some(count) = self - .arities + if let Some(abi) = self + .abis .get(&(imported.source_prefix.clone(), imported.name.clone())) { - imported.constructor_param_count = *count; + imported.constructor_param_count = abi.param_count; + // #10484: a no-own-ctor class emits a positional forwarder into + // its ancestor's symbol, so the `new` site here must pack the + // trailing arrays the ANCESTOR declares, not the (empty) set the + // forwarder's own class HIR shows. + imported.constructor_has_rest = abi.has_rest; + imported.constructor_has_synthetic_arguments = abi.has_synthetic_arguments; } } } diff --git a/crates/perry-codegen/src/codegen/ctor_arity.rs b/crates/perry-codegen/src/codegen/ctor_arity.rs index 4001fceb3e..f221453511 100644 --- a/crates/perry-codegen/src/codegen/ctor_arity.rs +++ b/crates/perry-codegen/src/codegen/ctor_arity.rs @@ -11,28 +11,74 @@ use std::collections::{BTreeMap, HashMap}; /// [`synthesized_ctor_param_count`]). pub const UNRESOLVED_PARENT_FWD_ARITY: usize = 8; -/// The standalone-constructor arity of `class` when it can be decided from the +/// The ABI of a class's standalone `_constructor` symbol: how many slots +/// it takes and which of the trailing ones are arrays a caller has to PACK — a +/// user `...rest` and/or the HIR-synthesized `arguments` slot (#10484). +/// +/// A class with no own constructor emits the `super(...args)` forwarder, which +/// passes every slot on to the ancestor's symbol unchanged, so it carries the +/// ancestor's ABI verbatim. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub struct CtorAbi { + pub param_count: usize, + pub has_rest: bool, + pub has_synthetic_arguments: bool, +} + +impl CtorAbi { + /// Positional-only, `param_count` slots. + pub(crate) fn positional(param_count: usize) -> Self { + CtorAbi { + param_count, + ..CtorAbi::default() + } + } + + /// Read a constructor's own parameter list. Synthesized `__perry_cap_*` + /// capture params trail the arrays and are bound from the class's capture + /// snapshot rather than from the call, so a capturing constructor reports + /// no packable slot and keeps positional marshaling. + pub(crate) fn from_params(params: &[perry_hir::Param]) -> Self { + if params.iter().any(|p| p.name.starts_with("__perry_cap_")) { + return CtorAbi::positional(params.len()); + } + CtorAbi { + param_count: params.len(), + has_rest: params + .iter() + .any(|p| p.is_rest && p.arguments_object.is_none()), + has_synthetic_arguments: params.iter().any(|p| p.arguments_object.is_some()), + } + } +} + +/// The standalone-constructor ABI of `class` when it can be decided from the /// class definition alone, without the defining module's class table or /// imports. The source-graph constructor-contract resolver uses this, so it /// MUST agree with [`synthesized_ctor_param_count`] for every case it answers: /// an own constructor, a native parent, no heritage, and a heritage that is only /// a runtime value (`extends_expr` with no resolvable `extends_name`), which -/// always synthesizes the fixed forwarding band. `None` means the arity depends -/// on the defining module's ancestor walk (#10258). -pub fn context_free_ctor_param_count(class: &perry_hir::Class) -> Option { +/// always synthesizes the fixed positional forwarding band. `None` means the ABI +/// depends on the defining module's ancestor walk (#10258). +pub fn context_free_ctor_abi(class: &perry_hir::Class) -> Option { if let Some(c) = class.constructor.as_ref() { - return Some(c.params.len()); + return Some(CtorAbi::from_params(&c.params)); } if class.native_extends.is_some() { - return Some(0); + return Some(CtorAbi::positional(0)); } match (&class.extends_name, &class.extends_expr) { - (None, None) => Some(0), - (None, Some(_)) => Some(UNRESOLVED_PARENT_FWD_ARITY), + (None, None) => Some(CtorAbi::positional(0)), + (None, Some(_)) => Some(CtorAbi::positional(UNRESOLVED_PARENT_FWD_ARITY)), _ => None, } } +/// [`context_free_ctor_abi`]'s arity half. +pub fn context_free_ctor_param_count(class: &perry_hir::Class) -> Option { + context_free_ctor_abi(class).map(|abi| abi.param_count) +} + /// The standalone-constructor arity Perry emits for `class`, accounting for the /// JS spec default ctor `constructor(...args) { super(...args) }` that a class /// with NO own constructor but WITH heritage inherits. Walks the ancestor chain @@ -115,3 +161,203 @@ pub(super) fn synthesized_ctor_param_count( // so over-declaring is correct for any (non-native) parent up to this band. UNRESOLVED_PARENT_FWD_ARITY } + +/// The parameter list whose trailing-array layout (user rest, synthesized +/// `arguments`) the emitted standalone `_constructor` symbol follows. +/// +/// An own constructor is its own layout. A class with NO own constructor emits +/// the `super(...args)` forwarder, whose `__forward_arg` params adopt the +/// nearest ctor-bearing LOCAL ancestor's params positionally and pass every slot +/// to that ancestor's symbol unchanged, so a caller has to fill them exactly as +/// it would fill the ancestor's (#10484: the ancestor's `arguments` slot must +/// receive the whole argument list, not one positional argument). +/// +/// `None` wherever the forwarder does not make that static positional call: a +/// dynamic heritage edge (`extends_expr`, forwarded through the runtime +/// dynamic-parent dispatcher as plain arguments), a native parent, an imported +/// ancestor, or an arity that does not match `emitted_param_count`. Also `None` +/// for an ancestor with `__perry_cap_*` params: the forwarder registers no +/// capture slots, so its callers bind every slot positionally as before. +pub(super) fn constructor_layout_params<'a>( + class: &'a perry_hir::Class, + class_table: &HashMap, + emitted_param_count: u32, +) -> Option<&'a [perry_hir::Param]> { + if let Some(ctor) = class.constructor.as_ref() { + return Some(&ctor.params); + } + if class.native_extends.is_some() || class.extends_expr.is_some() { + return None; + } + let mut parent = class.extends_name.as_deref(); + let mut depth = 0usize; + while let Some(name) = parent { + let ancestor = *class_table.get(name)?; + // Imported stubs carry id 0 and no constructor body in this module. + if ancestor.id == 0 || ancestor.native_extends.is_some() || depth > 64 { + return None; + } + if let Some(ctor) = ancestor.constructor.as_ref() { + let positional = ctor.params.len() == emitted_param_count as usize + && !ctor.params.iter().any(|p| p.name.starts_with("__perry_cap_")); + return positional.then_some(ctor.params.as_slice()); + } + if ancestor.extends_expr.is_some() { + return None; + } + parent = ancestor.extends_name.as_deref(); + depth += 1; + } + None +} + +#[cfg(test)] +mod tests { + use super::*; + use perry_hir::types::Type; + use perry_hir::{ArgumentsObjectMeta, Class, Function, Param}; + + fn param(name: &str, is_rest: bool, arguments_object: bool) -> Param { + Param { + id: 0, + name: name.to_string(), + ty: Type::Any, + default: None, + decorators: Vec::new(), + is_rest, + arguments_object: arguments_object.then(|| ArgumentsObjectMeta { + strict: true, + simple_parameters: false, + mapped_parameter_ids: Vec::new(), + restricted_callee: true, + }), + } + } + + fn class(name: &str, extends: Option<&str>, ctor_params: Option>) -> Class { + Class { + id: 1, + name: name.to_string(), + type_params: Vec::new(), + extends: None, + extends_name: extends.map(|e| e.to_string()), + native_extends: None, + extends_expr: None, + heritage_lexically_shadowed: false, + fields: Vec::new(), + constructor: ctor_params.map(|params| Function { + id: 7, + name: format!("{}_constructor", name), + type_params: Vec::new(), + params, + return_type: Type::Void, + body: Vec::new(), + is_async: false, + is_generator: false, + is_strict: true, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + was_plain_async: false, + was_unrolled: false, + }), + methods: Vec::new(), + getters: Vec::new(), + setters: Vec::new(), + static_accessor_names: Vec::new(), + static_accessor_fn_ids: Vec::new(), + static_fields: Vec::new(), + static_methods: Vec::new(), + computed_members: Vec::new(), + decorators: Vec::new(), + is_exported: false, + is_nested: false, + alloc_width_hint: 0, + specialized_from: None, + aliases: Vec::new(), + } + } + + fn arguments_ctor_params() -> Vec { + vec![ + param("p", false, false), + param("q", false, false), + param("arguments", true, true), + ] + } + + #[test] + fn reads_the_trailing_arrays_of_an_own_constructor() { + let abi = CtorAbi::from_params(&arguments_ctor_params()); + assert_eq!( + abi, + CtorAbi { + param_count: 3, + has_rest: false, + has_synthetic_arguments: true, + } + ); + let with_user_rest = vec![ + param("first", false, false), + param("rest", true, false), + param("arguments", false, true), + ]; + assert_eq!( + CtorAbi::from_params(&with_user_rest), + CtorAbi { + param_count: 3, + has_rest: true, + has_synthetic_arguments: true, + } + ); + } + + #[test] + fn a_capturing_constructor_reports_no_packable_slot() { + // The capture params trail the arrays, so the packed slots are not the + // last ones and every caller binds positionally instead. + let mut params = arguments_ctor_params(); + params.push(param("__perry_cap_4", false, false)); + assert_eq!(CtorAbi::from_params(¶ms), CtorAbi::positional(4)); + } + + #[test] + fn a_forwarder_adopts_its_local_ancestors_layout() { + let base = class("Base", None, Some(arguments_ctor_params())); + let middle = class("Middle", Some("Base"), None); + let leaf = class("Leaf", Some("Middle"), None); + let table: HashMap = [ + ("Base".to_string(), &base), + ("Middle".to_string(), &middle), + ("Leaf".to_string(), &leaf), + ] + .into_iter() + .collect(); + + let layout = constructor_layout_params(&leaf, &table, 3).expect("ancestor layout"); + assert!(layout.iter().any(|p| p.arguments_object.is_some())); + assert_eq!(layout.len(), 3); + + // An arity the emitted forwarder does not have must not claim a layout: + // the caller would pack an array into a slot nobody forwards. + assert!(constructor_layout_params(&leaf, &table, 2).is_none()); + } + + #[test] + fn a_dynamic_or_unknown_heritage_forwarder_has_no_layout() { + let base = class("Base", None, Some(arguments_ctor_params())); + let mut dynamic = class("Dynamic", Some("Base"), None); + dynamic.extends_expr = Some(Box::new(perry_hir::Expr::LocalGet(3))); + let orphan = class("Orphan", Some("Missing"), None); + let table: HashMap = [ + ("Base".to_string(), &base), + ("Dynamic".to_string(), &dynamic), + ("Orphan".to_string(), &orphan), + ] + .into_iter() + .collect(); + + assert!(constructor_layout_params(&dynamic, &table, 3).is_none()); + assert!(constructor_layout_params(&orphan, &table, 3).is_none()); + } +} diff --git a/crates/perry-codegen/src/codegen/mod.rs b/crates/perry-codegen/src/codegen/mod.rs index 592d956d69..dd853805ee 100644 --- a/crates/perry-codegen/src/codegen/mod.rs +++ b/crates/perry-codegen/src/codegen/mod.rs @@ -190,7 +190,9 @@ mod closure_collect; mod constructor_contracts; pub use constructor_contracts::{ConstructorContracts, ResolvedConstructorContracts}; mod ctor_arity; -pub use ctor_arity::{context_free_ctor_param_count, UNRESOLVED_PARENT_FWD_ARITY}; +pub use ctor_arity::{ + context_free_ctor_abi, context_free_ctor_param_count, CtorAbi, UNRESOLVED_PARENT_FWD_ARITY, +}; #[cfg(test)] mod declared_string_add_tests; #[cfg(test)] @@ -2475,6 +2477,7 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> has_own_constructor: ic.has_own_constructor, has_instance_fields: ic.has_instance_fields, has_rest: ic.constructor_has_rest, + has_synthetic_arguments: ic.constructor_has_synthetic_arguments, }, ) }) diff --git a/crates/perry-codegen/src/codegen/opts.rs b/crates/perry-codegen/src/codegen/opts.rs index 0b243882c6..cf6252b54f 100644 --- a/crates/perry-codegen/src/codegen/opts.rs +++ b/crates/perry-codegen/src/codegen/opts.rs @@ -553,14 +553,18 @@ 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 + /// Whether the source class's constructor declares a user `...rest` + /// parameter. 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 constructor reads `arguments`, i.e. its signature + /// ends in the HIR-synthesized `arguments` slot (after any user rest). That + /// slot receives a packed array of EVERY argument (#10484). + pub constructor_has_synthetic_arguments: bool, /// Whether the source class has instance fields that require initializer replay. pub has_instance_fields: bool, /// Method names defined on this class. @@ -677,6 +681,17 @@ pub struct ImportedClass { } impl ImportedClass { + /// The standalone-constructor ABI this class's defining module compiled, + /// as recorded on the import route (the constructor-contract resolver + /// overwrites all three fields for classes in the source graph). + pub(crate) fn ctor_abi(&self) -> super::ctor_arity::CtorAbi { + super::ctor_arity::CtorAbi { + param_count: self.constructor_param_count, + has_rest: self.constructor_has_rest, + has_synthetic_arguments: self.constructor_has_synthetic_arguments, + } + } + /// Consumer-side registry key for this class. pub fn effective_name(&self) -> String { let member = self.local_alias.as_deref().unwrap_or(&self.name); @@ -770,10 +785,13 @@ 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 + /// True when the constructor declares a user `...rest` param. 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, + /// True when the constructor's last param is the synthesized `arguments` + /// slot, which receives every argument packed into one array. + pub has_synthetic_arguments: bool, } impl ImportedCtor { diff --git a/crates/perry-codegen/src/codegen/string_pool.rs b/crates/perry-codegen/src/codegen/string_pool.rs index 6f68e99303..ef7b16e68d 100644 --- a/crates/perry-codegen/src/codegen/string_pool.rs +++ b/crates/perry-codegen/src/codegen/string_pool.rs @@ -7,6 +7,7 @@ use crate::module::LlModule; use crate::strings::StringPool; use crate::types::{DOUBLE, I32, I64, PTR, VOID}; +use super::ctor_arity::constructor_layout_params; use super::helpers::{sanitize, sanitize_member, scoped_static_method_name}; use super::retained_source_pool::{SourcePool, SourceRange}; use super::spec_function_length; @@ -950,12 +951,15 @@ pub(super) fn emit_string_pool( .unwrap_or(0) }); let ctor_symbol = format!("{}__{}_constructor", module_prefix, class_name); + // The trailing-array layout of the emitted standalone ctor. A class with + // no own ctor emits the `super(...args)` forwarder, which adopts the + // nearest local ancestor ctor's params positionally and hands every slot + // to that ctor unchanged, so it takes the same layout (#10484: a dynamic + // `new Sub(x)` must put all args in the ancestor's `arguments` slot). + let shape_params = constructor_layout_params(class, classes, ctor_params); // #wall3: record the rest-param position (in USER params) so the runtime // bundles trailing args at the dynamic member-new dispatch path. - if let Some(rest_idx) = class - .constructor - .as_ref() - .and_then(|c| c.params.iter().position(|p| p.is_rest)) + if let Some(rest_idx) = shape_params.and_then(|params| params.iter().position(|p| p.is_rest)) { ctor_rest_regs.push((ctor_symbol.clone(), rest_idx)); } @@ -964,13 +968,17 @@ pub(super) fn emit_string_pool( // a synthesized `arguments` slot receives ALL args (from index 0), a // user rest param only the args from the rest position onward. { - let last = class.constructor.as_ref().and_then(|c| c.params.last()); - let ctor_has_synth = last.map(|p| p.arguments_object.is_some()).unwrap_or(false); - let ctor_has_rest = class - .constructor - .as_ref() - .map(|c| { - c.params + // `any`, not `last`: a class declared inside a function carries + // synthesized `__perry_cap_*` params AFTER the `arguments` slot, so + // reading only the final param missed every capturing class — + // including every class in a compiled CommonJS module, whose + // wrapper function is what they capture from (#10484). + let ctor_has_synth = shape_params + .map(|params| params.iter().any(|p| p.arguments_object.is_some())) + .unwrap_or(false); + let ctor_has_rest = shape_params + .map(|params| { + params .iter() .any(|p| p.is_rest && p.arguments_object.is_none()) }) diff --git a/crates/perry-codegen/src/expr/readonly_collection_tests.rs b/crates/perry-codegen/src/expr/readonly_collection_tests.rs index 3e709d7b64..08682c916e 100644 --- a/crates/perry-codegen/src/expr/readonly_collection_tests.rs +++ b/crates/perry-codegen/src/expr/readonly_collection_tests.rs @@ -182,6 +182,7 @@ fn imported_archetype() -> ImportedClass { constructor_param_count: 0, has_own_constructor: true, constructor_has_rest: false, + constructor_has_synthetic_arguments: false, has_instance_fields: true, method_names: Vec::new(), proven_this_method_names: Vec::new(), diff --git a/crates/perry-codegen/src/lib.rs b/crates/perry-codegen/src/lib.rs index c8e5f33efa..708d0323b1 100644 --- a/crates/perry-codegen/src/lib.rs +++ b/crates/perry-codegen/src/lib.rs @@ -75,10 +75,11 @@ pub mod types; pub mod unit_cache; pub use codegen::{ - compile_module, context_free_ctor_param_count, namespace_member_class_key, - namespace_member_func_key, namespace_member_var_key, resolve_target_triple, - short_spread_method_capabilities, user_function_symbol, AppMetadata, CompileOptions, - ConstructorContracts, ExportedObjectLiteralCapability, FpContractMode, ImportedClass, + compile_module, context_free_ctor_abi, context_free_ctor_param_count, + namespace_member_class_key, namespace_member_func_key, namespace_member_var_key, + resolve_target_triple, short_spread_method_capabilities, user_function_symbol, AppMetadata, + CompileOptions, ConstructorContracts, CtorAbi, ExportedObjectLiteralCapability, FpContractMode, + ImportedClass, ImportedObjectLiteral, ImportedObjectLiteralMethod, NamespaceEntry, NamespaceEntryKind, ObjectLiteralMethodCandidate, ResolvedConstructorContracts, ShortSpreadMethodCandidate, }; diff --git a/crates/perry-codegen/src/lower_call/new_ctor_args.rs b/crates/perry-codegen/src/lower_call/new_ctor_args.rs index de9bd00491..24b72fbc5a 100644 --- a/crates/perry-codegen/src/lower_call/new_ctor_args.rs +++ b/crates/perry-codegen/src/lower_call/new_ctor_args.rs @@ -297,12 +297,13 @@ pub(super) fn lower_constructor_arg(ctx: &mut FnCtx<'_>, arg: &Expr) -> Result_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 +/// explicit slots, laid out as `[fixed..., user_rest?, arguments?]`. A user +/// `...rest` slot (`ctor.has_rest`) must receive a PACKED ARRAY of every +/// trailing arg — not the first trailing arg passed raw — and the synthesized +/// `arguments` slot (`ctor.has_synthetic_arguments`, #10484) a packed array of +/// EVERY arg. Mirrors the inline-ctor `inline_constructor_param_values` +/// packing and the `method_has_rest` path for imported methods (#672). Returns +/// exactly `ctor.param_count` value strings; missing fixed args are padded with /// `undefined`. pub(super) fn marshal_imported_ctor_args( ctx: &mut FnCtx<'_>, @@ -311,10 +312,9 @@ pub(super) fn marshal_imported_ctor_args( ) -> 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 trailing = usize::from(ctor.has_rest) + usize::from(ctor.has_synthetic_arguments); + if trailing > 0 && param_count >= trailing { + let n_positional = param_count - trailing; let mut out: Vec = Vec::with_capacity(param_count); for i in 0..n_positional { out.push( @@ -324,8 +324,13 @@ pub(super) fn marshal_imported_ctor_args( .unwrap_or_else(|| undef.clone()), ); } - let tail: Vec = lowered_args.iter().skip(n_positional).cloned().collect(); - out.push(pack_lowered_args_array(ctx, &tail)); + if ctor.has_rest { + let tail: Vec = lowered_args.iter().skip(n_positional).cloned().collect(); + out.push(pack_lowered_args_array(ctx, &tail)); + } + if ctor.has_synthetic_arguments { + out.push(pack_lowered_args_array(ctx, lowered_args)); + } out } else { // No rest: positional, padded to `param_count` with `undefined`. diff --git a/crates/perry-codegen/src/lower_call/typed_shape_bake_tests.rs b/crates/perry-codegen/src/lower_call/typed_shape_bake_tests.rs index fead87a8c6..ef8c19df45 100644 --- a/crates/perry-codegen/src/lower_call/typed_shape_bake_tests.rs +++ b/crates/perry-codegen/src/lower_call/typed_shape_bake_tests.rs @@ -492,6 +492,7 @@ fn imported_remote() -> ImportedClass { constructor_param_count: 1, has_own_constructor: true, constructor_has_rest: false, + constructor_has_synthetic_arguments: false, has_instance_fields: true, method_names: vec!["read".to_string()], proven_this_method_names: Vec::new(), diff --git a/crates/perry-hir/src/monomorph/defaults.rs b/crates/perry-hir/src/monomorph/defaults.rs index 1e87a1f7cd..1d5e7c21c3 100644 --- a/crates/perry-hir/src/monomorph/defaults.rs +++ b/crates/perry-hir/src/monomorph/defaults.rs @@ -34,6 +34,16 @@ pub(crate) fn fill_default_arguments(module: &mut Module) { let mut ctors: HashMap>> = HashMap::new(); for class in &module.classes { if let Some(ref ctor) = class.constructor { + // #10484: a constructor that reads `arguments` observes the call + // site's argument COUNT, and an appended `undefined` is + // indistinguishable from one the caller wrote (`new C("x")` + // reported `arguments.length === 2` for `constructor(p, q)`). + // Every constructor call path already binds an omitted parameter + // to `undefined` itself, so skip these exactly like the + // synth-`arguments` free functions below. + if ctor.params.iter().any(|p| p.arguments_object.is_some()) { + continue; + } // Stop at a trailing rest parameter. `constructor(...e)` (or // `constructor(a, b = 5, ...e)`) accepts zero or more trailing // args, so the call-site padding below must never synthesize an diff --git a/crates/perry-hir/src/monomorph/tests.rs b/crates/perry-hir/src/monomorph/tests.rs index 71b357be6f..533d60c064 100644 --- a/crates/perry-hir/src/monomorph/tests.rs +++ b/crates/perry-hir/src/monomorph/tests.rs @@ -1022,3 +1022,118 @@ fn a_specialized_class_reports_the_generics_display_name() { "only the specialization gets an entry" ); } + +/// #10484: a constructor that reads `arguments` must see the call site's own +/// argument count, so the default-fill pass may not pad its `new` sites. +#[test] +fn fill_defaults_skips_constructors_that_read_arguments() { + fn class_with_ctor(name: &str, id: u32, reads_arguments: bool) -> Class { + let mut params = vec![ + Param { + id: 1, + name: "p".to_string(), + ty: Type::Any, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + }, + Param { + id: 2, + name: "q".to_string(), + ty: Type::Any, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + }, + ]; + if reads_arguments { + params.push(Param { + id: 3, + name: "arguments".to_string(), + ty: Type::Any, + default: None, + decorators: Vec::new(), + is_rest: true, + arguments_object: Some(crate::ArgumentsObjectMeta { + strict: true, + simple_parameters: false, + mapped_parameter_ids: Vec::new(), + restricted_callee: true, + }), + }); + } + Class { + id, + name: name.to_string(), + type_params: Vec::new(), + extends: None, + extends_name: None, + native_extends: None, + extends_expr: None, + heritage_lexically_shadowed: false, + fields: Vec::new(), + constructor: Some(Function { + id: 100 + id, + name: format!("{}_constructor", name), + type_params: Vec::new(), + params, + return_type: Type::Void, + body: Vec::new(), + is_async: false, + is_generator: false, + is_strict: true, + was_plain_async: false, + was_unrolled: false, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + }), + methods: Vec::new(), + getters: Vec::new(), + setters: Vec::new(), + static_accessor_names: Vec::new(), + static_accessor_fn_ids: Vec::new(), + static_fields: Vec::new(), + static_methods: Vec::new(), + computed_members: Vec::new(), + decorators: Vec::new(), + is_exported: false, + is_nested: false, + alloc_width_hint: 0, + specialized_from: None, + aliases: Vec::new(), + } + } + + let mut module = Module::new("test"); + module.classes.push(class_with_ctor("Plain", 1, false)); + module.classes.push(class_with_ctor("Args", 2, true)); + for class_name in ["Plain", "Args"] { + module.init.push(Stmt::Expr(Expr::New { + class_name: class_name.to_string(), + args: vec![Expr::String("x".to_string())], + type_args: Vec::new(), + byte_offset: 0, + cap_args_appended: 0, + })); + } + + super::fill_default_arguments(&mut module); + + let arg_counts: Vec = module + .init + .iter() + .map(|stmt| match stmt { + Stmt::Expr(Expr::New { args, .. }) => args.len(), + other => panic!("unexpected statement {:?}", other), + }) + .collect(); + assert_eq!( + arg_counts, + vec![2, 1], + "the plain constructor keeps its `undefined` padding; the one reading \ + `arguments` must observe exactly the argument the call site passed" + ); +} diff --git a/crates/perry-runtime/src/object/class_constructors.rs b/crates/perry-runtime/src/object/class_constructors.rs index 6286afc168..f6d0ae2e4b 100644 --- a/crates/perry-runtime/src/object/class_constructors.rs +++ b/crates/perry-runtime/src/object/class_constructors.rs @@ -189,6 +189,78 @@ fn lookup_class_constructor_flags(class_id: u32) -> (bool, bool) { .unwrap_or((false, false)) } +/// Bind the USER parameter slots of `ctor_cid`'s registered constructor (every +/// slot before its trailing `__perry_cap_*` params) from a construct call's +/// full argument list. +/// +/// Codegen lowers the trailing array parameters as +/// `[fixed..., user_rest?, synthesized_arguments?]` and registers the position +/// of the first one in the closure-rest table (`ctor_rest_regs`). The flags say +/// which arrays follow: a user rest receives the arguments from that position +/// on, the synthesized `arguments` slot receives EVERY argument. +/// +/// #10484: the dynamic construct paths used to pack only a user-rest tail at +/// that position, so a constructor reading `arguments` saw just the arguments +/// past its declared parameters. `new K("x")` for `constructor(p, q)` reported +/// `arguments.length === 0`, which is how undici's `new Request(url)` failed +/// its own `argumentLengthCheck(arguments, 1)`. +/// +/// The returned words are not rooted. Callers hand them to the constructor call +/// without allocating in between. +unsafe fn constructor_user_arg_slots( + ctor_ptr: usize, + ctor_cid: u32, + user_params: usize, + args_ptr: *const f64, + args_len: usize, +) -> Vec { + let undef = f64::from_bits(crate::value::TAG_UNDEFINED); + let arg = |i: usize| { + if !args_ptr.is_null() && i < args_len { + *args_ptr.add(i) + } else { + undef + } + }; + let (has_synth, flagged_rest) = lookup_class_constructor_flags(ctor_cid); + // An unflagged registration is a plain `constructor(a, ...rest)`. + let has_rest = flagged_rest || !has_synth; + let trailing = usize::from(has_rest) + usize::from(has_synth); + let Some(fixed) = crate::closure::lookup_closure_rest(ctor_ptr as *const u8) + .map(|fixed| fixed as usize) + .filter(|fixed| fixed + trailing <= user_params) + else { + return (0..user_params).map(arg).collect(); + }; + + let scope = crate::gc::RuntimeHandleScope::new(); + let supplied: Vec = (0..args_len).map(arg).collect(); + let supplied_handles = scope.root_nanbox_f64_slice(&supplied); + let user_rest = has_rest.then(|| { + let refreshed = + crate::gc::RuntimeHandleScope::refreshed_nanbox_f64_slice(&supplied_handles); + let tail = &refreshed[fixed.min(refreshed.len())..]; + scope.root_nanbox_f64(crate::closure::build_rest_array(tail, false)) + }); + let arguments = has_synth.then(|| { + let refreshed = + crate::gc::RuntimeHandleScope::refreshed_nanbox_f64_slice(&supplied_handles); + scope.root_nanbox_f64(crate::closure::build_rest_array(&refreshed, true)) + }); + + let mut slots = Vec::with_capacity(user_params); + for i in 0..fixed { + slots.push( + supplied_handles + .get(i) + .map_or(undef, |handle| handle.get_nanbox_f64()), + ); + } + slots.extend(user_rest.map(|handle| handle.get_nanbox_f64())); + slots.extend(arguments.map(|handle| handle.get_nanbox_f64())); + slots +} + crate::perry_thread_local! { /// Decl-site snapshots of a function-nested class DECLARATION's captured /// outer locals, keyed by class_id. Filled by the codegen-emitted @@ -514,37 +586,16 @@ pub unsafe extern "C" fn js_super_construct_apply( // would swallow) — then append exactly `sig_caps` snapshot // values. Call with synth/rest OFF since we packed the trailing // slot manually. - let mut fa: Vec = Vec::with_capacity(total_params as usize); - let rest_idx = crate::closure::lookup_closure_rest(ctor_ptr as *const u8) - .map(|ri| ri as usize) - .filter(|ri| *ri < user_params); - if let Some(ri) = rest_idx { - for i in 0..ri { - fa.push(if i < n { - crate::array::js_array_get_f64(arr, i as u32) - } else { - undef - }); - } - let mut rest_arr = crate::array::js_array_alloc(0); - let mut i = ri; - while i < n { - rest_arr = crate::array::js_array_push_f64( - rest_arr, - crate::array::js_array_get_f64(arr, i as u32), - ); - i += 1; - } - fa.push(crate::value::js_nanbox_pointer(rest_arr as i64)); - } else { - for i in 0..user_params { - fa.push(if i < n { - crate::array::js_array_get_f64(arr, i as u32) - } else { - undef - }); - } - } + let spread: Vec = (0..n) + .map(|i| crate::array::js_array_get_f64(arr, i as u32)) + .collect(); + let mut fa = constructor_user_arg_slots( + ctor_ptr, + cur, + user_params, + spread.as_ptr(), + spread.len(), + ); for slot in 0..sig_caps as usize { fa.push(caps.get(slot).map(|b| f64::from_bits(*b)).unwrap_or(undef)); } @@ -923,33 +974,8 @@ pub(crate) unsafe fn run_class_constructor_on_this_flat( crate::closure::lookup_closure_rest(ctor_ptr as *const u8) ); } - let get = |i: usize| -> f64 { - if !args_ptr.is_null() && i < args_len { - unsafe { *args_ptr.add(i) } - } else { - undef - } - }; - let mut final_args: Vec = Vec::with_capacity(total_params as usize); - let rest_idx = crate::closure::lookup_closure_rest(ctor_ptr as *const u8) - .map(|ri| ri as usize) - .filter(|ri| *ri < user_params); - if let Some(ri) = rest_idx { - for i in 0..ri { - final_args.push(get(i)); - } - let mut rest_arr = crate::array::js_array_alloc(0); - let mut i = ri; - while i < args_len { - rest_arr = crate::array::js_array_push_f64(rest_arr, get(i)); - i += 1; - } - final_args.push(crate::value::js_nanbox_pointer(rest_arr as i64)); - } else { - for i in 0..user_params { - final_args.push(get(i)); - } - } + let mut final_args = + constructor_user_arg_slots(ctor_ptr, cur, user_params, args_ptr, args_len); for slot in 0..sig_caps as usize { final_args.push(caps.get(slot).map(|b| f64::from_bits(*b)).unwrap_or(undef)); } @@ -1276,45 +1302,17 @@ pub(crate) unsafe fn replay_class_object_constructor( // exists, and the old `max(per-eval, snapshot)` subtraction ate user args). let user_params = (total_params as usize).saturating_sub(sig_caps as usize); let undef = f64::from_bits(crate::value::TAG_UNDEFINED); - let mut final_args: Vec = Vec::with_capacity(total_params as usize); // #wall3: a `constructor(...args)` (rest param) called via the dynamic // member-new path (`new ns.Sub(opts)` → js_new_function_construct → // is_class_object_value → here) must BUNDLE the trailing call args into a JS // array for the rest slot. call_vtable_method's own `has_rest` can't do it // because the rest param is NOT last here — the positional `__perry_cap_*` - // capture params follow it — so we pack the rest array ourselves at the rest - // index, then append caps. Without this the rest binds to the first arg as a + // capture params follow it — so the trailing arrays are packed before the + // caps are appended. Without this the rest binds to the first arg as a // scalar (`args`=opts, not [opts]) and `super(...args)` spreads a bare object // → 0x400000000 mis-box → crash (Next.js `new c.AppPageRouteModule({...})`). - let rest_idx = crate::closure::lookup_closure_rest(ctor_ptr as *const u8) - .map(|ri| ri as usize) - .filter(|ri| *ri < user_params); - if let Some(ri) = rest_idx { - for i in 0..ri { - if !args_ptr.is_null() && i < args_len { - final_args.push(*args_ptr.add(i)); - } else { - final_args.push(undef); - } - } - let mut rest_arr = crate::array::js_array_alloc(0); - if !args_ptr.is_null() { - let mut i = ri; - while i < args_len { - rest_arr = crate::array::js_array_push_f64(rest_arr, *args_ptr.add(i)); - i += 1; - } - } - final_args.push(crate::value::js_nanbox_pointer(rest_arr as i64)); - } else { - for i in 0..user_params { - if !args_ptr.is_null() && i < args_len { - final_args.push(*args_ptr.add(i)); - } else { - final_args.push(undef); - } - } - } + let mut final_args = + constructor_user_arg_slots(ctor_ptr, ctor_cid, user_params, args_ptr, args_len); // Exactly `sig_caps` trailing cap slots: per-evaluation snapshot first // (class EXPRESSIONS carry `__perry_ctor_caps`), decl-site snapshot second // (class DECLARATIONS reached as heap values), undefined last. @@ -1407,46 +1405,18 @@ pub(crate) unsafe fn replay_registered_class_constructor( let user_params = (total_params as usize).saturating_sub(sig_caps as usize); let undef = f64::from_bits(crate::value::TAG_UNDEFINED); - let mut final_args: Vec = Vec::with_capacity(total_params as usize); // #wall3: a `constructor(...args)` reached via the dynamic class-REF member-new // path (`new ns.Sub(opts)` where ns.Sub resolves to an INT32 ClassRef at // runtime → js_new_function_construct → constructor_class_ref_id → // construct_registered_class_ref → here) must BUNDLE trailing call args into a // JS array for the rest slot. The rest is NOT the last ctor param (positional // `__perry_cap_*` capture params follow it), so call_vtable_method's own - // `has_rest` can't pack it — we pack the rest array ourselves at the rest - // index, then append caps. Without this the rest binds to the first arg as a + // `has_rest` can't pack it — the trailing arrays are packed before the caps + // are appended. Without this the rest binds to the first arg as a // scalar (`args`=opts, not [opts]) and `super(...args)` spreads a bare object // → 0x400000000 mis-box → crash (Next.js `new c.AppPageRouteModule({...})`). - let rest_idx = crate::closure::lookup_closure_rest(ctor_ptr as *const u8) - .map(|ri| ri as usize) - .filter(|ri| *ri < user_params); - if let Some(ri) = rest_idx { - for i in 0..ri { - if !args_ptr.is_null() && i < args_len { - final_args.push(*args_ptr.add(i)); - } else { - final_args.push(undef); - } - } - let mut rest_arr = crate::array::js_array_alloc(0); - if !args_ptr.is_null() { - let mut i = ri; - while i < args_len { - rest_arr = crate::array::js_array_push_f64(rest_arr, *args_ptr.add(i)); - i += 1; - } - } - final_args.push(crate::value::js_nanbox_pointer(rest_arr as i64)); - } else { - for i in 0..user_params { - if !args_ptr.is_null() && i < args_len { - final_args.push(*args_ptr.add(i)); - } else { - final_args.push(undef); - } - } - } + let mut final_args = + constructor_user_arg_slots(ctor_ptr, ctor_cid, user_params, args_ptr, args_len); for slot in 0..sig_caps as usize { final_args.push(caps.get(slot).map(|b| f64::from_bits(*b)).unwrap_or(undef)); } @@ -1460,3 +1430,110 @@ pub(crate) unsafe fn replay_registered_class_constructor( false, ) } + +#[cfg(test)] +mod constructor_arg_slot_tests { + use super::*; + + /// Distinct registry keys per case: `CLASS_CONSTRUCTOR_FLAGS` is + /// process-global and the closure-rest table is keyed by function pointer. + fn key(case: u32) -> (usize, u32) { + // Any stable non-null address works — the helper only READS the + // registrations under this key, it never calls through the pointer. + let ptr = (0x10_484_000usize) + case as usize * 0x40; + (ptr, 10_484_000 + case) + } + + fn array_of(value: f64) -> (usize, u32) { + let arr = crate::value::js_nanbox_get_pointer(value) as *const crate::array::ArrayHeader; + assert!(!arr.is_null(), "expected a packed array, got {value}"); + let len = unsafe { crate::array::js_array_length(arr) }; + (arr as usize, len) + } + + fn element(value: f64, index: u32) -> f64 { + let arr = crate::value::js_nanbox_get_pointer(value) as *const crate::array::ArrayHeader; + unsafe { crate::array::js_array_get_f64(arr, index) } + } + + #[test] + fn the_synthesized_arguments_slot_takes_every_argument() { + let (ptr, cid) = key(1); + // `constructor(p, q)` reading `arguments`: two fixed slots, then the + // synthesized array at index 2. + js_register_class_constructor_flags(cid as i64, 1, 0); + crate::closure::js_register_closure_rest(ptr as *const u8, 2); + + let args = [11.0, 22.0, 33.0]; + let slots = + unsafe { constructor_user_arg_slots(ptr, cid, 3, args.as_ptr(), args.len()) }; + assert_eq!(slots.len(), 3); + assert_eq!(slots[0], 11.0); + assert_eq!(slots[1], 22.0); + let (arr, len) = array_of(slots[2]); + assert_eq!(len, 3, "`arguments` must hold every supplied argument"); + assert_eq!(element(slots[2], 2), 33.0); + assert!( + unsafe { + crate::array::array_has_arguments_object_flag( + arr as *const crate::array::ArrayHeader, + ) + }, + "the packed array must be marked as an Arguments object" + ); + + // Fewer arguments than declared parameters: the fixed slots pad with + // `undefined` while `arguments.length` stays at what was passed. + let one = [11.0]; + let slots = unsafe { constructor_user_arg_slots(ptr, cid, 3, one.as_ptr(), one.len()) }; + assert_eq!(slots[1].to_bits(), crate::value::TAG_UNDEFINED); + assert_eq!(array_of(slots[2]).1, 1); + + let slots = unsafe { constructor_user_arg_slots(ptr, cid, 3, std::ptr::null(), 0) }; + assert_eq!(array_of(slots[2]).1, 0); + } + + #[test] + fn a_user_rest_and_arguments_constructor_fills_both_arrays() { + let (ptr, cid) = key(2); + // `constructor(first, ...rest)` reading `arguments`: one fixed slot, + // the rest array at index 1, the full argument list at index 2. + js_register_class_constructor_flags(cid as i64, 1, 1); + crate::closure::js_register_closure_rest(ptr as *const u8, 1); + + let args = [11.0, 22.0, 33.0]; + let slots = + unsafe { constructor_user_arg_slots(ptr, cid, 3, args.as_ptr(), args.len()) }; + assert_eq!(slots.len(), 3); + assert_eq!(slots[0], 11.0); + assert_eq!(array_of(slots[1]).1, 2, "rest holds the tail only"); + assert_eq!(element(slots[1], 0), 22.0); + assert_eq!(array_of(slots[2]).1, 3, "`arguments` holds all three"); + } + + #[test] + fn an_unflagged_rest_constructor_keeps_tail_only_packing() { + let (ptr, cid) = key(3); + // No flags registered at all — a plain `constructor(a, ...rest)`. + crate::closure::js_register_closure_rest(ptr as *const u8, 1); + + let args = [11.0, 22.0, 33.0]; + let slots = + unsafe { constructor_user_arg_slots(ptr, cid, 2, args.as_ptr(), args.len()) }; + assert_eq!(slots.len(), 2); + assert_eq!(slots[0], 11.0); + assert_eq!(array_of(slots[1]).1, 2); + } + + #[test] + fn a_constructor_with_no_trailing_array_stays_positional() { + let (_, cid) = key(4); + // An unregistered constructor pointer: positional, padded to the + // declared parameter count. + let args = [11.0]; + let slots = unsafe { constructor_user_arg_slots(0x10_484_900, cid, 2, args.as_ptr(), 1) }; + assert_eq!(slots.len(), 2); + assert_eq!(slots[0], 11.0); + assert_eq!(slots[1].to_bits(), crate::value::TAG_UNDEFINED); + } +} diff --git a/crates/perry/src/commands/compile/object_cache.rs b/crates/perry/src/commands/compile/object_cache.rs index 64e7f9f927..30433f1e59 100644 --- a/crates/perry/src/commands/compile/object_cache.rs +++ b/crates/perry/src/commands/compile/object_cache.rs @@ -645,6 +645,14 @@ fn compute_object_cache_key_with_env( buf.push_str(namespace); buf.push('|'); } + // #10484: the constructor's trailing-array layout decides how a + // `new` site packs its arguments. Only constructors reading + // `arguments` add a component, so other keys stay byte-identical. + if c.constructor_has_synthetic_arguments { + buf.push_str(":ctor_arguments=1:ctor_rest="); + buf.push_str(if c.constructor_has_rest { "1" } else { "0" }); + buf.push('|'); + } buf.push_str("method_rest="); buf.push_str( &c.method_has_rest diff --git a/crates/perry/src/commands/compile/object_cache/object_cache_tests.rs b/crates/perry/src/commands/compile/object_cache/object_cache_tests.rs index 77662c1c25..4962c52b1b 100644 --- a/crates/perry/src/commands/compile/object_cache/object_cache_tests.rs +++ b/crates/perry/src/commands/compile/object_cache/object_cache_tests.rs @@ -354,6 +354,7 @@ fn key_stable_for_nested_type_hashmap_order() { constructor_param_count: 0, has_own_constructor: false, constructor_has_rest: false, + constructor_has_synthetic_arguments: false, has_instance_fields: true, method_names: vec![], proven_this_method_names: vec![], @@ -415,6 +416,7 @@ fn key_changes_with_imported_class_signature() { constructor_param_count: 1, has_own_constructor: true, constructor_has_rest: false, + constructor_has_synthetic_arguments: false, has_instance_fields: true, method_names: vec!["bar".into()], proven_this_method_names: vec![], @@ -449,6 +451,7 @@ fn key_changes_with_imported_class_signature() { constructor_param_count: 2, // different arity has_own_constructor: true, constructor_has_rest: false, + constructor_has_synthetic_arguments: false, has_instance_fields: true, method_names: vec!["bar".into()], proven_this_method_names: vec![], @@ -491,6 +494,7 @@ fn key_changes_with_imported_class_codegen_surface() { constructor_param_count: 1, has_own_constructor: true, constructor_has_rest: false, + constructor_has_synthetic_arguments: false, has_instance_fields: true, method_names: vec!["bar".into()], proven_this_method_names: vec![], diff --git a/crates/perry/src/commands/compile/run_pipeline.rs b/crates/perry/src/commands/compile/run_pipeline.rs index 16e2fb4457..a4091b216d 100644 --- a/crates/perry/src/commands/compile/run_pipeline.rs +++ b/crates/perry/src/commands/compile/run_pipeline.rs @@ -288,6 +288,7 @@ fn imported_class_from_hir( proven_this_method_names: Vec, proven_this_tower_method_names: Vec, ) -> perry_codegen::ImportedClass { + let ctor_abi = perry_codegen::context_free_ctor_abi(class).unwrap_or_default(); perry_codegen::ImportedClass { name: class.name.clone(), local_alias, @@ -300,11 +301,11 @@ fn imported_class_from_hir( .as_ref() .map_or(0, |ctor| ctor.params.len()), has_own_constructor: class.constructor.is_some(), - constructor_has_rest: class - .constructor - .as_ref() - .map(|ctor| ctor.params.iter().any(|param| param.is_rest)) - .unwrap_or(false), + // Trailing array slots this class's own constructor declares. A + // no-own-ctor class emits a positional forwarder, so it reports none + // until the constructor contract resolves its ancestor's ABI (#10484). + constructor_has_rest: ctor_abi.has_rest, + constructor_has_synthetic_arguments: ctor_abi.has_synthetic_arguments, has_instance_fields: !class.fields.is_empty(), method_names: class .methods @@ -436,6 +437,7 @@ fn imported_object_literal_from_capability( constructor_param_count: capability.field_names.len(), has_own_constructor: true, constructor_has_rest: false, + constructor_has_synthetic_arguments: false, has_instance_fields: !capability.field_names.is_empty(), method_names: Vec::new(), proven_this_method_names: Vec::new(), diff --git a/test-files/fixtures/issue_10484_ctor_arguments/classes.ts b/test-files/fixtures/issue_10484_ctor_arguments/classes.ts new file mode 100644 index 0000000000..7130c87f27 --- /dev/null +++ b/test-files/fixtures/issue_10484_ctor_arguments/classes.ts @@ -0,0 +1,55 @@ +// #10484: classes whose constructors read `arguments`, imported by +// test_gap_10484_class_constructor_arguments.ts through ESM bindings. + +export function fmtArgs(a: ArrayLike): string { + return a.length + ":" + JSON.stringify(Array.from(a)); +} + +export class ExpTwo { + r: string; + constructor(p?: any, q?: any) { + this.r = fmtArgs(arguments); + } +} + +export class ExpDefault { + r: string; + constructor(p: any, q: any = "dq") { + this.r = fmtArgs(arguments) + " q=" + q; + } +} + +export class ExpRest { + r: string; + constructor(first?: any, ...rest: any[]) { + this.r = fmtArgs(arguments) + " rest=" + JSON.stringify(rest); + } +} + +export class ExpLength { + n: number; + constructor(p?: any, q?: any) { + this.n = arguments.length; + } +} + +export class ExpDerived extends ExpTwo { + constructor(x?: any) { + super(...arguments); + } +} + +export class ExpNoCtorDerived extends ExpTwo {} + +export class ExpNoCtorRest extends ExpRest {} + +const tag = "cap"; +export function makeCapturing() { + const local = tag + "-" + "tured"; + return class Capturing { + r: string; + constructor(p?: any) { + this.r = fmtArgs(arguments) + " " + local; + } + }; +} diff --git a/test-files/fixtures/issue_10484_ctor_arguments/request.cjs b/test-files/fixtures/issue_10484_ctor_arguments/request.cjs new file mode 100644 index 0000000000..80108b1375 --- /dev/null +++ b/test-files/fixtures/issue_10484_ctor_arguments/request.cjs @@ -0,0 +1,46 @@ +'use strict'; +// #10484: undici 8.x `lib/web/fetch/request.js` / webidl2js shapes. Every class +// in a compiled CommonJS package is constructed through a runtime value. + +function argumentLengthCheck({ length }, min, ctx) { + if (length < min) { + throw new TypeError(`${ctx}: ${min} argument${min !== 1 ? 's' : ''} required, but only ${length} found.`); + } +} + +class Request { + constructor(input, init = {}) { + argumentLengthCheck(arguments, 1, 'Request constructor'); + this.url = input; + this.init = init; + this.argc = arguments.length; + } +} + +class Headers { + constructor(init = undefined) { + this.argc = arguments.length; + this.list = Array.from(arguments); + } +} + +// A subclass that forwards `arguments` the way transpiled code does. +class StrictRequest extends Request { + constructor() { + super(...arguments); + this.forwarded = arguments.length; + } +} + +// `fn.apply(this, arguments)` inside a constructor. +function record(self, a, b) { + self.applied = [a, b]; +} +class Applier { + constructor(a, b) { + record.apply(null, [this].concat(Array.prototype.slice.call(arguments))); + this.argc = arguments.length; + } +} + +module.exports = { Request, Headers, StrictRequest, Applier, argumentLengthCheck }; diff --git a/test-files/test_gap_10484_class_constructor_arguments.ts b/test-files/test_gap_10484_class_constructor_arguments.ts new file mode 100644 index 0000000000..b3cf23e410 --- /dev/null +++ b/test-files/test_gap_10484_class_constructor_arguments.ts @@ -0,0 +1,291 @@ +// #10484: `arguments` inside a class CONSTRUCTOR must describe the call site. +// +// Two defects, one object: +// - a static `new C(x)` reported the DECLARED parameter count, because the +// call site was padded with `undefined` up to the declared arity before +// the arguments list was packed; +// - construction through a runtime value (`const K = C; new K(x)`, a class +// returned from a function, an imported or CommonJS class, +// `Reflect.construct`) reported an EMPTY list, because the dynamic +// construct path bound the synthesized `arguments` slot like a user rest +// parameter (only the arguments past the declared ones). +// +// undici 8.9.0's `new Request(url)` (webidl `argumentLengthCheck(arguments, 1)`) +// and whatwg-url's `new URL(href)` (a class declared inside `install()` that +// checks `arguments.length < 1`) both threw "1 argument required, but 0 found". +import cjs from "./fixtures/issue_10484_ctor_arguments/request.cjs"; +import * as esm from "./fixtures/issue_10484_ctor_arguments/classes.ts"; +import { ExpTwo, ExpDefault, fmtArgs } from "./fixtures/issue_10484_ctor_arguments/classes.ts"; + +function row(label: string, f: () => any): void { + try { + const o = f(); + console.log(label, "=>", o.r ?? o.n ?? o.argc ?? o.href ?? o.url); + } catch (e: any) { + console.log(label, "=> threw", e.constructor.name, e.message); + } +} + +// ── module-level classes ── +class Two { + r: string; + constructor(p?: any, q?: any) { + this.r = fmtArgs(arguments); + } +} +class NoParams { + r: string; + constructor() { + this.r = fmtArgs(arguments); + } +} +class WithDefault { + r: string; + constructor(p: any, q: any = "dq") { + this.r = fmtArgs(arguments) + " q=" + q; + } +} +class WithRest { + r: string; + constructor(first?: any, ...rest: any[]) { + this.r = fmtArgs(arguments) + " rest=" + JSON.stringify(rest); + } +} +class LengthOnly { + n: number; + constructor(p?: any, q?: any) { + this.n = arguments.length; + } +} +class Indexed { + r: string; + constructor(p?: any) { + this.r = [arguments[0], arguments[1], arguments[2]].map(String).join(","); + } +} +class WithField { + tag = "field"; + p: any; + r: string; + constructor(p?: any) { + this.p = p; + this.r = fmtArgs(arguments) + " " + this.tag + " p=" + this.p; + } +} + +console.log("-- static new --"); +row("Two()", () => new Two()); +row("Two(a)", () => new Two("a")); +row("Two(a,b)", () => new Two("a", "b")); +row("Two(a,b,c)", () => new Two("a", "b", "c")); +row("Two(undefined)", () => new Two(undefined)); +row("Two(a,undefined)", () => new Two("a", undefined)); +row("NoParams()", () => new NoParams()); +row("NoParams(a,b)", () => new NoParams("a", "b")); +row("WithDefault(a)", () => new WithDefault("a")); +row("WithDefault(a,b,c)", () => new WithDefault("a", "b", "c")); +row("WithRest()", () => new WithRest()); +row("WithRest(a)", () => new WithRest("a")); +row("WithRest(a,b,c)", () => new WithRest("a", "b", "c")); +row("LengthOnly()", () => new LengthOnly()); +row("LengthOnly(a)", () => new LengthOnly("a")); +row("LengthOnly(a,b,c)", () => new LengthOnly("a", "b", "c")); +row("Indexed(a)", () => new Indexed("a")); +row("Indexed(a,b,c)", () => new Indexed("a", "b", "c")); +row("WithField(a)", () => new WithField("a")); +row("WithField()", () => new WithField()); + +console.log("-- spread and Reflect.construct --"); +const none: any[] = []; +const one: any[] = ["s1"]; +const three: any[] = ["s1", "s2", "s3"]; +row("Two(...[])", () => new Two(...none)); +row("Two(...[1])", () => new Two(...one)); +row("Two(...[3])", () => new Two(...three)); +row("WithRest(...[3])", () => new WithRest(...three)); +row("Reflect.construct(Two,[])", () => Reflect.construct(Two, [])); +row("Reflect.construct(Two,[1])", () => Reflect.construct(Two, ["x"])); +row("Reflect.construct(Two,[3])", () => Reflect.construct(Two, ["x", "y", "z"])); +row("Reflect.construct(WithDefault,[1])", () => Reflect.construct(WithDefault, ["x"])); +row("Reflect.construct(LengthOnly,[1])", () => Reflect.construct(LengthOnly, ["x"])); + +console.log("-- class stored in a variable --"); +const V: any = Two; +const VD: any = WithDefault; +const VR: any = WithRest; +const VL: any = LengthOnly; +const VN: any = NoParams; +row("V()", () => new V()); +row("V(a)", () => new V("a")); +row("V(a,b,c)", () => new V("a", "b", "c")); +row("V(...[3])", () => new V(...three)); +row("VD(a)", () => new VD("a")); +row("VR(a,b,c)", () => new VR("a", "b", "c")); +row("VL(a)", () => new VL("a")); +row("VN(a,b)", () => new VN("a", "b")); +const table: Record = { Two, WithField }; +row("table.Two(a)", () => new table.Two("a")); +row("table[WithField](a)", () => new table["WithField"]("a")); + +console.log("-- class returned from / declared inside a function --"); +function makeInner() { + return class Inner { + r: string; + constructor(p?: any) { + this.r = fmtArgs(arguments); + } + }; +} +const Inner = makeInner(); +row("Inner()", () => new Inner()); +row("Inner(a)", () => new Inner("a")); +row("Inner(a,b)", () => new (Inner as any)("a", "b")); + +function makeCapturing(suffix: string) { + class Local { + r: string; + constructor(p?: any, q?: any) { + this.r = fmtArgs(arguments) + " " + suffix; + } + } + const direct = new Local("inside"); + console.log("Local(inside) static =>", direct.r); + return Local; +} +const Local: any = makeCapturing("captured"); +row("Local(a)", () => new Local("a")); +row("Local(a,b,c)", () => new Local("a", "b", "c")); + +// whatwg-url shape: class declared inside `install`, arity checked by hand. +function install(globalObject: any) { + const prefix = "Failed to construct 'URL': "; + class URL { + href: string; + constructor(url: any) { + if (arguments.length < 1) { + throw new TypeError(prefix + "1 argument required, but only " + arguments.length + " present."); + } + const args: string[] = []; + { + const curArg = arguments[0]; + args.push(String(curArg)); + } + { + const curArg = arguments[1]; + if (curArg !== undefined) args.push(String(curArg)); + } + this.href = args.join(" @ "); + } + } + globalObject.URL = URL; +} +const exportsObj: any = {}; +install(exportsObj); +row("URL(href)", () => new exportsObj.URL("http://a/")); +row("URL(href,base)", () => new exportsObj.URL("/p", "http://b/")); +row("URL()", () => new exportsObj.URL()); + +console.log("-- subclasses --"); +class SpreadArgs extends Two { + constructor(x?: any) { + super(...arguments); + } +} +class NoCtor extends Two {} +class Explicit extends Two { + d: string; + constructor(x?: any, y?: any, z?: any) { + super(x); + this.d = fmtArgs(arguments); + } +} +class RestForward extends Two { + constructor(...args: any[]) { + super(...args); + } +} +row("SpreadArgs(a)", () => new SpreadArgs("a")); +row("SpreadArgs(a,b,c)", () => new (SpreadArgs as any)("a", "b", "c")); +row("NoCtor()", () => new NoCtor()); +row("NoCtor(a)", () => new NoCtor("a")); +row("NoCtor(a,b,c)", () => new (NoCtor as any)("a", "b", "c")); +row("Explicit(a,b) base", () => new Explicit("a", "b")); +console.log("Explicit(a,b) own =>", new Explicit("a", "b").d); +row("RestForward(a)", () => new RestForward("a")); +row("RestForward(a,b,c)", () => new RestForward("a", "b", "c")); +const VS: any = SpreadArgs; +const VNC: any = NoCtor; +row("VS(a,b)", () => new VS("a", "b")); +row("VNC(a)", () => new VNC("a")); +row("VNC(a,b,c)", () => new VNC("a", "b", "c")); +const nt: any = Reflect.construct(Two, ["n1"], RestForward); +console.log("Reflect.construct(Two,[1],RestForward) =>", nt.r, nt instanceof RestForward); +class FromValue extends (V as any) { + constructor(a?: any) { + super(a, "extra"); + } +} +row("FromValue(a)", () => new FromValue("a")); + +console.log("-- imported ESM classes --"); +row("ExpTwo()", () => new ExpTwo()); +row("ExpTwo(a)", () => new ExpTwo("a")); +row("ExpTwo(a,b,c)", () => new (ExpTwo as any)("a", "b", "c")); +row("ExpDefault(a)", () => new ExpDefault("a")); +row("esm.ExpTwo(a)", () => new esm.ExpTwo("a")); +row("esm.ExpRest(a,b,c)", () => new esm.ExpRest("a", "b", "c")); +row("esm.ExpLength(a)", () => new esm.ExpLength("a")); +row("esm.ExpDerived(a)", () => new esm.ExpDerived("a")); +row("esm.ExpNoCtorDerived(a)", () => new esm.ExpNoCtorDerived("a")); +row("esm.ExpNoCtorDerived(a,b,c)", () => new (esm.ExpNoCtorDerived as any)("a", "b", "c")); +row("esm.ExpNoCtorRest(a,b,c)", () => new esm.ExpNoCtorRest("a", "b", "c")); +const ENCD: any = esm.ExpNoCtorDerived; +row("value ExpNoCtorDerived(a)", () => new ENCD("a")); +const Capturing: any = esm.makeCapturing(); +row("Capturing(a,b)", () => new Capturing("a", "b")); +const EV: any = esm.ExpTwo; +row("EV(a)", () => new EV("a")); + +console.log("-- CommonJS classes --"); +row("cjs.Request(url)", () => new cjs.Request("http://127.0.0.1:1/")); +row("cjs.Request(url,init)", () => new cjs.Request("http://127.0.0.1:1/", { method: "POST" })); +row("cjs.Request()", () => new cjs.Request()); +row("cjs.Headers()", () => new cjs.Headers()); +row("cjs.Headers(a,b)", () => new cjs.Headers("a", "b")); +row("cjs.StrictRequest(url)", () => new cjs.StrictRequest("http://x/")); +row("cjs.StrictRequest()", () => new cjs.StrictRequest()); +const applier = new cjs.Applier("p", "q"); +console.log("cjs.Applier(p,q) =>", applier.argc, JSON.stringify(applier.applied)); +const { Request: DestructuredRequest } = cjs; +row("DestructuredRequest(url)", () => new DestructuredRequest("http://d/")); + +// webidl shape declared in TypeScript. +function argumentLengthCheck({ length }: { length: number }, min: number, ctx: string) { + if (length < min) throw new TypeError(`${ctx}: ${min} argument required, but only ${length} found.`); +} +class WebRequest { + url: any; + constructor(input: any, init: any = {}) { + argumentLengthCheck(arguments, 1, "Request constructor"); + this.url = input; + } +} +row("WebRequest(url)", () => new WebRequest("http://w/")); +row("WebRequest() via value", () => new (WebRequest as any)()); + +console.log("-- function constructor controls --"); +function FnCtor(this: any, p?: any, q?: any) { + this.r = fmtArgs(arguments); +} +const FV: any = FnCtor; +row("FnCtor(a)", () => new (FnCtor as any)("a")); +row("FV()", () => new FV()); +row("FV(a,b,c)", () => new FV("a", "b", "c")); +row("Reflect.construct(FnCtor,[1])", () => Reflect.construct(FnCtor as any, ["x"])); + +console.log("-- hot loop --"); +let total = 0; +for (let i = 0; i < 1000; i++) { + total += new LengthOnly(i).n + new V(i, i).r.length + (i % 2 ? new VL() : new VL(i, i, i)).n; +} +console.log("total", total); From ca7dcb19bb2efa706b2f01d6687e7bbe6b741a5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 08:17:56 +0000 Subject: [PATCH 2/3] style: cargo fmt --- crates/perry-codegen/src/codegen/ctor_arity.rs | 5 ++++- crates/perry-codegen/src/codegen/string_pool.rs | 3 ++- crates/perry-codegen/src/lib.rs | 6 +++--- crates/perry-runtime/src/object/class_constructors.rs | 9 +++------ 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/crates/perry-codegen/src/codegen/ctor_arity.rs b/crates/perry-codegen/src/codegen/ctor_arity.rs index f221453511..2a796a9634 100644 --- a/crates/perry-codegen/src/codegen/ctor_arity.rs +++ b/crates/perry-codegen/src/codegen/ctor_arity.rs @@ -199,7 +199,10 @@ pub(super) fn constructor_layout_params<'a>( } if let Some(ctor) = ancestor.constructor.as_ref() { let positional = ctor.params.len() == emitted_param_count as usize - && !ctor.params.iter().any(|p| p.name.starts_with("__perry_cap_")); + && !ctor + .params + .iter() + .any(|p| p.name.starts_with("__perry_cap_")); return positional.then_some(ctor.params.as_slice()); } if ancestor.extends_expr.is_some() { diff --git a/crates/perry-codegen/src/codegen/string_pool.rs b/crates/perry-codegen/src/codegen/string_pool.rs index ef7b16e68d..d6570e7a6b 100644 --- a/crates/perry-codegen/src/codegen/string_pool.rs +++ b/crates/perry-codegen/src/codegen/string_pool.rs @@ -959,7 +959,8 @@ pub(super) fn emit_string_pool( let shape_params = constructor_layout_params(class, classes, ctor_params); // #wall3: record the rest-param position (in USER params) so the runtime // bundles trailing args at the dynamic member-new dispatch path. - if let Some(rest_idx) = shape_params.and_then(|params| params.iter().position(|p| p.is_rest)) + if let Some(rest_idx) = + shape_params.and_then(|params| params.iter().position(|p| p.is_rest)) { ctor_rest_regs.push((ctor_symbol.clone(), rest_idx)); } diff --git a/crates/perry-codegen/src/lib.rs b/crates/perry-codegen/src/lib.rs index 708d0323b1..84c4a8786c 100644 --- a/crates/perry-codegen/src/lib.rs +++ b/crates/perry-codegen/src/lib.rs @@ -79,9 +79,9 @@ pub use codegen::{ namespace_member_class_key, namespace_member_func_key, namespace_member_var_key, resolve_target_triple, short_spread_method_capabilities, user_function_symbol, AppMetadata, CompileOptions, ConstructorContracts, CtorAbi, ExportedObjectLiteralCapability, FpContractMode, - ImportedClass, - ImportedObjectLiteral, ImportedObjectLiteralMethod, NamespaceEntry, NamespaceEntryKind, - ObjectLiteralMethodCandidate, ResolvedConstructorContracts, ShortSpreadMethodCandidate, + ImportedClass, ImportedObjectLiteral, ImportedObjectLiteralMethod, NamespaceEntry, + NamespaceEntryKind, ObjectLiteralMethodCandidate, ResolvedConstructorContracts, + ShortSpreadMethodCandidate, }; pub use collectors::CjsPreambleCensus; // #9843: the segment-view for-of matcher's counter. Exported so the diff --git a/crates/perry-runtime/src/object/class_constructors.rs b/crates/perry-runtime/src/object/class_constructors.rs index f6d0ae2e4b..72f1bc3359 100644 --- a/crates/perry-runtime/src/object/class_constructors.rs +++ b/crates/perry-runtime/src/object/class_constructors.rs @@ -1465,8 +1465,7 @@ mod constructor_arg_slot_tests { crate::closure::js_register_closure_rest(ptr as *const u8, 2); let args = [11.0, 22.0, 33.0]; - let slots = - unsafe { constructor_user_arg_slots(ptr, cid, 3, args.as_ptr(), args.len()) }; + let slots = unsafe { constructor_user_arg_slots(ptr, cid, 3, args.as_ptr(), args.len()) }; assert_eq!(slots.len(), 3); assert_eq!(slots[0], 11.0); assert_eq!(slots[1], 22.0); @@ -1502,8 +1501,7 @@ mod constructor_arg_slot_tests { crate::closure::js_register_closure_rest(ptr as *const u8, 1); let args = [11.0, 22.0, 33.0]; - let slots = - unsafe { constructor_user_arg_slots(ptr, cid, 3, args.as_ptr(), args.len()) }; + let slots = unsafe { constructor_user_arg_slots(ptr, cid, 3, args.as_ptr(), args.len()) }; assert_eq!(slots.len(), 3); assert_eq!(slots[0], 11.0); assert_eq!(array_of(slots[1]).1, 2, "rest holds the tail only"); @@ -1518,8 +1516,7 @@ mod constructor_arg_slot_tests { crate::closure::js_register_closure_rest(ptr as *const u8, 1); let args = [11.0, 22.0, 33.0]; - let slots = - unsafe { constructor_user_arg_slots(ptr, cid, 2, args.as_ptr(), args.len()) }; + let slots = unsafe { constructor_user_arg_slots(ptr, cid, 2, args.as_ptr(), args.len()) }; assert_eq!(slots.len(), 2); assert_eq!(slots[0], 11.0); assert_eq!(array_of(slots[1]).1, 2); From 99b660e2f0f809330401cfe0585e5c95169bf313 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 10:52:23 +0000 Subject: [PATCH 3/3] docs(changelog): add fragment for #10612 --- changelog.d/10612-class-ctor-arguments.md | 34 +++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 changelog.d/10612-class-ctor-arguments.md diff --git a/changelog.d/10612-class-ctor-arguments.md b/changelog.d/10612-class-ctor-arguments.md new file mode 100644 index 0000000000..05d22ec5ec --- /dev/null +++ b/changelog.d/10612-class-ctor-arguments.md @@ -0,0 +1,34 @@ +### Fixed: `arguments` inside a class constructor now reflects the call site (#10484) + +The `arguments` object inside a class constructor didn't match the call: constructing +through a runtime value (a class stored in a variable, returned from a function, or +imported — including every CommonJS class) saw an **empty** `arguments`, and a static +`new C(x)` reported the constructor's **declared** parameter count instead of the +number of arguments actually passed. undici's `new Request(url)` and whatwg-url's +`new URL(href)` both threw spurious "argument required" errors as a result. + +**Root cause**, three layers: HIR padded a `new`-site's argument list with `undefined` +up to the declared arity before packing `arguments`, making a caller-omitted argument +indistinguishable from a declared one; the four runtime dynamic-construct paths (super- +apply, flat-ctor replay, class-object/registered-class replay) packed the synthesized +`arguments` slot like a user `...rest` parameter (only the arguments past the declared +count, empty for the common case); and codegen read a constructor's trailing-array +layout off its last declared parameter only, which misses every capturing constructor +— i.e. every CommonJS class, since Perry adds capture params mechanically. + +**Fix.** HIR skips arity padding for a constructor that reads `arguments`. The four +runtime dynamic-construct sites now share one helper, `constructor_user_arg_slots`, +that packs `arguments` from every call argument. Codegen gains a `CtorAbi` (param +count, has-rest, has-synthetic-arguments) computed from the constructor's actual +layout instead of its last parameter, threaded through constructor-contract +resolution, imported-class metadata, and cross-module `new`-site argument marshaling. + +**Validation.** New gap test `test_gap_10484_class_constructor_arguments.ts` (with an +undici-`Request`-shaped CommonJS fixture) fails on the base commit and passes here, +byte-identical to Node. A 45-test constructor/ABI regression sweep and the full +`perry-runtime`/`perry-codegen` unit suites are clean. Static construction and +value-construction without `arguments` usage are unaffected in instruction-count +perf; value-construction with an `arguments`-reading constructor — the case the bug +was actually about — costs roughly 20% more instructions on that specific path, a +correctness-necessitated cost of materializing a real `arguments` array where the +buggy path previously built nothing.