diff --git a/crates/js/src/bytecode.rs b/crates/js/src/bytecode.rs index 5344e9f..5bb6ba8 100644 --- a/crates/js/src/bytecode.rs +++ b/crates/js/src/bytecode.rs @@ -381,8 +381,12 @@ impl BytecodeBuilder { /// Patch a jump offset at the given position to point to the current offset. pub fn patch_jump(&mut self, pos: usize) { - let target = self.offset() as i32; - let offset = target - (pos as i32 + 4); // relative to after the i32 + self.patch_jump_to(pos, self.offset()); + } + + /// Patch a jump offset at the given position to point to an arbitrary target. + pub fn patch_jump_to(&mut self, pos: usize, target: usize) { + let offset = target as i32 - (pos as i32 + 4); // relative to after the i32 let bytes = offset.to_le_bytes(); self.func.code[pos] = bytes[0]; self.func.code[pos + 1] = bytes[1]; diff --git a/crates/js/src/compiler.rs b/crates/js/src/compiler.rs index dd9d200..5243971 100644 --- a/crates/js/src/compiler.rs +++ b/crates/js/src/compiler.rs @@ -28,10 +28,10 @@ struct Local { struct LoopCtx { /// Label, if this is a labeled loop. label: Option, - /// Bytecode offset of the loop condition check (for `continue`). - continue_target: usize, /// Patch positions for break jumps. break_patches: Vec, + /// Patch positions for continue jumps (patched after body compilation). + continue_patches: Vec, } impl FunctionCompiler { @@ -223,8 +223,8 @@ fn compile_stmt(fc: &mut FunctionCompiler, stmt: &Stmt, result_reg: Reg) -> Resu StmtKind::Continue(label) => { let idx = find_loop_ctx(&fc.loop_stack, label.as_deref()) .ok_or_else(|| JsError::SyntaxError("continue outside of loop".into()))?; - let target = fc.loop_stack[idx].continue_target; - fc.builder.emit_jump_to(target); + let patch = fc.builder.emit_jump(Op::Jump); + fc.loop_stack[idx].continue_patches.push(patch); } StmtKind::Labeled { label, body } => { @@ -470,9 +470,13 @@ fn compile_class_decl(fc: &mut FunctionCompiler, class_def: &ClassDef) -> Result fc.builder.emit_reg_u16(Op::CreateClosure, reg, func_idx); } } else { - // No constructor: create an empty function. - let empty = Function::new(name.clone(), 0); - let func_idx = fc.builder.add_function(empty); + // No constructor: create a minimal function that returns undefined. + let mut empty = BytecodeBuilder::new(name.clone(), 0); + let r = 0u8; + empty.func.register_count = 1; + empty.emit_reg(Op::LoadUndefined, r); + empty.emit_reg(Op::Return, r); + let func_idx = fc.builder.add_function(empty.finish()); fc.builder.emit_reg_u16(Op::CreateClosure, reg, func_idx); } @@ -579,8 +583,8 @@ fn compile_while( fc.loop_stack.push(LoopCtx { label, - continue_target: loop_start, break_patches: Vec::new(), + continue_patches: Vec::new(), }); compile_stmt(fc, body, result_reg)?; @@ -591,6 +595,10 @@ fn compile_while( for patch in ctx.break_patches { fc.builder.patch_jump(patch); } + // In a while loop, continue jumps back to the condition check (loop_start). + for patch in ctx.continue_patches { + fc.builder.patch_jump_to(patch, loop_start); + } Ok(()) } @@ -606,12 +614,15 @@ fn compile_do_while( fc.loop_stack.push(LoopCtx { label, - continue_target: loop_start, break_patches: Vec::new(), + continue_patches: Vec::new(), }); compile_stmt(fc, body, result_reg)?; + // continue in do-while should jump here (the condition check). + let cond_start = fc.builder.offset(); + let cond = fc.alloc_reg(); compile_expr(fc, test, cond)?; fc.builder @@ -622,6 +633,9 @@ fn compile_do_while( for patch in ctx.break_patches { fc.builder.patch_jump(patch); } + for patch in ctx.continue_patches { + fc.builder.patch_jump_to(patch, cond_start); + } Ok(()) } @@ -670,21 +684,16 @@ fn compile_for( None }; - // continue_target points to the update expression (or loop_start if no update). - // We'll set this after compiling the body. - let continue_placeholder = fc.builder.offset(); fc.loop_stack.push(LoopCtx { label, - continue_target: continue_placeholder, // will be updated break_patches: Vec::new(), + continue_patches: Vec::new(), }); compile_stmt(fc, body, result_reg)?; - // Set the continue target to the update expression position. + // continue in a for-loop should jump here (the update expression). let continue_target = fc.builder.offset(); - let loop_idx = fc.loop_stack.len() - 1; - fc.loop_stack[loop_idx].continue_target = continue_target; // Update. if let Some(update) = update { @@ -703,6 +712,9 @@ fn compile_for( for patch in ctx.break_patches { fc.builder.patch_jump(patch); } + for patch in ctx.continue_patches { + fc.builder.patch_jump_to(patch, continue_target); + } fc.locals.truncate(saved_locals); fc.next_reg = saved_next; @@ -722,15 +734,16 @@ fn compile_switch( // Use a loop context for break statements. fc.loop_stack.push(LoopCtx { label: None, - continue_target: 0, // not applicable for switch break_patches: Vec::new(), + continue_patches: Vec::new(), }); - let mut case_patches = Vec::new(); - let mut default_patch = None; + // Phase 1: emit comparison jumps for each non-default case. + // Store (case_index, patch_position) for each case with a test. + let mut case_jump_patches: Vec<(usize, usize)> = Vec::new(); + let mut default_index: Option = None; - // Phase 1: emit comparison jumps for each case. - for case in cases { + for (i, case) in cases.iter().enumerate() { if let Some(test) = &case.test { let test_reg = fc.alloc_reg(); compile_expr(fc, test, test_reg)?; @@ -740,42 +753,44 @@ fn compile_switch( let patch = fc.builder.emit_cond_jump(Op::JumpIfTrue, cmp_reg); fc.free_reg(cmp_reg); fc.free_reg(test_reg); - case_patches.push(patch); + case_jump_patches.push((i, patch)); } else { - // Default case: jump is emitted after all test comparisons. - case_patches.push(0); // placeholder - default_patch = Some(case_patches.len() - 1); + default_index = Some(i); } } - // Jump to default or end if no case matched. - let end_jump = if default_patch.is_some() { - None - } else { - Some(fc.builder.emit_jump(Op::Jump)) - }; + // After all comparisons: jump to default body or end. + let fallthrough_patch = fc.builder.emit_jump(Op::Jump); - // Phase 2: emit case bodies. + // Phase 2: emit case bodies in order (fall-through semantics). + let mut body_offsets: Vec<(usize, usize)> = Vec::new(); for (i, case) in cases.iter().enumerate() { - if default_patch == Some(i) { - // Patch the "no match" jump to here if this is the default. - if let Some(end_j) = end_jump { - fc.builder.patch_jump(end_j); - } - // We need to update the default case patch to point here. - let default_jump_target = fc.builder.offset(); - // The default_patch entry is a placeholder; we don't jump TO it. - // Instead, if no case matched and there's a default, we jump here. - // Let's fix: emit a Jump before the first case body that jumps to default. - let _ = default_jump_target; - } - - fc.builder.patch_jump(case_patches[i]); + body_offsets.push((i, fc.builder.offset())); compile_stmts(fc, &case.consequent, result_reg)?; } - if let Some(end_j) = end_jump { - fc.builder.patch_jump(end_j); + let end_offset = fc.builder.offset(); + + // Patch case test jumps to their respective body offsets. + for (case_idx, patch) in &case_jump_patches { + let body_offset = body_offsets + .iter() + .find(|(i, _)| i == case_idx) + .map(|(_, off)| *off) + .unwrap(); + fc.builder.patch_jump_to(*patch, body_offset); + } + + // Patch fallthrough: jump to default body if present, otherwise to end. + if let Some(def_idx) = default_index { + let default_offset = body_offsets + .iter() + .find(|(i, _)| *i == def_idx) + .map(|(_, off)| *off) + .unwrap(); + fc.builder.patch_jump_to(fallthrough_patch, default_offset); + } else { + fc.builder.patch_jump_to(fallthrough_patch, end_offset); } fc.free_reg(disc_reg); @@ -1195,10 +1210,44 @@ fn compile_expr(fc: &mut FunctionCompiler, expr: &Expr, dst: Reg) -> Result<(), fc.builder.emit_reg_u16(Op::CreateClosure, dst, func_idx); } } else { - let empty = Function::new(name, 0); - let func_idx = fc.builder.add_function(empty); + let mut empty = BytecodeBuilder::new(name, 0); + let r = 0u8; + empty.func.register_count = 1; + empty.emit_reg(Op::LoadUndefined, r); + empty.emit_reg(Op::Return, r); + let func_idx = fc.builder.add_function(empty.finish()); fc.builder.emit_reg_u16(Op::CreateClosure, dst, func_idx); } + + // Compile methods as properties on the constructor. + for member in &class_def.body { + match &member.kind { + ClassMemberKind::Method { + key, + value, + kind, + is_static: _, + computed: _, + } => { + if matches!(kind, MethodKind::Constructor) { + continue; + } + let method_name = match key { + PropertyKey::Identifier(s) | PropertyKey::String(s) => s.clone(), + _ => continue, + }; + let inner = compile_function_body(value)?; + let func_idx = fc.builder.add_function(inner); + let method_reg = fc.alloc_reg(); + fc.builder + .emit_reg_u16(Op::CreateClosure, method_reg, func_idx); + let name_idx = fc.builder.add_name(&method_name); + fc.builder.emit_set_prop_name(dst, name_idx, method_reg); + fc.free_reg(method_reg); + } + ClassMemberKind::Property { .. } => {} + } + } } ExprKind::Sequence(exprs) => { @@ -1747,4 +1796,78 @@ mod tests { let dis = f.disassemble(); assert!(dis.contains("Jump"), "got:\n{dis}"); } + + #[test] + fn test_for_continue_targets_update() { + // `continue` in a for-loop must jump to the update expression, not back + // to the condition check. Verify the continue jump goes to the Add (i + 1) + // rather than to the LessThan condition. + let f = compile_src("for (var i = 0; i < 10; i = i + 1) { continue; }"); + let dis = f.disassemble(); + // The for-loop should contain: LessThan (test), JumpIfFalse (exit), + // Jump (continue), Add (update), Jump (back to test). + assert!(dis.contains("LessThan"), "missing test: {dis}"); + assert!(dis.contains("Add"), "missing update: {dis}"); + // There should be at least 2 Jump instructions (continue + back-edge). + let jump_count = dis.matches("Jump ").count(); + assert!( + jump_count >= 2, + "expected >= 2 jumps for continue + back-edge, got {jump_count}: {dis}" + ); + } + + #[test] + fn test_do_while_continue_targets_condition() { + // `continue` in do-while must jump to the condition, not the body start. + let f = compile_src("var i = 0; do { i = i + 1; continue; } while (i < 5);"); + let dis = f.disassemble(); + assert!(dis.contains("LessThan"), "missing condition: {dis}"); + assert!(dis.contains("JumpIfTrue"), "missing back-edge: {dis}"); + } + + #[test] + fn test_switch_default_case() { + // Default case must not corrupt bytecode. + let f = compile_src("switch (1) { case 1: 10; break; default: 20; break; }"); + let dis = f.disassemble(); + assert!(dis.contains("StrictEq"), "missing case test: {dis}"); + // The first instruction should NOT be corrupted. + assert!( + dis.contains("LoadUndefined r0"), + "first instruction corrupted: {dis}" + ); + } + + #[test] + fn test_switch_only_default() { + // Switch with only a default case. + let f = compile_src("switch (42) { default: 99; }"); + let dis = f.disassemble(); + // Should compile without panicking and contain the default body. + assert!(dis.contains("LoadInt8"), "got:\n{dis}"); + } + + #[test] + fn test_class_empty_constructor_has_return() { + // A class without an explicit constructor should produce a function with Return. + let f = compile_src("class Foo {}"); + assert!(!f.functions.is_empty(), "should have constructor function"); + let ctor = &f.functions[0]; + let dis = ctor.disassemble(); + assert!( + dis.contains("Return"), + "empty constructor must have Return: {dis}" + ); + } + + #[test] + fn test_class_expression_compiles_methods() { + // Class expression should compile methods, not just the constructor. + let f = compile_src("var C = class { greet() { return 1; } };"); + let dis = f.disassemble(); + assert!( + dis.contains("SetPropertyByName"), + "method should be set as property: {dis}" + ); + } }