diff --git a/changelog.d/10650-symbol-iterator-generator-this.md b/changelog.d/10650-symbol-iterator-generator-this.md new file mode 100644 index 0000000000..360d715b5e --- /dev/null +++ b/changelog.d/10650-symbol-iterator-generator-this.md @@ -0,0 +1,26 @@ +Fixed: a class generator method keyed by `[Symbol.iterator]`, when a `for…of` +loop inside it iterated a method call on `this` (`for (const x of +this.gen()) yield x;`), saw `this === undefined` and threw `Cannot read +properties of undefined (reading 'gen')`. Every consumer that dispatches +through the class's iterator protocol (`for…of`, spread, `Array.from`) hit it +identically; the identical body under an ordinary method name worked. + +Root cause: lifting a `*[Symbol.iterator]()` method to its top-level +generator (`synthesize_symbol_iterator_wrapper`) rewrites `this` to an +explicit parameter via `replace_this_in_stmts`/`replace_this_in_expr` +(`crates/perry-hir/src/analysis.rs`). That rewrite had no arm for +`Expr::GetIterator`/`GetAsyncIterator`/`MapEntries`/`SetValues` — the wrapper +expressions a `for…of` iterable lowers to when it can't be proven a plain +Array/Map/Set — so a `this` buried inside one of those wrappers fell through +to the catch-all and was never rewritten. + +Fix: added the four missing arms, recursing into the wrapped expression the +same way the existing `Await`/`TypeOf`/`Void` arms do. + +Validation: new gap test (`test_gap_10445_symbol_iterator_generator_this.ts`) +covering the issue repro plus a two-level generator chain, a +`Symbol.iterator` generator on a class expression, and `yield*` delegation +alongside a `for…of` over `this.method()` — proven to fail on the pre-fix +tree and pass on this one, byte-identical to Node 26.5.1. `cargo test +--release -p perry-hir --tests`: 748 passed. Lint: 76/77 gates (the one red +is the pre-existing, repo-wide benchmark-freshness check). diff --git a/crates/perry-hir/src/analysis.rs b/crates/perry-hir/src/analysis.rs index b9fba65cee..0b62440897 100644 --- a/crates/perry-hir/src/analysis.rs +++ b/crates/perry-hir/src/analysis.rs @@ -1154,6 +1154,25 @@ fn replace_this_in_expr(expr: &mut Expr, this_id: LocalId) { replace_this_in_expr(else_expr, this_id); } Expr::Await(inner) => replace_this_in_expr(inner, this_id), + // #10445: a `for…of`/`for await…of` iterable that can't be proven a + // plain Array/Map/Set lowers to one of these wrapper exprs around the + // ORIGINAL receiver expression (see `stmt_loops.rs`'s + // `lower_stmt_for_of_inner` — `Expr::GetIterator`/`GetAsyncIterator` + // wrap the lazy-iterator-protocol receiver, `MapEntries`/`SetValues` + // wrap a Map/Set whose fast path is disabled). Missing them here left + // a `for (const x of this.gen())` inside a lifted + // `*[Symbol.iterator]()` generator (`synthesize_symbol_iterator_wrapper` + // below, which lifts the method to a top-level function and replaces + // `this` with an explicit param) with an unreplaced `Expr::This` deep + // inside the wrapper — it fell through to the catch-all and evaluated + // to `undefined` outside any method body, `Cannot read properties of + // undefined (reading 'gen')`. Hoisting the same call into a local + // first (`const it = this.gen(); for (const x of it)`) sidestepped + // the bug because the plain `Stmt::Let` init IS a matched `Expr::Call`. + Expr::GetIterator(inner) | Expr::GetAsyncIterator(inner) => { + replace_this_in_expr(inner, this_id) + } + Expr::MapEntries(inner) | Expr::SetValues(inner) => replace_this_in_expr(inner, this_id), Expr::Yield { value, .. } => { if let Some(v) = value { replace_this_in_expr(v, this_id); diff --git a/test-files/test_gap_10445_symbol_iterator_generator_this.ts b/test-files/test_gap_10445_symbol_iterator_generator_this.ts new file mode 100644 index 0000000000..a90843efb7 --- /dev/null +++ b/test-files/test_gap_10445_symbol_iterator_generator_this.ts @@ -0,0 +1,89 @@ +// #10445: a generator method keyed by `[Symbol.iterator]`, whose `for…of` +// iterable is a method call on `this` (`for (const x of this.gen()) …`), +// saw `this === undefined` inside the callee. Root cause: +// `synthesize_symbol_iterator_wrapper` (lower_decl/class_decl.rs) lifts the +// method's body to a top-level generator taking `this` as an explicit +// param, then `replace_this_in_stmts`/`replace_this_in_expr` (analysis.rs) +// rewrites every `Expr::This` in that body to the param. A `for…of` whose +// iterable can't be proven a plain Array/Map/Set lowers to one of +// `GetIterator`/`GetAsyncIterator`/`MapEntries`/`SetValues` wrapping the +// receiver expression (stmt_loops.rs's `lower_stmt_for_of_inner`) -- and +// `replace_this_in_expr` had no arm for any of those wrappers, so a `this` +// buried inside one fell through to the catch-all and was left unreplaced. +// Every consumer that dispatches through the lifted function (spread, +// `for…of`, `Array.from`) hit the same bug identically. + +class Bag { + items = [1, 2]; + *gen() { + yield* this.items; + } + *[Symbol.iterator]() { + for (const x of this.gen()) yield x; // the repro shape + } + *viaLocal() { + for (const x of this.gen()) yield x; // same body, ordinary name: control + } +} + +// Two-level: the iterator method's for-of iterable is ANOTHER method whose +// OWN for-of iterable is a THIRD method -- this must survive two hops of +// the lifted-generator's this-substitution, not just one. +class TwoLevel { + items = [10, 20, 30]; + *inner() { + for (const x of this.items) yield x * 2; + } + *middle() { + for (const x of this.inner()) yield x + 1; + } + *[Symbol.iterator]() { + for (const x of this.middle()) yield x; + } +} + +// Class EXPRESSION (not a declaration) -- the lift/this-rewrite must not be +// keyed off a named-declaration-only path. +const ExprClass = class { + items = ["a", "b", "c"]; + *gen() { + yield* this.items; + } + *[Symbol.iterator]() { + for (const x of this.gen()) yield x; + } +}; + +// `yield*` delegation alongside a for-of over `this.method()` in the SAME +// generator -- confirms the fix doesn't disturb the already-working +// yield*-over-this.gen() path while also fixing the for-of one. +class Mixed { + items = [1, 2, 3]; + *gen() { + yield* this.items; + } + *[Symbol.iterator]() { + yield* this.gen(); + for (const x of this.gen()) yield x * 10; + } +} + +const show = (label: string, f: () => unknown) => { + try { + console.log(label, JSON.stringify(f())); + } catch (e: any) { + console.log(label, "threw:", e.message); + } +}; + +show("spread over *[Symbol.iterator]:", () => [...new Bag()]); +show("for-of over *[Symbol.iterator]:", () => { + const out: number[] = []; + for (const x of new Bag()) out.push(x); + return out; +}); +show("Array.from(bag):", () => Array.from(new Bag())); +show("named generator, same body:", () => [...new Bag().viaLocal()]); +show("two-level generator:", () => [...new TwoLevel()]); +show("class expression generator:", () => [...new ExprClass()]); +show("yield* + for-of mixed:", () => [...new Mixed()]);