diff --git a/.isu/issues.json b/.isu/issues.json index 0bfc5a8..c0a6d78 100644 --- a/.isu/issues.json +++ b/.isu/issues.json @@ -1,5 +1,5 @@ { - "next_id": 375, + "next_id": 376, "issues": [ { "id": 1, @@ -4515,7 +4515,7 @@ ], "assigned": [], "author": "piefev", - "state": "open", + "state": "closed", "created_at": "2026-06-05T05:30:30Z" }, { @@ -4528,7 +4528,7 @@ ], "assigned": [], "author": "piefev", - "state": "open", + "state": "closed", "created_at": "2026-06-05T05:30:31Z" }, { @@ -4541,7 +4541,7 @@ ], "assigned": [], "author": "piefev", - "state": "open", + "state": "closed", "created_at": "2026-06-05T05:30:31Z" }, { @@ -4554,7 +4554,7 @@ ], "assigned": [], "author": "piefev", - "state": "open", + "state": "closed", "created_at": "2026-06-05T05:30:31Z" }, { @@ -4580,6 +4580,19 @@ "author": "piefev", "state": "open", "created_at": "2026-06-05T08:30:39Z" + }, + { + "id": 375, + "repo": "we", + "title": "Bing modules_wrapper remains empty after JS hydration blocker fixes", + "body": "Parent: isu issue 280. Related older blocker: isu issue 374.\n\nAfter fixing and closing these JS real-web blockers:\n- isu issue 369 object literal spread\n- isu issue 370 rest parameters\n- isu issue 371 object literal accessors\n- isu issue 372 arrow lexical this\n\nRepro:\n\ncargo run -p we-e2e -- --scenario crates/e2e/scenarios/real-web/bing.com.we --out-dir crates/e2e/artifacts\n\nCurrent result after the fixes:\n- desktop screenshot match still fails at 2.47% match, 1198216/1228500 px differ, tol=4, max_diff=0.1000%\n- mobile screenshot match still fails at 5.31% match, 311681/329160 px differ, tol=4, max_diff=0.1000%\n- desktop_console.txt and mobile_console.txt are empty\n- scenario stdout still reports the seeded 404 for https://r.bing.com/rb/4O/jnc,nj/cKzGnam6-Fytml03VT3ixBZ9tTs.js?... \n- desktop_dom.txt still has an empty modules_wrapper\n\nArtifacts:\n- crates/e2e/artifacts/real-web/bing.com/desktop.png\n- crates/e2e/artifacts/real-web/bing.com/desktop.png.diff.png\n- crates/e2e/artifacts/real-web/bing.com/desktop_dom.txt\n- crates/e2e/artifacts/real-web/bing.com/desktop_console.txt\n- crates/e2e/artifacts/real-web/bing.com/mobile.png\n- crates/e2e/artifacts/real-web/bing.com/mobile.png.diff.png\n- crates/e2e/artifacts/real-web/bing.com/mobile_dom.txt\n- crates/e2e/artifacts/real-web/bing.com/mobile_console.txt\n\nExpected: the offline Bing snapshot and cached carousel/postload responses hydrate the modules_wrapper carousel/news content closely enough to remove the xfail from crates/e2e/scenarios/real-web/bing.com.we.\n\nActual: the page remains the static Sichuan Tea shell while the Chromium golden shows the hydrated Jaipur/news-card page. The next investigation should focus on why the Bing bundle stops before populating modules_wrapper now that spread, rest params, object accessors, and arrow lexical this are implemented.", + "labels": [ + "real-web" + ], + "assigned": [], + "author": "piefev", + "state": "open", + "created_at": "2026-06-05T14:28:43Z" } ] } diff --git a/crates/js/src/ast.rs b/crates/js/src/ast.rs index 548a23a..9c9f9b0 100644 --- a/crates/js/src/ast.rs +++ b/crates/js/src/ast.rs @@ -442,6 +442,8 @@ pub enum PatternKind { left: Box, right: Box, }, + /// Rest parameter: `...args` + Rest(Box), } #[derive(Debug, Clone, PartialEq)] diff --git a/crates/js/src/bytecode.rs b/crates/js/src/bytecode.rs index f91ef57..539198b 100644 --- a/crates/js/src/bytecode.rs +++ b/crates/js/src/bytecode.rs @@ -285,6 +285,9 @@ pub enum Op { /// HasPrivate dst, obj_reg, name_idx(u16) — `#x in obj`: true iff `obj` /// carries the private name. Never throws. HasPrivate = 0x6C, + /// CopyDataProperties target_reg, source_reg — object literal spread: + /// copy source's own enumerable properties into target. + CopyDataProperties = 0x6D, // ── Misc ──────────────────────────────────────────────── /// Delete dst, obj_reg, key_reg @@ -327,6 +330,9 @@ pub enum Op { /// The resulting object has indexed properties 0..N-1, a `length` property of N, /// and inherits from Object.prototype. BuildArguments = 0x83, + /// BuildRestArguments dst, start_idx(u16) — materialize arguments[start_idx..] + /// as an Array object for a rest parameter. + BuildRestArguments = 0x84, } impl Op { @@ -392,6 +398,7 @@ impl Op { 0x6A => Some(Op::GetPrivate), 0x6B => Some(Op::SetPrivate), 0x6C => Some(Op::HasPrivate), + 0x6D => Some(Op::CopyDataProperties), 0x70 => Some(Op::Delete), 0x71 => Some(Op::LoadInt8), 0x72 => Some(Op::ForInInit), @@ -409,6 +416,7 @@ impl Op { 0x81 => Some(Op::Spread), 0x82 => Some(Op::Await), 0x83 => Some(Op::BuildArguments), + 0x84 => Some(Op::BuildRestArguments), _ => None, } } @@ -802,6 +810,20 @@ impl BytecodeBuilder { self.emit_u16(name_idx); } + /// Emit: CopyDataProperties target, source + pub fn emit_copy_data_properties(&mut self, target: Reg, source: Reg) { + self.emit_u8(Op::CopyDataProperties as u8); + self.emit_reg_operand(target); + self.emit_reg_operand(source); + } + + /// Emit: BuildRestArguments dst, start_idx + pub fn emit_build_rest_arguments(&mut self, dst: Reg, start_idx: u16) { + self.emit_u8(Op::BuildRestArguments as u8); + self.emit_reg_operand(dst); + self.emit_u16(start_idx); + } + /// Emit: LoadInt8 dst, value pub fn emit_load_int8(&mut self, dst: Reg, value: i8) { self.emit_u8(Op::LoadInt8 as u8); @@ -1126,7 +1148,7 @@ impl Function { .get(idx as usize) .map(|s| s.as_str()) .unwrap_or("?"); - let k = if kind == 0 { "get" } else { "set" }; + let k = if kind & 1 == 0 { "get" } else { "set" }; format!("DefineAccessor r{obj}, @{idx}(\"{name}\"), {k}, r{f}") } Op::SetHomeObject => { @@ -1171,6 +1193,11 @@ impl Function { .unwrap_or("?"); format!("HasPrivate r{dst}, r{obj}, @{idx}(\"{name}\")") } + Op::CopyDataProperties => { + let target = read_reg_operand(code, &mut pc); + let source = read_reg_operand(code, &mut pc); + format!("CopyDataProperties r{target}, r{source}") + } Op::Delete => { let dst = read_reg_operand(code, &mut pc); let obj = read_reg_operand(code, &mut pc); @@ -1255,6 +1282,11 @@ impl Function { let dst = read_reg_operand(code, &mut pc); format!("BuildArguments r{dst}") } + Op::BuildRestArguments => { + let dst = read_reg_operand(code, &mut pc); + let start = read_u16_operand(code, &mut pc); + format!("BuildRestArguments r{dst}, {start}") + } }; out.push_str(&format!(" {offset:04X} {line}\n")); } @@ -1342,6 +1374,10 @@ mod tests { Op::DefineAccessor, Op::SetHomeObject, Op::LoadSuperBase, + Op::GetPrivate, + Op::SetPrivate, + Op::HasPrivate, + Op::CopyDataProperties, Op::Delete, Op::LoadInt8, Op::ForInInit, @@ -1359,6 +1395,7 @@ mod tests { Op::Spread, Op::Await, Op::BuildArguments, + Op::BuildRestArguments, ]; for op in ops { assert_eq!( diff --git a/crates/js/src/compiler.rs b/crates/js/src/compiler.rs index 0f23f11..02e3be7 100644 --- a/crates/js/src/compiler.rs +++ b/crates/js/src/compiler.rs @@ -242,6 +242,10 @@ fn collect_free_vars(params: &[Pattern], body: &[Stmt]) -> HashSet { // reference it bubble up to this function (which materializes the // binding) rather than to the parent or to a global lookup. declared.insert("arguments".to_string()); + // Non-arrow functions also have their own dynamic `this`. Nested arrows + // should capture it at their creation site, but it must not bubble up as a + // free variable of this function. + declared.insert("this".to_string()); // Collect declarations and references from the body. for stmt in body { @@ -310,14 +314,190 @@ fn collect_free_vars_arrow(params: &[Pattern], body: &ArrowBody) -> HashSet bool { + match body { + ArrowBody::Expr(expr) => expr_uses_lexical_this(expr), + ArrowBody::Block(stmts) => stmts_use_lexical_this(stmts), + } +} + +fn stmts_use_lexical_this(stmts: &[Stmt]) -> bool { + stmts.iter().any(stmt_uses_lexical_this) +} + +fn stmt_uses_lexical_this(stmt: &Stmt) -> bool { + match &stmt.kind { + StmtKind::Expr(expr) | StmtKind::Return(Some(expr)) | StmtKind::Throw(expr) => { + expr_uses_lexical_this(expr) + } + StmtKind::Return(None) + | StmtKind::Break(_) + | StmtKind::Continue(_) + | StmtKind::Debugger + | StmtKind::Empty + | StmtKind::Import { .. } => false, + StmtKind::Block(stmts) => stmts_use_lexical_this(stmts), + StmtKind::VarDecl { declarators, .. } => declarators + .iter() + .any(|decl| decl.init.as_ref().is_some_and(expr_uses_lexical_this)), + StmtKind::FunctionDecl(_) | StmtKind::ClassDecl(_) => false, + StmtKind::If { + test, + consequent, + alternate, + } => { + expr_uses_lexical_this(test) + || stmt_uses_lexical_this(consequent) + || alternate + .as_ref() + .is_some_and(|alt| stmt_uses_lexical_this(alt)) + } + StmtKind::For { + init, + test, + update, + body, + } => { + init.as_ref().is_some_and(for_init_uses_lexical_this) + || test.as_ref().is_some_and(expr_uses_lexical_this) + || update.as_ref().is_some_and(expr_uses_lexical_this) + || stmt_uses_lexical_this(body) + } + StmtKind::ForIn { left, right, body } + | StmtKind::ForOf { + left, right, body, .. + } => { + for_in_of_left_uses_lexical_this(left) + || expr_uses_lexical_this(right) + || stmt_uses_lexical_this(body) + } + StmtKind::While { test, body } => { + expr_uses_lexical_this(test) || stmt_uses_lexical_this(body) + } + StmtKind::DoWhile { body, test } => { + stmt_uses_lexical_this(body) || expr_uses_lexical_this(test) + } + StmtKind::Switch { + discriminant, + cases, + } => { + expr_uses_lexical_this(discriminant) + || cases.iter().any(|case| { + case.test.as_ref().is_some_and(expr_uses_lexical_this) + || stmts_use_lexical_this(&case.consequent) + }) + } + StmtKind::Try { + block, + handler, + finalizer, + } => { + stmts_use_lexical_this(block) + || handler + .as_ref() + .is_some_and(|h| stmts_use_lexical_this(&h.body)) + || finalizer + .as_ref() + .is_some_and(|f| stmts_use_lexical_this(f)) + } + StmtKind::Labeled { body, .. } => stmt_uses_lexical_this(body), + StmtKind::With { object, body } => { + expr_uses_lexical_this(object) || stmt_uses_lexical_this(body) + } + StmtKind::Export(export) => export_decl_uses_lexical_this(export), + } +} + +fn for_init_uses_lexical_this(init: &ForInit) -> bool { + match init { + ForInit::VarDecl { declarators, .. } => declarators + .iter() + .any(|decl| decl.init.as_ref().is_some_and(expr_uses_lexical_this)), + ForInit::Expr(expr) => expr_uses_lexical_this(expr), + } +} + +fn for_in_of_left_uses_lexical_this(left: &ForInOfLeft) -> bool { + match left { + ForInOfLeft::VarDecl { .. } | ForInOfLeft::Pattern(_) => false, + } +} + +fn export_decl_uses_lexical_this(export: &ExportDecl) -> bool { + match export { + ExportDecl::Default(expr) => expr_uses_lexical_this(expr), + ExportDecl::Declaration(stmt) => stmt_uses_lexical_this(stmt), + ExportDecl::Named { .. } | ExportDecl::AllFrom(_) => false, + } +} + +fn expr_uses_lexical_this(expr: &Expr) -> bool { + match &expr.kind { + ExprKind::This => true, + ExprKind::Function(_) | ExprKind::Class(_) => false, + ExprKind::Arrow { body, .. } => arrow_body_uses_lexical_this(body), + ExprKind::Unary { argument, .. } + | ExprKind::Update { argument, .. } + | ExprKind::OptionalChain { base: argument } + | ExprKind::OptionalChainExpr { chain: argument } + | ExprKind::Spread(argument) + | ExprKind::Await(argument) => expr_uses_lexical_this(argument), + ExprKind::Yield { argument, .. } => argument + .as_ref() + .is_some_and(|arg| expr_uses_lexical_this(arg)), + ExprKind::Binary { left, right, .. } + | ExprKind::Logical { left, right, .. } + | ExprKind::Assignment { left, right, .. } => { + expr_uses_lexical_this(left) || expr_uses_lexical_this(right) + } + ExprKind::Conditional { + test, + consequent, + alternate, + } => { + expr_uses_lexical_this(test) + || expr_uses_lexical_this(consequent) + || expr_uses_lexical_this(alternate) + } + ExprKind::Call { callee, arguments } | ExprKind::New { callee, arguments } => { + expr_uses_lexical_this(callee) || arguments.iter().any(expr_uses_lexical_this) + } + ExprKind::Member { + object, + property, + computed, + } => expr_uses_lexical_this(object) || (*computed && expr_uses_lexical_this(property)), + ExprKind::Array(elements) => elements.iter().flatten().any(|elem| match elem { + ArrayElement::Expr(expr) | ArrayElement::Spread(expr) => expr_uses_lexical_this(expr), + }), + ExprKind::Object(props) => props.iter().any(|prop| { + matches!(&prop.key, PropertyKey::Computed(expr) if expr_uses_lexical_this(expr)) + || prop.value.as_ref().is_some_and(expr_uses_lexical_this) + }), + ExprKind::TemplateLiteral { expressions, .. } => { + expressions.iter().any(expr_uses_lexical_this) + } + ExprKind::TaggedTemplate { tag, quasi } => { + expr_uses_lexical_this(tag) || expr_uses_lexical_this(quasi) + } + ExprKind::Sequence(exprs) => exprs.iter().any(expr_uses_lexical_this), + _ => false, + } +} + fn collect_pattern_names(pat: &Pattern, names: &mut HashSet) { match &pat.kind { PatternKind::Identifier(name) => { names.insert(name.clone()); } + PatternKind::Rest(inner) => collect_pattern_names(inner, names), PatternKind::Array { elements, rest } => { for elem in elements.iter().flatten() { collect_pattern_names(elem, names); @@ -340,6 +520,23 @@ fn collect_pattern_names(pat: &Pattern, names: &mut HashSet) { } } +fn rest_identifier_name(pat: &Pattern) -> Option<&str> { + if let PatternKind::Rest(inner) = &pat.kind { + if let PatternKind::Identifier(name) = &inner.kind { + return Some(name.as_str()); + } + } + None +} + +fn simple_param_name(pat: &Pattern) -> Option<&str> { + match &pat.kind { + PatternKind::Identifier(name) => Some(name.as_str()), + PatternKind::Rest(_) => rest_identifier_name(pat), + _ => None, + } +} + fn collect_pattern_names_many(patterns: &[Pattern], names: &mut HashSet) { for pattern in patterns { collect_pattern_names(pattern, names); @@ -1975,6 +2172,9 @@ fn compile_destructuring_pattern( // Don't free temps — rest pattern allocates locals. } } + PatternKind::Rest(inner) => { + compile_destructuring_pattern(fc, inner, src)?; + } PatternKind::Assign { left, right } => { // Default value: if src is undefined, use default. let val_reg = fc.alloc_reg(); @@ -2008,14 +2208,53 @@ fn compile_destructuring_pattern( /// patterns and chained defaults work exactly as in `var`/`let` destructuring. fn bind_complex_params(inner: &mut FunctionCompiler, params: &[Pattern]) -> Result<(), JsError> { for (i, p) in params.iter().enumerate() { - if !matches!(p.kind, PatternKind::Identifier(_)) { - let param_reg = i as Reg; - compile_destructuring_pattern(inner, p, param_reg)?; + let param_reg = i as Reg; + match &p.kind { + PatternKind::Identifier(_) => {} + PatternKind::Rest(rest_pat) => { + inner + .builder + .emit_build_rest_arguments(param_reg, i.min(u16::MAX as usize) as u16); + bind_rest_param(inner, rest_pat, param_reg)?; + } + _ => { + compile_destructuring_pattern(inner, p, param_reg)?; + } } } Ok(()) } +fn bind_rest_param( + inner: &mut FunctionCompiler, + rest_pat: &Pattern, + src: Reg, +) -> Result<(), JsError> { + if let PatternKind::Identifier(name) = &rest_pat.kind { + let existing = inner + .find_local_info(name) + .map(|local| (local.reg, local.is_captured)); + let is_captured = existing + .map(|(_, captured)| captured) + .unwrap_or_else(|| inner.captured_names.contains(name.as_str())); + let reg = existing + .map(|(reg, _)| reg) + .unwrap_or_else(|| inner.define_local_ext(name, is_captured, false)); + if is_captured { + let tmp = inner.alloc_reg(); + inner.builder.emit_reg_reg(Op::Move, tmp, src); + inner.builder.emit_reg(Op::NewCell, reg); + inner.builder.emit_reg_reg(Op::CellStore, reg, tmp); + inner.free_reg(tmp); + } else if reg != src { + inner.builder.emit_reg_reg(Op::Move, reg, src); + } + Ok(()) + } else { + compile_destructuring_pattern(inner, rest_pat, src) + } +} + /// Collect free-variable references that appear inside formal-parameter default /// expressions (and computed destructuring keys). `function f(a = outer)` must /// capture `outer` as an upvalue; `collect_pattern_names` only records the @@ -2027,6 +2266,7 @@ fn collect_pattern_default_refs( ) { match &pat.kind { PatternKind::Identifier(_) => {} + PatternKind::Rest(inner) => collect_pattern_default_refs(inner, declared, referenced), PatternKind::Assign { left, right } => { collect_expr_refs(right, declared, referenced); collect_pattern_default_refs(left, declared, referenced); @@ -2472,8 +2712,8 @@ fn compile_function_body_inner( // Allocate registers for parameters. for p in &func_def.params { - if let PatternKind::Identifier(pname) = &p.kind { - let is_captured = inner.captured_names.contains(pname.as_str()); + if let Some(pname) = simple_param_name(p) { + let is_captured = inner.captured_names.contains(pname); inner.define_local_ext(pname, is_captured, false); } else { let _ = inner.alloc_reg(); @@ -3485,9 +3725,13 @@ fn compile_expr(fc: &mut FunctionCompiler, expr: &Expr, dst: Reg) -> Result<(), } ExprKind::This => { - // `this` is loaded as a global named "this" (the VM binds it). - let ni = fc.builder.add_name("this"); - fc.builder.emit_load_global(dst, ni); + if let Some(uv_idx) = fc.find_upvalue("this") { + fc.builder.emit_load_upvalue(dst, uv_idx); + } else { + // `this` is loaded as a global named "this" (the VM binds it). + let ni = fc.builder.add_name("this"); + fc.builder.emit_load_global(dst, ni); + } } ExprKind::NewTarget => { @@ -4031,6 +4275,27 @@ fn compile_expr(fc: &mut FunctionCompiler, expr: &Expr, dst: Reg) -> Result<(), } } + if is_object_spread_property(prop) { + fc.builder.emit_copy_data_properties(dst, val_reg); + fc.free_reg(val_reg); + continue; + } + + if !matches!(prop.kind, PropertyKind::Init) { + if let Some(name) = static_property_name(&prop.key) { + let ni = fc.builder.add_name(&name); + let accessor_kind = match prop.kind { + PropertyKind::Get => 0b10, + PropertyKind::Set => 0b11, + PropertyKind::Init => unreachable!(), + }; + fc.builder + .emit_define_accessor(dst, ni, accessor_kind, val_reg); + fc.free_reg(val_reg); + continue; + } + } + match &prop.key { PropertyKey::Identifier(name) | PropertyKey::String(name) => { let ni = fc.builder.add_name(name); @@ -4072,8 +4337,38 @@ fn compile_expr(fc: &mut FunctionCompiler, expr: &Expr, dst: Reg) -> Result<(), // Resolve upvalues against the parent scope. let mut upvalue_entries = Vec::new(); + let mut lexical_this_cell = None; for name in &free_vars { - if let Some(local) = fc.find_local_info(name) { + if name == "this" { + if let Some(parent_uv_idx) = fc.find_upvalue(name) { + let is_const = fc.is_upvalue_const(parent_uv_idx); + upvalue_entries.push(UpvalueEntry { + name: name.clone(), + def: UpvalueDef { + is_local: false, + index: parent_uv_idx, + }, + is_const, + }); + } else { + let cell_reg = fc.alloc_reg(); + let value_reg = fc.alloc_reg(); + let this_ni = fc.builder.add_name("this"); + fc.builder.emit_reg(Op::NewCell, cell_reg); + fc.builder.emit_load_global(value_reg, this_ni); + fc.builder.emit_reg_reg(Op::CellStore, cell_reg, value_reg); + fc.free_reg(value_reg); + lexical_this_cell = Some(cell_reg); + upvalue_entries.push(UpvalueEntry { + name: name.clone(), + def: UpvalueDef { + is_local: true, + index: cell_reg, + }, + is_const: true, + }); + } + } else if let Some(local) = fc.find_local_info(name) { if fc.is_top_level && local.is_top_level_var { continue; } @@ -4131,8 +4426,8 @@ fn compile_expr(fc: &mut FunctionCompiler, expr: &Expr, dst: Reg) -> Result<(), } for p in params { - if let PatternKind::Identifier(pname) = &p.kind { - let is_captured = inner.captured_names.contains(pname.as_str()); + if let Some(pname) = simple_param_name(p) { + let is_captured = inner.captured_names.contains(pname); inner.define_local_ext(pname, is_captured, false); } else { let _ = inner.alloc_reg(); @@ -4187,6 +4482,9 @@ fn compile_expr(fc: &mut FunctionCompiler, expr: &Expr, dst: Reg) -> Result<(), inner_func.is_async = *is_async; let func_idx = fc.builder.add_function(inner_func); fc.builder.emit_reg_u16(Op::CreateClosure, dst, func_idx); + if let Some(reg) = lexical_this_cell { + fc.free_reg(reg); + } } ExprKind::Class(class_def) => { @@ -4392,6 +4690,24 @@ fn compile_expr(fc: &mut FunctionCompiler, expr: &Expr, dst: Reg) -> Result<(), Ok(()) } +fn is_object_spread_property(prop: &Property) -> bool { + matches!(&prop.key, PropertyKey::Identifier(name) if name.is_empty()) + && prop.value.is_some() + && matches!(prop.kind, PropertyKind::Init) + && !prop.computed + && !prop.shorthand + && !prop.method +} + +fn static_property_name(key: &PropertyKey) -> Option { + match key { + PropertyKey::Identifier(name) | PropertyKey::String(name) => Some(name.clone()), + PropertyKey::Number(n) if n.fract() == 0.0 => Some(format!("{}", *n as i64)), + PropertyKey::Number(n) => Some(n.to_string()), + PropertyKey::Computed(_) | PropertyKey::PrivateName(_) => None, + } +} + /// Compile a store operation (assignment target). fn compile_store(fc: &mut FunctionCompiler, target: &Expr, src: Reg) -> Result<(), JsError> { match &target.kind { diff --git a/crates/js/src/jit/compiler.rs b/crates/js/src/jit/compiler.rs index a8dbeec..f8bcce2 100644 --- a/crates/js/src/jit/compiler.rs +++ b/crates/js/src/jit/compiler.rs @@ -646,6 +646,7 @@ impl BaselineJit { | Op::GetPrivate | Op::SetPrivate | Op::HasPrivate + | Op::CopyDataProperties | Op::SetHomeObject | Op::LoadSuperBase | Op::ForInInit @@ -655,7 +656,8 @@ impl BaselineJit { | Op::Yield | Op::Spread | Op::Await - | Op::BuildArguments => { + | Op::BuildArguments + | Op::BuildRestArguments => { self.emit_bail(bail_label); ip += instruction_operand_size(op); } @@ -878,6 +880,10 @@ fn instruction_operand_size(op: Op) -> usize { // GetPrivate/HasPrivate: dst(2) + obj(2) + name_idx(2) // SetPrivate: obj(2) + name_idx(2) + val(2) Op::GetPrivate | Op::SetPrivate | Op::HasPrivate => 6, + // CopyDataProperties: target(2) + source(2) + Op::CopyDataProperties => 4, + // BuildRestArguments: dst(2) + start_idx(2) + Op::BuildRestArguments => 4, } } diff --git a/crates/js/src/parser.rs b/crates/js/src/parser.rs index 8850997..0b70df3 100644 --- a/crates/js/src/parser.rs +++ b/crates/js/src/parser.rs @@ -995,8 +995,13 @@ impl Parser { let mut params = Vec::new(); while !self.at(&TokenKind::RParen) && !self.at_eof() { if self.at(&TokenKind::Ellipsis) { + let start = self.start_span(); self.advance(); - params.push(self.parse_binding_pattern()?); + let inner = self.parse_binding_pattern()?; + params.push(Pattern { + kind: PatternKind::Rest(Box::new(inner)), + span: self.span_from(start), + }); self.eat(&TokenKind::Comma); break; } @@ -2784,13 +2789,24 @@ impl Parser { ExprKind::Sequence(exprs) => { let mut params = Vec::new(); for e in exprs { - params.push(self.expr_to_pattern(e)?); + if let ExprKind::Spread(inner) = &e.kind { + let pat = self.expr_to_pattern(inner)?; + params.push(Pattern { + kind: PatternKind::Rest(Box::new(pat)), + span: e.span, + }); + } else { + params.push(self.expr_to_pattern(e)?); + } } Ok(params) } ExprKind::Spread(inner) => { let pat = self.expr_to_pattern(inner)?; - Ok(vec![pat]) + Ok(vec![Pattern { + kind: PatternKind::Rest(Box::new(pat)), + span: expr.span, + }]) } _ => { // Single expression as single parameter @@ -3429,8 +3445,9 @@ mod tests { ExprKind::Arrow { params, .. } => { assert_eq!(params.len(), 2); assert!(matches!( - params[1].kind, - PatternKind::Identifier(ref n) if n == "rest" + ¶ms[1].kind, + PatternKind::Rest(inner) + if matches!(&inner.kind, PatternKind::Identifier(n) if n == "rest") )); } _ => panic!("expected arrow"), @@ -3633,8 +3650,9 @@ mod tests { StmtKind::FunctionDecl(def) => { assert_eq!(def.params.len(), 1); assert!(matches!( - def.params[0].kind, - PatternKind::Identifier(ref n) if n == "args" + &def.params[0].kind, + PatternKind::Rest(inner) + if matches!(&inner.kind, PatternKind::Identifier(n) if n == "args") )); } _ => panic!("expected function decl"), diff --git a/crates/js/src/vm.rs b/crates/js/src/vm.rs index 1f4face..bbb728c 100644 --- a/crates/js/src/vm.rs +++ b/crates/js/src/vm.rs @@ -1608,6 +1608,58 @@ impl Vm { Ok(Value::Undefined) } + fn copy_data_properties( + &mut self, + target_ref: GcRef, + source: Value, + ) -> Result<(), RuntimeError> { + let entries: Vec<(String, Property)> = match &source { + Value::Undefined | Value::Null => return Ok(()), + Value::Object(source_ref) => match self.gc.get(*source_ref) { + Some(HeapObject::Object(data)) => data + .property_entries(&self.shapes) + .into_iter() + .filter(|(_, prop)| prop.enumerable) + .collect(), + _ => Vec::new(), + }, + Value::Function(source_ref) => match self.gc.get(*source_ref) { + Some(HeapObject::Function(data)) => data + .properties + .iter() + .filter(|(_, prop)| prop.enumerable) + .map(|(key, prop)| (key.clone(), prop.clone())) + .collect(), + _ => Vec::new(), + }, + Value::String(ref s) => s + .chars() + .enumerate() + .map(|(idx, ch)| { + ( + idx.to_string(), + Property::data(Value::String(ch.to_string())), + ) + }) + .collect(), + _ => Vec::new(), + }; + + for (key, prop) in entries { + let value = match &source { + Value::Object(source_ref) => self.get_object_property_value(*source_ref, &key)?, + Value::Function(source_ref) => { + self.property_value(prop, Value::Function(*source_ref))? + } + _ => prop.value, + }; + if let Some(HeapObject::Object(target)) = self.gc.get_mut(target_ref) { + target.insert_property(key, Property::data(value), &mut self.shapes); + } + } + Ok(()) + } + /// Walk the prototype chain looking for a property that would intercept an /// assignment of `key`. Returns an accessor setter to invoke, a marker that /// an accessor without a setter shadows the write, or `DataOrNone` when an @@ -2811,6 +2863,31 @@ impl Vm { self.gc.alloc(HeapObject::Object(obj)) } + fn build_array_from_values(&mut self, items: &[Value]) -> GcRef { + let mut obj = ObjectData::new(); + obj.prototype = self.array_prototype; + for (i, item) in items.iter().enumerate() { + obj.insert_property( + i.to_string(), + Property::data(item.clone()), + &mut self.shapes, + ); + } + obj.insert_property( + "length".to_string(), + Property { + value: Value::Number(items.len() as f64), + getter: None, + setter: None, + writable: true, + enumerable: false, + configurable: false, + }, + &mut self.shapes, + ); + self.gc.alloc(HeapObject::Object(obj)) + } + /// Create a {value, done} iterator result object. fn make_iterator_result(&mut self, value: Value, done: bool) -> Value { let mut obj = ObjectData::new(); @@ -5269,15 +5346,16 @@ impl Vm { } _ => (None, None), }; - if kind == 0 { + if kind & 1 == 0 { getter = accessor_fn; } else { setter = accessor_fn; } + let enumerable = kind & 0b10 != 0; if let Some(HeapObject::Object(data)) = self.gc.get_mut(obj_ref) { data.insert_property( name, - Property::accessor(getter, setter, false, true), + Property::accessor(getter, setter, enumerable, true), &mut self.shapes, ); } @@ -5292,15 +5370,17 @@ impl Vm { } _ => (None, None), }; - if kind == 0 { + if kind & 1 == 0 { getter = accessor_fn; } else { setter = accessor_fn; } + let enumerable = kind & 0b10 != 0; if let Some(HeapObject::Function(fdata)) = self.gc.get_mut(fn_ref) { - fdata - .properties - .insert(name, Property::accessor(getter, setter, false, true)); + fdata.properties.insert( + name, + Property::accessor(getter, setter, enumerable, true), + ); } } _ => {} @@ -5522,6 +5602,22 @@ impl Vm { let has = self.has_private_field(&receiver, &name); self.registers[base + dst as usize] = Value::Boolean(has); } + Op::CopyDataProperties => { + let target_r = Self::read_reg(&mut self.frames[fi]); + let source_r = Self::read_reg(&mut self.frames[fi]); + let base = self.frames[fi].base; + let source = self.registers[base + source_r as usize].clone(); + let target_ref = match self.registers[base + target_r as usize] { + Value::Object(gc_ref) => gc_ref, + _ => { + self.throw_runtime_error(RuntimeError::type_error( + "object spread target must be an object", + ))?; + continue; + } + }; + self.copy_data_properties(target_ref, source)?; + } // ── Exception handling ───────────────────────────── Op::PushExceptionHandler => { @@ -5716,6 +5812,19 @@ impl Vm { let obj_ref = self.build_arguments_object(&args); self.registers[base + dst as usize] = Value::Object(obj_ref); } + Op::BuildRestArguments => { + let dst = Self::read_reg(&mut self.frames[fi]); + let start = Self::read_u16(&mut self.frames[fi]) as usize; + let base = self.frames[fi].base; + let args = self.frames[fi].args.clone(); + let rest = if start < args.len() { + &args[start..] + } else { + &[] + }; + let arr_ref = self.build_array_from_values(rest); + self.registers[base + dst as usize] = Value::Object(arr_ref); + } Op::Spread => { let dst = Self::read_reg(&mut self.frames[fi]); let src = Self::read_reg(&mut self.frames[fi]); @@ -7401,6 +7510,50 @@ mod tests { } } + #[test] + fn rest_parameter_collects_trailing_arguments() { + let src = r#" + function f(first, ...rest) { + return rest.length + ":" + rest[0] + ":" + rest[1] + ":" + typeof rest.push; + } + f(9, 1, 2) + "#; + match eval(src).unwrap() { + Value::String(s) => assert_eq!(s, "2:1:2:function"), + v => panic!("expected rest arguments array, got {v:?}"), + } + } + + #[test] + fn arrow_rest_parameter_collects_arguments() { + match eval("var f = (...args) => args.length + ':' + args[1]; f(1, 2, 3)").unwrap() { + Value::String(s) => assert_eq!(s, "3:2"), + v => panic!("expected arrow rest arguments, got {v:?}"), + } + } + + #[test] + fn captured_rest_parameter_is_boxed_after_array_build() { + let src = r#" + function f(...args) { + return function() { return args.length + args[1]; }; + } + f(1, 2)() + "#; + match eval(src).unwrap() { + Value::Number(n) => assert_eq!(n, 4.0), + v => panic!("expected captured rest array, got {v:?}"), + } + } + + #[test] + fn destructured_rest_parameter_binds_from_rest_array() { + match eval("function f(...[first, second]) { return first * second; } f(3, 4)").unwrap() { + Value::Number(n) => assert_eq!(n, 12.0), + v => panic!("expected destructured rest parameter, got {v:?}"), + } + } + #[test] fn test_function_call() { let mut inner_b = BytecodeBuilder::new("add1".into(), 1); @@ -7914,6 +8067,110 @@ mod tests { } } + #[test] + fn object_literal_spread_copies_own_enumerable_properties() { + let src = r#" + var source = { x: 1 }; + Object.defineProperty(source, "hidden", { value: 9, enumerable: false }); + var out = { ...source, y: 2 }; + out.x + out.y + (typeof out.hidden === "undefined" ? 0 : 100) + "#; + match eval(src).unwrap() { + Value::Number(n) => assert_eq!(n, 3.0), + v => panic!("expected copied properties, got {v:?}"), + } + } + + #[test] + fn object_literal_spread_obeys_overwrite_order() { + match eval("var out = { x: 1, ...{ x: 2 }, y: 3, ...{ y: 4 } }; out.x + out.y").unwrap() { + Value::Number(n) => assert_eq!(n, 6.0), + v => panic!("expected spread overwrite order, got {v:?}"), + } + } + + #[test] + fn object_literal_spread_skips_nullish_sources() { + match eval("var out = { a: 1, ...null, ...undefined, b: 2 }; out.a + out.b").unwrap() { + Value::Number(n) => assert_eq!(n, 3.0), + v => panic!("expected nullish spread to be skipped, got {v:?}"), + } + } + + #[test] + fn object_literal_spread_reads_getters() { + let src = r#" + var count = 0; + var source = {}; + Object.defineProperty(source, "x", { + enumerable: true, + get: function() { count = count + 1; return 7; } + }); + var out = { ...source }; + out.x + count + "#; + match eval(src).unwrap() { + Value::Number(n) => assert_eq!(n, 8.0), + v => panic!("expected getter-backed spread, got {v:?}"), + } + } + + #[test] + fn object_literal_spread_overrides_existing_accessor_with_data_property() { + let src = r#" + var seen = 0; + var out = { set x(v) { seen = v; }, ...{ x: 4 } }; + out.x + ":" + seen + "#; + match eval(src).unwrap() { + Value::String(s) => assert_eq!(s, "4:0"), + v => panic!("expected spread data property to replace accessor, got {v:?}"), + } + } + + #[test] + fn object_literal_spread_copies_string_indices() { + match eval("var out = { ...'ab' }; out[0] + out[1]").unwrap() { + Value::String(s) => assert_eq!(s, "ab"), + v => panic!("expected string indices, got {v:?}"), + } + } + + #[test] + fn object_literal_getter_returns_accessor_value() { + match eval("var o = { get x() { return 4; } }; o.x + (typeof o.x === 'number' ? 1 : 0)") + .unwrap() + { + Value::Number(n) => assert_eq!(n, 5.0), + v => panic!("expected object-literal getter value, got {v:?}"), + } + } + + #[test] + fn object_literal_getter_setter_share_property() { + let src = r#" + var seen = 0; + var o = { + get x() { return seen; }, + set x(v) { seen = v; } + }; + o.x = 9; + o.x + "#; + match eval(src).unwrap() { + Value::Number(n) => assert_eq!(n, 9.0), + v => panic!("expected getter/setter pair, got {v:?}"), + } + } + + #[test] + fn object_literal_accessors_are_enumerable() { + match eval("var o = { get x() { return 1; } }; Object.keys(o)[0]").unwrap() { + Value::String(s) => assert_eq!(s, "x"), + v => panic!("expected enumerable object-literal accessor, got {v:?}"), + } + } + #[test] fn test_e2e_for_loop() { match eval("var s = 0; for (var i = 0; i < 5; i = i + 1) { s = s + i; } s").unwrap() { @@ -9412,6 +9669,61 @@ mod tests { } } + #[test] + fn arrow_inside_method_captures_lexical_this() { + let src = r#" + var obj = { + x: 2, + f: function() { + var g = () => this.x; + return g(); + } + }; + obj.f() + "#; + match eval(src).unwrap() { + Value::Number(n) => assert_eq!(n, 2.0), + v => panic!("expected lexical this value, got {v:?}"), + } + } + + #[test] + fn returned_arrow_keeps_method_this_after_call_returns() { + let src = r#" + var obj = { + x: 7, + make: function() { return () => this.x; } + }; + var f = obj.make(); + var other = { x: 1, f: f }; + other.f() + "#; + match eval(src).unwrap() { + Value::Number(n) => assert_eq!(n, 7.0), + v => panic!("expected returned arrow to keep lexical this, got {v:?}"), + } + } + + #[test] + fn arrow_inside_nested_normal_function_uses_inner_this() { + let src = r#" + var outer = { + x: 1, + make: function() { + return function() { + return (() => this.x)(); + }; + } + }; + var inner = outer.make(); + inner.call({ x: 5 }) + "#; + match eval(src).unwrap() { + Value::Number(n) => assert_eq!(n, 5.0), + v => panic!("expected nested normal function this, got {v:?}"), + } + } + // ── Object built-in tests ──────────────────────────────── #[test]