diff --git a/crates/perry-codegen/src/expr/index_set.rs b/crates/perry-codegen/src/expr/index_set.rs index 3f26681b0..12b336841 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 846ae5cf5..7821bfd2d 100644 --- a/crates/perry-hir/src/lower/global_eval_hoist.rs +++ b/crates/perry-hir/src/lower/global_eval_hoist.rs @@ -74,39 +74,70 @@ 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 = ( = ); }` — 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(sink: &str, 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 Some(ast::Stmt::Decl(ast::Decl::Var(var))) = b.stmts.first_mut() else { return None; }; - let ast::Expr::Unary(u) = es.expr.as_mut() else { + let decl = var.decls.first_mut()?; + let ast::Pat::Ident(binding) = &mut decl.name else { return None; }; - u.arg = inner_es.expr; - Some(wrapper) + binding.id.sym = sink.into(); + 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 +152,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} }}); }} }}" )) } @@ -705,12 +736,13 @@ impl GlobalEvalHoist { } // A top-level `var` is CreateGlobalVarBinding: pre-create each // name (`undefined`, not reinitialized if it already exists) via - // the prelude and rewrite `var x = init` to a `void (x = init)` - // global publish (the `void` 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. + // the prelude and rewrite `var x = init` to a completion-inert + // `{ let = (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 @@ -725,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_void_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; @@ -1172,19 +1208,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 +1257,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 +1316,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 +1326,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 +1349,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 f492f66e3..0551727a7 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 30f24c37f..ca8ebc136 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 000000000..fae9b27cf --- /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}"); +}