diff --git a/crates/perry-codegen/src/lower_array_method.rs b/crates/perry-codegen/src/lower_array_method.rs index 48004ea37e..4440152585 100644 --- a/crates/perry-codegen/src/lower_array_method.rs +++ b/crates/perry-codegen/src/lower_array_method.rs @@ -45,24 +45,12 @@ //! so `arr.slice(1, 2)`, `arr.includes(x)`, `arr.join(",")` and every no-arg //! method emit byte-identical IR. //! -//! ## Known window deliberately NOT closed here: #7213 -//! -//! The `toString` arm unboxes the interned `","` through -//! `js_get_string_pointer_unified`, whose SSO branch allocates, while the -//! receiver handle is already in a register. `js_string_materialize_to_heap`'s -//! rustdoc records why that is unexploitable today (the alloc-point arm forces -//! a conservative stack scan, which both makes the copying minor ineligible and -//! finds the register) and why it is tracked as #7213 rather than papered over. -//! Closing it needs a combinator that roots across a collection point this -//! module *emits* rather than one it infers from an operand list; that -//! combinator should arrive with the slice that needs it, not ahead of one. - use anyhow::{bail, Result}; use perry_hir::Expr; use crate::expr::{ emit_root_nanbox_store_on_block, emit_write_barrier, nanbox_pointer_inline, - nanbox_string_inline, unbox_str_handle, unbox_to_i64, FnCtx, + nanbox_string_inline, unbox_to_i64, FnCtx, }; use crate::nanbox::{double_literal, TAG_UNDEFINED}; use crate::rooting; @@ -147,10 +135,10 @@ pub(crate) fn emit_grow_mutator_writeback( /// migration, so no behaviour changes in this slice. fn lowered_arg_count(property: &str, args: &[Expr]) -> usize { let declared = match property { - // Receiver-only. `arr.toString()` and `arr.reverse()` ignore their - // arguments entirely today; lowering them here would be a behaviour - // change (an added evaluation), not a rooting fix. - "toString" | "reverse" => 0, + // Receiver-only. `arr.reverse()` ignores its arguments entirely today; + // lowering them here would be a behaviour change (an added evaluation), + // not a rooting fix. + "reverse" => 0, // One: separator, depth, callback, comparator or index. "join" | "flat" | "flatMap" | "sort" | "at" | "findLast" | "findLastIndex" => 1, // Two: callback + thisArg, value + fromIndex, start + end, source + @@ -282,27 +270,12 @@ pub(crate) fn lower_array_method( )) } "toString" => { - // arr.toString() == arr.join(",") - let key_idx = ctx.strings.intern(","); - let handle_global = format!("@{}", ctx.strings.entry(key_idx).handle_global); - let blk = ctx.block(); - let sep_box = blk.load(DOUBLE, &handle_global); - let recv_handle = unbox_to_i64(blk, recv_box); - // Interned literal "," — heap allocated at module init, so - // `unbox_to_i64` would technically work, but routing through - // `unbox_str_handle` keeps the path uniform with the `join` - // arm and is robust if interning ever changes to SSO-eligible. - // - // This call is the #7213 window described in the module header: - // its SSO branch allocates while `recv_handle` is already in a - // register. Left as-is, with its reasoning recorded there. - let sep_handle = unbox_str_handle(blk, &sep_box); - let result_handle = blk.call( - I64, - "js_array_join", - &[(I64, &recv_handle), (I64, &sep_handle)], - ); - Ok(nanbox_string_inline(blk, &result_handle)) + // A source-level method call must resolve the current property: + // `Array.prototype.toString` is writable, and replacing it with + // `Object.prototype.toString` must affect even statically-known + // arrays. The runtime dispatcher preserves own-property + // precedence and invokes the live prototype method. + emit_native_method_dispatch(ctx, recv_box, property, arg_vals) } "concat" => { // #2805: arr.concat(...args) — spec-complete, non-mutating, variadic. @@ -1236,7 +1209,6 @@ mod tests { // (property, args supplied, arguments lowered) let cases: &[(&str, usize, usize)] = &[ // Receiver-only: arguments are not evaluated at all today. - ("toString", 2, 0), ("reverse", 1, 0), // One leading argument. ("join", 0, 0), @@ -1268,6 +1240,8 @@ mod tests { ("pop", 2, 2), ("shift", 1, 1), ("entries", 1, 1), + // Live method dispatch forwards every argument to a user override. + ("toString", 2, 2), ("next", 2, 2), ("someUserMethod", 3, 3), ]; diff --git a/crates/perry-hir/src/lower/expr_call/intrinsics/apply_call.rs b/crates/perry-hir/src/lower/expr_call/intrinsics/apply_call.rs index d6ec74993f..f8c1952fba 100644 --- a/crates/perry-hir/src/lower/expr_call/intrinsics/apply_call.rs +++ b/crates/perry-hir/src/lower/expr_call/intrinsics/apply_call.rs @@ -409,6 +409,27 @@ fn is_string_prototype_generic_method(recv: &ast::Expr, method: &str) -> bool { ) } +/// `Array.prototype.toString` is itself a generic dispatch operation: it gets +/// `this.join`, calls that value when callable, and otherwise calls the +/// intrinsic `Object.prototype.toString`. Folding `.call(receiver)` into +/// `receiver.toString()` changes all three operations, so leave this one method +/// on the reflective runtime path. +fn is_array_prototype_to_string(recv: &ast::Expr, method: &str) -> bool { + if method != "toString" { + return false; + } + let ast::Expr::Member(member) = recv else { + return false; + }; + let ast::MemberProp::Ident(prop) = &member.prop else { + return false; + }; + if prop.sym.as_ref() != "prototype" { + return false; + } + matches!(member.obj.as_ref(), ast::Expr::Ident(base) if base.sym.as_ref() == "Array") +} + pub(crate) fn try_builtin_prototype_method_apply_call( ctx: &mut LoweringContext, call: &ast::CallExpr, @@ -475,6 +496,9 @@ pub(crate) fn try_builtin_prototype_method_apply_call( if is_string_prototype_generic_method(inner.obj.as_ref(), method_ident.sym.as_ref()) { return Ok(None); } + if is_array_prototype_to_string(inner.obj.as_ref(), method_ident.sym.as_ref()) { + return Ok(None); + } method_ident.clone() } ast::Expr::Ident(id) => match ctx.builtin_proto_method_locals.get(id.sym.as_ref()) { @@ -745,6 +769,9 @@ pub(crate) fn as_builtin_proto_method_ref( if is_string_prototype_generic_method(&member.obj, method.sym.as_ref()) { return None; } + if is_array_prototype_to_string(&member.obj, method.sym.as_ref()) { + return None; + } // For a `.prototype` receiver, any method ident is accepted (mirrors // the existing `.call`/`.apply` rewrite, which doesn't gate on the method // name). For an array/string literal receiver, gate on the known @@ -834,4 +861,15 @@ mod tests { assert!(!is_string_prototype_generic_method(&recv, "toString")); assert!(!is_string_prototype_generic_method(&recv, "valueOf")); } + + #[test] + fn array_to_string_stays_reflective() { + let recv = prototype_member("Array"); + assert!(is_array_prototype_to_string(&recv, "toString")); + assert!(!is_array_prototype_to_string(&recv, "join")); + assert!(!is_array_prototype_to_string( + &prototype_member("String"), + "toString" + )); + } } diff --git a/crates/perry-runtime/src/object/global_this/array_error.rs b/crates/perry-runtime/src/object/global_this/array_error.rs index baa64bb167..89ae1e42af 100644 --- a/crates/perry-runtime/src/object/global_this/array_error.rs +++ b/crates/perry-runtime/src/object/global_this/array_error.rs @@ -877,6 +877,19 @@ pub(crate) extern "C" fn array_proto_join_thunk( let this = crate::object::js_implicit_this_get(); crate::array::js_arraylike_join(this, sep) } + +/// Spec `Array.prototype.toString`: `ToObject(this)`, get the receiver's live +/// `join` property, call it when callable, and otherwise use the intrinsic +/// `Object.prototype.toString`. This must be a real thunk because keeping +/// `Array.prototype.toString.call(x)` reflective is what preserves the generic +/// receiver semantics; rewriting it to `x.toString()` loses the Array method. +pub(crate) extern "C" fn array_prototype_to_string_thunk( + _c: *const crate::closure::ClosureHeader, +) -> f64 { + let this = crate::object::js_implicit_this_get(); + crate::value::array_prototype_to_string(this) +} + pub(crate) extern "C" fn array_prototype_concat_thunk( _c: *const crate::closure::ClosureHeader, rest: f64, diff --git a/crates/perry-runtime/src/object/global_this/proto_methods.rs b/crates/perry-runtime/src/object/global_this/proto_methods.rs index d63cf18478..5f68060b5c 100644 --- a/crates/perry-runtime/src/object/global_this/proto_methods.rs +++ b/crates/perry-runtime/src/object/global_this/proto_methods.rs @@ -30,10 +30,11 @@ const OBJECT_PROTO_METHODS: &[(&str, u32)] = &[ /// `typeof Array.prototype.map === "function"` and `.name === "map"` /// agree with Node when the value is read through indirection. /// -/// Two of these methods retain dedicated thunks for spec-accurate call -/// behavior — `Array.prototype.slice` (ramda's curry/variadic helpers +/// These methods retain dedicated thunks for spec-accurate call behavior — +/// `Array.prototype.slice` (ramda's curry/variadic helpers /// reach through `Array.prototype.slice.call(args, …)` and depend on it -/// returning a real sliced array, even via indirection) and +/// returning a real sliced array, even via indirection), +/// `Array.prototype.toString` (generic `join` dispatch), and /// `Object.prototype.toString` (ramda's `_isArguments.js` IIFE calls /// `Object.prototype.toString.call(arguments)` at module-init time). /// All other methods are noop-backed: typeof + `.name` introspection @@ -116,6 +117,12 @@ pub(crate) fn populate_builtin_prototype_methods(builtin_name: &str, proto_obj: array_prototype_slice_thunk as *const u8, 2, ); + install_proto_method( + proto_obj, + "toString", + array_prototype_to_string_thunk as *const u8, + 0, + ); install_noop_proto_methods( proto_obj, &[ @@ -127,7 +134,6 @@ pub(crate) fn populate_builtin_prototype_methods(builtin_name: &str, proto_obj: ("toReversed", 0), ("toSorted", 1), ("toSpliced", 2), - ("toString", 0), ("with", 2), ], ); diff --git a/crates/perry-runtime/src/object/native_call_method/handle_methods.rs b/crates/perry-runtime/src/object/native_call_method/handle_methods.rs index d4c150ae9c..84a8f058a4 100644 --- a/crates/perry-runtime/src/object/native_call_method/handle_methods.rs +++ b/crates/perry-runtime/src/object/native_call_method/handle_methods.rs @@ -268,12 +268,10 @@ pub(super) unsafe fn dispatch_handle( } match method_name { "toString" => { - let arr = raw_ptr as *const crate::array::ArrayHeader; - let s = crate::array::js_array_join_value( - arr, - f64::from_bits(crate::value::TAG_UNDEFINED), - ); - return Some(f64::from_bits(JSValue::string_ptr(s).bits())); + return Some(crate::value::call_array_prototype_to_string_method( + object_handle.get_nanbox_f64(), + arg_handles, + )); } "map" if args_len >= 1 && !args_ptr.is_null() => { let arr = raw_ptr as *const crate::array::ArrayHeader; diff --git a/crates/perry-runtime/src/value/mod.rs b/crates/perry-runtime/src/value/mod.rs index 6b9333de75..bb14298f70 100644 --- a/crates/perry-runtime/src/value/mod.rs +++ b/crates/perry-runtime/src/value/mod.rs @@ -115,7 +115,8 @@ pub use dyn_index::{js_dyn_index_get, js_dyn_index_set, js_is_undefined_or_bare_ // ----- to-string conversion helpers ----- pub(crate) use to_string::{ - coerce_validate_radix, function_to_primitive_for_add, function_to_string_method_result, + array_prototype_to_string, call_array_prototype_to_string_method, coerce_validate_radix, + function_to_primitive_for_add, function_to_string_method_result, ordinary_to_primitive_for_toprimitive, ordinary_to_primitive_number_for_add, to_primitive_number, OrdinaryToPrimitiveOutcome, }; diff --git a/crates/perry-runtime/src/value/to_string.rs b/crates/perry-runtime/src/value/to_string.rs index 2db7306c55..1dd7b8f60e 100644 --- a/crates/perry-runtime/src/value/to_string.rs +++ b/crates/perry-runtime/src/value/to_string.rs @@ -660,8 +660,16 @@ enum ArrayToStringOutcome { } unsafe fn array_prototype_to_string_override(value: f64) -> ArrayToStringOutcome { + // `value`, the prototype, its key, and the resolved method are all live + // across allocating operations below. Keep each one visible to the moving + // collector and re-read its address after every allocation. + let scope = crate::gc::RuntimeHandleScope::new(); + let value_handle = scope.root_nanbox_f64(value); + let key_handle = + scope.root_raw_mut_ptr(crate::string::js_string_from_bytes(b"toString".as_ptr(), 8)); let proto = crate::object::builtin_prototype_value("Array"); - let proto_bits = proto.to_bits(); + let proto_handle = scope.root_nanbox_f64(proto); + let proto_bits = proto_handle.get_nanbox_f64().to_bits(); if (proto_bits & 0xFFFF_0000_0000_0000) != POINTER_TAG { return ArrayToStringOutcome::UseDefaultJoin; } @@ -669,9 +677,10 @@ unsafe fn array_prototype_to_string_override(value: f64) -> ArrayToStringOutcome if proto_ptr.is_null() { return ArrayToStringOutcome::UseDefaultJoin; } - let key = crate::string::js_string_from_bytes(b"toString".as_ptr(), 8); + let key = key_handle.get_raw_mut_ptr(); let method = crate::object::js_object_get_field_by_name(proto_ptr, key); - let method_bits = method.bits(); + let method_handle = scope.root_nanbox_u64(method.bits()); + let method_bits = method_handle.get_nanbox_u64(); if (method_bits & 0xFFFF_0000_0000_0000) != POINTER_TAG { return ArrayToStringOutcome::UseDefaultJoin; } @@ -683,10 +692,14 @@ unsafe fn array_prototype_to_string_override(value: f64) -> ArrayToStringOutcome if (*closure).func_ptr == crate::object::global_this_builtin_noop_thunk as *const u8 { return ArrayToStringOutcome::UseDefaultJoin; } - let bound = crate::closure::clone_closure_rebind_this(method_bits, value); - let prev_this = crate::object::js_implicit_this_set(value); - let ret = crate::closure::js_native_call_value(f64::from_bits(bound), std::ptr::null(), 0); - crate::object::js_implicit_this_set(prev_this); + let receiver = value_handle.get_nanbox_f64(); + let bound = crate::closure::clone_closure_rebind_this(method_bits, receiver); + let bound_handle = scope.root_nanbox_u64(bound); + let prev_this = crate::object::js_implicit_this_set(receiver); + let prev_this_handle = scope.root_nanbox_f64(prev_this); + let ret = + crate::closure::js_native_call_value(bound_handle.get_nanbox_f64(), std::ptr::null(), 0); + crate::object::js_implicit_this_set(prev_this_handle.get_nanbox_f64()); if is_primitive_value(ret) { ArrayToStringOutcome::Primitive(ret) } else { @@ -694,6 +707,97 @@ unsafe fn array_prototype_to_string_override(value: f64) -> ArrayToStringOutcome } } +/// Resolve and invoke the current `Array.prototype.toString` method for a +/// source-level `array.toString()` call. Static array lowering must not replace +/// this with `join(",")`: the prototype property is writable and its live value +/// (for example `Object.prototype.toString`) must win. +pub(crate) fn call_array_prototype_to_string_method( + value: f64, + arg_handles: &[crate::gc::RuntimeHandle<'_>], +) -> f64 { + unsafe { + let scope = crate::gc::RuntimeHandleScope::new(); + let receiver_handle = scope.root_nanbox_f64(value); + let key_handle = + scope.root_raw_mut_ptr(crate::string::js_string_from_bytes(b"toString".as_ptr(), 8)); + let prototype_handle = + scope.root_nanbox_f64(crate::object::builtin_prototype_value("Array")); + let prototype_bits = prototype_handle.get_nanbox_f64().to_bits(); + if (prototype_bits & TAG_MASK) != POINTER_TAG { + crate::error::js_throw_type_error_not_a_function( + std::ptr::null(), + 0, + b"toString".as_ptr(), + 8, + ); + } + + let prototype = (prototype_bits & POINTER_MASK) as *const crate::object::ObjectHeader; + let method = + crate::object::js_object_get_field_by_name(prototype, key_handle.get_raw_mut_ptr()); + let method_handle = scope.root_nanbox_u64(method.bits()); + if !crate::object::value_is_callable(method_handle.get_nanbox_f64()) { + crate::error::js_throw_type_error_not_a_function( + std::ptr::null(), + 0, + b"toString".as_ptr(), + 8, + ); + } + + let receiver = receiver_handle.get_nanbox_f64(); + let rebound = + crate::closure::rebind_explicit_this(method_handle.get_nanbox_f64(), receiver); + let rebound_handle = scope.root_nanbox_f64(rebound); + let previous = crate::object::js_implicit_this_set(receiver); + let previous_handle = scope.root_nanbox_f64(previous); + let args = crate::gc::RuntimeHandleScope::refreshed_nanbox_f64_slice(arg_handles); + let result = crate::closure::js_native_call_value( + rebound_handle.get_nanbox_f64(), + args.as_ptr(), + args.len(), + ); + crate::object::js_implicit_this_set(previous_handle.get_nanbox_f64()); + result + } +} + +/// Execute the generic `Array.prototype.toString` algorithm for a call-site +/// receiver. Kept here so both the reflective prototype thunk and the native +/// array method dispatcher use the same live `join` lookup and intrinsic +/// Object-toString fallback. +pub(crate) fn array_prototype_to_string(value: f64) -> f64 { + let value_kind = JSValue::from_bits(value.to_bits()); + if value_kind.is_undefined() || value_kind.is_null() { + crate::object::has_own_helpers::throw_to_object_nullish_type_error(); + } + + let scope = crate::gc::RuntimeHandleScope::new(); + let receiver_handle = scope.root_nanbox_f64(value); + let join = unsafe { + crate::value::js_get_property( + receiver_handle.get_nanbox_f64(), + b"join".as_ptr() as i64, + b"join".len() as i64, + ) + }; + let join_handle = scope.root_nanbox_f64(join); + if !crate::object::value_is_callable(join_handle.get_nanbox_f64()) { + return unsafe { crate::object::js_object_to_string(receiver_handle.get_nanbox_f64()) }; + } + + let receiver = receiver_handle.get_nanbox_f64(); + let rebound = crate::closure::rebind_explicit_this(join_handle.get_nanbox_f64(), receiver); + let rebound_handle = scope.root_nanbox_f64(rebound); + let previous = crate::object::js_implicit_this_set(receiver); + let previous_handle = scope.root_nanbox_f64(previous); + let result = unsafe { + crate::closure::js_native_call_value(rebound_handle.get_nanbox_f64(), std::ptr::null(), 0) + }; + crate::object::js_implicit_this_set(previous_handle.get_nanbox_f64()); + result +} + unsafe fn call_method_for_primitive( scope: &crate::gc::RuntimeHandleScope, value_handle: &crate::gc::RuntimeHandle<'_>, diff --git a/crates/perry/tests/fixtures/issue_5898_array_to_string.ts b/crates/perry/tests/fixtures/issue_5898_array_to_string.ts new file mode 100644 index 0000000000..a0bbb89020 --- /dev/null +++ b/crates/perry/tests/fixtures/issue_5898_array_to_string.ts @@ -0,0 +1,18 @@ +const originalToString = Array.prototype.toString; + +(Array.prototype as any).toString = Object.prototype.toString; +console.log(Array.prototype.toString === Object.prototype.toString); +console.log(Array().toString()); +console.log(Array(0, 1, 2).toString()); +console.log(new Array().toString()); +console.log(new Array(0, 1, 2).toString()); +console.log(new Array(0).toString()); + +(Array.prototype as any).toString = function () { return "custom toString"; }; +console.log(Array().toString()); + +(Array.prototype as any).toString = originalToString; +console.log(Array.prototype.toString.call(true)); +console.log(Array.prototype.toString.call(false)); +console.log(Array.prototype.toString.call({ join() { return "custom join"; } })); +console.log(Array.prototype.toString.call({ join: null })); diff --git a/crates/perry/tests/issue_5898_array_to_string.rs b/crates/perry/tests/issue_5898_array_to_string.rs new file mode 100644 index 0000000000..61197cd2c8 --- /dev/null +++ b/crates/perry/tests/issue_5898_array_to_string.rs @@ -0,0 +1,66 @@ +//! Regression coverage for the Array.prototype.toString subcluster in #5898. +//! The method must remain reflective through `.call`, dispatch a callable +//! `join`, and fall back to the Object.prototype.toString intrinsic otherwise. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +#[test] +fn array_to_string_uses_the_live_prototype_method_and_generic_receiver() { + let dir = tempfile::tempdir().expect("tempdir"); + let entry = dir.path().join("main.ts"); + let output = dir.path().join("main_bin"); + std::fs::write( + &entry, + include_str!("fixtures/issue_5898_array_to_string.ts"), + ) + .expect("write entry"); + + let compile = Command::new(perry_bin()) + .current_dir(dir.path()) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .arg("--no-cache") + .env("PERRY_NO_AUTO_OPTIMIZE", "1") + .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.path()) + .output() + .expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&run.stdout), + concat!( + "true\n", + "[object Array]\n", + "[object Array]\n", + "[object Array]\n", + "[object Array]\n", + "[object Array]\n", + "custom toString\n", + "[object Boolean]\n", + "[object Boolean]\n", + "custom join\n", + "[object Object]\n", + ) + ); +}