[codex] Respect symbol enumerability in Object.assign

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

@@ -805,6 +805,9 @@ pub unsafe extern "C" fn js_object_assign_one(target_f64: f64, source_f64: f64)
     //    Mutex; holding the lock across the iteration would deadlock.
     let entries = crate::symbol::clone_symbol_entries_for_obj_ptr(src_raw);
     for (sym_ptr, value_bits) in entries {
+        if !crate::symbol::symbol_property_is_enumerable(src_raw, sym_ptr) {
+            continue;
+        }
         let sym_f64 = f64::from_bits(JSValue::pointer(sym_ptr as *const u8).bits());
         let value_f64 = f64::from_bits(value_bits);
         crate::symbol::js_object_set_symbol_property(target_f64, sym_f64, value_f64);

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

@@ -127,6 +127,34 @@ pub extern "C" fn js_object_get_own_property_descriptor(obj_value: f64, key_valu
             }
         }
 
+        if crate::symbol::js_is_symbol(key_value) != 0 {
+            let owner = crate::symbol::obj_key_from_f64(obj_value);
+            let sym_key = crate::symbol::sym_key_from_f64(key_value);
+            if owner == 0 || sym_key == 0 {
+                return f64::from_bits(crate::value::TAG_UNDEFINED);
+            }
+            let attrs = crate::symbol::get_symbol_property_attrs(owner, sym_key)
+                .unwrap_or(PropertyAttrs::new(true, true, true));
+            if let Some((get, set)) = crate::symbol::symbol_accessor_descriptor_bits(owner, sym_key)
+            {
+                return build_accessor_descriptor(
+                    f64::from_bits(get),
+                    f64::from_bits(set),
+                    attrs.enumerable(),
+                    attrs.configurable(),
+                );
+            }
+            if let Some(value_bits) = crate::symbol::symbol_property_root_bits(owner, sym_key) {
+                return build_data_descriptor(
+                    f64::from_bits(value_bits),
+                    attrs.writable(),
+                    attrs.enumerable(),
+                    attrs.configurable(),
+                );
+            }
+            return f64::from_bits(crate::value::TAG_UNDEFINED);
+        }
+
         if let Some(class_id) = class_ref_id(obj_value) {
             let method_name = metadata_key_to_string(key_value);
             if let Some(method_name) = method_name {

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

@@ -1006,31 +1006,55 @@ pub extern "C" fn js_object_define_property(
                         js_object_get_field_by_name(desc_ptr as *const ObjectHeader, get_key);
                     let set_field =
                         js_object_get_field_by_name(desc_ptr as *const ObjectHeader, set_key);
-                    if !get_field.is_undefined() || !set_field.is_undefined() {
-                        let get_bits = if get_field.is_undefined() {
+                    let has_get = own_key_present(desc_ptr, get_key);
+                    let has_set = own_key_present(desc_ptr, set_key);
+                    let has_accessor = has_get || has_set;
+                    if has_accessor {
+                        let get_bits = if !has_get || get_field.is_undefined() {
                             0
                         } else {
                             crate::closure::clone_closure_rebind_this(get_field.bits(), obj_value)
                         };
-                        let set_bits = if set_field.is_undefined() {
+                        let set_bits = if !has_set || set_field.is_undefined() {
                             0
                         } else {
                             crate::closure::clone_closure_rebind_this(set_field.bits(), obj_value)
                         };
                         crate::symbol::set_symbol_accessor_property(
                             obj_value, key_value, get_bits, set_bits,
                         );
+                    } else {
+                        let value_key = crate::string::js_string_from_bytes(b"value".as_ptr(), 5);
+                        if own_key_present(desc_ptr, value_key) {
+                            let value_field = js_object_get_field_by_name(
+                                desc_ptr as *const ObjectHeader,
+                                value_key,
+                            );
+                            crate::symbol::js_object_set_symbol_property(
+                                obj_value,
+                                key_value,
+                                f64::from_bits(value_field.bits()),
+                            );
+                        }
                     }
-                    let value_key = crate::string::js_string_from_bytes(b"value".as_ptr(), 5);
-                    let value_field =
-                        js_object_get_field_by_name(desc_ptr as *const ObjectHeader, value_key);
-                    if !value_field.is_undefined() {
-                        crate::symbol::js_object_set_symbol_property(
-                            obj_value,
-                            key_value,
-                            f64::from_bits(value_field.bits()),
-                        );
-                    }
+                    let read_bool = |name: &[u8]| -> Option<bool> {
+                        let k =
+                            crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32);
+                        let v = js_object_get_field_by_name(desc_ptr as *const ObjectHeader, k);
+                        if v.is_undefined() {
+                            None
+                        } else {
+                            Some(crate::value::js_is_truthy(f64::from_bits(v.bits())) != 0)
+                        }
+                    };
+                    let writable = read_bool(b"writable").unwrap_or(has_accessor);
+                    let enumerable = read_bool(b"enumerable").unwrap_or(false);
+                    let configurable = read_bool(b"configurable").unwrap_or(false);
+                    crate::symbol::set_symbol_property_attrs(
+                        obj as usize,
+                        raw_ptr as usize,
+                        PropertyAttrs::new(writable, enumerable, configurable),
+                    );
                 }
                 return obj_value;
             }

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

@@ -147,6 +147,93 @@ fn closure_name_can_be_redefined_with_define_property() {
     }
 }
 
+#[test]
+fn symbol_define_property_attrs_round_trip_descriptor() {
+    crate::symbol::test_clear_symbol_side_table_roots();
+    unsafe {
+        let obj = js_object_alloc(0, 0);
+        assert!(!obj.is_null());
+        let obj_value = crate::value::js_nanbox_pointer(obj as i64);
+        let symbol_key = crate::symbol::js_symbol_new_empty();
+        let symbol_ptr = crate::symbol::sym_key_from_f64(symbol_key);
+        assert_ne!(symbol_ptr, 0);
+
+        let value_key = crate::string::js_string_from_bytes(b"value".as_ptr(), 5);
+        let writable_key = crate::string::js_string_from_bytes(b"writable".as_ptr(), 8);
+        let enumerable_key = crate::string::js_string_from_bytes(b"enumerable".as_ptr(), 10);
+        let configurable_key = crate::string::js_string_from_bytes(b"configurable".as_ptr(), 12);
+
+        let descriptor = js_object_alloc(0, 0);
+        assert!(!descriptor.is_null());
+        js_object_set_field_by_name(descriptor, value_key, 42.0);
+        js_object_set_field_by_name(
+            descriptor,
+            writable_key,
+            f64::from_bits(crate::value::TAG_FALSE),
+        );
+        js_object_set_field_by_name(
+            descriptor,
+            enumerable_key,
+            f64::from_bits(crate::value::TAG_FALSE),
+        );
+        js_object_set_field_by_name(
+            descriptor,
+            configurable_key,
+            f64::from_bits(crate::value::TAG_TRUE),
+        );
+
+        let descriptor_value = crate::value::js_nanbox_pointer(descriptor as i64);
+        js_object_define_property(obj_value, symbol_key, descriptor_value);
+
+        assert_eq!(
+            crate::symbol::symbol_property_root_bits(obj as usize, symbol_ptr),
+            Some(42.0f64.to_bits())
+        );
+        assert!(!crate::symbol::symbol_property_is_enumerable(
+            obj as usize,
+            symbol_ptr
+        ));
+
+        let own_descriptor = js_object_get_own_property_descriptor(obj_value, symbol_key);
+        let own_descriptor_obj =
+            crate::value::js_nanbox_get_pointer(own_descriptor) as *const ObjectHeader;
+        assert!(!own_descriptor_obj.is_null());
+        let value = js_object_get_field_by_name(own_descriptor_obj, value_key);
+        assert!(value.is_number());
+        assert_eq!(value.as_number(), 42.0);
+        assert_eq!(
+            js_object_get_field_by_name(own_descriptor_obj, writable_key).bits(),
+            crate::value::TAG_FALSE
+        );
+        assert_eq!(
+            js_object_get_field_by_name(own_descriptor_obj, enumerable_key).bits(),
+            crate::value::TAG_FALSE
+        );
+        assert_eq!(
+            js_object_get_field_by_name(own_descriptor_obj, configurable_key).bits(),
+            crate::value::TAG_TRUE
+        );
+
+        let attr_descriptor = js_object_alloc(0, 0);
+        assert!(!attr_descriptor.is_null());
+        js_object_set_field_by_name(
+            attr_descriptor,
+            enumerable_key,
+            f64::from_bits(crate::value::TAG_TRUE),
+        );
+        let attr_descriptor_value = crate::value::js_nanbox_pointer(attr_descriptor as i64);
+        js_object_define_property(obj_value, symbol_key, attr_descriptor_value);
+        assert_eq!(
+            crate::symbol::symbol_property_root_bits(obj as usize, symbol_ptr),
+            Some(42.0f64.to_bits())
+        );
+        assert!(crate::symbol::symbol_property_is_enumerable(
+            obj as usize,
+            symbol_ptr
+        ));
+    }
+}
+
 #[test]
 fn test_object_alloc_and_fields() {
     let obj = js_object_alloc(1, 3);

crates/perry-runtime/src/symbol.rs

@@ -203,6 +203,12 @@ pub(crate) fn is_global_registered_symbol(ptr: usize) -> bool {
 // Storage stays intentionally linear because per-object symbol keys are rare.
 static SYMBOL_PROPERTIES: Mutex<Option<HashMap<usize, Vec<(usize, u64)>>>> = Mutex::new(None);
 
+// Descriptor attributes for symbol-keyed properties installed through
+// Object.defineProperty. Direct symbol assignment uses the normal data-property
+// defaults, so absence here means writable/enumerable/configurable are all true.
+static SYMBOL_PROPERTY_ATTRS: Mutex<Option<HashMap<(usize, usize), crate::object::PropertyAttrs>>> =
+    Mutex::new(None);
+
 // Monotonic id counter for fresh symbols. Not thread-safe per-thread but
 // Symbol semantics are compatible with coarse locking.
 static NEXT_SYMBOL_ID: Mutex<u64> = Mutex::new(1);
@@ -483,6 +489,50 @@ pub(crate) fn clone_symbol_entries_for_obj_ptr(src_obj_ptr: usize) -> Vec<(usize
         .unwrap_or_default()
 }
 
+pub(crate) fn symbol_property_root_bits(owner: usize, sym_key: usize) -> Option<u64> {
+    let guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTIES);
+    guard.as_ref().and_then(|map| {
+        map.get(&owner)
+            .and_then(|entries| entries.iter().find(|(key, _)| *key == sym_key))
+            .map(|(_, value_bits)| *value_bits)
+    })
+}
+
+pub(crate) fn get_symbol_property_attrs(
+    owner: usize,
+    sym_key: usize,
+) -> Option<crate::object::PropertyAttrs> {
+    let guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTY_ATTRS);
+    guard
+        .as_ref()
+        .and_then(|map| map.get(&(owner, sym_key)).copied())
+}
+
+pub(crate) fn set_symbol_property_attrs(
+    owner: usize,
+    sym_key: usize,
+    attrs: crate::object::PropertyAttrs,
+) {
+    if owner == 0 || sym_key == 0 {
+        return;
+    }
+    let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTY_ATTRS);
+    if guard.is_none() {
+        *guard = Some(HashMap::new());
+    }
+    guard.as_mut().unwrap().insert((owner, sym_key), attrs);
+}
+
+pub(crate) fn symbol_property_is_enumerable(owner: usize, sym_key: usize) -> bool {
+    get_symbol_property_attrs(owner, sym_key)
+        .map(|attrs| attrs.enumerable())
+        .unwrap_or(true)
+}
+
+pub(crate) fn symbol_accessor_descriptor_bits(owner: usize, sym_key: usize) -> Option<(u64, u64)> {
+    accessors::symbol_accessor_property_by_key(owner, sym_key).map(|acc| (acc.get, acc.set))
+}
+
 /// Extract the raw object pointer from a NaN-boxed JSValue. Returns 0 if the
 /// value isn't a pointer-tagged object (and 0 is also a valid "no entries"
 /// sentinel for the side table).
@@ -764,6 +814,7 @@ pub fn scan_symbol_side_table_roots(mark: &mut dyn FnMut(f64)) {
 
 pub fn scan_symbol_side_table_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) {
     scan_symbol_property_roots_mut(visitor);
+    scan_symbol_property_attrs_mut(visitor);
     accessors::scan_symbol_accessor_roots_mut(visitor);
     scan_class_static_symbol_roots_mut(visitor);
     scan_symbol_pointer_metadata_roots_mut(visitor);
@@ -802,6 +853,31 @@ fn scan_symbol_property_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_
     }
 }
 
