fix(runtime): improve test262 built-ins/Object parity (#5588)

crates/perry-runtime/src/object/field_get_set/enumeration.rs

@@ -243,21 +243,84 @@ pub extern "C" fn js_for_in_keys_value(value: f64) -> *mut ArrayHeader {
 }
 
 fn closure_dynamic_enumerable_props(ptr: usize) -> Vec<(String, f64)> {
-    let mut props = crate::closure::closure_dynamic_props_snapshot(ptr)
+    let mut props: Vec<(String, f64)> = Vec::new();
+
+    // Built-in function properties `length` and `name` are non-enumerable by
+    // default. If the caller redefined them via `Object.defineProperty` with
+    // `enumerable: true`, include them here BEFORE user-added dynamic props
+    // so their relative order matches the spec insertion order (built-ins
+    // precede dynamically-added own properties).
+    for builtin_key in &["length", "name"] {
+        if crate::closure::closure_is_key_deleted(ptr, builtin_key) {
+            continue;
+        }
+        // Only include if the side table explicitly marks them enumerable.
+        // Default (no entry in descriptor side table) = non-enumerable for
+        // built-in function properties.
+        if !get_property_attrs(ptr, builtin_key)
+            .map(|attrs| attrs.enumerable())
+            .unwrap_or(false)
+        {
+            continue;
+        }
+        // Value: prefer a side-table override written by defineProperty, then
+        // fall back to the built-in computed value so Object.keys / entries
+        // returns the right thing even when defineProperty only changed attrs.
+        // Use `closure_has_own_dynamic_prop` to distinguish "has an explicit
+        // dynamic value (possibly undefined)" from "no override" — using
+        // `closure_get_dynamic_prop` as a sentinel conflates both cases and
+        // also invokes getters, which is wrong for the keys-only path.
+        let value = if crate::closure::closure_has_own_dynamic_prop(ptr, builtin_key) {
+            f64::from_bits(crate::closure::closure_get_dynamic_prop(ptr, builtin_key).to_bits())
+        } else if *builtin_key == "length" {
+            let closure_value = crate::value::js_nanbox_pointer(ptr as i64);
+            let len = unsafe {
+                super::super::native_module::bound_native_callable_value_arity(closure_value)
+            }
+            .map(|a| a as f64)
+            .or_else(|| super::super::native_module::builtin_closure_length(ptr).map(|l| l as f64))
+            .or_else(|| {
+                crate::closure::closure_length(ptr as *const crate::closure::ClosureHeader)
+                    .map(|l| l as f64)
+            })
+            .unwrap_or(0.0);
+            len
+        } else {
+            // "name"
+            let func_ptr =
+                unsafe { (*(ptr as *const crate::closure::ClosureHeader)).func_ptr as usize };
+            let fname = crate::builtins::function_name_for_ptr(func_ptr).unwrap_or_default();
+            let s = crate::string::js_string_from_bytes(fname.as_ptr(), fname.len() as u32);
+            f64::from_bits(JSValue::string_ptr(s).bits())
+        };
+        props.push((builtin_key.to_string(), value));
+    }
+
+    // User-added dynamic props (skip "length"/"name" — handled above so we
+    // don't double-count if defineProperty also wrote a value to dynamic props).
+    let user_props = crate::closure::closure_dynamic_props_snapshot(ptr)
         .into_iter()
         .filter(|(name, _)| {
+            if matches!(name.as_str(), "length" | "name") {
+                return false;
+            }
             get_property_attrs(ptr, name)
                 .map(|attrs| attrs.enumerable())
                 .unwrap_or(true)
         })
         .collect::<Vec<_>>();
+    props.extend(user_props);
+
     for name in super::super::accessor_descriptor_keys_for_obj(ptr) {
         if props.iter().any(|(existing, _)| existing == &name) {
             continue;
         }
         if crate::closure::closure_is_key_deleted(ptr, &name) {
             continue;
         }
+        if matches!(name.as_str(), "length" | "name") {
+            continue;
+        }
         if get_property_attrs(ptr, &name)
             .map(|attrs| attrs.enumerable())
             .unwrap_or(false)

crates/perry-runtime/src/object/native_call_method/object_proto.rs

@@ -94,14 +94,48 @@ pub(crate) unsafe fn js_object_default_to_locale_string(receiver: f64) -> f64 {
     if crate::temporal::is_temporal_value(receiver) {
         return crate::temporal::dispatch::call_method(receiver, "toLocaleString", &[]);
     }
-    if !jsval.is_pointer() {
-        return js_native_call_method(
-            receiver,
-            b"toString".as_ptr() as *const i8,
-            "toString".len(),
-            std::ptr::null(),
-            0,
-        );
+    // Symbols are POINTER-tagged, so `!jsval.is_pointer()` would be false for
+    // them — check before the pointer guard so the branch is reachable.
+    let is_symbol = unsafe { crate::symbol::js_is_symbol(receiver) } != 0;
+    if !jsval.is_pointer() || is_symbol {
+        // Spec 20.1.3.6 Object.prototype.toLocaleString: step 1 is "Let O be
+        // the this value" (NOT ToObject), step 2 is "Return ? Invoke(O,
+        // 'toString')". Invoke resolves the method on the primitive's prototype
+        // chain and calls it with the original primitive as `this`. A
+        // user-patched Boolean/Number/BigInt/String prototype toString must be
+        // honoured, and a strict callee must receive the raw primitive (not a
+        // boxed wrapper) — call_primitive_closure_value handles both.
+        let builtin_name: &[u8] = if jsval.is_bool() {
+            b"Boolean"
+        } else if jsval.is_bigint() {
+            b"BigInt"
+        } else if jsval.is_any_string() {
+            b"String"
+        } else if is_symbol {
+            b"Symbol"
+        } else {
+            b""
+        };
+        if !builtin_name.is_empty() {
+            if let Some(patched) =
+                unsafe { super::builtin_proto_user_method(builtin_name, "toString") }
+            {
+                if let Some(result) =
+                    unsafe { call_primitive_closure_value(receiver, patched, std::ptr::null(), 0) }
+                {
+                    return result;
+                }
+            }
+        }
+        return unsafe {
+            js_native_call_method(
+                receiver,
+                b"toString".as_ptr() as *const i8,
+                "toString".len(),
+                std::ptr::null(),
+                0,
+            )
+        };
     }
     // An own `toLocaleString` closure wins over the default rendering —
     // notably `%TypedArray%.prototype.toLocaleString()` invoked as a method ON

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

@@ -196,6 +196,49 @@ pub extern "C" fn js_object_set_prototype_of(obj_value: f64, proto: f64) -> f64
         return obj_value;
     }
 
+    // OrdinarySetPrototypeOf step 7: detect prototype cycles.
+    // Walk the prototype chain of the proposed new prototype; if any ancestor
+    // equals the target object, setting the prototype would form a cycle.
+    // Use Floyd's tortoise-and-hare so a pre-existing multi-node cycle in the
+    // chain (A→B→A) terminates instead of looping forever. The `tortoise`
+    // advances one step; the `hare` advances two. If they meet, the chain is
+    // cyclic and contains a loop (so it will never reach null), meaning we also
+    // can't form a fresh cycle by setting obj's proto to `proto`.
+    if !proto_is_null {
+        const TAG_NULL_U64: u64 = 0x7FFC_0000_0000_0002;
+        let advance = |bits: u64| -> u64 {
+            let val = f64::from_bits(bits);
+            let next = js_object_get_prototype_of(val);
+            let nb = next.to_bits();
+            if nb == TAG_NULL_U64 {
+                TAG_NULL_U64
+            } else {
+                nb
+            }
+        };
+        let mut tortoise = proto_bits;
+        let mut hare = proto_bits;
+        loop {
+            // Check current tortoise position first (catches `proto == obj`
+            // on the very first iteration without an extra advance).
+            if tortoise == obj_bits {
+                throw_object_type_error(b"Cyclic __proto__ value");
+            }
+            if tortoise == TAG_NULL_U64 {
+                break;
+            }
+            // Advance tortoise one step, hare two steps.
+            tortoise = advance(tortoise);
+            hare = advance(advance(hare));
+            // If they meet, the existing chain already has a cycle — the walk
+            // will never reach null, so we also can never form a new one by
+            // setting obj's proto. Just break; the set is safe.
+            if hare == tortoise {
+                break;
+            }
+        }
+    }
+
     // #2820: setting the prototype of a primitive target is a spec no-op that
     // returns the (boxed) primitive value. `value_is_object_like` is false for
     // numbers/strings/booleans, and class refs are handled by the recording