From 6ff0124dae031963d7a4cf0c32d1323d4f0cc24a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 16 Jul 2026 08:54:56 +0200 Subject: [PATCH] fix(runtime): resolve `[Symbol.iterator]` on a class declaration (#6454) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `yield* SomeTag` — effect's most central idiom, where the tag IS a class: class HttpRouter extends Context.Tag("HttpRouter")<...>() {} const router = yield* HttpRouter threw `TypeError: is not iterable` in Perry. So did `for…of`, destructuring and spread over any class declaration; `Array.from` silently returned `[]`. Perry lowers a class declaration to an INT32-tagged ClassRef. `js_get_iterator` guarded its generic `[Symbol.iterator]` lookup with "not a pointer and not a string ⇒ primitive ⇒ not iterable", which a ClassRef fails — so it threw before reaching the lookup at the bottom of the same function. That lookup already handles class refs correctly: `js_object_get_symbol_property` routes `bits >> 48 == 0x7FFE` through the static symbol table, `resolve_proto_chain_symbol`, and `class_parent_closure` — the last added (#36/#321) precisely for `class Svc extends Context.Tag(id)<...>() {}`. The machinery was all there; the value never reached it. Four surfaces, one rule — resolve the class ref's (possibly inherited) `[Symbol.iterator]` and drive it: - `js_get_iterator` (yield* / lazy for-of / destructuring): admit a *registered* class ref past the primitive guard; throw at the tail if it resolved no iterator. The registry probe is paid only by values the guard was already about to throw on — the array/object/string hot path costs exactly what it did before. - `js_for_of_to_array` + `array_from_spread_value` (eager for-of, spread): resolve-and-drive before the raw-pointer logic, which would otherwise misread the class id as a heap address and sniff a GC header at `id - 8` — a latent SIGSEGV, not just a wrong error. No iterator ⇒ throw, like node. - `js_array_from_value` (`Array.from`): same resolve-and-drive, but with NO iterator it falls through to the array-like branch (node: `Array.from(class C {})` → `[]`, not a throw — unlike spread/for-of). The tail throw in `js_get_iterator` is not belt-and-braces. `INT32_TAG | 2` (the number 2) and a ClassRef with `class_id == 2` are bit-identical — `class_ref_id`'s registry check is the only thing that separates them. Returning an unresolved class ref as its own iterator would turn `for (const x of 37) {}` from a clean TypeError into a misleading "next is not a function" whenever class id 37 happened to exist. Throwing keeps every primitive behaving exactly as before; only a class ref that genuinely resolves an iterator changes behavior. Found via the emitted IR for effect's web.ts, where the operand is a bare constant: if.then.13: ; generator state == 0 %r43 = call double @js_get_iterator(double 0x7FFE000000000002) `0x7FFE…0002` is INT32 2 — a class id, not the number 2. 6 of 709 `js_get_iterator` calls in that 311-module build pass a constant class ref. --- crates/perry-runtime/src/array/from_concat.rs | 11 ++++ crates/perry-runtime/src/array/iterator.rs | 28 ++++++++++ crates/perry-runtime/src/symbol.rs | 1 + crates/perry-runtime/src/symbol/iterator.rs | 55 ++++++++++++++++++- 4 files changed, 94 insertions(+), 1 deletion(-) diff --git a/crates/perry-runtime/src/array/from_concat.rs b/crates/perry-runtime/src/array/from_concat.rs index 3f03442733..36626fabe8 100644 --- a/crates/perry-runtime/src/array/from_concat.rs +++ b/crates/perry-runtime/src/array/from_concat.rs @@ -81,6 +81,17 @@ pub extern "C" fn js_array_from_value(boxed: f64) -> *mut ArrayHeader { if bits == TAG_NULL { throw_not_iterable("object null"); } + // #6454: `Array.from(SomeClass)` where the class DECLARATION (an + // INT32-tagged ClassRef) carries a — possibly inherited, #36/#321 — + // `[Symbol.iterator]`: drive it. A class WITHOUT one falls through to the + // non-iterable branch below (node: `Array.from(class C {})` → `[]` via the + // array-like path, not a throw — unlike spread/for-of). + if crate::object::class_ref_id(boxed).is_some() + && crate::symbol::class_ref_resolves_iterator(boxed) + { + let iter = crate::symbol::js_get_iterator(boxed); + return crate::array::js_iterator_to_array(iter); + } // Numbers / booleans / strings handled inside js_array_clone: // - numbers/booleans aren't pointers → empty array. // - strings → per-codepoint materialization. diff --git a/crates/perry-runtime/src/array/iterator.rs b/crates/perry-runtime/src/array/iterator.rs index 4e336bd466..140ad2deef 100644 --- a/crates/perry-runtime/src/array/iterator.rs +++ b/crates/perry-runtime/src/array/iterator.rs @@ -73,6 +73,22 @@ pub extern "C" fn js_for_of_to_array(val_f64: f64) -> f64 { return js_nanbox_pointer(arr_i64); } + // #6454: a class DECLARATION is an INT32-tagged ClassRef whose low bits are + // the class id — `js_nanbox_get_pointer` below would misread that id as a + // heap address and the GC-header sniff would dereference `id - 8`. Resolve + // its (possibly inherited, #36/#321) `[Symbol.iterator]` and drive it; + // a class with none is not iterable, exactly like node + // (`for (const x of Plain) {}` → TypeError). Must run BEFORE the raw-pointer + // logic below. + if crate::object::class_ref_id(val_f64).is_some() { + if crate::symbol::class_ref_resolves_iterator(val_f64) { + let iter = crate::symbol::js_get_iterator(val_f64); + let arr = js_iterator_to_array(iter); + return js_nanbox_pointer(arr as i64); + } + throw_not_iterable(val_f64); + } + // Non-pointer scalars (number/bool/null/undefined/symbol) are not // iterable. Per ECMA-262 §13.7.5.13 (ForIn/OfHeadEvaluation → // GetIterator → ToObject/GetMethod) these MUST throw a TypeError: @@ -703,6 +719,18 @@ pub(crate) fn array_from_spread_value(value: f64) -> *mut ArrayHeader { return crate::string::js_string_to_char_array(str_bits as i64) as *mut ArrayHeader; } + // #6454: `[...SomeClass]` / `fn(...SomeClass)` on a class DECLARATION — an + // INT32-tagged ClassRef. Drive its (possibly inherited) `[Symbol.iterator]`; + // with none it is not iterable, like node. Must run before the raw-pointer + // reads below, which would misread the class id as a heap address. + if crate::object::class_ref_id(value).is_some() { + if crate::symbol::class_ref_resolves_iterator(value) { + let iter = crate::symbol::js_get_iterator(value); + return js_iterator_to_array(iter); + } + throw_not_iterable(value); + } + let raw_ptr = js_nanbox_get_pointer(value) as usize; if raw_ptr == 0 { throw_not_iterable(value); diff --git a/crates/perry-runtime/src/symbol.rs b/crates/perry-runtime/src/symbol.rs index 2d9c21afe7..e6a122f656 100644 --- a/crates/perry-runtime/src/symbol.rs +++ b/crates/perry-runtime/src/symbol.rs @@ -54,6 +54,7 @@ pub use get::js_object_get_symbol_property; pub(crate) use get::{inherited_symbol_property, own_symbol_property}; // Iterator protocol, getOwnPropertySymbols, ToPrimitive. +pub(crate) use iterator::class_ref_resolves_iterator; pub use iterator::{ js_get_iterator, js_iterator_result_validate, js_object_get_own_property_symbols, js_to_primitive, diff --git a/crates/perry-runtime/src/symbol/iterator.rs b/crates/perry-runtime/src/symbol/iterator.rs index 1365d6add7..b8c3858e66 100644 --- a/crates/perry-runtime/src/symbol/iterator.rs +++ b/crates/perry-runtime/src/symbol/iterator.rs @@ -130,6 +130,24 @@ fn throw_value_not_iterable() -> ! { crate::exception::js_throw(crate::value::js_nanbox_pointer(err as i64)); } +/// #6454: does this value — already known to be a *registered class ref* +/// (INT32-tagged, `class_ref_id(..).is_some()`) — resolve a `[Symbol.iterator]` +/// method? Used by the eager materializers (`array_from_spread_value`, +/// `js_array_from_value`, `js_for_of_to_array`) to decide between driving the +/// iterator and their per-construct fallback (spread/for-of throw, `Array.from` +/// takes its array-like branch), mirroring node. The resolution walks the same +/// chain `js_get_iterator`'s generic tail uses: own static symbols → +/// `resolve_proto_chain_symbol` → `class_parent_closure` (#36/#321). +pub(crate) fn class_ref_resolves_iterator(val_f64: f64) -> bool { + let iter_wk = well_known_symbol("iterator"); + if iter_wk.is_null() { + return false; + } + let sym_f64 = f64::from_bits(crate::value::JSValue::pointer(iter_wk as *const u8).bits()); + let method = unsafe { js_object_get_symbol_property(val_f64, sym_f64) }; + method.to_bits() != TAG_UNDEFINED +} + /// Spec IteratorNext / IteratorClose step "If innerResult is not an Object, /// throw a TypeError". The for-of lazy-loop desugar wraps each `__iter.next()` /// / guarded `__iter.return()` call in this validator. Returns the result @@ -294,10 +312,35 @@ pub extern "C" fn js_get_iterator(val_f64: f64) -> f64 { // lookup, which would otherwise dereference a raw (non-NaN-boxed) double as // an object pointer and crash (`for (x of 37) {}`). Strings ARE iterable, so // they fall through to the symbol lookup below. + // + // #6454: a class DECLARATION is an INT32-tagged ClassRef, not a pointer, so + // this guard used to reject it as a primitive number — `yield* SomeTag` / + // `for (const x of SomeClass)` threw "is not iterable" without ever reaching + // the lookup at the bottom, even though `js_object_get_symbol_property` + // resolves class refs (own static symbols → `resolve_proto_chain_symbol` → + // `class_parent_closure`, the last of which exists precisely for effect's + // `class Svc extends Context.Tag(id)<...>() {}`, #36/#321). Let a registered + // class ref through to that lookup; if it resolves no `[Symbol.iterator]` it + // still throws, at the tail of this function. + // + // Note `INT32_TAG | 2` (the number 2) and a ClassRef with `class_id == 2` + // are bit-identical — `class_ref_id`'s registry check is the only thing + // separating them. That is why the tail must throw rather than return the + // value as its own iterator: it keeps `for (const x of 37) {}` a TypeError + // even when class id 37 happens to be registered. + // + // The `class_ref_id` registry probe (an RwLock read + hash lookup) is paid + // ONLY by values this guard was already about to throw on — every pointer / + // string receiver, i.e. every array, object and string for-of, skips it. The + // hot path costs exactly what it did before #6454. + let mut is_registered_class_ref = false; { let jsv = crate::value::JSValue::from_bits(val_f64.to_bits()); if !jsv.is_pointer() && !jsv.is_any_string() { - throw_value_not_iterable(); + is_registered_class_ref = crate::object::class_ref_id(val_f64).is_some(); + if !is_registered_class_ref { + throw_value_not_iterable(); + } } } // A string PRIMITIVE (heap STRING_TAG or inline SSO short string) iterates @@ -383,6 +426,16 @@ pub extern "C" fn js_get_iterator(val_f64: f64) -> f64 { throw_value_not_iterable(); } } + // #6454: the class ref admitted past the primitive guard above resolved no + // `[Symbol.iterator]`, so it is genuinely not iterable — `class C {}` with no + // iterator, or (because the encodings are bit-identical) a plain number whose + // value collides with a registered class id. Returning it would hand the + // caller an INT32 as its own "iterator" and surface a misleading + // "next is not a function" later; throw here, exactly as before #6454 for + // every non-pointer value. + if is_registered_class_ref { + throw_value_not_iterable(); + } val_f64 }