+fn scan_symbol_property_attrs_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) {
+    let mut rewrites = Vec::new();
+    let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTY_ATTRS);
+    let Some(map) = guard.as_mut() else {
+        return;
+    };
+
+    for (old_owner, old_sym_key) in map.keys().copied().collect::<Vec<_>>() {
+        let mut new_owner = old_owner;
+        let mut new_sym_key = old_sym_key;
+        let owner_changed =
+            visitor.visit_metadata_usize_slot(&mut new_owner) && new_owner != old_owner;
+        let sym_changed = visitor.visit_usize_slot(&mut new_sym_key) && new_sym_key != old_sym_key;
+        if owner_changed || sym_changed {
+            rewrites.push(((old_owner, old_sym_key), (new_owner, new_sym_key)));
+        }
+    }
+
+    for (old_key, new_key) in rewrites {
+        if let Some(attrs) = map.remove(&old_key) {
+            map.insert(new_key, attrs);
+        }
+    }
+}
+
 fn scan_class_static_symbol_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) {
     let mut key_rewrites = Vec::new();
     let mut guard = crate::gc::lock_gc_root_registry(&CLASS_STATIC_SYMBOLS);
@@ -851,6 +927,7 @@ fn scan_symbol_pointer_metadata_roots_mut(visitor: &mut crate::gc::RuntimeRootVi
 enum SymbolSideTableRootSlot {
     SymbolPropertyOwner { owner: usize },
     SymbolPropertyEntry { owner: usize, sym_key: usize },
+    SymbolPropertyAttrs { owner: usize, sym_key: usize },
     ClassStaticSymbol { class_id: u32, sym_key: usize },
     SymbolPointer { ptr: usize },
 }
@@ -898,6 +975,15 @@ fn symbol_side_table_root_snapshot() -> Vec<SymbolSideTableRootSlot> {
         }
     }
 
+    {
+        let guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTY_ATTRS);
+        if let Some(map) = guard.as_ref() {
+            for &(owner, sym_key) in map.keys() {
+                slots.push(SymbolSideTableRootSlot::SymbolPropertyAttrs { owner, sym_key });
+            }
+        }
+    }
+
     {
         let guard = crate::gc::lock_gc_root_registry(&CLASS_STATIC_SYMBOLS);
         if let Some(map) = guard.as_ref() {
@@ -939,6 +1025,9 @@ fn scan_symbol_side_table_root_slot(
             visitor.visit_usize_slot(entry_sym);
             visitor.visit_nanbox_u64_slot(value_bits);
         }
+        SymbolSideTableRootSlot::SymbolPropertyAttrs { owner, sym_key } => {
+            rewrite_symbol_property_attrs_if_forwarded(visitor, owner, sym_key);
+        }
         SymbolSideTableRootSlot::ClassStaticSymbol { class_id, sym_key } => {
             rewrite_class_static_symbol_entry_if_forwarded(visitor, class_id, sym_key);
         }
@@ -971,6 +1060,29 @@ fn rewrite_symbol_property_owner_if_forwarded(
     }
 }
 
+fn rewrite_symbol_property_attrs_if_forwarded(
+    visitor: &mut crate::gc::RuntimeRootVisitor<'_>,
+    owner: usize,
+    sym_key: usize,
+) {
+    let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTY_ATTRS);
+    let Some(map) = guard.as_mut() else {
+        return;
+    };
+    if !map.contains_key(&(owner, sym_key)) {
+        return;
+    }
+    let mut new_owner = owner;
+    let mut new_sym_key = sym_key;
+    let owner_moved = visitor.visit_metadata_usize_slot(&mut new_owner);
+    let sym_moved = visitor.visit_usize_slot(&mut new_sym_key);
+    if (owner_moved && new_owner != owner) || (sym_moved && new_sym_key != sym_key) {
+        if let Some(attrs) = map.remove(&(owner, sym_key)) {
+            map.insert((new_owner, new_sym_key), attrs);
+        }
+    }
+}
+
 fn rewrite_class_static_symbol_entry_if_forwarded(
     visitor: &mut crate::gc::RuntimeRootVisitor<'_>,
     class_id: u32,
@@ -1013,6 +1125,7 @@ fn rewrite_symbol_pointer_metadata_if_forwarded(
 #[cfg(test)]
 pub(crate) fn test_clear_symbol_side_table_roots() {
     *crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTIES) = None;
+    *crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTY_ATTRS) = None;
     *crate::gc::lock_gc_root_registry(&CLASS_STATIC_SYMBOLS) = None;
     accessors::test_clear_symbol_accessor_roots();