From c3083d288e5e36f577e233aa79f602bec868794a Mon Sep 17 00:00:00 2001 From: Ralph Kuepper Date: Wed, 5 Aug 2026 21:55:13 +0200 Subject: [PATCH 1/2] =?UTF-8?q?fix(hir):=20boxed=20primitive=20wrappers=20?= =?UTF-8?q?are=20not=20arrays=20=E2=80=94=20stop=20the=20array=20fold=20fr?= =?UTF-8?q?om=20hijacking=20their=20methods=20(#5902)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A local typed `Named("Object")` / `Named("Number")` / `Named("Boolean")` — i.e. `new Object(true)`, `new Number(1)`, `new Boolean` — walked straight into the array fast path in `try_local_array_methods`, because the gate that guards it (`is_known_not_string`) reads "definitely not a string" as positive evidence of array-ness. `inst.indexOf(x)` then lowered to `Expr::ArrayIndexOf`, which reads the wrapper's ObjectHeader as an ArrayHeader and answers -1 — even when the receiver owns or inherits a real `indexOf`, so a borrowed `String.prototype.indexOf` never ran. `Named("String")` was already excluded one arm earlier (`is_boxed_string_wrapper`, which routes to the ToString-coercing string dispatch); this extends the same reasoning to the remaining boxed primitive wrappers and to `Object`, routing them to the generic runtime dispatch instead, which resolves the own/inherited property first and still reaches the array engine for a genuine array. `new Array()` is unaffected: `Expr::New { class_name: "Array" }` already infers `Type::Array`, not `Type::Named("Array")`, so real arrays keep the fast path (verified: the `Array*` folds are still emitted, and a mixed array/string/boxed-string/class probe matches node byte-for-byte). Adds `receiver_is_non_array_builtin_wrapper` as a standalone predicate with three per-PR-visible unit tests (`cargo test -p perry-hir --lib`), including one asserting `Named("String")` is deliberately NOT claimed here. --- .../lower/expr_call/local_array_methods.rs | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/crates/perry-hir/src/lower/expr_call/local_array_methods.rs b/crates/perry-hir/src/lower/expr_call/local_array_methods.rs index cd9e527f7f..2c376315c4 100644 --- a/crates/perry-hir/src/lower/expr_call/local_array_methods.rs +++ b/crates/perry-hir/src/lower/expr_call/local_array_methods.rs @@ -42,6 +42,37 @@ fn receiver_is_class_instance(recv_ty: Option<&Type>) -> bool { } } +/// #5902: is `recv_ty` a boxed primitive wrapper (`new Number(1)`, +/// `new Boolean`, …) or a plain `Object` wrapper (`new Object(true)`)? +/// +/// These are never arrays, but they are also not strings, and the array +/// fast-path gate reads "definitely not a string" as positive evidence of +/// array-ness (`is_known_not_string`). So `Named("Object")` / `Named("Number")` +/// / `Named("Boolean")` walked straight into the array block and lowered +/// `inst.indexOf(x)` to `Expr::ArrayIndexOf`, which reads the wrapper's +/// `ObjectHeader` as an `ArrayHeader` and answers `-1` — even when the receiver +/// owns or inherits a real `indexOf`. test262 `S15.5.4.7_A1_T1` borrows +/// `String.prototype.indexOf` onto `new Object(true)`, and `S15.5.4.7_A4_T4` +/// puts it on `Number.prototype`; both expect the borrowed method to run. +/// +/// `Named("String")` is deliberately absent — it is handled one step earlier by +/// `is_boxed_string_wrapper`, which routes to the *string* dispatch (so the +/// wrapper gets `ToString`-coerced) rather than to generic dispatch. +/// +/// Declining the fold is always safe: the generic runtime dispatch resolves the +/// own/inherited property first and still reaches the array engine for a +/// genuine array, so a mistyped-but-really-an-array receiver keeps working. +fn receiver_is_non_array_builtin_wrapper(recv_ty: Option<&Type>) -> bool { + matches!( + recv_ty, + Some(Type::Named(n)) + if matches!( + n.as_str(), + "Object" | "Number" | "Boolean" | "Symbol" | "BigInt" + ) + ) +} + pub(super) fn try_local_array_methods( ctx: &mut LoweringContext, call: &ast::CallExpr, @@ -107,6 +138,7 @@ pub(super) fn try_local_array_methods( // file because they aren't Array methods. let is_boxed_string_wrapper = matches!(type_info, Some(Type::Named(n)) if n == "String"); + let is_non_array_builtin_wrapper = receiver_is_non_array_builtin_wrapper(type_info); let is_known_string = type_info .map(|ty| matches!(ty, Type::String)) .unwrap_or(false) @@ -245,6 +277,8 @@ pub(super) fn try_local_array_methods( false // user class — must dispatch to class method, skip array fast-path } else if is_object_type { false // object type literal — dispatch via method call, not array ops + } else if is_non_array_builtin_wrapper { + false // boxed primitive wrapper / plain Object — never an array } else if is_buffer_type { false // Buffer/Uint8Array — runtime dispatch handles byte-level methods } else if is_node_stream_readable_type { @@ -1085,3 +1119,58 @@ pub(super) fn try_local_array_methods( } Ok(Err(args)) } + +#[cfg(test)] +mod tests { + use super::*; + + fn named(n: &str) -> Option { + Some(Type::Named(n.to_string())) + } + + #[test] + fn boxed_primitive_wrappers_decline_the_array_fold() { + // #5902: `new Object(true)` / `new Boolean` / `new Number(1)` are not + // strings, but they are not arrays either — the array fast path must + // not claim `indexOf`/`lastIndexOf`/`slice`/`includes` on them. + for n in ["Object", "Number", "Boolean", "Symbol", "BigInt"] { + assert!( + receiver_is_non_array_builtin_wrapper(named(n).as_ref()), + "{n} wrapper must decline the array fast path" + ); + } + } + + #[test] + fn string_wrapper_is_not_claimed_here() { + // `Named("String")` routes to the STRING dispatch via + // `is_boxed_string_wrapper`, one arm earlier, so this predicate must + // leave it alone — claiming it here would send a boxed String to + // generic dispatch instead of the ToString-coercing string path. + assert!(!receiver_is_non_array_builtin_wrapper( + named("String").as_ref() + )); + } + + #[test] + fn real_array_receivers_keep_the_fold() { + assert!(!receiver_is_non_array_builtin_wrapper(Some(&Type::Array( + Box::new(Type::Number) + )))); + assert!(!receiver_is_non_array_builtin_wrapper(Some( + &Type::Generic { + base: "Array".to_string(), + type_args: vec![Type::Number], + } + ))); + // An unknown receiver is gated elsewhere (`is_ambiguous_method` / + // `is_arraylike_mutator_method`), not here. + assert!(!receiver_is_non_array_builtin_wrapper(Some(&Type::Any))); + assert!(!receiver_is_non_array_builtin_wrapper(None)); + // A user class named e.g. `Number`-adjacent must not be confused with + // the builtin set; only the exact builtin names are claimed. + assert!(!receiver_is_non_array_builtin_wrapper( + named("NumberLike").as_ref() + )); + } +} From 83482623454b4ee5ef336b551868fc37704df81f Mon Sep 17 00:00:00 2001 From: Ralph Kuepper Date: Wed, 5 Aug 2026 22:04:03 +0200 Subject: [PATCH 2/2] docs: changelog fragment for #7470 --- changelog.d/7470-boxed-wrapper-array-fold.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/7470-boxed-wrapper-array-fold.md diff --git a/changelog.d/7470-boxed-wrapper-array-fold.md b/changelog.d/7470-boxed-wrapper-array-fold.md new file mode 100644 index 0000000000..8bd1cb00f9 --- /dev/null +++ b/changelog.d/7470-boxed-wrapper-array-fold.md @@ -0,0 +1 @@ +fix(hir): a boxed primitive wrapper receiver is not an array, so the array fast path no longer hijacks its methods (#5902). `try_local_array_methods` gates the `Expr::Array*` fold on `is_known_not_string`, which reads "definitely not a string" as positive evidence of *array*-ness — so a local typed `Named("Object")` / `Named("Number")` / `Named("Boolean")` (`new Object(true)`, `new Number(1)`, `new Boolean`) walked straight in and lowered `inst.indexOf(x)` to `Expr::ArrayIndexOf`, which reads the wrapper's `ObjectHeader` as an `ArrayHeader` and answers `-1` — so a borrowed `String.prototype.indexOf` stored on the receiver (or on `Number.prototype`) never ran. `Named("String")` was already excluded one arm earlier by `is_boxed_string_wrapper`, which routes a boxed String to the `ToString`-coercing *string* dispatch; the new `receiver_is_non_array_builtin_wrapper` predicate extends that reasoning to the remaining boxed wrappers and to `Object`, routing them to the generic runtime dispatch, which resolves the own/inherited property first and still reaches the array engine for a genuine array. Real arrays are structurally unaffected — `Expr::New { class_name: "Array" }` already infers `Type::Array`, not `Type::Named("Array")` — and `--print-hir` confirms the `Array*` folds are still emitted, so the optimization is preserved rather than disabled. test262 `built-ins/String`: pass 916→922, runtime-fail 16→10 (6 flips, zero regressions), parity 98.3%→98.9%; the flipped cases are `indexOf/S15.5.4.7_{A1_T1,A1_T2,A4_T4}` and `lastIndexOf/S15.5.4.8_{A1_T1,A1_T2,A4_T4}`. New per-PR-visible coverage: three `perry-hir` unit tests on the extracted predicate, one of which asserts `Named("String")` is deliberately *not* claimed here. Still open in this cluster and left for its own PR: `concat/S15.5.4.6_A4_T1` has a different root — `classify_own_slot` (`perry-runtime/src/array/generic.rs`) labels *any* borrowed builtin an *Array* builtin, so `obj.concat = String.prototype.concat` runs the array engine.