From 1efe9d5f6b289a2c370a148138d5adb9600da13c Mon Sep 17 00:00:00 2001 From: James Tippett Date: Thu, 9 Jul 2026 15:50:59 +0700 Subject: [PATCH] fix: give switch a break frame so bare break cannot jump to program start A bare break inside switch emitted Jump(0) with no patch target (switch pushed no loop frame), an infinite jump back to program start that ran until the allocation limit tripped. switch now registers a break-only frame that continue skips, and break/continue outside any loop or switch is a compile error instead of miscompiled code. --- crates/zapcode-core/src/compiler/mod.rs | 42 ++++++++++++++++++++--- crates/zapcode-core/tests/control_flow.rs | 36 +++++++++++++++++++ 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/crates/zapcode-core/src/compiler/mod.rs b/crates/zapcode-core/src/compiler/mod.rs index 9d2284e..59b80e7 100644 --- a/crates/zapcode-core/src/compiler/mod.rs +++ b/crates/zapcode-core/src/compiler/mod.rs @@ -37,6 +37,9 @@ struct Compiler { struct LoopInfo { break_patches: Vec, continue_patches: Vec, + // A `switch` participates in `break` targeting (break exits the switch) but + // NOT `continue` — a `continue` inside a switch targets the enclosing loop. + is_switch: bool, } impl Compiler { @@ -222,6 +225,7 @@ impl Compiler { self.loop_stack.push(LoopInfo { break_patches: Vec::new(), continue_patches: Vec::new(), + is_switch: false, }); self.compile_expr(test)?; @@ -248,6 +252,7 @@ impl Compiler { self.loop_stack.push(LoopInfo { break_patches: Vec::new(), continue_patches: Vec::new(), + is_switch: false, }); for s in body { @@ -282,6 +287,7 @@ impl Compiler { self.loop_stack.push(LoopInfo { break_patches: Vec::new(), continue_patches: Vec::new(), + is_switch: false, }); let exit_jump = if let Some(test) = test { @@ -329,6 +335,7 @@ impl Compiler { self.loop_stack.push(LoopInfo { break_patches: Vec::new(), continue_patches: Vec::new(), + is_switch: false, }); self.emit(Instruction::Dup); @@ -413,15 +420,28 @@ impl Compiler { } } Statement::Break { .. } => { + // `break` exits the nearest loop OR switch. let idx = self.emit(Instruction::Jump(0)); - if let Some(loop_info) = self.loop_stack.last_mut() { - loop_info.break_patches.push(idx); + match self.loop_stack.last_mut() { + Some(loop_info) => loop_info.break_patches.push(idx), + None => { + return Err(ZapcodeError::CompileError( + "illegal break statement (not inside a loop or switch)".to_string(), + )) + } } } Statement::Continue { .. } => { + // `continue` targets the nearest enclosing *loop*, skipping any + // switch frames in between (a switch is break-only). let idx = self.emit(Instruction::Jump(0)); - if let Some(loop_info) = self.loop_stack.last_mut() { - loop_info.continue_patches.push(idx); + match self.loop_stack.iter_mut().rev().find(|l| !l.is_switch) { + Some(loop_info) => loop_info.continue_patches.push(idx), + None => { + return Err(ZapcodeError::CompileError( + "illegal continue statement (not inside a loop)".to_string(), + )) + } } } Statement::FunctionDecl { func_index, .. } => { @@ -487,6 +507,14 @@ impl Compiler { let jump_end = self.emit(Instruction::Jump(0)); + // A `break` inside a case must exit the switch. Register a + // switch frame so `break` jumps here (and `continue` skips it). + self.loop_stack.push(LoopInfo { + break_patches: Vec::new(), + continue_patches: Vec::new(), + is_switch: true, + }); + // Compile case bodies let mut body_starts = Vec::new(); for case in cases { @@ -499,6 +527,12 @@ impl Compiler { let end = self.current_offset(); self.emit(Instruction::Pop); // pop discriminant + // `break` targets the Pop, so it also cleans up the discriminant. + let switch_frame = self.loop_stack.pop().expect("switch frame present"); + for patch in switch_frame.break_patches { + self.patch_jump(patch, end); + } + // Patch jumps for (i, &jump) in case_jumps.iter().enumerate() { if jump != 0 { diff --git a/crates/zapcode-core/tests/control_flow.rs b/crates/zapcode-core/tests/control_flow.rs index 3de06d8..859250a 100644 --- a/crates/zapcode-core/tests/control_flow.rs +++ b/crates/zapcode-core/tests/control_flow.rs @@ -75,3 +75,39 @@ fn test_nullish_coalescing_defined() { let result = eval_ts("0 ?? 42").unwrap(); assert_eq!(result, Value::Int(0)); } + +// ── switch (regression: bare `break` looped forever → allocation blowup) ───── + +#[test] +fn test_switch_basic_match_and_break() { + let r = eval_ts( + "let r = 'none'; switch (2) { case 1: r = 'one'; break; case 2: r = 'two'; break; } r", + ) + .unwrap(); + assert_eq!(r, Value::String("two".into())); +} + +#[test] +fn test_switch_default() { + let r = eval_ts("let r = 'x'; switch (9) { case 1: r = 'one'; break; default: r = 'def'; } r") + .unwrap(); + assert_eq!(r, Value::String("def".into())); +} + +#[test] +fn test_switch_fallthrough() { + let r = eval_ts( + "let r = 0; switch (1) { case 1: r += 1; case 2: r += 10; break; case 3: r += 100; } r", + ) + .unwrap(); + assert_eq!(r, Value::Int(11)); +} + +#[test] +fn test_switch_break_inside_loop_breaks_switch_only() { + // break exits the switch (not the loop): n=1 adds 1, n=2 hits default (+100); + // the loop still runs both iterations and the trailing `t += 1000` each time. + let src = "let t = 0; for (const n of [1, 2]) { switch (n) { case 1: t += 1; break; default: t += 100; } t += 1000; } t"; + let r = eval_ts(src).unwrap(); + assert_eq!(r, Value::Int(2101)); +}