fix(runtime): observe class iterator prototype replacements

changelog.d/9788-iterator-prototype-symbols.md

@@ -0,0 +1,4 @@
+Keep class iterator methods symbol-only in prototype own-key enumeration, and
+observe replacements of Symbol.iterator during direct calls, spread, Array.from,
+and for-of loops. Prototype accessors receive the instance and run once; an
+explicit undefined replacement shadows the original class method.

crates/perry-hir/src/lower/stmt_loops.rs

@@ -866,8 +866,8 @@ pub(super) fn lower_stmt_for_of_inner(
     // Also detect: for (const x of new Range(...)) where Range
     // defines `*[Symbol.iterator]()`. We lowered that method as
     // a synthesized top-level generator function taking `this`
-    // as its first parameter; the for-of here dispatches by
-    // calling that function with the lowered receiver.
+    // as its first parameter; this identifies the iterator-protocol loop.
+    // The actual iterator method is looked up at runtime so mutations count.
     let iter_from_class: Option<crate::types::FuncId> =
         if let ast::Expr::New(new_expr) = &*for_of_stmt.right {
             if let ast::Expr::Ident(ident) = new_expr.callee.as_ref() {
@@ -902,22 +902,17 @@ pub(super) fn lower_stmt_for_of_inner(
     {
         // Lower to iterator protocol:
         //   let __iter = genFunc(...);                     // generator-fn path
-        //   let __iter = __perry_iter_Range(new Range(...));  // class path
+        //   let __iter = GetIterator(new Range(...));      // class path
         //   let __iter = readable.iterator();              // node:stream path
         //   let __result = __iter.next();
         //   while (!__result.done) { const x = __result.value; body; __result = __iter.next(); }
         let for_scope_mark = ctx.push_block_scope();
         let iter_expr = lower_expr(ctx, &for_of_stmt.right)?;
-        // For the class path we wrap the lowered `new Range(..)`
-        // in a direct FuncRef call to the synthesized iterator
-        // function (which has `this` as its first parameter).
-        let iter_expr = if let Some(iter_fn_id) = iter_from_class {
-            Expr::Call {
-                callee: Box::new(Expr::FuncRef(iter_fn_id)),
-                args: vec![iter_expr],
-                type_args: vec![],
-                byte_offset: 0,
-            }
+        // A fresh instance still observes prototype mutations at loop entry.
+        let iter_expr = if iter_from_class.is_some() {
+            // Resolve the current Symbol.iterator property, including
+            // prototype replacements, once at loop entry (#9788).
+            Expr::GetIterator(Box::new(iter_expr))
         } else if is_filehandle_readlines_for_await || is_fs_dir_for_await {
             async_iterator_method_call(iter_expr)
         } else if is_node_readable_for_await {

crates/perry-hir/src/lower_decl/body_stmt.rs

@@ -1326,13 +1326,10 @@ fn lower_body_stmt_impl(ctx: &mut LoweringContext, stmt: &ast::Stmt) -> Result<V
             {
                 let scope_mark = ctx.push_block_scope();
                 let iter_expr_raw = lower_expr(ctx, &for_of_stmt.right)?;
-                let iter_expr = if let Some(iter_fn_id) = iter_from_class {
-                    Expr::Call {
-                        callee: Box::new(Expr::FuncRef(iter_fn_id)),
-                        args: vec![iter_expr_raw],
-                        type_args: vec![],
-                        byte_offset: 0,
-                    }
+                let iter_expr = if iter_from_class.is_some() {
+                    // Resolve the current Symbol.iterator property, including
+                    // prototype replacements, once at loop entry (#9788).
+                    Expr::GetIterator(Box::new(iter_expr_raw))
                 } else if is_filehandle_readlines_for_await || is_fs_dir_for_await {
                     async_iterator_method_call(iter_expr_raw)
                 } else if is_node_readable_for_await {

crates/perry-hir/src/lower_decl/class_decl.rs

@@ -822,17 +822,10 @@ pub fn lower_class_decl(
                             let ast::PropName::Computed(computed) = &method.key else {
                                 unreachable!("@@iterator generator key must be computed");
                             };
-                            // #9226 registers the wrapper as a prototype own
-                            // key so `Object.getOwnPropertyNames` /
-                            // `getOwnPropertySymbols` list it. That registration
-                            // does NOT install an instance vtable entry, and the
-                            // wrapper exists precisely to provide one (#5128) —
-                            // dropping it left `const C = class { *[Symbol.iterator]
-                            // () {…} }` throwing `TypeError: value is not
-                            // iterable`. Both registrations are required: the
-                            // vtable entry makes the instance iterable, the
-                            // computed member makes the key enumerable.
-                            methods.push(wrapper.clone());
+                            // The computed-symbol registration installs the
+                            // runtime dispatch alias too. Registering the wrapper
+                            // as a string method also exposed an own "@@iterator"
+                            // property that the source never declared (#9788).
                             computed_members.push(ClassComputedMember {
                                 key_expr: lower_expr(ctx, &computed.expr)?,
                                 function: wrapper,
@@ -1702,17 +1695,10 @@ pub fn lower_class_from_ast(
                             let ast::PropName::Computed(computed) = &method.key else {
                                 unreachable!("@@iterator generator key must be computed");
                             };
-                            // #9226 registers the wrapper as a prototype own
-                            // key so `Object.getOwnPropertyNames` /
-                            // `getOwnPropertySymbols` list it. That registration
-                            // does NOT install an instance vtable entry, and the
-                            // wrapper exists precisely to provide one (#5128) —
-                            // dropping it left `const C = class { *[Symbol.iterator]
-                            // () {…} }` throwing `TypeError: value is not
-                            // iterable`. Both registrations are required: the
-                            // vtable entry makes the instance iterable, the
-                            // computed member makes the key enumerable.
-                            methods.push(wrapper.clone());
+                            // The computed-symbol registration installs the
+                            // runtime dispatch alias too. Registering the wrapper
+                            // as a string method also exposed an own "@@iterator"
+                            // property that the source never declared (#9788).
                             computed_members.push(ClassComputedMember {
                                 key_expr: lower_expr(ctx, &computed.expr)?,
                                 function: wrapper,

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

@@ -652,7 +652,9 @@ pub unsafe extern "C-unwind" fn js_native_call_method_value(
     let key_jsval = JSValue::from_bits(key.to_bits());
     let is_symbol_key = crate::symbol::js_is_symbol(key) != 0;
 
-    if is_symbol_key {
+    // Well-known symbol calls must use the current property value below;
+    // direct registry dispatch bypasses own/prototype replacements (#9788).
+    if is_symbol_key && !crate::symbol::is_well_known_symbol(crate::symbol::sym_key_from_f64(key)) {
         let sym_key = crate::symbol::sym_key_from_f64(key);
         if sym_key != 0 {
             let bits = object.to_bits();
@@ -886,7 +888,7 @@ pub unsafe extern "C-unwind" fn js_native_call_method_value(
     // (whose slot is already the receiver), effect's Tag-class symbol *statics*
     // (plain data values), and any closure that doesn't read `this` are all left
     // untouched — keeping the #1758/#36/#321 closure-proto-chain paths intact.
-    let field = if is_symbol_key && crate::symbol::own_symbol_property(object, key).is_none() {
+    let field = if is_symbol_key && !crate::symbol::has_own_symbol_property(object, key) {
         f64::from_bits(crate::closure::clone_closure_rebind_this(
             field.to_bits(),
             object,

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

@@ -1302,7 +1302,7 @@ pub extern "C" fn js_object_define_property(
                                 desc_ptr as *const ObjectHeader,
                                 value_key,
                             );
-                            crate::symbol::js_object_set_symbol_property(
+                            crate::symbol::define_symbol_data_property(
                                 obj_value,
                                 key_value,
                                 f64::from_bits(value_field.bits()),

crates/perry-runtime/src/symbol/get.rs

@@ -25,6 +25,41 @@ fn well_known_symbol_method_name(sym_key: usize) -> Option<&'static str> {
     None
 }
 
+/// An iterator dispatch alias is a fallback for the method declared in source.
+/// Check writes on the intervening prototypes first, but stop at the nearest
+/// declaration so a subclass method still shadows a replaced base method.
+unsafe fn class_iterator_prototype_override(
+    receiver: f64,
+    sym: f64,
+    mut class_id: u32,
+    method_owner: u32,
+) -> Option<f64> {
+    for _ in 0..32 {
+        let declared = crate::object::class_decl_prototype_object(class_id);
+        let dynamic = crate::object::class_prototype_object(class_id);
+        for proto in [declared, dynamic] {
+            if proto.is_null() {
+                continue;
+            }
+            let proto_value = crate::value::js_nanbox_pointer(proto as i64);
+            if let Some(acc) = accessors::symbol_accessor_property(proto_value, sym) {
+                return Some(accessors::invoke_symbol_accessor_getter(acc.get, receiver));
+            }
+            if let Some(value) = own_symbol_property(proto_value, sym) {
+                return Some(value);
+            }
+        }
+        if class_id == method_owner {
+            break;
+        }
+        match crate::object::get_parent_class_id(class_id) {
+            Some(parent) if parent != 0 && parent != class_id => class_id = parent,
+            _ => break,
+        }
+    }
+    None
+}
+
 /// Does `obj` carry an OWN symbol-keyed property under `sym`, **without
 /// invoking** an accessor for it?
 ///
@@ -812,7 +847,14 @@ pub unsafe extern "C" fn js_object_get_symbol_property(obj_f64: f64, sym_f64: f6
                     // on `method_owner_class_id` first: `js_class_method_bind`
                     // otherwise mints a bound closure for a non-existent method.
                     if let Some(method_name) = well_known_symbol_method_name(sym_key) {
-                        if crate::object::method_owner_class_id(class_id, method_name).is_some() {
+                        if let Some(owner) =
+                            crate::object::method_owner_class_id(class_id, method_name)
+                        {
+                            if let Some(value) =
+                                class_iterator_prototype_override(obj_f64, sym_f64, class_id, owner)
+                            {
+                                return value;
+                            }
                             return crate::object::js_class_method_bind(
                                 obj_f64,
                                 method_name.as_ptr(),

test-files/test_gap_9788_iterator_protocol_mutation.ts

@@ -0,0 +1,49 @@
+// Exercise declaration/expression vtables, own overrides, prototype mutation,
+// IteratorClose, and symbol enumeration in one deterministic matrix.
+
+class DeclaredRange {
+  lo: number;
+  hi: number;
+  constructor(lo: number, hi: number) {
+    this.lo = lo;
+    this.hi = hi;
+  }
+  *[Symbol.iterator]() {
+    for (let i = this.lo; i <= this.hi; i++) yield i;
+  }
+}
+
+const ExpressionRange = class {
+  *[Symbol.iterator]() {
+    yield "expr-a";
+    yield "expr-b";
+  }
+};
+
+console.log([...new DeclaredRange(2, 4)].join(","));
+console.log([...new ExpressionRange()].join(","));
+console.log(
+  Object.getOwnPropertySymbols(ExpressionRange.prototype).map(String).join(","),
+  Object.getOwnPropertyNames(ExpressionRange.prototype).join(","),
+);
+
+const own: any = new DeclaredRange(1, 2);
+own[Symbol.iterator] = function* () {
+  yield 99;
+};
+console.log([...own].join(","));
+
+(DeclaredRange.prototype as any)[Symbol.iterator] = function* () {
+  yield 70;
+  yield 71;
+};
+console.log([...new DeclaredRange(1, 2)].join(","));
+
+const iterator: any = new Map([[1, "a"], [2, "b"]]).entries();
+let closed = 0;
+iterator.return = () => {
+  closed++;
+  return { done: true, value: undefined };
+};
+for (const _entry of iterator) break;
+console.log("closed", closed);

test-files/test_gap_9788_iterator_prototype_overrides.ts

@@ -0,0 +1,60 @@
+// #9788: declaration/expression and module/function loop paths must all read
+// the current Symbol.iterator, including prototype accessors and inheritance.
+class Range {
+  name = "range";
+  *[Symbol.iterator]() { yield 1; yield 2; }
+}
+function functionLoop() {
+  const result: unknown[] = [];
+  for (const value of new Range()) result.push(value);
+  return result.join(",");
+}
+console.log("before", functionLoop());
+Range.prototype[Symbol.iterator] = function* () { yield 7; yield 8; };
+const moduleValues: unknown[] = [];
+for (const value of new Range()) moduleValues.push(value);
+console.log("loops", moduleValues.join(","), functionLoop());
+console.log("call", new Range()[Symbol.iterator]().next().value);
+console.log("array-from", Array.from(new Range()).join(","));
+
+class Inherited extends Range {}
+class Own extends Range { *[Symbol.iterator]() { yield 3; } }
+console.log("inherit", [...new Inherited()].join(","), [...new Own()].join(","));
+Inherited.prototype[Symbol.iterator] = function* () { yield 4; };
+console.log("sub-override", [...new Inherited()].join(","), [...new Range()].join(","));
+
+let accessorReceiver: any;
+let gets = 0;
+Object.defineProperty(Range.prototype, Symbol.iterator, {
+  configurable: true,
+  get() {
+    gets++;
+    accessorReceiver = this;
+    return function* () { yield this.name; };
+  },
+});
+const instance = new Range();
+console.log("getter", [...instance].join(","), gets, accessorReceiver === instance);
+Object.defineProperty(Range.prototype, Symbol.iterator, { value: undefined, configurable: true });
+try { console.log([...instance]); } catch (error) { console.log("undefined", error instanceof TypeError); }
+
+const Expression = class { *[Symbol.iterator]() { yield "old"; } };
+Expression.prototype[Symbol.iterator] = function* () { yield "new"; };
+console.log("expression", [...new Expression()].join(","));
+console.log("expression-names", Object.getOwnPropertyNames(Expression.prototype).join(","));
+
+class Plain {
+  [Symbol.iterator]() { return [5, 6][Symbol.iterator](); }
+}
+console.log("non-generator", [...new Plain()].join(","));
+console.log("plain-names", Object.getOwnPropertyNames(Plain.prototype).join(","));
+class Literal {
+  "@@iterator"() { return "literal"; }
+}
+console.log("literal", Object.getOwnPropertyNames(Literal.prototype).join(","), new Literal()["@@iterator"]());
+const ownGetter: any = new Own();
+let ownGets = 0;
+Object.defineProperty(ownGetter, Symbol.iterator, {
+  get() { ownGets++; return function* () { yield 9; }; },
+});
+console.log("own-getter-call", ownGetter[Symbol.iterator]().next().value, ownGets);