fix: #5735 — declaration completion-value (5) + typed-array symbol-key writes

crates/perry-codegen/src/expr/index_set.rs

@@ -165,6 +165,33 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
                 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));

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

@@ -74,39 +74,70 @@ fn synth_ident_assign_stmt(name: &str, ident: &str) -> Option<ast::Stmt> {
     )
 }
 
-/// `void (<name> = <init>);` — 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<ast::Expr>) -> Option<ast::Stmt> {
+/// `{ let <sink> = (<name> = <init>); }` — 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 (`<name> = <init>`) 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<ast::Expr>) -> Option<ast::Stmt> {
     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 `<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, "<name>");
 ///   if (__perry_d === void 0 || __perry_d.configurable)
-///        Object.defineProperty(globalThis, "<name>",
+///        void Object.defineProperty(globalThis, "<name>",
 ///                              { value: <hidden>, writable: true, enumerable: true, configurable: true });
-///   else Object.defineProperty(globalThis, "<name>", { value: <hidden> }); }
+///   else void Object.defineProperty(globalThis, "<name>", { value: <hidden> }); }
 /// ```
 ///
 /// 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<ast::Stmt>
     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 <hidden> = (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<String> {
+    /// 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<String> {
         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,18 +1316,21 @@ 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!(
             matches!(out.first(), Some(ast::Stmt::If(_))),
             "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!(

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)

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}"
+    );
+}