From 97ad1eae93dcd450237387709c3f6c25cce44d3e Mon Sep 17 00:00:00 2001 From: Ralph Date: Sun, 28 Jun 2026 08:13:32 -0700 Subject: [PATCH 1/2] =?UTF-8?q?fix(eval,typedarray):=20#5735=20=E2=80=94?= =?UTF-8?q?=20declaration=20completion-value=20(5)=20+=20typed-array=20sym?= =?UTF-8?q?bol-key=20writes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two coherent test262 clusters from the 2026-06-27 sweep. ## Cluster 2 — declaration completion-value (5 cases, fully fixed) The completion value of a statement list whose last evaluated statement is a `function`/`async function`/`generator`/`var` *declaration* must be empty, so it falls through to the prior statement (`eval("1; function f(){}")` is `1`; `eval("function f(){}")` is `undefined`; `eval("9; var x = 10")` is `9`). In global-script mode the eval-fold hoist published these declarations to the global environment with statements the completion tracker mis-counted: - A top-level `function` is published via `Object.defineProperty(globalThis, …)`, which *returns* `globalThis`. The tracker rewrote that to `__perry_cv = define…`, so a declaration-only body yielded the global object. Fix: `void`-wrap the defineProperty calls (the publish block is prepended, so an empty completion is enough). - A top-level `var x = init` was rewritten in place to `void (x = init)`, still an expression statement, so the tracker wrote `__perry_cv = undefined` and *clobbered* a preceding value (`9; var x = 10` wrongly became `undefined`). Fix: emit a completion-inert lexical declaration `{ let __perry_eval_void = (x = init); }` — a declaration's completion is empty, so the prior value shows through. Fixes test262 `language/statements/{function,async-function,generators,variable}/cptn-*` and `language/statementList/eval-fn-block` (all pass). ## Cluster 1 — typed-array symbol-key writes A Symbol write on a *statically-typed* multi-byte typed array (`const t = new Float64Array(2); t[sym] = v`) lowered through the width-tracked native store, which `fptosi`-coerced the NaN-boxed Symbol to index 0 and *clobbered element 0* instead of storing a symbol property. Fix: route non-numeric keys on the width-tracked store path to the symbol-aware runtime dispatcher (mirroring the symmetric IndexGet guard), and make `js_typed_array_index_{get,set}_dynamic` triage Symbol keys into the symbol side table. Note: the two `OwnPropertyKeys/integer-indexes-and-string-and-symbol-keys` cases remain — they exercise a multi-byte view over a buffer from a discarded temporary (`new TA(new TA([…]).buffer)`) whose receiver is mis-inferred onto a typed-feedback array path; that is a separate type-inference issue tracked for follow-up. Tests: extends issue_5579 completion test with the declaration cases; adds issue_5735_typed_array_symbol_keys for the symbol-write fix. perry-hir, perry-codegen, perry-runtime suites green. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/perry-codegen/src/expr/index_set.rs | 27 +++++ .../perry-hir/src/lower/global_eval_hoist.rs | 104 +++++++++++------ crates/perry-runtime/src/typedarray_props.rs | 24 ++++ ...ue_5579_indirect_eval_global_completion.rs | 78 +++++++++++++ .../issue_5735_typed_array_symbol_keys.rs | 109 ++++++++++++++++++ 5 files changed, 307 insertions(+), 35 deletions(-) create mode 100644 crates/perry/tests/issue_5735_typed_array_symbol_keys.rs diff --git a/crates/perry-codegen/src/expr/index_set.rs b/crates/perry-codegen/src/expr/index_set.rs index 3f26681b01..12b336841a 100644 --- a/crates/perry-codegen/src/expr/index_set.rs +++ b/crates/perry-codegen/src/expr/index_set.rs @@ -165,6 +165,33 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { return Ok(val_double); } if is_width_tracked_typed_array_receiver(ctx, object) { + // A non-numeric index (a Symbol, or a string property name) is + // never an integer-indexed element. The width-tracked native + // store coerces the index with `fptosi`, which truncates a + // NaN-boxed Symbol to 0 and clobbers element 0 instead of + // storing the symbol property (test262 TypedArray symbol-key + // internals, #5735). Route such keys through the runtime + // dispatcher, which triages symbol / string / numeric keys — + // mirroring the symmetric IndexGet guard (index_get.rs). A + // literal / loop-counter index stays `is_numeric_expr`, so every + // proven element fast path below is preserved. + if !is_numeric_expr(ctx, index) { + let arr_box = lower_expr(ctx, object)?; + let idx_double = lower_expr(ctx, index)?; + let val_double = lower_expr(ctx, value)?; + let blk = ctx.block(); + let arr_bits = blk.bitcast_double_to_i64(&arr_box); + let arr_i64 = blk.and(I64, &arr_bits, POINTER_MASK_I64); + return Ok(blk.call( + DOUBLE, + "js_typed_array_index_set_dynamic", + &[ + (I64, &arr_i64), + (DOUBLE, &idx_double), + (DOUBLE, &val_double), + ], + )); + } if let Some(store) = lower_typed_array_store(ctx, object, index, value)? { if ctx.discard_expr_value { return Ok(double_literal(0.0)); diff --git a/crates/perry-hir/src/lower/global_eval_hoist.rs b/crates/perry-hir/src/lower/global_eval_hoist.rs index 846ae5cf5b..2603cfb385 100644 --- a/crates/perry-hir/src/lower/global_eval_hoist.rs +++ b/crates/perry-hir/src/lower/global_eval_hoist.rs @@ -74,39 +74,61 @@ fn synth_ident_assign_stmt(name: &str, ident: &str) -> Option { ) } -/// `void ( = );` — like [`synth_assign_stmt`] but wrapped in `void` -/// so the resulting expression statement keeps the *empty* completion value of -/// the `var` / `function` declaration it replaces (a bare `x = init` statement -/// would otherwise make the eval call yield `init`, e.g. breaking -/// `(0,eval)("var x = 1")` which must return `undefined`). The wrapper is built -/// by swapping the operand of a parsed `void 0;` to dodge version-sensitive SWC -/// `UnaryExpr` construction. -fn synth_void_assign_stmt(name: &str, init: Box) -> Option { +/// `{ let __perry_eval_void = ( = ); }` — like [`synth_assign_stmt`] +/// but wrapped as a *completion-inert* lexical declaration so it keeps the +/// *empty* completion value of the `var` declaration it replaces. A +/// VariableStatement's completion is empty (§14.3.2): `(0,eval)("var x = 1")` +/// must yield `undefined`, and — crucially — `eval("9; var x = 1")` must yield +/// `9` (the empty `var` completion falls through to the prior statement). A bare +/// `x = init` expression statement would yield `init`; a `void (x = init)` +/// statement is still an expression statement, so the completion tracker rewrote +/// it to `__perry_cv = void(x = init)` = `undefined`, which *clobbered* a +/// preceding value (`eval("9; var x = 1")` wrongly became `undefined`). Wrapping +/// the publish as a `let` declaration inside its own block makes it a +/// declaration — which the completion tracker leaves untouched (like the +/// original `var`) — while the inner assignment still publishes to the (global) +/// variable environment. Each call gets a fresh block scope, so repeated +/// `__perry_eval_void` bindings never collide. The throwaway is `let` (not +/// `var`) so it stays lexically scoped to the completion IIFE and is never +/// re-hoisted to the global environment. (test262 +/// `language/statements/variable/cptn-value`, #5735.) +fn synth_inert_assign_stmt(name: &str, init: Box) -> Option { let inner = synth_assign_stmt(name, init)?; let ast::Stmt::Expr(inner_es) = inner else { return None; }; - let mut wrapper = parse_single_stmt("void 0;")?; - let ast::Stmt::Expr(es) = &mut wrapper else { + let mut block = parse_single_stmt("{ let __perry_eval_void = 0; }")?; + let ast::Stmt::Block(b) = &mut block else { return None; }; - let ast::Expr::Unary(u) = es.expr.as_mut() else { + let Some(ast::Stmt::Decl(ast::Decl::Var(var))) = b.stmts.first_mut() else { return None; }; - u.arg = inner_es.expr; - Some(wrapper) + let decl = var.decls.first_mut()?; + decl.init = Some(inner_es.expr); + Some(block) } /// CreateGlobalFunctionBinding for a renamed hidden *top-level* function: /// publish its value to the global name `` with the spec's descriptor -/// rules, emitted as a block so its completion value stays empty. +/// rules. This is declaration-instantiation machinery, not a statement of the +/// eval source, so it must contribute an *empty* completion value: the +/// `Object.defineProperty(...)` calls are `void`-wrapped because `defineProperty` +/// returns the target object (`globalThis`) — without the wrapper the completion +/// tracker rewrites the call to `__perry_cv = Object.defineProperty(...)` and a +/// declaration-only eval body (`eval("function f() {}")`) yields `globalThis` +/// instead of `undefined` (test262 `language/statements/*/cptn-decl`). Same +/// empty-completion reasoning as [`synth_inert_assign_stmt`] for the `var` +/// publish (this block is *prepended* before the body, so `void`-wrapping its +/// stores to `undefined` suffices — a later user statement overwrites it, and a +/// declaration-only body correctly ends at `undefined`). /// /// ```text /// { let __perry_d = Object.getOwnPropertyDescriptor(globalThis, ""); /// if (__perry_d === void 0 || __perry_d.configurable) -/// Object.defineProperty(globalThis, "", +/// void Object.defineProperty(globalThis, "", /// { value: , writable: true, enumerable: true, configurable: true }); -/// else Object.defineProperty(globalThis, "", { value: }); } +/// else void Object.defineProperty(globalThis, "", { value: }); } /// ``` /// /// An absent or configurable binding is (re)defined as a writable, enumerable, @@ -121,9 +143,9 @@ fn synth_create_global_fn_binding(name: &str, ident: &str) -> Option parse_single_stmt(&format!( "{{ let __perry_d = Object.getOwnPropertyDescriptor(globalThis, {name:?}); \ if (__perry_d === void 0 || __perry_d.configurable) \ - {{ Object.defineProperty(globalThis, {name:?}, \ + {{ void Object.defineProperty(globalThis, {name:?}, \ {{ value: {ident}, writable: true, enumerable: true, configurable: true }}); }} \ - else {{ Object.defineProperty(globalThis, {name:?}, {{ value: {ident} }}); }} }}" + else {{ void Object.defineProperty(globalThis, {name:?}, {{ value: {ident} }}); }} }}" )) } @@ -725,7 +747,7 @@ impl GlobalEvalHoist { let name = binding.id.sym.to_string(); self.var_prelude_names.push(name.clone()); if let Some(init) = &d.init { - match synth_void_assign_stmt(&name, init.clone()) { + match synth_inert_assign_stmt(&name, init.clone()) { Some(s) => publishes.push(s), None => { self.ok = false; @@ -1172,19 +1194,25 @@ mod global_eval_hoist_tests { ); } - /// Names assigned by a top-level `void (name = …)` publish statement. - fn void_publish_targets(stmts: &[ast::Stmt]) -> Vec { + /// Names assigned by a top-level completion-inert `var` publish — + /// `{ let __perry_eval_void = (name = …); }`. The assignment publishes to the + /// (global) variable environment while the enclosing lexical declaration + /// keeps the `var` statement's empty completion value. + fn inert_publish_targets(stmts: &[ast::Stmt]) -> Vec { let mut out = Vec::new(); for s in stmts { - if let ast::Stmt::Expr(es) = s { - if let ast::Expr::Unary(u) = es.expr.as_ref() { - if matches!(u.op, ast::UnaryOp::Void) { - if let ast::Expr::Assign(a) = u.arg.as_ref() { - if let ast::AssignTarget::Simple(ast::SimpleAssignTarget::Ident(b)) = - &a.left - { - out.push(b.id.sym.to_string()); - } + let ast::Stmt::Block(b) = s else { continue }; + for inner in &b.stmts { + let ast::Stmt::Decl(ast::Decl::Var(v)) = inner else { + continue; + }; + for d in &v.decls { + let Some(init) = &d.init else { continue }; + if let ast::Expr::Assign(a) = init.as_ref() { + if let ast::AssignTarget::Simple(ast::SimpleAssignTarget::Ident(bind)) = + &a.left + { + out.push(bind.id.sym.to_string()); } } } @@ -1215,6 +1243,9 @@ mod global_eval_hoist_tests { expr(&c.cons, out); expr(&c.alt, out); } + // The function publish wraps each `Object.defineProperty(...)` + // call in `void (...)` to keep an empty completion value. + ast::Expr::Unary(u) => expr(&u.arg, out), _ => {} } } @@ -1271,7 +1302,9 @@ mod global_eval_hoist_tests { #[test] fn top_level_var_is_published_to_the_global() { // A *top-level* `var` is CreateGlobalVarBinding: a create-if-absent slot - // (`if (...) { globalThis[x] = void 0 }`) plus a `void (x = init)` publish. + // (`if (...) { globalThis[x] = void 0 }`) plus a completion-inert + // `{ let __perry_eval_void = (x = init); }` publish (the lexical + // declaration keeps the VariableStatement's empty completion value). let out = apply_global_eval_hoist(&parse_body("initial = x; var x = 9;")) .expect("publishes the top-level var"); assert!( @@ -1279,10 +1312,11 @@ mod global_eval_hoist_tests { "create-if-absent prelude" ); assert!( - void_publish_targets(&out).iter().any(|t| t == "x"), - "expected a void-wrapped publish of `x`" + inert_publish_targets(&out).iter().any(|t| t == "x"), + "expected an inert publish of `x`" ); - // No `var` declaration may remain (it was rewritten to the publish). + // No top-level `var` declaration may remain (it was rewritten to the + // publish block). assert!( !out.iter() .any(|s| matches!(s, ast::Stmt::Decl(ast::Decl::Var(_)))), @@ -1301,7 +1335,7 @@ mod global_eval_hoist_tests { "create-if-absent prelude" ); assert!( - void_publish_targets(&out).is_empty(), + inert_publish_targets(&out).is_empty(), "no publish for a bare var" ); assert!( diff --git a/crates/perry-runtime/src/typedarray_props.rs b/crates/perry-runtime/src/typedarray_props.rs index f492f66e39..0551727a78 100644 --- a/crates/perry-runtime/src/typedarray_props.rs +++ b/crates/perry-runtime/src/typedarray_props.rs @@ -583,6 +583,17 @@ pub(crate) unsafe fn typed_array_index_get_dynamic(owner_bits: usize, key: f64) let Some(owner) = typed_array_addr_from_value(f64::from_bits(owner_bits as u64)) else { return f64::from_bits(crate::value::TAG_UNDEFINED); }; + // A Symbol key is never an integer-indexed element — read it from the symbol + // side table, exactly as the ordinary `obj[sym]` path does. Without this a + // symbol coerces to a numeric index (NaN → undefined) and a previously + // stored `ta[sym]` reads back `undefined` (test262 TypedArray symbol-key + // internals, #5735). + if crate::symbol::js_is_symbol(key) != 0 { + return crate::symbol::js_object_get_symbol_property( + crate::value::js_nanbox_pointer(owner as i64), + key, + ); + } let jsval = crate::value::JSValue::from_bits(key.to_bits()); if jsval.is_string() || jsval.is_short_string() { let key_ptr = @@ -621,6 +632,19 @@ pub extern "C" fn js_typed_array_index_set_dynamic( let Some(owner) = typed_array_addr_from_value(f64::from_bits(ta as u64)) else { return value; }; + // A Symbol key is never an integer-indexed element — store it in the + // symbol side table, exactly as the ordinary `obj[sym] = v` path does. + // Without this the symbol is silently dropped (and, on the codegen + // width-tracked store path, `fptosi` coerces it to index 0 and clobbers + // element 0) — test262 TypedArray symbol-key internals, #5735. + if crate::symbol::js_is_symbol(key) != 0 { + crate::symbol::js_object_set_symbol_property( + crate::value::js_nanbox_pointer(owner as i64), + key, + value, + ); + return value; + } let jsval = crate::value::JSValue::from_bits(key.to_bits()); if jsval.is_string() || jsval.is_short_string() { let key_ptr = crate::value::js_get_string_pointer_unified(key) diff --git a/crates/perry/tests/issue_5579_indirect_eval_global_completion.rs b/crates/perry/tests/issue_5579_indirect_eval_global_completion.rs index 30f24c37fa..ca8ebc136c 100644 --- a/crates/perry/tests/issue_5579_indirect_eval_global_completion.rs +++ b/crates/perry/tests/issue_5579_indirect_eval_global_completion.rs @@ -182,3 +182,81 @@ fn indirect_eval_nested_does_not_capture_locals() { "indirect eval must not capture the enclosing function's `local`\n{out}" ); } + +/// Regression (#5735, cluster 2): the completion value of a statement list whose +/// last evaluated statement is a *declaration* must fall through to the prior +/// statement — a declaration produces an *empty* completion (test262 +/// `language/statements/{function,async-function,generators,variable}/cptn-*`, +/// `language/statementList/eval-fn-block`). +/// +/// In global-script mode a top-level `function` declaration inside the eval body +/// is published to the global environment via CreateGlobalFunctionBinding, whose +/// `Object.defineProperty(globalThis, …)` call *returns `globalThis`*. The +/// completion tracker rewrote that publish into `__perry_cv = Object.define…`, +/// so a declaration-only body (`eval("function f() {}")`) wrongly yielded the +/// global object instead of `undefined`. The publish is now `void`-wrapped (it +/// is declaration-instantiation machinery, not a statement of the source), so it +/// keeps an empty completion. A preceding value statement still shows through +/// (`eval("1; function f() {}")` === 1). +const CPTN_DECL: &str = r#" +console.log("fn:", eval("function f() {}")); // -> undefined +console.log("fn1:", eval("1; function f1() {}")); // -> 1 +console.log("gen:", eval("function* g() {}")); // -> undefined +console.log("gen1:", eval("1; function* g1() {}")); // -> 1 +console.log("async:", eval("async function af() {}")); // -> undefined +console.log("var:", eval("var v1;")); // -> undefined +console.log("varinit:", eval("var v2 = 2;")); // -> undefined +console.log("var7:", eval("7; var v8;")); // -> 7 +console.log("var9:", eval("9; var v10 = 10;")); // -> 9 (init falls through) +console.log("var11:", eval("11; var v12 = 12, v13;")); // -> 11 +console.log("var14:", eval("14; var v15, v16 = 16;")); // -> 14 +console.log("fnblock:", eval("function fn() {}{}")); // -> undefined +console.log("DONE"); +"#; + +#[test] +fn eval_declaration_completion_is_empty_global_script() { + let (ok, out) = compile_and_run(CPTN_DECL, /* global_script */ true); + assert!(ok, "binary did not exit cleanly\n{out}"); + assert!( + out.contains("fn: undefined"), + "`function f(){{}}` -> undefined\n{out}" + ); + assert!(out.contains("fn1: 1"), "`1; function f1(){{}}` -> 1\n{out}"); + assert!( + out.contains("gen: undefined"), + "`function* g(){{}}` -> undefined\n{out}" + ); + assert!( + out.contains("gen1: 1"), + "`1; function* g1(){{}}` -> 1\n{out}" + ); + assert!( + out.contains("async: undefined"), + "`async function af(){{}}` -> undefined\n{out}" + ); + assert!( + out.contains("var: undefined"), + "`var v1;` -> undefined\n{out}" + ); + assert!( + out.contains("varinit: undefined"), + "`var v2 = 2;` -> undefined\n{out}" + ); + assert!(out.contains("var7: 7"), "`7; var v8;` -> 7\n{out}"); + // The empty completion of a `var` declaration (even with an initializer) + // falls through to the preceding statement's value, not `undefined`. + assert!(out.contains("var9: 9"), "`9; var v10 = 10;` -> 9\n{out}"); + assert!( + out.contains("var11: 11"), + "`11; var v12 = 12, v13;` -> 11\n{out}" + ); + assert!( + out.contains("var14: 14"), + "`14; var v15, v16 = 16;` -> 14\n{out}" + ); + assert!( + out.contains("fnblock: undefined"), + "`function fn(){{}}{{}}` -> undefined\n{out}" + ); +} diff --git a/crates/perry/tests/issue_5735_typed_array_symbol_keys.rs b/crates/perry/tests/issue_5735_typed_array_symbol_keys.rs new file mode 100644 index 0000000000..fae9b27cfe --- /dev/null +++ b/crates/perry/tests/issue_5735_typed_array_symbol_keys.rs @@ -0,0 +1,109 @@ +//! Regression test for #5735 (cluster 1): Symbol-keyed property writes on a +//! *statically-typed* typed array (`const t = new Float64Array(2); t[sym] = v`). +//! +//! A statically-typed multi-byte typed-array receiver lowers through the +//! width-tracked native store path (`lower_typed_array_store`), which coerced +//! the index with `fptosi`. A NaN-boxed Symbol truncates to index 0, so +//! `t[sym] = v` silently *clobbered element 0* instead of storing a symbol +//! property — corrupting the array's data and dropping the symbol. The fix +//! routes non-numeric keys on the width-tracked store path to the symbol-aware +//! runtime dispatcher (mirroring the symmetric IndexGet guard), and makes the +//! `js_typed_array_index_{get,set}_dynamic` helpers triage Symbol keys into the +//! symbol side table. This test pins that a Symbol write lands in the symbol +//! table, leaves the element data untouched, and survives `reverse()` / +//! `Reflect.ownKeys` ordering — no test262 harness needed, so it runs in CI. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +#[test] +fn typed_array_symbol_keys_are_stored_not_coerced_to_element_zero() { + let dir = tempfile::tempdir().expect("tempdir"); + let entry = dir.path().join("main.ts"); + let output = dir.path().join("main_bin"); + + std::fs::write( + &entry, + r#" +const s1 = Symbol("1"); +const s2 = Symbol("2"); + +// (A) A symbol write on a statically-typed multi-byte typed array must NOT +// clobber element 0; it lands in the symbol side table and reads back. +const f = new Float64Array(3); +f[0] = 11; f[1] = 22; f[2] = 33; +f[s1] = 99; +console.log("A=" + f[s1] + "," + f[0] + "," + f[1] + "," + f[2]); // 99,11,22,33 + +// (B) Multiple symbols + string expandos: all retained, and Reflect.ownKeys +// orders integer indexes, then string keys (insertion order), then symbols. +const g = new Int16Array(2); +(g as any).foo = 42; +g[s1] = 1; +g[s2] = 2; +console.log("B=" + Object.getOwnPropertySymbols(g).length); // 2 +const keys = Reflect.ownKeys(g).map(String).join(","); +console.log("Bkeys=" + keys); // 0,1,foo,Symbol(1),Symbol(2) + +// (C) reverse() preserves symbol-keyed (and string-keyed) properties while +// reversing the elements. +const r = new Int8Array(2); +r[0] = 5; r[1] = 6; +(r as any).bar = "bar"; +r[s1] = 7; +const rr = r.reverse(); +console.log("C=" + rr[s1] + "," + (rr as any).bar + "," + rr[0] + "," + rr[1]); // 7,bar,6,5 + +// (D) Symbol read on a fresh typed array (no prior write) is undefined, and a +// numeric index still reads the element (the guard only diverts non-numerics). +const h = new Uint32Array(1); +h[0] = 1234; +console.log("D=" + (h[s2] === undefined) + "," + h[0]); // true,1234 + +console.log("DONE"); +"#, + ) + .expect("write entry"); + + let compiled = Command::new(perry_bin()) + .current_dir(dir.path()) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .output() + .expect("run perry compile"); + assert!( + compiled.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compiled.stdout), + String::from_utf8_lossy(&compiled.stderr) + ); + + let run = Command::new(&output).output().expect("run compiled binary"); + let out = String::from_utf8_lossy(&run.stdout).to_string(); + assert!(run.status.success(), "binary did not exit cleanly\n{out}"); + + assert!( + out.contains("A=99,11,22,33"), + "symbol write clobbered element data\n{out}" + ); + assert!(out.contains("B=2"), "both symbols must be retained\n{out}"); + assert!( + out.contains("Bkeys=0,1,foo,Symbol(1),Symbol(2)"), + "ownKeys ordering (indexes, strings, symbols)\n{out}" + ); + assert!( + out.contains("C=7,bar,6,5"), + "reverse must preserve symbol + reverse elements\n{out}" + ); + assert!( + out.contains("D=true,1234"), + "symbol read undefined; numeric read intact\n{out}" + ); + assert!(out.contains("DONE"), "program did not finish\n{out}"); +} From 773052d378128d8d7eaf58062b041316d1cd18a1 Mon Sep 17 00:00:00 2001 From: Ralph Date: Sun, 28 Jun 2026 08:29:59 -0700 Subject: [PATCH 2/2] =?UTF-8?q?fix(hir):=20#5735=20=E2=80=94=20use=20a=20f?= =?UTF-8?q?resh=20hidden=20sink=20for=20the=20inert=20var-init=20publish?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses CodeRabbit review: the completion-inert `var x = init` publish wrapped the assignment in `{ let __perry_eval_void = (x = init); }` with a *fixed* sink name. The lexical sink is in TDZ while its own initializer (`x = init`) evaluates, so eval source whose `init` referenced that exact name (`eval("var x = __perry_eval_void")`) would throw a spurious ReferenceError and diverge from the pre-rewrite resolution. Generate a fresh per-publish hidden name via `fresh_hidden()` instead, matching the existing renamed-binding convention. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../perry-hir/src/lower/global_eval_hoist.rs | 64 +++++++++++-------- 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/crates/perry-hir/src/lower/global_eval_hoist.rs b/crates/perry-hir/src/lower/global_eval_hoist.rs index 2603cfb385..7821bfd2d8 100644 --- a/crates/perry-hir/src/lower/global_eval_hoist.rs +++ b/crates/perry-hir/src/lower/global_eval_hoist.rs @@ -74,25 +74,30 @@ fn synth_ident_assign_stmt(name: &str, ident: &str) -> Option { ) } -/// `{ let __perry_eval_void = ( = ); }` — like [`synth_assign_stmt`] -/// but wrapped as a *completion-inert* lexical declaration so it keeps the -/// *empty* completion value of the `var` declaration it replaces. A -/// VariableStatement's completion is empty (§14.3.2): `(0,eval)("var x = 1")` -/// must yield `undefined`, and — crucially — `eval("9; var x = 1")` must yield -/// `9` (the empty `var` completion falls through to the prior statement). A bare -/// `x = init` expression statement would yield `init`; a `void (x = init)` -/// statement is still an expression statement, so the completion tracker rewrote -/// it to `__perry_cv = void(x = init)` = `undefined`, which *clobbered* a -/// preceding value (`eval("9; var x = 1")` wrongly became `undefined`). Wrapping -/// the publish as a `let` declaration inside its own block makes it a -/// declaration — which the completion tracker leaves untouched (like the -/// original `var`) — while the inner assignment still publishes to the (global) -/// variable environment. Each call gets a fresh block scope, so repeated -/// `__perry_eval_void` bindings never collide. The throwaway is `let` (not -/// `var`) so it stays lexically scoped to the completion IIFE and is never -/// re-hoisted to the global environment. (test262 +/// `{ let = ( = ); }` — like [`synth_assign_stmt`] but +/// wrapped as a *completion-inert* lexical declaration so it keeps the *empty* +/// completion value of the `var` declaration it replaces. A VariableStatement's +/// completion is empty (§14.3.2): `(0,eval)("var x = 1")` must yield `undefined`, +/// and — crucially — `eval("9; var x = 1")` must yield `9` (the empty `var` +/// completion falls through to the prior statement). A bare `x = init` expression +/// statement would yield `init`; a `void (x = init)` statement is still an +/// expression statement, so the completion tracker rewrote it to +/// `__perry_cv = void(x = init)` = `undefined`, which *clobbered* a preceding +/// value (`eval("9; var x = 1")` wrongly became `undefined`). Wrapping the +/// publish as a `let` declaration inside its own block makes it a declaration — +/// which the completion tracker leaves untouched (like the original `var`) — +/// while the inner assignment still publishes to the (global) variable +/// environment. The throwaway is `let` (not `var`) so it stays lexically scoped +/// to the completion IIFE and is never re-hoisted to the global environment. +/// +/// `sink` must be a caller-generated hidden name ([`GlobalEvalHoist::fresh_hidden`]), +/// not a fixed literal: the lexical `sink` binding is in TDZ while its own +/// initializer (` = `) evaluates, so a fixed name that the user's +/// `init` could reference (`eval("var x = __perry_eval_void")`) would throw a +/// spurious `ReferenceError`. A fresh per-publish name keeps `init`'s references +/// resolving exactly as they did before the rewrite. (test262 /// `language/statements/variable/cptn-value`, #5735.) -fn synth_inert_assign_stmt(name: &str, init: Box) -> Option { +fn synth_inert_assign_stmt(sink: &str, name: &str, init: Box) -> Option { let inner = synth_assign_stmt(name, init)?; let ast::Stmt::Expr(inner_es) = inner else { return None; @@ -105,6 +110,10 @@ fn synth_inert_assign_stmt(name: &str, init: Box) -> Option = (x = init); }` global publish (the lexical + // declaration keeps the VariableStatement's empty completion + // value). A non-simple declarator (destructuring) the rewrite + // can't model bails the whole fold. (test262 `*/var-env-var-*`.) + // A *nested* `var` and all `let`/`const` stay put — the IIFE + // models the eval's own variable / lexical env. ast::Stmt::Decl(ast::Decl::Var(var_decl)) if top_level && var_decl.kind == ast::VarDeclKind::Var @@ -747,7 +757,11 @@ impl GlobalEvalHoist { let name = binding.id.sym.to_string(); self.var_prelude_names.push(name.clone()); if let Some(init) = &d.init { - match synth_inert_assign_stmt(&name, init.clone()) { + // A fresh hidden sink per publish: the lexical binding + // is in TDZ while `init` evaluates, so a name the + // user's `init` could reference would throw spuriously. + let sink = self.fresh_hidden(); + match synth_inert_assign_stmt(&sink, &name, init.clone()) { Some(s) => publishes.push(s), None => { self.ok = false;