diff --git a/changelog.d/11235-capture-ctor-omitted-args.md b/changelog.d/11235-capture-ctor-omitted-args.md new file mode 100644 index 0000000000..fe96a7349f --- /dev/null +++ b/changelog.d/11235-capture-ctor-omitted-args.md @@ -0,0 +1,3 @@ +- **A capturing class constructed with fewer arguments than its constructor declares now sees `undefined`, and so its defaults, in the omitted parameters (#11229).** A class that reads an enclosing binding receives the captured values through synthesized trailing `__perry_cap_*` constructor params. That covers every class in a CommonJS module that reads a module-scope binding. The `new` site appends the captured values after the user arguments and records their count in `Expr::New::cap_args_appended`. The monomorph default-fill pass (`monomorph/defaults.rs`) then padded the call out to the constructor's full param count, which included the cap params, by appending `undefined` **after** those captures. So the captures shifted into the omitted user parameters, and the padding landed in the capture slots. mongodb 7.5.0's `new OnDemandDocument(this.bson, offset)` bound the module-scope `BSONElementOffset` object to `isArray`, and every cursor operation (`findOne`, `find().toArray()`, `countDocuments`, …) threw `TypeError: Cannot convert undefined or null to object`. The fill boundary now excludes the cap params, and the padding is spliced in between the user arguments and the appended captures. A constructor that reads `arguments` still skips padding (#10484), so `arguments.length` stays exact. +- `Reflect.construct(C, args)` now accepts a per-evaluation class object, meaning any capturing class or class returned from a factory. `is_constructor_function` only recognized closures and ClassRefs, so it threw `[object Function] is not a constructor` while `new C(...args)` worked. With a distinct `newTarget`, the result now takes `newTarget.prototype` (`construct_class_object_with_new_target`, the same approach as the Date arm). Before, it fell through to the plain-function tail. +- Files: `crates/perry-hir/src/monomorph/defaults.rs`, `crates/perry-runtime/src/proxy/apply_construct.rs`, `crates/perry-runtime/src/object/class_registry/construct.rs` + `construct/class_object.rs`. Unit test `monomorph::tests::fill_defaults_pads_before_appended_class_captures`. Gap test `test-files/test_gap_11229_capture_ctor_omitted_args.ts` (+ `fixtures/issue_11229_capture_ctor_arity/`) covers the mongodb shape, `new` from inside and outside the class, `new this.constructor`, defaults reading earlier params, rest params, spread arguments, explicit and implicit `super()` chains, `arguments.length`, `Reflect.construct` with and without `newTarget`, and a TypeScript factory class. diff --git a/crates/perry-hir/src/monomorph/defaults.rs b/crates/perry-hir/src/monomorph/defaults.rs index 1d5e7c21c3..77c4289cd8 100644 --- a/crates/perry-hir/src/monomorph/defaults.rs +++ b/crates/perry-hir/src/monomorph/defaults.rs @@ -52,10 +52,16 @@ pub(crate) fn fill_default_arguments(module: &mut Module) { // bogus element throws (marked's `new q` / hono's verb-method // setup). Only the leading fixed params (which DO get default-fill // checks prepended to the ctor body) are eligible for padding. + // + // #11229: the synthesized `__perry_cap_*` params a capturing + // class's constructor carries are not user parameters either -- + // the `new` site appends their values itself (`cap_args_appended`) + // -- so they never count toward the fill boundary. let defaults: Vec> = ctor .params .iter() .take_while(|p| !p.is_rest) + .filter(|p| !p.name.starts_with(crate::cap_fields::CAP_FIELD_PREFIX)) .map(|p| p.default.clone()) .collect(); ctors.insert(class.name.clone(), defaults); @@ -190,7 +196,10 @@ fn fill_defaults_in_stmt(stmt: &mut Stmt, cx: &DefaultFill) { fn fill_defaults_in_expr(expr: &mut Expr, cx: &DefaultFill) { match expr { Expr::New { - class_name, args, .. + class_name, + args, + cap_args_appended, + .. } => { // First, recurse into the arguments for arg in args.iter_mut() { @@ -200,16 +209,28 @@ fn fill_defaults_in_expr(expr: &mut Expr, cx: &DefaultFill) { // Check if we need to fill in defaults if let Some(defaults) = cx.ctors.get(class_name) { let param_count = defaults.len(); - let arg_count = args.len(); + // #11229: the trailing `cap_args_appended` args are the + // capturing class's captured values, which the constructor + // receives in its synthesized `__perry_cap_*` params AFTER + // every user param. Padding goes between the user args and + // them. Appending it after them instead left the captures in + // the omitted user params (`new Doc(bson, off)` against + // `constructor(bson, offset = 0, isArray = false, elements)` + // bound a captured module object to `isArray`) and handed the + // padding to the capture slots. + let caps = (*cap_args_appended as usize).min(args.len()); + let user_arg_count = args.len() - caps; - if arg_count < param_count { + if user_arg_count < param_count { // Fill missing constructor slots with `undefined`. // Constructor bodies already prepend default-param // checks, so default expressions must run in the // constructor boundary rather than at the `new` site. - for _ in arg_count..param_count { - args.push(Expr::Undefined); - } + let padding = param_count - user_arg_count; + args.splice( + user_arg_count..user_arg_count, + std::iter::repeat_n(Expr::Undefined, padding), + ); } } } diff --git a/crates/perry-hir/src/monomorph/tests.rs b/crates/perry-hir/src/monomorph/tests.rs index 533d60c064..f16d6f5a9a 100644 --- a/crates/perry-hir/src/monomorph/tests.rs +++ b/crates/perry-hir/src/monomorph/tests.rs @@ -1137,3 +1137,80 @@ fn fill_defaults_skips_constructors_that_read_arguments() { `arguments` must observe exactly the argument the call site passed" ); } + +/// #11229: a capturing class's `new` site appends its captured values after +/// the user arguments (`cap_args_appended`). When the call passes fewer +/// arguments than the constructor declares, the default-fill padding must go +/// BETWEEN the user args and those captures, and the synthesized +/// `__perry_cap_*` params never count toward the fill boundary. +#[test] +fn fill_defaults_pads_before_appended_class_captures() { + let source = r#" + function outer() { + const K = { tag: "K" }; + class Doc { + constructor(bson, offset = 0, isArray = false, elements) { + this.bson = bson; this.offset = offset; this.isArray = isArray; + this.elements = elements ?? [K.tag]; + } + child(o) { return new Doc(this.bson, o); } + } + return Doc; + } + "#; + let parsed = perry_parser::parse_typescript(source, "t.ts").expect("source parses"); + let mut module = crate::lower_module(&parsed, "t", "t.ts").expect("source lowers"); + crate::monomorph::monomorphize_module(&mut module); + let doc = module + .classes + .iter() + .find(|class| class.name == "Doc") + .expect("Doc is lowered"); + let ctor_caps = doc + .constructor + .as_ref() + .expect("Doc has a constructor") + .params + .iter() + .filter(|p| p.name.starts_with(crate::cap_fields::CAP_FIELD_PREFIX)) + .count(); + assert!(ctor_caps > 0, "Doc must be a capturing class for this test"); + let child = doc + .methods + .iter() + .find(|m| m.name == "child") + .expect("child method"); + let mut found = None; + fn find_new<'a>(stmts: &'a [Stmt], out: &mut Option<(&'a Vec, u32)>) { + for stmt in stmts { + if let Stmt::Return(Some(Expr::New { + class_name, + args, + cap_args_appended, + .. + })) = stmt + { + if class_name == "Doc" { + *out = Some((args, *cap_args_appended)); + } + } + } + } + find_new(&child.body, &mut found); + let (args, caps) = found.expect("child returns `new Doc(...)`"); + let caps = caps as usize; + assert_eq!(caps, ctor_caps, "every capture is appended at the new site"); + assert_eq!( + args.len(), + 4 + caps, + "two user args, padded to the four user params, then the captures: {args:?}" + ); + assert!( + matches!(args[2], Expr::Undefined) && matches!(args[3], Expr::Undefined), + "the padding must fill the omitted USER params: {args:?}" + ); + assert!( + args[4..].iter().all(|a| !matches!(a, Expr::Undefined)), + "the trailing capture args must be the captured values, not padding: {args:?}" + ); +} diff --git a/crates/perry-runtime/src/object/class_registry/construct.rs b/crates/perry-runtime/src/object/class_registry/construct.rs index fdd1c0953a..aab0f1fcdc 100644 --- a/crates/perry-runtime/src/object/class_registry/construct.rs +++ b/crates/perry-runtime/src/object/class_registry/construct.rs @@ -1686,6 +1686,10 @@ pub unsafe extern "C" fn js_new_function_construct_with_new_target( let instance_cid = new_target_class_id(nt).unwrap_or(target_cid); return construct_registered_class_ref(target_cid, instance_cid, nt, args_ptr, args_len); } + // #11229: a per-evaluation class object with a distinct newTarget. + if is_class_object_value(func_value) { + return construct_class_object_with_new_target(func_value, args_ptr, args_len, nt); + } // `Reflect.construct(Int8Array, [len], newTarget)` — a typed-array // constructor invoked with a distinct newTarget. Build the typed array the // normal way, then honor `GetPrototypeFromConstructor(newTarget)`: when diff --git a/crates/perry-runtime/src/object/class_registry/construct/class_object.rs b/crates/perry-runtime/src/object/class_registry/construct/class_object.rs index ca1852e707..2ea6b1ea88 100644 --- a/crates/perry-runtime/src/object/class_registry/construct/class_object.rs +++ b/crates/perry-runtime/src/object/class_registry/construct/class_object.rs @@ -65,3 +65,35 @@ unsafe fn construct_object_with_new_target(new_target: f64) -> f64 { } instance.with_mut_ptr::(|i| crate::value::js_nanbox_pointer(i as i64)) } + +/// #11229: `Reflect.construct(C, args, newTarget)` where `C` is a +/// per-evaluation class object (a capturing class) and `newTarget` is a +/// different constructor. Construct `C` the normal way -- that replays its +/// constructor with its own captures -- then honor +/// `GetPrototypeFromConstructor(newTarget)`, as the Date arm does. Falling +/// through to the generic tail instead ran the class as a plain function +/// against a bare object, so the result was not `instanceof newTarget`. +unsafe fn construct_class_object_with_new_target( + func_value: f64, + args_ptr: *const f64, + args_len: usize, + new_target: f64, +) -> f64 { + let scope = crate::gc::RuntimeHandleScope::new(); + let nt = scope.root_nanbox_f64(new_target); + let func = scope.root_nanbox_f64(func_value); + let proto = new_target_custom_object_prototype(nt.get_nanbox_f64()) + .map(|bits| scope.root_heap_word_u64(bits)); + let result = js_new_function_construct(func.get_nanbox_f64(), args_ptr, args_len); + if let Some(proto) = proto { + let jv = crate::value::JSValue::from_bits(result.to_bits()); + if jv.is_pointer() { + let addr = (jv.bits() & crate::value::POINTER_MASK) as usize; + super::super::prototype_chain::object_set_static_prototype( + addr, + proto.get_heap_word_u64(), + ); + } + } + result +} diff --git a/crates/perry-runtime/src/proxy/apply_construct.rs b/crates/perry-runtime/src/proxy/apply_construct.rs index d9beaa14b6..4ccaaebc76 100644 --- a/crates/perry-runtime/src/proxy/apply_construct.rs +++ b/crates/perry-runtime/src/proxy/apply_construct.rs @@ -43,6 +43,16 @@ pub(crate) fn is_constructor_function(value: f64) -> bool { .is_some_and(|e| e.constructable) }); } + // #11229: a per-evaluation class object (a class that captures its + // enclosing scope -- every such class in a CommonJS module, and any class + // returned from a factory) is a heap object, not a closure, so the + // callable check below rejected it and `Reflect.construct(C, args)` threw + // `is not a constructor` while `new C(...args)` worked. It is always a + // constructor, and `js_new_function_construct_with_new_target` already + // replays its constructor with its own captures. + if crate::object::is_class_object_value(value) { + return true; + } if !is_callable_function(value) { return false; } diff --git a/test-files/fixtures/issue_11229_capture_ctor_arity/document.cjs b/test-files/fixtures/issue_11229_capture_ctor_arity/document.cjs new file mode 100644 index 0000000000..9c114edbb7 --- /dev/null +++ b/test-files/fixtures/issue_11229_capture_ctor_arity/document.cjs @@ -0,0 +1,20 @@ +"use strict"; +// Mirrors mongodb 7.5.0's lib/cmap/wire_protocol/on_demand/document.js: the +// class reads module-scope bindings, so it is a capturing class, and it +// constructs itself with FEWER arguments than its constructor declares. +Object.defineProperty(exports, "__esModule", { value: true }); +exports.OnDemandDocument = void 0; +const BSONElementOffset = { type: 0, nameOffset: 1, nameLength: 2, offset: 3, length: 4 }; +function parseToElementsToArray(bson, offset) { return [bson.length, offset]; } +class OnDemandDocument { + constructor(bson, offset = 0, isArray = false, elements) { + this.cache = Object.create(null); + this.bson = bson; + this.offset = offset; + this.isArray = isArray; + this.elements = elements ?? parseToElementsToArray(this.bson, offset); + } + child(offset) { return new OnDemandDocument(this.bson, offset + BSONElementOffset.offset); } + childArray(offset) { return new OnDemandDocument(this.bson, offset, true); } +} +exports.OnDemandDocument = OnDemandDocument; diff --git a/test-files/fixtures/issue_11229_capture_ctor_arity/shapes.cjs b/test-files/fixtures/issue_11229_capture_ctor_arity/shapes.cjs new file mode 100644 index 0000000000..cd41c197fd --- /dev/null +++ b/test-files/fixtures/issue_11229_capture_ctor_arity/shapes.cjs @@ -0,0 +1,68 @@ +"use strict"; +const K = { tag: "K" }; +function helper(x) { return "h" + x; } +function norm(v) { + if (v === undefined) return ""; + if (Array.isArray(v)) return v.map(norm); + if (v !== null && typeof v === "object") { const out = {}; for (const k of Object.keys(v)) out[k] = norm(v[k]); return out; } + return v; +} +function show(o) { return JSON.stringify(norm(o)); } +class Doc { + constructor(bson, offset = 0, isArray = false, elements) { + this.n = arguments.length; + this.bson = bson; this.offset = offset; this.isArray = isArray; + this.elements = elements ?? [helper(bson), K.tag]; + } + child(o) { return new Doc(this.bson, o); } + childArr(o) { return new Doc(this.bson, o, true); } + viaThisCtor(o) { return new this.constructor(this.bson, o); } +} +class Dep { + constructor(a, b = a + 1, c = b * 2) { this.v = [a, b, c, arguments.length, K.tag]; } + static make1(a) { return new Dep(a); } + static make2(a, b) { return new Dep(a, b); } +} +class Rest { + constructor(first, ...more) { this.v = [first, more, arguments.length, helper(1)]; } + static none() { return new Rest(); } + static one() { return new Rest(1); } + static three() { return new Rest(1, 2, 3); } + static spread(xs) { return new Rest(...xs); } +} +class Spread { + constructor(a, b = "B", c) { this.v = [a, b, c, arguments.length, K.tag]; } + static s(xs) { return new Spread(...xs); } +} +class Base2 { + constructor(a, b = "b-default", c) { this.base = [a, b, c, arguments.length, K.tag]; } +} +class Mid extends Base2 { + constructor(a) { super(a); this.mid = helper(a); } +} +class Leaf extends Mid {} +class Implicit extends Base2 {} +class Many { + constructor(a, b, c, d, e) { this.v = [a, b, c, d, e, arguments.length, K.tag, helper(0)]; } + static zero() { return new Many(); } +} +// The same shapes WITHOUT `arguments` in the constructor: those are the ones +// the call-site padding pass rewrites (a ctor reading `arguments` is skipped). +class DepNA { + constructor(a, b = a + 1, c = b * 2) { this.v = [a, b, c, K.tag]; } + static make1(a) { return new DepNA(a); } + static make0() { return new DepNA(); } +} +class BaseNA { + constructor(a, b = "b-default", c = helper(a)) { this.base = [a, b, c, K.tag]; } +} +class MidNA extends BaseNA { + constructor(a) { super(a); this.mid = helper(a); } + static again(a) { return new MidNA(a); } +} +class LeafNA extends MidNA {} +class RestNA { + constructor(first, second = "S", ...more) { this.v = [first, second, more, helper(2)]; } + static one() { return new RestNA(1); } +} +module.exports = { Doc, Dep, Rest, Spread, Base2, Mid, Leaf, Implicit, Many, DepNA, BaseNA, MidNA, LeafNA, RestNA, show, K }; diff --git a/test-files/test_gap_11229_capture_ctor_omitted_args.ts b/test-files/test_gap_11229_capture_ctor_omitted_args.ts new file mode 100644 index 0000000000..fcc06c29c0 --- /dev/null +++ b/test-files/test_gap_11229_capture_ctor_omitted_args.ts @@ -0,0 +1,102 @@ +// #11229: a capturing class constructed with FEWER arguments than its +// constructor declares must see `undefined` (and so its defaults) in the +// omitted parameters. +// +// A class that reads an enclosing binding receives the captured values as +// synthesized trailing constructor params, which the `new` site appends +// after the user arguments. The monomorph default-fill pass then padded the +// call out to the constructor's full param count by appending `undefined` +// AFTER those captures, so the captures landed in the omitted user +// parameters. mongodb 7.5.0's `new OnDemandDocument(this.bson, offset)` bound +// the module-scope `BSONElementOffset` object to `isArray`, and every cursor +// operation threw `Cannot convert undefined or null to object`. +// +// Also covered: construction from outside the class, `new this.constructor`, +// defaults that read earlier params, rest params, spread arguments, super() +// chains (explicit and implicit constructors), `arguments.length`, and +// `Reflect.construct` (which rejected every per-evaluation class object). +import { OnDemandDocument } from "./fixtures/issue_11229_capture_ctor_arity/document.cjs"; +import * as S from "./fixtures/issue_11229_capture_ctor_arity/shapes.cjs"; + +const M: any = S; +const { show } = M; +function say(label: string, f: () => any) { + try { + console.log(label, show(f())); + } catch (e: any) { + console.log(label, "THREW", e?.constructor?.name, e?.message); + } +} +const pick = (d: any) => ({ offset: d.offset, isArray: d.isArray, elements: d.elements }); + +// 1. The mongodb shape. +const root: any = new (OnDemandDocument as any)("abc"); +say("odd.root", () => pick(root)); +say("odd.child", () => pick(root.child(1))); +say("odd.childArray", () => pick(root.childArray(2))); +say("odd.direct", () => pick(new (OnDemandDocument as any)("xy", 5))); +say("odd.reflect", () => pick(Reflect.construct(OnDemandDocument as any, ["r", 3]))); + +// 2. Inside/outside the class, arguments.length, new this.constructor. +const d = new M.Doc("abc"); +say("doc.root", () => d); +say("doc.child", () => d.child(4)); +say("doc.childArr", () => d.childArr(2)); +say("doc.viaThisCtor", () => d.viaThisCtor(9)); +say("doc.outside1", () => new M.Doc("xy")); +say("doc.outside2", () => new M.Doc("xy", 5)); + +// 3. Defaults that reference earlier params. +say("dep.make1", () => M.Dep.make1(1)); +say("dep.make2", () => M.Dep.make2(1, 5)); +say("dep.outside", () => new M.Dep(3)); + +// 4. Rest params and spread arguments. +say("rest.none", () => M.Rest.none()); +say("rest.one", () => M.Rest.one()); +say("rest.three", () => M.Rest.three()); +say("rest.spread", () => M.Rest.spread([4, 5])); +say("rest.outsideSpread", () => new M.Rest(...[7, 8, 9])); +say("spread.s1", () => M.Spread.s([1])); +say("spread.s3", () => M.Spread.s([1, 2, 3])); + +// 5. super() chains: explicit ctor forwarding fewer args, and implicit ctors. +say("mid", () => new M.Mid("m")); +say("leaf", () => new M.Leaf("l")); +say("implicit", () => new M.Implicit("i")); + +// 6. Reflect.construct on per-evaluation class objects. +say("reflect.doc", () => Reflect.construct(M.Doc, ["r"])); +say("reflect.dep", () => Reflect.construct(M.Dep, [2])); +say("reflect.mid", () => Reflect.construct(M.Mid, ["rm"])); +say("reflect.newTarget", () => Reflect.construct(M.Base2, ["nt"], M.Mid) instanceof M.Mid); + +// 7. Many omitted params, zero args. +say("many.zero", () => M.Many.zero()); +say("many.outside", () => new M.Many(1)); + +// 8. The same shapes without `arguments` in the constructor. +say("na.dep1", () => M.DepNA.make1(1)); +say("na.dep0", () => M.DepNA.make0()); +say("na.depOutside", () => new M.DepNA(3)); +say("na.mid", () => new M.MidNA("m")); +say("na.midAgain", () => M.MidNA.again("x")); +say("na.leaf", () => new M.LeafNA("l")); +say("na.rest", () => M.RestNA.one()); +say("na.reflectLeaf", () => Reflect.construct(M.LeafNA, ["rl"])); + +// 9. A TS factory class capturing a parameter, constructed with omitted args +// from inside and outside. +function makeNode(tag: string) { + return class Node { + tag: string; a: any; b: any; c: any; n: number; + constructor(a?: any, b: any = "B", c?: any) { + this.tag = tag; this.a = a; this.b = b; this.c = c; this.n = arguments.length; + } + kid() { return new Node(1); } + }; +} +const N: any = makeNode("t"); +say("ts.outside", () => new N()); +say("ts.kid", () => new N(0).kid()); +say("ts.reflect", () => Reflect.construct(N, [5]));