From 6328a79bcf842a07823802b3f82d8576339c7004 Mon Sep 17 00:00:00 2001 From: Ralph Date: Mon, 20 Jul 2026 10:28:06 -0700 Subject: [PATCH 1/2] fix(symbol): resolve well-known symbols via optional chaining `Symbol?.iterator` (#6719) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Symbol?.iterator` (and `Symbol?.["iterator"]` / `Symbol?.[name]`) returned `undefined` because only the non-optional dot (`Symbol.iterator`) and bracket (`Symbol["iterator"]`, #6676) forms were folded to the well-known-symbol sentinel in `lower_member_inner`. The optional-chaining forms lower through a separate arm (`arm_optchain.rs`) that never routed through that resolver, so a `?.` read fell through to a generic property get on the `Symbol` constructor and returned `undefined`. Downstream, a class/object keyed with `[Symbol?.iterator]` was keyed under `undefined` and so was not iterable (`[...new R()]` → "value is not iterable"). - HIR: extract the Symbol well-known resolver from `lower_member_inner` into a shared `try_fold_symbol_well_known_member(ctx, obj, prop)` covering the dot, string-bracket, and runtime-key (`js_symbol_computed_member`) forms, and wire it into the `OptChainBase::Member` arm. `Symbol` is a non-nullish global, so `Symbol?.iterator === Symbol.iterator` and the `?.` short-circuit is dead — the resolver returns the fold directly. - The extracted resolver also applies the `#6676` real-global gate (`lookup_local("Symbol").is_none()`) to the dot form, which previously lacked it: a locally-shadowed `Symbol` now resolves normally instead of folding. - test: test_gap_6719_optional_chain_symbol.ts (byte-identical to Node) — optional dot/bracket/runtime-key forms, class & object-literal computed method keys, non-well-known keys, local shadowing, and nullish short-circuit. Co-authored-by: Ralph Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/perry-hir/src/lower/expr_member.rs | 160 +++++++++--------- .../src/lower/lower_expr/arm_optchain.rs | 18 ++ .../test_gap_6719_optional_chain_symbol.ts | 87 ++++++++++ 3 files changed, 189 insertions(+), 76 deletions(-) create mode 100644 test-files/test_gap_6719_optional_chain_symbol.ts diff --git a/crates/perry-hir/src/lower/expr_member.rs b/crates/perry-hir/src/lower/expr_member.rs index 3216a1281d..6545559dd1 100644 --- a/crates/perry-hir/src/lower/expr_member.rs +++ b/crates/perry-hir/src/lower/expr_member.rs @@ -127,6 +127,82 @@ pub(crate) fn is_well_known_symbol_member(name: &str) -> bool { ) } +/// Fold a well-known-symbol member access on the `Symbol` constructor to the +/// `@@__perry_wk_` sentinel that `js_symbol_for` resolves from the +/// well-known cache. This is the ONE resolver shared by every access form that +/// reaches `Symbol.`, so they all yield the identity-equal cached +/// symbol: +/// - dot `Symbol.iterator` +/// - string bracket `Symbol["iterator"]` (#6676) +/// - optional dot `Symbol?.iterator` (#6719) +/// - optional bracket `Symbol?.["iterator"]` (#6719) +/// +/// The *runtime*-key computed form `Symbol[name]` / `Symbol?.[name]` (esbuild's +/// `__knownSymbol` helper) can't be constant-folded, so it lowers to a +/// `js_symbol_computed_member` call — a strict superset of a plain `Symbol[key]` +/// read (maps a well-known name to the cached symbol, else reads normally). +/// +/// Returns `Ok(None)` unless this is a well-known-symbol read on the real +/// (unshadowed) `Symbol` global, so the caller falls through to its ordinary +/// member lowering. `Symbol` is a non-nullish global, so the optional-chain +/// forms resolve identically to their non-optional twins — the `?.` +/// short-circuit is dead and the caller may drop it. +pub(crate) fn try_fold_symbol_well_known_member( + ctx: &mut LoweringContext, + obj: &ast::Expr, + prop: &ast::MemberProp, +) -> Result> { + let ast::Expr::Ident(obj_ident) = unwrap_transparent(obj) else { + return Ok(None); + }; + // Gated on `Symbol` being the real global — a local binding of that name + // resolves normally, matching JS scoping. + if obj_ident.sym.as_ref() != "Symbol" || ctx.lookup_local("Symbol").is_some() { + return Ok(None); + } + let fold = + |name: &str| Expr::SymbolFor(Box::new(Expr::String(format!("@@__perry_wk_{}", name)))); + match prop { + ast::MemberProp::Ident(prop_ident) => { + let prop_name = prop_ident.sym.as_ref(); + if is_well_known_symbol_member(prop_name) { + return Ok(Some(fold(prop_name))); + } + } + ast::MemberProp::Computed(computed) => match computed.expr.as_ref() { + ast::Expr::Lit(ast::Lit::Str(s)) => { + if let Some(prop_name) = s.value.as_str() { + if is_well_known_symbol_member(prop_name) { + return Ok(Some(fold(prop_name))); + } + } + } + _ => { + let key_expr = lower_expr(ctx, &computed.expr)?; + return Ok(Some(Expr::Call { + callee: Box::new(Expr::ExternFuncRef { + name: "js_symbol_computed_member".to_string(), + param_types: vec![Type::Any, Type::Any], + return_type: Type::Any, + }), + args: vec![ + Expr::PropertyGet { + byte_offset: 0, + object: Box::new(Expr::GlobalGet(0)), + property: "Symbol".to_string(), + }, + key_expr, + ], + type_args: vec![], + byte_offset: 0, + })); + } + }, + ast::MemberProp::PrivateName(_) => {} + } + Ok(None) +} + pub(super) fn lower_member(ctx: &mut LoweringContext, member: &ast::MemberExpr) -> Result { // #1723: when THIS access is the auditable `ns[dynamicKey].staticMember` // shape — a dynamic stdlib SUB-namespace selection (`path.win32` / @@ -633,82 +709,14 @@ fn lower_member_inner(ctx: &mut LoweringContext, member: &ast::MemberExpr) -> Re } } - // Check if this is Symbol. — Symbol.toPrimitive, - // Symbol.hasInstance, Symbol.toStringTag, Symbol.match, Symbol.iterator, - // Symbol.asyncIterator, Symbol.dispose, Symbol.asyncDispose, and the - // remaining standard protocol constants. - // Lowered to `SymbolFor(String("@@__perry_wk_"))` which the - // runtime's `js_symbol_for` sniffs via prefix and resolves from - // the well-known cache (not the registry). Gives each well-known - // symbol a stable pointer without needing a new HIR variant. - if let ast::Expr::Ident(obj_ident) = unwrap_transparent(member.obj.as_ref()) { - if obj_ident.sym.as_ref() == "Symbol" { - if let ast::MemberProp::Ident(prop_ident) = &member.prop { - let prop_name = prop_ident.sym.as_ref(); - if is_well_known_symbol_member(prop_name) { - return Ok(Expr::SymbolFor(Box::new(Expr::String(format!( - "@@__perry_wk_{}", - prop_name - ))))); - } - } - } - } - - // #6676: the COMPUTED form `Symbol["iterator"]` / `Symbol[name]`. The dot - // fold above only matches `MemberProp::Ident`, so a bracket read fell - // through to a generic property get on the `Symbol` constructor and returned - // `undefined`. This is exactly what breaks esbuild's `__knownSymbol` helper - // — `(symbol = Symbol[name]) ? symbol : Symbol.for("Symbol." + name)` — - // which every downleveled `yield*`/`for-of`/spread routes through at - // `--target=es2015|es2017`: keyed under `undefined`, the delegate iterator - // dies with "Cannot read properties of undefined (reading 'next')". - // - // A string-literal key folds to the same `@@__perry_wk_` sentinel the dot - // form uses, so `Symbol["iterator"] === Symbol.iterator` holds by - // construction. A *runtime* key can't be folded (the esbuild helper passes - // `name` as a parameter), so it resolves through `js_symbol_computed_member`, - // which maps a well-known name to the cached symbol and otherwise falls back - // to the ordinary `Symbol[key]` read — a strict superset of prior behavior. - // Gated on `Symbol` being the real global (a local binding of that name - // resolves normally, matching JS scoping). - if let ast::Expr::Ident(obj_ident) = unwrap_transparent(member.obj.as_ref()) { - if obj_ident.sym.as_ref() == "Symbol" && ctx.lookup_local("Symbol").is_none() { - if let ast::MemberProp::Computed(computed) = &member.prop { - match computed.expr.as_ref() { - ast::Expr::Lit(ast::Lit::Str(s)) => { - if let Some(prop_name) = s.value.as_str() { - if is_well_known_symbol_member(prop_name) { - return Ok(Expr::SymbolFor(Box::new(Expr::String(format!( - "@@__perry_wk_{}", - prop_name - ))))); - } - } - } - _ => { - let key_expr = lower_expr(ctx, &computed.expr)?; - return Ok(Expr::Call { - callee: Box::new(Expr::ExternFuncRef { - name: "js_symbol_computed_member".to_string(), - param_types: vec![Type::Any, Type::Any], - return_type: Type::Any, - }), - args: vec![ - Expr::PropertyGet { - byte_offset: 0, - object: Box::new(Expr::GlobalGet(0)), - property: "Symbol".to_string(), - }, - key_expr, - ], - type_args: vec![], - byte_offset: 0, - }); - } - } - } - } + // `Symbol.` in every form — dot (`Symbol.iterator`), string + // bracket (`Symbol["iterator"]`, #6676), and the runtime-key computed form + // (`Symbol[name]`, esbuild's `__knownSymbol`) — folds to the + // `@@__perry_wk_` sentinel (or a `js_symbol_computed_member` call) via + // the shared resolver. See `try_fold_symbol_well_known_member`. + if let Some(folded) = try_fold_symbol_well_known_member(ctx, member.obj.as_ref(), &member.prop)? + { + return Ok(folded); } // `util.inspect.custom` / `inspect.custom` and diff --git a/crates/perry-hir/src/lower/lower_expr/arm_optchain.rs b/crates/perry-hir/src/lower/lower_expr/arm_optchain.rs index 0ffc02f9a9..23b5a59a8a 100644 --- a/crates/perry-hir/src/lower/lower_expr/arm_optchain.rs +++ b/crates/perry-hir/src/lower/lower_expr/arm_optchain.rs @@ -48,6 +48,24 @@ pub(crate) fn lower_opt_chain_expr( } } } + // #6719: `Symbol?.iterator` (and `Symbol?.["iterator"]` / + // `Symbol?.[name]`) must resolve the well-known symbol, exactly like + // the non-optional `Symbol.iterator` / `Symbol["iterator"]` (#6676) + // forms — the dot/bracket folds live in `lower_member_inner`, which + // this optional-chain arm never reaches, so without this a + // well-known symbol read via `?.` fell through to a generic property + // get on the `Symbol` constructor and returned `undefined` (a class + // keyed with `[Symbol?.iterator]` was then not iterable). `Symbol` is + // a non-nullish global, so `Symbol?.iterator === Symbol.iterator` and + // the `?.` short-circuit is dead — the resolver returns the fold + // directly. + if let Some(folded) = expr_member::try_fold_symbol_well_known_member( + ctx, + member.obj.as_ref(), + &member.prop, + )? { + return Ok(folded); + } // obj?.prop -> obj == null ? undefined : obj.prop let obj_expr = lower_expr(ctx, &member.obj)?; diff --git a/test-files/test_gap_6719_optional_chain_symbol.ts b/test-files/test_gap_6719_optional_chain_symbol.ts new file mode 100644 index 0000000000..42b4d26baf --- /dev/null +++ b/test-files/test_gap_6719_optional_chain_symbol.ts @@ -0,0 +1,87 @@ +// #6719: reading a well-known symbol off the `Symbol` constructor through +// OPTIONAL chaining (`Symbol?.iterator`) must return the same symbol as dot +// access (`Symbol.iterator`). #6676 fixed the computed bracket form +// (`Symbol["iterator"]`); the optional-chain form lowers through a different +// arm (`arm_optchain.rs`) that never routed through the well-known-symbol +// resolver, so it fell through to a generic property read on the `Symbol` +// constructor and returned `undefined`. `Symbol` is a non-nullish global, so +// `Symbol?.iterator` is exactly `Symbol.iterator`. +// +// Downstream impact: a class/object keyed with `[Symbol?.iterator]` was keyed +// under `undefined` and so was not iterable. +// +// Validated byte-for-byte against `node --experimental-strip-types`. + +// ── optional dot (`Symbol?.iterator`) === dot access ──────────────────────── +console.log(Symbol?.iterator === Symbol.iterator, typeof Symbol?.iterator); +console.log( + Symbol?.asyncIterator === Symbol.asyncIterator, + typeof Symbol?.asyncIterator, +); +console.log( + Symbol?.toPrimitive === Symbol.toPrimitive, + typeof Symbol?.toPrimitive, +); +console.log( + Symbol?.hasInstance === Symbol.hasInstance, + typeof Symbol?.hasInstance, +); +console.log( + Symbol?.toStringTag === Symbol.toStringTag, + typeof Symbol?.toStringTag, +); + +// ── optional computed key (`Symbol?.["iterator"]`) — string literal ───────── +console.log( + Symbol?.["iterator"] === Symbol.iterator, + typeof Symbol?.["iterator"], +); +console.log( + Symbol?.["species"] === Symbol.species, + typeof Symbol?.["species"], +); + +// ── optional computed key with a runtime key (`Symbol?.[name]`) ───────────── +// Routes through `js_symbol_computed_member`, identity-equal to dot access. +const grab = (name: string): any => Symbol?.[name as any]; +for (const k of ["iterator", "asyncIterator", "toPrimitive", "match"]) { + console.log(k, grab(k) === (Symbol as any)[k], typeof grab(k)); +} + +// ── a non-well-known key stays undefined ──────────────────────────────────── +console.log(typeof Symbol?.["definitelyNotAWellKnownSymbol" as any]); +console.log(typeof grab("notAKnownSymbol")); + +// ── the real manifestation: a class computed method key via `?.` ──────────── +class R { + *[Symbol?.iterator]() { + yield 1; + yield 2; + } +} +console.log(JSON.stringify([...new R()])); // spread → [1,2] +const collected: number[] = []; +for (const v of new R()) collected.push(v); // for-of +console.log(collected.join(",")); + +// an object literal keyed with `[Symbol?.iterator]` is iterable too +const obj = { + *[Symbol?.iterator]() { + yield 10; + yield 20; + yield 30; + }, +}; +console.log([...obj].join(",")); +console.log(Math.max(...obj)); // spread into a call + +// ── a locally-shadowed `Symbol` must NOT fold — normal scoping wins ───────── +function shadowed(): string { + const Symbol = { iterator: "local-value" } as any; + return typeof Symbol?.iterator + ":" + Symbol?.iterator; +} +console.log(shadowed()); // node: "string:local-value" + +// ── `?.` still short-circuits a genuinely nullish receiver ────────────────── +const maybe: any = null; +console.log(maybe?.iterator); // undefined From 46709b9c99931fc5a67cedeaa7921f198cf2f595 Mon Sep 17 00:00:00 2001 From: Ralph Date: Mon, 20 Jul 2026 10:41:50 -0700 Subject: [PATCH 2/2] fix(symbol): gate Symbol fold on shadows_unqualified_global (CodeRabbit #6729) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shared resolver gated on `lookup_local("Symbol").is_none()` (inherited from #6676), which only detects `let`/`const`/param bindings — a `class Symbol {}` or `function Symbol() {}` shadow slipped through and still folded `Symbol.iterator` to the global well-known symbol (`class Symbol { static iterator }; Symbol?.iterator` printed `Symbol(Symbol.iterator)` instead of the class's static field). Switch to `shadows_unqualified_global("Symbol")`, the established idiom in this file (used for `process`), which is a strict superset covering local, function, imported- function, and class bindings. Fixes the dot, bracket (#6676), and optional-chain (#6719) forms in one place. - test: add `class Symbol` and `function Symbol` shadowing cases (byte-identical to Node) alongside the existing `const Symbol` case. Co-authored-by: Ralph Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/perry-hir/src/lower/expr_member.rs | 8 ++++--- .../test_gap_6719_optional_chain_symbol.ts | 22 +++++++++++++++++-- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/crates/perry-hir/src/lower/expr_member.rs b/crates/perry-hir/src/lower/expr_member.rs index 6545559dd1..97c0b11f62 100644 --- a/crates/perry-hir/src/lower/expr_member.rs +++ b/crates/perry-hir/src/lower/expr_member.rs @@ -155,9 +155,11 @@ pub(crate) fn try_fold_symbol_well_known_member( let ast::Expr::Ident(obj_ident) = unwrap_transparent(obj) else { return Ok(None); }; - // Gated on `Symbol` being the real global — a local binding of that name - // resolves normally, matching JS scoping. - if obj_ident.sym.as_ref() != "Symbol" || ctx.lookup_local("Symbol").is_some() { + // Gated on `Symbol` being the real, unshadowed global — a `let`/`const`, + // `function`, `class`, or imported binding of that name resolves normally, + // matching JS scoping. `shadows_unqualified_global` covers all four forms + // (`lookup_local` alone would miss `class Symbol` / `function Symbol`). + if obj_ident.sym.as_ref() != "Symbol" || ctx.shadows_unqualified_global("Symbol") { return Ok(None); } let fold = diff --git a/test-files/test_gap_6719_optional_chain_symbol.ts b/test-files/test_gap_6719_optional_chain_symbol.ts index 42b4d26baf..291ebe1eb0 100644 --- a/test-files/test_gap_6719_optional_chain_symbol.ts +++ b/test-files/test_gap_6719_optional_chain_symbol.ts @@ -76,11 +76,29 @@ console.log([...obj].join(",")); console.log(Math.max(...obj)); // spread into a call // ── a locally-shadowed `Symbol` must NOT fold — normal scoping wins ───────── -function shadowed(): string { +// Every binding form that shadows the global must be honored: `const`, `class`, +// and `function` (the resolver gates on `shadows_unqualified_global`, not just +// `lookup_local`, which alone would miss the class/function declarations). +function shadowedConst(): string { const Symbol = { iterator: "local-value" } as any; return typeof Symbol?.iterator + ":" + Symbol?.iterator; } -console.log(shadowed()); // node: "string:local-value" +console.log(shadowedConst()); // node: "string:local-value" + +function shadowedClass(): string { + class Symbol { + static iterator = "class-static-iter"; + } + return String(Symbol?.iterator); +} +console.log(shadowedClass()); // node: "class-static-iter" + +function shadowedFunc(): string { + function Symbol() {} + (Symbol as any).iterator = "fn-prop-iter"; + return String(Symbol?.iterator); +} +console.log(shadowedFunc()); // node: "fn-prop-iter" // ── `?.` still short-circuits a genuinely nullish receiver ────────────────── const maybe: any = null;