fix: dynamic array-method dispatch dropped the thisArg — extracted builtin method as callback with thisArg (#6658)

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

@@ -260,11 +260,132 @@ fn install_collection_size_getter(proto_obj: *mut ObjectHeader, name: &str, func
     }
 }
 
-/// Throw `TypeError: Method <proto>.<method> called on incompatible receiver`.
-/// Mirrors V8's wording closely; Test262's brand-check tests assert only the
-/// error *type*, so the exact message is informational. Never returns.
-fn throw_incompatible_receiver(proto: &str, method: &str) -> ! {
-    let msg = format!("Method {proto}.{method} called on incompatible receiver");
+/// Owned UTF-8 copy of a heap `StringHeader`'s bytes.
+fn string_header_owned(sp: *const crate::string::StringHeader) -> String {
+    if sp.is_null() {
+        return String::new();
+    }
+    unsafe {
+        let bytes = (sp as *const u8).add(std::mem::size_of::<crate::string::StringHeader>());
+        let len = (*sp).byte_len as usize;
+        std::str::from_utf8(std::slice::from_raw_parts(bytes, len))
+            .unwrap_or("")
+            .to_string()
+    }
+}
+
+/// Render the brand-check receiver the way V8's `NoSideEffectsToString` does —
+/// node APPENDS it to the incompatible-receiver TypeError (`Method
+/// Set.prototype.add called on incompatible receiver #<Object>` /
+/// `... receiver undefined` / `... receiver [object Array]`), and #6658's
+/// parity fixtures assert the full message. V8 never CALLS a potentially
+/// side-effecting user `toString` here: primitives print their ToString
+/// (`undefined`, `null`, `5.5`, the raw string chars, `Symbol(desc)`, bigint
+/// digits); objects whose `toString` is the default `Object.prototype.toString`
+/// print `#<CtorName>` (`#<Object>`, `#<Set>`, `#<Map>`, `#<Promise>`,
+/// `#<Foo>`); objects that override it (Array, Date, RegExp, an own user
+/// `toString`) print the `Object.prototype.toString` tag instead; errors print
+/// `Name: message`. node prints a function receiver's SOURCE — perry keeps no
+/// source, so the native-code form is the closest stable rendering.
+/// Unclassifiable heap values fall back to `#<Object>`.
+fn render_incompatible_receiver(bits: u64) -> String {
+    use crate::value::JSValue;
+    let value = f64::from_bits(bits);
+    let jv = JSValue::from_bits(bits);
+    if !jv.is_pointer() {
+        // undefined / null / bool / number / bigint / heap+SSO string: plain
+        // side-effect-free ToString.
+        return string_header_owned(crate::value::js_jsvalue_to_string(value));
+    }
+    let ptr = (bits & crate::value::POINTER_MASK) as usize;
+    if crate::symbol::is_registered_symbol(ptr) {
+        // "Symbol(desc)" — Symbol.prototype.toString is side-effect free.
+        return string_header_owned(unsafe { crate::symbol::js_symbol_to_string(value) }
+            as *const crate::string::StringHeader);
+    }
+    if crate::set::is_registered_set(ptr) {
+        return "#<Set>".to_string();
+    }
+    if crate::map::is_registered_map(ptr) {
+        return "#<Map>".to_string();
+    }
+    match crate::weakref::weak_class_id_from_receiver(value) {
+        Some(crate::weakref::CLASS_ID_WEAKSET) => return "#<WeakSet>".to_string(),
+        Some(crate::weakref::CLASS_ID_WEAKMAP) => return "#<WeakMap>".to_string(),
+        _ => {}
+    }
+    if crate::date::is_registered_date_bits(bits) {
+        return "[object Date]".to_string();
+    }
+    if crate::regex::is_registered_regex(ptr) {
+        return "[object RegExp]".to_string();
+    }
+    // Heap-header classification — only a real heap pointer above the
+    // synthetic handle band may be dereferenced (mirrors `as_real_array`).
+    if crate::value::addr_class::is_above_handle_band(ptr)
+        && crate::object::is_valid_obj_ptr(ptr as *const u8)
+    {
+        let obj_type = unsafe {
+            (*((ptr as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader))
+                .obj_type
+        };
+        match obj_type {
+            crate::gc::GC_TYPE_ARRAY | crate::gc::GC_TYPE_LAZY_ARRAY => {
+                return "[object Array]".to_string();
+            }
+            crate::gc::GC_TYPE_PROMISE => return "#<Promise>".to_string(),
+            crate::gc::GC_TYPE_ERROR => {
+                let err = ptr as *mut crate::error::ErrorHeader;
+                let name = string_header_owned(crate::error::js_error_get_name(err));
+                let message = string_header_owned(crate::error::js_error_get_message(err));
+                return if message.is_empty() {
+                    name
+                } else {
+                    format!("{name}: {message}")
+                };
+            }
+            crate::gc::GC_TYPE_CLOSURE => {
+                return "function () { [native code] }".to_string();
+            }
+            crate::gc::GC_TYPE_OBJECT => {
+                // An OWN user `toString` overrides the default — V8 refuses to
+                // call it and falls back to the Object.prototype.toString tag.
+                // js_string_from_bytes allocates and can trigger a GC that
+                // relocates the receiver: root it and re-derive the pointer
+                // through the (rewritten) handle after the allocation.
+                let scope = crate::gc::RuntimeHandleScope::new();
+                let handle = scope.root_nanbox_f64(f64::from_bits(bits));
+                let key = crate::string::js_string_from_bytes(b"toString".as_ptr(), 8);
+                let obj = crate::value::JSValue::from_bits(handle.get_nanbox_f64().to_bits())
+                    .as_pointer::<u8>() as *mut ObjectHeader;
+                if unsafe { super::own_data_field_by_name(obj, key) }.is_some() {
+                    return "[object Object]".to_string();
+                }
+                let cid = unsafe { (*obj).class_id };
+                if cid != 0 {
+                    if let Some(name) = crate::object::class_name_for_id(cid) {
+                        if !name.is_empty() {
+                            return format!("#<{name}>");
+                        }
+                    }
+                }
+                return "#<Object>".to_string();
+            }
+            _ => {}
+        }
+    }
+    "#<Object>".to_string()
+}
+
+/// Throw `TypeError: Method <proto>.<method> called on incompatible receiver
+/// <receiver>`. Matches V8/node's wording INCLUDING the appended receiver
+/// rendering (#6658 asserts the full message; Test262's brand-check tests
+/// assert only the error *type*). Never returns.
+fn throw_incompatible_receiver(proto: &str, method: &str, receiver_bits: u64) -> ! {
+    let msg = format!(
+        "Method {proto}.{method} called on incompatible receiver {}",
+        render_incompatible_receiver(receiver_bits)
+    );
     let s = crate::string::js_string_from_bytes(msg.as_ptr(), msg.len() as u32);
     let err = crate::error::js_typeerror_new(s);
     crate::exception::js_throw(f64::from_bits(
@@ -277,7 +398,7 @@ fn set_receiver_or_throw(method: &str) -> *mut crate::set::SetHeader {
     let bits = IMPLICIT_THIS.with(|c| c.get());
     match crate::set::set_ptr_from_receiver_bits(bits) {
         Some(p) => p,
-        None => throw_incompatible_receiver("Set.prototype", method),
+        None => throw_incompatible_receiver("Set.prototype", method, bits),
     }
 }
 
@@ -286,7 +407,7 @@ fn map_receiver_or_throw(method: &str) -> *mut crate::map::MapHeader {
     let bits = IMPLICIT_THIS.with(|c| c.get());
     match crate::map::map_ptr_from_receiver_bits(bits) {
         Some(p) => p,
-        None => throw_incompatible_receiver("Map.prototype", method),
+        None => throw_incompatible_receiver("Map.prototype", method, bits),
     }
 }
 
@@ -295,7 +416,7 @@ fn weak_receiver_or_throw(expected: u32, proto: &str, method: &str) -> f64 {
     let receiver = f64::from_bits(IMPLICIT_THIS.with(|c| c.get()));
     match crate::weakref::weak_class_id_from_receiver(receiver) {
         Some(cid) if cid == expected => receiver,
-        _ => throw_incompatible_receiver(proto, method),
+        _ => throw_incompatible_receiver(proto, method, receiver.to_bits()),
     }
 }
 

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

@@ -219,6 +219,45 @@ pub(super) unsafe fn dispatch_handle(
                         return Some(result);
                     }
                 }
+                // #6658: an explicit `thisArg` (2nd argument) must bind the
+                // callback's `this`. The dense helpers the arms below dispatch
+                // to deliberately bind `undefined` (spec: absent thisArg) and
+                // take no thisArg parameter, so routing a 2-arg call through
+                // them silently DROPS the thisArg — an extracted builtin
+                // method used as the callback (`arr.forEach(set.add, set)`,
+                // @babel/types' alias-expansion loop) then brand-checks
+                // `this === undefined` and throws "Method Set.prototype.add
+                // called on incompatible receiver". Mirror the static lowering
+                // rule (lower_array_method.rs): with a thisArg present, route
+                // through the spec-generic array-like engine, which binds it
+                // for each callback call (real-array receivers keep a fast
+                // element path there). `flatMap` has no engine entry (the
+                // static path shares that thisArg gap) and falls through
+                // unchanged.
+                if args_len >= 2
+                    && !args_ptr.is_null()
+                    && matches!(
+                        method_name,
+                        "forEach"
+                            | "map"
+                            | "filter"
+                            | "some"
+                            | "every"
+                            | "find"
+                            | "findIndex"
+                            | "findLast"
+                            | "findLastIndex"
+                    )
+                {
+                    if let Some(result) = crate::array::dispatch_arraylike_read_method(
+                        object,
+                        method_name,
+                        args_ptr,
+                        args_len,
+                    ) {
+                        return Some(result);
+                    }
+                }
                 match method_name {
                     "toString" => {
                         let arr = raw_ptr as *const crate::array::ArrayHeader;

crates/perry/tests/issue_6658_extracted_builtin_method_thisarg.rs

@@ -0,0 +1,187 @@
+//! Regression tests for #6658 (pi wall #7): an extracted builtin method used
+//! as a callback with an explicit `thisArg` — `arr.forEach(set.add, set)` —
+//! must invoke the builtin with `this = thisArg`, exactly as node does.
+//!
+//! Trigger in the wild: @babel/types' alias-expansion loop
+//! (`e5 ? e5.forEach(t4.add, t4) : t4.add(r4)`, pi-bundle.mjs:203827) threw
+//! "TypeError: Method Set.prototype.add called on incompatible receiver"
+//! during pi-native module init.
+//!
+//! Root cause: the DYNAMIC method-dispatch tower (`js_native_call_method` →
+//! the dense-array arms in `native_call_method/handle_methods.rs`) dropped
+//! `args[1]` for the whole Array.prototype callback family and dispatched to
+//! the dense helpers, which bind the callback's `this` to undefined (the spec
+//! rule for an ABSENT thisArg). The STATIC lowering already routed
+//! explicit-thisArg calls through the this-binding `js_arraylike_*` engine —
+//! a receiver only reaches the dynamic tower when codegen can't prove its
+//! type (here: an object member read through a DYNAMIC key), which is why
+//! only the combined @babel/types shape reproduced and every
+//! statically-provable simplification worked. The fix mirrors the static
+//! rule in the dynamic tower: with a thisArg present, route through
+//! `dispatch_arraylike_read_method`.
+//!
+//! Also pinned: node/V8's brand-check TypeError message, receiver rendering
+//! included (`NoSideEffectsToString`: "#<Object>", "undefined", "5.5", ...).
+//!
+//! Expected outputs are node v26's, byte for byte.
+
+use std::path::PathBuf;
+use std::process::Command;
+
+fn perry_bin() -> PathBuf {
+    PathBuf::from(env!("CARGO_BIN_EXE_perry"))
+}
+
+fn compile_and_run(dir: &std::path::Path, source: &str) -> String {
+    let entry = dir.join("main.ts");
+    let output = dir.join("main_bin");
+    std::fs::write(&entry, source).expect("write entry");
+
+    let compile = Command::new(perry_bin())
+        .current_dir(dir)
+        .arg("compile")
+        .arg(&entry)
+        .arg("-o")
+        .arg(&output)
+        .output()
+        .expect("run perry compile");
+    assert!(
+        compile.status.success(),
+        "perry compile failed\nstdout:\n{}\nstderr:\n{}",
+        String::from_utf8_lossy(&compile.stdout),
+        String::from_utf8_lossy(&compile.stderr)
+    );
+
+    let run = Command::new(&output)
+        .current_dir(dir)
+        .output()
+        .expect("run compiled binary");
+    assert!(
+        run.status.success(),
+        "compiled binary failed\nstatus: {:?}\nstdout:\n{}\nstderr:\n{}",
+        run.status,
+        String::from_utf8_lossy(&run.stdout),
+        String::from_utf8_lossy(&run.stderr)
+    );
+    String::from_utf8_lossy(&run.stdout).into_owned()
+}
+
+/// The issue's combined shape verbatim, the five previously-ruled-out simpler
+/// shapes as anchors, and the minimal dynamic-tower trigger (a dynamic-key
+/// member read defeats flow typing, so `.forEach` dispatches through the
+/// runtime tower that dropped the thisArg) with Set.prototype.add,
+/// Map.prototype.set, and Array.prototype.push extracted as callbacks.
+#[test]
+fn extracted_builtin_method_callback_thisarg_matches_node() {
+    let dir = tempfile::tempdir().expect("tempdir");
+    let stdout = compile_and_run(
+        dir.path(),
+        r#"
+const FLIPPED: any = { A: ["x", "y"], B: null };
+const allExpandedTypes: any[] = [
+  { types: ["A", "B"], set: new Set() },
+  { types: ["B"], set: new Set() },
+];
+for (const { types: e4, set: t4 } of allExpandedTypes) {
+  for (const r4 of e4) {
+    const e5 = FLIPPED[r4];
+    e5 ? e5.forEach(t4.add, t4) : t4.add(r4);
+  }
+}
+console.log("sizes:", allExpandedTypes[0].set.size, allExpandedTypes[1].set.size);
+
+const s = new Set<string>();
+["a", "b"].forEach(s.add, s);
+const t4: any = new Set();
+["a"].forEach(t4.add, t4);
+const add = t4.add;
+add.call(t4, "q");
+const rows: any[] = [{ set: new Set() }];
+for (const { set: r } of rows) ["a"].forEach(r.add, r);
+const e5t: any = ["z1", "z2"];
+const t5: any = new Set();
+e5t ? e5t.forEach(t5.add, t5) : t5.add("z");
+console.log("anchors:", s.size, t4.size, rows[0].set.size, t5.size);
+
+const SRC: any = { A: ["x", "y"] };
+const KEYS = ["A"];
+const dyn = SRC[KEYS[0]];
+const ds: any = new Set();
+dyn.forEach(ds.add, ds);
+const m: any = new Map();
+dyn.forEach(m.set, m);
+const a2: any = [];
+dyn.forEach(a2.push, a2);
+console.log("dynamic:", ds.size, m.size, m.get("x"), m.get("y"), a2.length);
+
+const marker = { tag: "T" };
+const seen: boolean[] = [];
+function observe(this: any): boolean {
+  seen.push(this === marker);
+  return true;
+}
+dyn.forEach(observe, marker);
+dyn.map(observe, marker);
+dyn.filter(observe, marker);
+dyn.every(observe, marker);
+dyn.some(function (this: any) { seen.push(this === marker); return false; }, marker);
+dyn.find(function (this: any) { seen.push(this === marker); return false; }, marker);
+dyn.findIndex(function (this: any) { seen.push(this === marker); return false; }, marker);
+dyn.findLast(function (this: any) { seen.push(this === marker); return false; }, marker);
+dyn.findLastIndex(function (this: any) { seen.push(this === marker); return false; }, marker);
+console.log("family this-bound:", seen.length, seen.every(Boolean));
+"#,
+    );
+    assert_eq!(
+        stdout,
+        "sizes: 3 1\n\
+         anchors: 2 2 1 2\n\
+         dynamic: 2 2 0 1 6\n\
+         family this-bound: 18 true\n"
+    );
+}
+
+/// The brand-check TypeError cases where node DOES throw — message-identical,
+/// including V8's `NoSideEffectsToString` receiver rendering.
+#[test]
+fn incompatible_receiver_message_matches_node() {
+    let dir = tempfile::tempdir().expect("tempdir");
+    let stdout = compile_and_run(
+        dir.path(),
+        r#"
+function thrown(fn: () => void): string {
+  try {
+    fn();
+    return "NO_THROW";
+  } catch (e: any) {
+    return e.constructor.name + ": " + e.message;
+  }
+}
+const SRC: any = { A: ["x"] };
+const KEYS = ["A"];
+const dyn = SRC[KEYS[0]];
+const t4: any = new Set();
+const m: any = new Map();
+console.log("undef:", thrown(() => dyn.forEach(t4.add)));
+console.log("obj:", thrown(() => dyn.forEach(t4.add, {})));
+console.log("cross:", thrown(() => dyn.forEach(m.set, t4)));
+console.log("num:", thrown(() => dyn.forEach(t4.add, 5.5 as any)));
+console.log("str:", thrown(() => dyn.forEach(t4.add, "abc" as any)));
+console.log("null:", thrown(() => (Set.prototype.add as any).call(null, 1)));
+class Foo {}
+console.log("inst:", thrown(() => (Set.prototype.add as any).call(new Foo(), 1)));
+console.log("arr:", thrown(() => (Set.prototype.add as any).call([1, 2], 1)));
+"#,
+    );
+    assert_eq!(
+        stdout,
+        "undef: TypeError: Method Set.prototype.add called on incompatible receiver undefined\n\
+         obj: TypeError: Method Set.prototype.add called on incompatible receiver #<Object>\n\
+         cross: TypeError: Method Map.prototype.set called on incompatible receiver #<Set>\n\
+         num: TypeError: Method Set.prototype.add called on incompatible receiver 5.5\n\
+         str: TypeError: Method Set.prototype.add called on incompatible receiver abc\n\
+         null: TypeError: Method Set.prototype.add called on incompatible receiver null\n\
+         inst: TypeError: Method Set.prototype.add called on incompatible receiver #<Foo>\n\
+         arr: TypeError: Method Set.prototype.add called on incompatible receiver [object Array]\n"
+    );
+}