perf(codegen): build ObjectRest's excluded-key array as a literal, not N calls (−8.7%)

changelog.d/10814-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.

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<String> {
             object,
             exclude_keys,
         } => {
-            let obj_box = lower_expr(ctx, object)?;
-            let key_handle_globals: Vec<String> = 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<Expr> = 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",

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));
+}