fix: preserve Symbol properties in object rest

changelog.d/11023-object-rest-symbols.md

@@ -0,0 +1,5 @@
+### Fixed
+
+Object rest destructuring now preserves enumerable own Symbol properties,
+including accessor values and Symbol-only objects, while continuing to omit
+excluded and non-enumerable Symbol keys.

crates/perry-runtime/src/object/delete_rest.rs

@@ -1125,8 +1125,81 @@ pub extern "C" fn js_object_delete_dynamic(obj: *mut ObjectHeader, key: f64) ->
     1
 }
 
-/// Create a rest object from destructuring: copies all properties from src except excluded keys.
-/// exclude_keys is an array of NaN-boxed string pointers (the explicitly destructured keys).
+unsafe fn copy_rest_symbol_properties(
+    src_handle: &crate::gc::RuntimeHandle<'_>,
+    exclude_handle: &crate::gc::RuntimeHandle<'_>,
+    rest_handle: &crate::gc::RuntimeHandle<'_>,
+) {
+    let src_value =
+        crate::value::js_nanbox_pointer(src_handle.get_raw_const_ptr::<ObjectHeader>() as i64);
+    // Snapshot [[OwnPropertyKeys]] before running any getters. The returned
+    // array is a GC object, so keep it rooted while each property read may run
+    // arbitrary user code and collect.
+    let symbols = crate::symbol::js_object_get_own_property_symbols(src_value)
+        as *mut crate::array::ArrayHeader;
+    let scope = crate::gc::RuntimeHandleScope::new();
+    let symbols_handle = scope.root_raw_mut_ptr(symbols);
+    let symbol_count = crate::array::js_array_length(
+        symbols_handle.get_raw_const_ptr::<crate::array::ArrayHeader>(),
+    );
+    let exclude_ptr = exclude_handle.get_raw_const_ptr::<crate::array::ArrayHeader>();
+    let exclude_count = if exclude_ptr.is_null() {
+        0
+    } else {
+        crate::array::js_array_length(exclude_ptr)
+    };
+
+    for i in 0..symbol_count {
+        let symbol = crate::array::js_array_get(
+            symbols_handle.get_raw_const_ptr::<crate::array::ArrayHeader>(),
+            i,
+        );
+        let symbol_bits = symbol.bits();
+        if (0..exclude_count).any(|j| {
+            crate::array::js_array_get(
+                exclude_handle.get_raw_const_ptr::<crate::array::ArrayHeader>(),
+                j,
+            )
+            .bits()
+                == symbol_bits
+        }) {
+            continue;
+        }
+
+        let iter_scope = crate::gc::RuntimeHandleScope::new();
+        let symbol_handle = iter_scope.root_nanbox_u64(symbol_bits);
+        let src_value =
+            crate::value::js_nanbox_pointer(src_handle.get_raw_const_ptr::<ObjectHeader>() as i64);
+        let symbol_value = symbol_handle.get_nanbox_f64();
+        let symbol_key = (symbol_value.to_bits() & crate::value::POINTER_MASK) as usize;
+        let owner = src_handle.get_raw_const_ptr::<ObjectHeader>() as usize;
+
+        // CopyDataProperties rechecks the descriptor after collecting keys:
+        // an earlier getter may have deleted this key, and non-enumerable
+        // symbols must not appear in the rest object.
+        let Some(slot) = crate::symbol::own_symbol_slot(src_value, symbol_value) else {
+            continue;
+        };
+        if !crate::symbol::symbol_property_is_enumerable(owner, symbol_key) {
+            continue;
+        }
+
+        let value_handle = iter_scope.root_nanbox_f64(slot.read(src_value));
+        let rest_value =
+            crate::value::js_nanbox_pointer(rest_handle.get_raw_mut_ptr::<ObjectHeader>() as i64);
+        // CreateDataProperty semantics: install a fresh enumerable data
+        // property without invoking an inherited setter on Object.prototype.
+        crate::symbol::define_symbol_data_property(
+            rest_value,
+            symbol_handle.get_nanbox_f64(),
+            value_handle.get_nanbox_f64(),
+        );
+    }
+}
+
+/// Create a rest object from destructuring: copies all own enumerable
+/// properties from `src` except the explicitly destructured keys.
+/// `exclude_keys` contains NaN-boxed strings and may also contain Symbols.
 /// Returns a pointer to a new object with the remaining key-value pairs.
 #[no_mangle]
 pub extern "C" fn js_object_rest(
@@ -1137,19 +1210,35 @@ pub extern "C" fn js_object_rest(
         return js_object_alloc(0, 0);
     }
     unsafe {
+        let scope = crate::gc::RuntimeHandleScope::new();
+        let src_handle = scope.root_raw_const_ptr(src);
+        let exclude_handle = scope.root_raw_const_ptr(exclude_keys);
+
         if super::string_wrapper::length(src as usize).is_some() {
-            return super::string_wrapper::rest(src, exclude_keys);
-        }
-        let keys = crate::object::object_keys_array(src);
-        if keys.is_null() {
-            return js_object_alloc(0, 0);
+            let rest = super::string_wrapper::rest(
+                src_handle.get_raw_const_ptr::<ObjectHeader>(),
+                exclude_handle.get_raw_const_ptr::<ArrayHeader>(),
+            );
+            let rest_handle = scope.root_raw_mut_ptr(rest);
+            copy_rest_symbol_properties(&src_handle, &exclude_handle, &rest_handle);
+            return rest_handle.get_raw_mut_ptr::<ObjectHeader>();
         }
+        let keys = crate::object::object_keys_array(src_handle.get_raw_const_ptr::<ObjectHeader>());
+        let keys_handle = scope.root_raw_mut_ptr(keys);
 
-        let key_count = crate::array::js_array_length(keys) as usize;
+        let key_count = if keys.is_null() {
+            0
+        } else {
+            crate::array::js_array_length(
+                keys_handle.get_raw_const_ptr::<crate::array::ArrayHeader>(),
+            ) as usize
+        };
         let exclude_count = if exclude_keys.is_null() {
             0
         } else {
-            crate::array::js_array_length(exclude_keys) as usize
+            crate::array::js_array_length(
+                exclude_handle.get_raw_const_ptr::<crate::array::ArrayHeader>(),
+            ) as usize
         };
 
         // Collect indices of keys to include (not in exclude list and not undefined/deleted).
@@ -1161,22 +1250,29 @@ pub extern "C" fn js_object_rest(
         let mut include_indices: Vec<usize> = Vec::new();
         let mut src_buf = [0u8; crate::value::SHORT_STRING_MAX_LEN];
         for i in 0..key_count {
-            let key_val = crate::array::js_array_get(keys, i as u32);
+            let key_val = crate::array::js_array_get(
+                keys_handle.get_raw_const_ptr::<crate::array::ArrayHeader>(),
+                i as u32,
+            );
             let key_bytes = match crate::string::js_string_key_bytes(key_val, &mut src_buf) {
                 Some(b) => b.to_vec(),
                 None => continue,
             };
 
             // Check if field was deleted
-            let field_val = js_object_get_field(src, i as u32);
+            let field_val =
+                js_object_get_field(src_handle.get_raw_const_ptr::<ObjectHeader>(), i as u32);
             if field_val.is_undefined() {
                 continue;
             }
 
             // Check if this key is in the exclude list
             let mut excluded = false;
             for j in 0..exclude_count {
-                let ex_val = crate::array::js_array_get(exclude_keys, j as u32);
+                let ex_val = crate::array::js_array_get(
+                    exclude_handle.get_raw_const_ptr::<crate::array::ArrayHeader>(),
+                    j as u32,
+                );
                 if crate::string::js_string_key_matches_bytes(ex_val, &key_bytes) {
                     excluded = true;
                     break;
@@ -1189,22 +1285,41 @@ pub extern "C" fn js_object_rest(
 
         // Allocate new object with the right number of fields
         let rest_count = include_indices.len() as u32;
-        let rest_obj = js_object_alloc(0, rest_count);
+        let rest_handle = scope.root_raw_mut_ptr(js_object_alloc(0, rest_count));
 
         // Create keys array for the rest object
-        let rest_keys = crate::array::js_array_alloc_with_length(rest_count);
-        set_object_keys_array(rest_obj, rest_keys);
+        let rest_keys_handle =
+            scope.root_raw_mut_ptr(crate::array::js_array_alloc_with_length(rest_count));
+        set_object_keys_array(
+            rest_handle.get_raw_mut_ptr::<ObjectHeader>(),
+            rest_keys_handle.get_raw_mut_ptr::<crate::array::ArrayHeader>(),
+        );
 
         // Copy included key-value pairs
         for (new_idx, &src_idx) in include_indices.iter().enumerate() {
-            let key_val = crate::array::js_array_get(keys, src_idx as u32);
-            crate::array::js_array_set(rest_keys, new_idx as u32, key_val);
+            let key_val = crate::array::js_array_get(
+                keys_handle.get_raw_const_ptr::<crate::array::ArrayHeader>(),
+                src_idx as u32,
+            );
+            crate::array::js_array_set(
+                rest_keys_handle.get_raw_mut_ptr::<crate::array::ArrayHeader>(),
+                new_idx as u32,
+                key_val,
+            );
 
-            let field_val = js_object_get_field(src, src_idx as u32);
-            js_object_set_field(rest_obj, new_idx as u32, field_val);
+            let field_val = js_object_get_field(
+                src_handle.get_raw_const_ptr::<ObjectHeader>(),
+                src_idx as u32,
+            );
+            js_object_set_field(
+                rest_handle.get_raw_mut_ptr::<ObjectHeader>(),
+                new_idx as u32,
+                field_val,
+            );
         }
 
-        rest_obj
+        copy_rest_symbol_properties(&src_handle, &exclude_handle, &rest_handle);
+        rest_handle.get_raw_mut_ptr::<ObjectHeader>()
     }
 }
 

test-files/test_gap_object_destructuring_field_and_rest_guard.ts

@@ -42,12 +42,40 @@
   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.
+  const kept = Symbol("kept");
+  const omitted = Symbol("omitted");
+  const hidden = Symbol("hidden");
+  const getter = Symbol("getter");
+  let getterCalls = 0;
+  const symbolSource: any = {
+    plain: 1,
+    [kept]: 2,
+    [omitted]: 3,
+    get [getter]() {
+      getterCalls++;
+      return 4;
+    },
+  };
+  Object.defineProperty(symbolSource, hidden, { value: 5, enumerable: false });
+  const { [omitted]: omittedValue, ...symbolRest } = symbolSource;
+  console.log(
+    "rest-symbols",
+    omittedValue,
+    symbolRest[kept],
+    symbolRest[omitted],
+    symbolRest[hidden],
+    symbolRest[getter],
+    getterCalls,
+    Object.getOwnPropertySymbols(symbolRest).map(String).join(","),
+  );
+
+  const only = Symbol("only");
+  const { ...symbolOnlyRest } = { [only]: 6 };
+  console.log(
+    "rest-symbol-only",
+    symbolOnlyRest[only],
+    Object.getOwnPropertySymbols(symbolOnlyRest).map(String).join(","),
+  );
 
   // computed-key exclusion (evaluated once) + rest
   let evalCount = 0;