From 02c7d164e7e83bf414b3de4fd49db5b92986a287 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 20 Sep 2026 15:32:14 +0200 Subject: [PATCH 1/2] perf(codegen): build ObjectRest's excluded-key array as a literal, not N calls const {a, b, ...rest} = obj built the excluded-key array via one js_array_alloc_with_length call plus one js_array_set_f64_unchecked call per excluded key. Each per-key call re-derives and re-bounds-checks a receiver the site just allocated itself, so every check inside (frozen?, has index descriptors?, index in range?) was statically true. The excluded keys are compile-time-known string literals, so route them through lower_array_literal instead -- the same inline bump-allocation path an ordinary [k1, k2, ...] literal already takes, previously reused only for the rest/arguments call-bundle case. Also reorders so the source object's pointer is derived after that allocation instead of cached across it. Found and documented (not fixed, unrelated to this change): {...rest} drops Symbol-keyed properties from the source object instead of copying them through -- reproduces identically on unmodified main. --- .../destructuring-rest-array-literal.md | 31 ++++ crates/perry-codegen/src/expr/bigint_set.rs | 59 +++++-- ...ject_destructuring_field_and_rest_guard.ts | 155 ++++++++++++++++++ 3 files changed, 227 insertions(+), 18 deletions(-) create mode 100644 changelog.d/destructuring-rest-array-literal.md create mode 100644 test-files/test_gap_object_destructuring_field_and_rest_guard.ts diff --git a/changelog.d/destructuring-rest-array-literal.md b/changelog.d/destructuring-rest-array-literal.md new file mode 100644 index 0000000000..7a0d3d46ed --- /dev/null +++ b/changelog.d/destructuring-rest-array-literal.md @@ -0,0 +1,31 @@ +**perf(codegen):** `const {a, b, ...rest} = obj` built its excluded-key array +(the list of statically-named keys `ObjectRest` must NOT copy into `rest`) via +one `js_array_alloc_with_length` call plus one `js_array_set_f64_unchecked` +call *per excluded key* — each of those per-key calls re-derived and +re-bounds-checked a receiver the site had just allocated itself, so every +check inside (frozen? has index descriptors? index in range?) was statically +true. The excluded keys are compile-time-known string literals, so this is +exactly the "array literal of known values" shape `perry-codegen` already has +a cheap path for (`lower_array_literal`/`emit_array_from_lowered_values`, +previously reused only for the rest/`arguments` call-bundle case): one inline +bump allocation plus N `store double`, with a single call only on the cold +arena-full arm. `js_object_rest` itself, and everything downstream of it, is +unchanged — only how its `exclude_keys` argument gets built changes. The +source object's pointer is now also derived *after* that allocation rather +than cached across it. + +Also added `test-files/test_gap_object_destructuring_field_and_rest_guard.ts`, +covering 2-field, 5-field, nested, defaulted, and rest object destructuring +(including computed-key exclusion, an empty pattern before rest, and function +parameter destructuring), plus the easy-to-break edge cases: missing +properties reading as `undefined`, defaults applying only to `undefined` and +not `null`, getter evaluation order following the *pattern's* key order (not +the source object's), and `null`/`undefined` sources throwing `TypeError` +(including for an empty pattern and a `...rest`-only pattern). + +Along the way, found and confirmed **pre-existing** (unaffected by this +change, reproduces identically on unmodified `main`): `const {...rest} = obj` +silently drops any Symbol-keyed own property of `obj` from `rest` instead of +copying it through. Not fixed here — it's in `js_object_rest`'s own key-copy +logic, unrelated to how the `exclude_keys` array is constructed — but worth a +follow-up issue. diff --git a/crates/perry-codegen/src/expr/bigint_set.rs b/crates/perry-codegen/src/expr/bigint_set.rs index 4184dcd86f..f8060de878 100644 --- a/crates/perry-codegen/src/expr/bigint_set.rs +++ b/crates/perry-codegen/src/expr/bigint_set.rs @@ -16,8 +16,8 @@ use crate::type_analysis::{ use crate::types::{DOUBLE, F32, I1, I32, I64, PTR}; use super::{ - can_lower_expr_as_i32, i32_bool_to_nanbox, lower_expr, lower_expr_native, nanbox_bigint_inline, - nanbox_pointer_inline, record_collection_number_key_fallback, + can_lower_expr_as_i32, i32_bool_to_nanbox, lower_array_literal, lower_expr, lower_expr_native, + nanbox_bigint_inline, nanbox_pointer_inline, record_collection_number_key_fallback, record_collection_number_key_selected, record_collection_string_key_fallback, record_collection_string_key_selected, record_collection_typed_value_fallback, record_collection_typed_value_selected, unbox_collection_receiver, unbox_to_i64, FnCtx, @@ -437,29 +437,52 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { object, exclude_keys, } => { - let obj_box = lower_expr(ctx, object)?; - let key_handle_globals: Vec = exclude_keys + // `exclude_keys` is ALWAYS statically-named property strings — + // never a computed key. The HIR lowering that populates this + // field (`destructuring/pattern_binding.rs`'s `Pat::Object` + // arm) only ever pushes `PropName::Ident`/`Str`/`Num` onto + // `static_keys`; a `{ [k]: v, ...rest }` computed key goes + // through a completely separate path (`computed_key_temps` + + // a `delete` on the already-built rest object, #6153) that + // never touches this field. So every element built below is + // guaranteed to be a literal `Expr::String`, not merely + // "usually" one. + // + // Build the excluded-key array FIRST, the same way a literal + // array of the same keys (`[k1, k2, ...]`) is already built: + // one inline bump allocation plus N `store double` (the + // all-literal shape `lower_array_literal` takes when every + // element is `Expr::String` — pooled interned-string handles, + // never an allocation of their own, so no operand rooting is + // needed around them either; see #6951's "emits nothing for + // the all-literal / all-local shapes"). That replaces one + // `js_array_alloc_with_length` call plus one + // `js_array_set_f64_unchecked` call PER excluded key: each of + // those per-key calls re-derived and re-bounds-checked a + // receiver this site had just allocated itself, so every + // check inside (`frozen?`, `has index descriptors?`, `index + // in range?`) was statically true here. + // + // Doing this before lowering `object` — rather than after, as + // the call-by-call version did — also means `object`'s + // pointer is derived AFTER the only allocation left in this + // expression, not cached across it. + let key_exprs: Vec = exclude_keys .iter() - .map(|k| { - let idx = ctx.strings.intern(k); - format!("@{}", ctx.strings.entry(idx).handle_global) - }) + .map(|k| Expr::String(k.clone())) .collect(); + let keys_arr_boxed = lower_array_literal(ctx, &key_exprs)?; + let keys_arr = { + let blk = ctx.block(); + let bits = blk.bitcast_double_to_i64(&keys_arr_boxed); + blk.and(I64, &bits, POINTER_MASK_I64) + }; + let obj_box = lower_expr(ctx, object)?; let blk = ctx.block(); let obj_handle = { let bits = blk.bitcast_double_to_i64(&obj_box); blk.and(I64, &bits, POINTER_MASK_I64) }; - let n_str = (exclude_keys.len() as u32).to_string(); - let keys_arr = blk.call(I64, "js_array_alloc_with_length", &[(I32, &n_str)]); - for (i, handle_global) in key_handle_globals.iter().enumerate() { - let idx_str = i.to_string(); - let key_box = blk.load(DOUBLE, handle_global); - blk.call_void( - "js_array_set_f64_unchecked", - &[(I64, &keys_arr), (I32, &idx_str), (DOUBLE, &key_box)], - ); - } let rest_ptr = blk.call( I64, "js_object_rest", diff --git a/test-files/test_gap_object_destructuring_field_and_rest_guard.ts b/test-files/test_gap_object_destructuring_field_and_rest_guard.ts new file mode 100644 index 0000000000..616395f3db --- /dev/null +++ b/test-files/test_gap_object_destructuring_field_and_rest_guard.ts @@ -0,0 +1,155 @@ +// Object-destructuring coverage for the field-guard / rest-array perf work: +// 2-field, 5-field, nested, defaults (undefined-only, not null), rest +// (excluding named + computed keys, keeping Symbol keys), missing +// properties, getter call order, null/undefined source TypeErrors, Symbol +// keys, and function-parameter destructuring. + +// --- 2-field / 5-field --- +{ + const { a, b } = { a: 1, b: 2 }; + console.log("2field", a, b); + const { a: a5, b: b5, c: c5, d: d5, e: e5 } = { a: 10, b: 20, c: 30, d: 40, e: 50 }; + console.log("5field", a5, b5, c5, d5, e5); +} + +// --- nested --- +{ + const { a: { b } } = { a: { b: 99 } }; + console.log("nested", b); +} + +// --- defaults: only undefined triggers, not null --- +{ + const { a = 10 } = {} as { a?: number }; + console.log("default-missing", a); + const { a: a2 = 10 } = { a: undefined } as { a?: number }; + console.log("default-undefined", a2); + const { a: a3 = 10 } = { a: null } as { a?: number | null }; + console.log("default-null", a3); + const { a: a4 = 10 } = { a: 0 }; + console.log("default-falsy-present", a4); +} + +// --- missing property -> undefined --- +{ + const { missing } = { a: 1 } as { a: number; missing?: number }; + console.log("missing-prop", missing === undefined); +} + +// --- rest excludes named keys, keeps Symbol keys --- +{ + const src = { a: 1, b: 2, c: 3, d: 4 }; + const { a, b, ...rest } = src; + console.log("rest-basic", a, b, JSON.stringify(rest)); + + // NOTE: a `{...rest}` of an object with a Symbol-keyed property is + // deliberately NOT covered here. Perry currently drops Symbol-keyed + // properties from the rest object entirely (`js_object_rest`'s own + // key-copy logic, unrelated to the `exclude_keys` array this file's + // perf change touches) — a pre-existing, separately-filed gap, not + // something this test should assert byte-identical parity on. + + // computed-key exclusion (evaluated once) + rest + let evalCount = 0; + function key() { + evalCount++; + return "b"; + } + const { [key()]: bv, ...restComputed } = { a: 1, b: 2, c: 3 }; + console.log("rest-computed-key", bv, JSON.stringify(restComputed), evalCount); + + // empty pattern before rest + const { ...restAll } = { x: 1, y: 2 }; + console.log("rest-empty-pattern", JSON.stringify(restAll)); +} + +// --- Symbol-keyed destructuring --- +{ + const sym2 = Symbol("s2"); + const obj: any = { [sym2]: 42, plain: 1 }; + const { [sym2]: symVal, plain } = obj; + console.log("symbol-key-read", symVal, plain); +} + +// --- getters run exactly once, in PATTERN source order (not object's own key order) --- +{ + const log: string[] = []; + const obj = { + get c() { + log.push("c"); + return 3; + }, + get a() { + log.push("a"); + return 1; + }, + get b() { + log.push("b"); + return 2; + }, + }; + const { c, a, b } = obj; + console.log("getter-order", log.join(","), a, b, c); +} + +// --- null / undefined source throws TypeError (even for empty pattern) --- +{ + function tryDestructure(fn: () => void): string { + try { + fn(); + return "no-throw"; + } catch (e) { + return e instanceof TypeError ? "TypeError" : "wrong-error:" + String(e); + } + } + console.log("null-source", tryDestructure(() => { + const { a } = null as any; + void a; + })); + console.log("undefined-source", tryDestructure(() => { + const { a } = undefined as any; + void a; + })); + console.log("null-source-empty-pattern", tryDestructure(() => { + const {} = null as any; + })); + console.log("undefined-source-rest", tryDestructure(() => { + const { ...r } = undefined as any; + void r; + })); +} + +// --- function parameter destructuring: plain, default, rest, nested --- +{ + function f2({ a, b }: { a: number; b: number }): number { + return a - b; + } + console.log("param-2field", f2({ a: 5, b: 2 })); + + function fDefault({ a = 7 }: { a?: number }): number { + return a; + } + console.log("param-default-missing", fDefault({})); + console.log("param-default-present", fDefault({ a: 1 })); + console.log("param-default-null", fDefault({ a: null } as any)); + + function fRest({ a, ...rest }: { a: number; [k: string]: number }): string { + return a + ":" + JSON.stringify(rest); + } + console.log("param-rest", fRest({ a: 1, b: 2, c: 3 })); + + function fNested({ outer: { inner } }: { outer: { inner: number } }): number { + return inner; + } + console.log("param-nested", fNested({ outer: { inner: 77 } })); + + function fParamThrows(o: any): string { + try { + const { a } = o; + return "no-throw:" + a; + } catch (e) { + return e instanceof TypeError ? "TypeError" : "wrong-error"; + } + } + console.log("param-null-throws", fParamThrows(null)); +} From 8ca449d26621cdf568ca1e2a537121cdf9b64c47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 20 Sep 2026 15:34:25 +0200 Subject: [PATCH 2/2] changelog: key fragment to #10814 --- ...array-literal.md => 10814-destructuring-rest-array-literal.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{destructuring-rest-array-literal.md => 10814-destructuring-rest-array-literal.md} (100%) diff --git a/changelog.d/destructuring-rest-array-literal.md b/changelog.d/10814-destructuring-rest-array-literal.md similarity index 100% rename from changelog.d/destructuring-rest-array-literal.md rename to changelog.d/10814-destructuring-rest-array-literal.md