diff --git a/ndc_vm/src/chunk.rs b/ndc_vm/src/chunk.rs index bad6e33..ad56283 100644 --- a/ndc_vm/src/chunk.rs +++ b/ndc_vm/src/chunk.rs @@ -3,6 +3,41 @@ use ndc_lexer::Span; use ndc_parser::CaptureSource; use std::rc::Rc; +/// A signed displacement applied to the instruction pointer to perform a jump. +/// +/// Wraps `isize` so the contract — "relative offset, in instructions, from the +/// instruction *after* the jump opcode" — has a single definition site. Future +/// stages of a compile pipeline may swap this for an `enum JumpTarget { … }` +/// without touching every call site. +#[derive(Copy, Clone, PartialEq, Eq)] +pub struct JumpOffset(isize); + +impl JumpOffset { + pub const ZERO: Self = Self(0); + + #[inline] + pub fn new(offset: isize) -> Self { + Self(offset) + } + + #[inline] + pub fn raw(self) -> isize { + self.0 + } + + /// Advance `ip` by this offset using wrapping signed arithmetic. + #[inline] + pub fn apply(self, ip: usize) -> usize { + ip.wrapping_add_signed(self.0) + } +} + +impl std::fmt::Debug for JumpOffset { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + std::fmt::Debug::fmt(&self.0, f) + } +} + /// A single bytecode instruction. /// /// ## Stack effects @@ -57,11 +92,11 @@ pub enum OpCode { /// Pops top of stack. `[… value → …]` Pop, /// Unconditional jump. `[…] → […]` - Jump(isize), + Jump(JumpOffset), /// Peeks top; jumps if true. `[… bool → … bool]` - JumpIfTrue(isize), + JumpIfTrue(JumpOffset), /// Peeks top; jumps if false. `[… bool → … bool]` - JumpIfFalse(isize), + JumpIfFalse(JumpOffset), /// Pushes a constant. `[… → … value]` Constant(usize), /// Copies local slot onto stack. `[… → … value]` @@ -88,7 +123,7 @@ pub enum OpCode { /// Pops value, pushes iterator. No-op if already an iterator. `[… value → … iter]` GetIterator, /// Peeks iterator; pushes next value or jumps if exhausted. `[… iter → … iter value]` - IterNext(isize), + IterNext(JumpOffset), /// Pops value, appends to list at local slot. `[… value → …]` ListPush(usize), /// Pops value then key, inserts into map at local slot. `[… key value → …]` @@ -113,9 +148,9 @@ impl std::fmt::Debug for OpCode { Self::Call(n) => write!(f, "Call({n})"), Self::CallVec(n) => write!(f, "CallVec({n})"), Self::Pop => write!(f, "Pop"), - Self::Jump(n) => write!(f, "Jump({n})"), - Self::JumpIfTrue(n) => write!(f, "JumpIfTrue({n})"), - Self::JumpIfFalse(n) => write!(f, "JumpIfFalse({n})"), + Self::Jump(n) => write!(f, "Jump({n:?})"), + Self::JumpIfTrue(n) => write!(f, "JumpIfTrue({n:?})"), + Self::JumpIfFalse(n) => write!(f, "JumpIfFalse({n:?})"), Self::Constant(n) => write!(f, "Constant({n})"), Self::GetLocal(n) => write!(f, "GetLocal({n})"), Self::SetLocal(n) => write!(f, "SetLocal({n})"), @@ -141,7 +176,7 @@ impl std::fmt::Debug for OpCode { write!(f, ")") } Self::GetIterator => write!(f, "GetIterator"), - Self::IterNext(n) => write!(f, "IterNext({n})"), + Self::IterNext(n) => write!(f, "IterNext({n:?})"), Self::ListPush(n) => write!(f, "ListPush({n})"), Self::MapInsert(n) => write!(f, "MapInsert({n})"), Self::MakeRange { inclusive, bounded } => { @@ -180,27 +215,20 @@ impl Chunk { self.code.len() - 1 } - pub fn patch_jump(&mut self, op_idx: usize) { - let offset = - isize::try_from(self.code.len() - op_idx - 1).expect("jump too large to patch"); - match self.code.get_mut(op_idx) { + /// Overwrites the jump operand of a `Jump`, `JumpIfTrue`, `JumpIfFalse`, or + /// `IterNext` already written at `idx`. Panics on any other opcode. + pub fn set_jump_offset(&mut self, idx: usize, offset: JumpOffset) { + match self.code.get_mut(idx) { Some( OpCode::JumpIfFalse(n) | OpCode::JumpIfTrue(n) | OpCode::Jump(n) | OpCode::IterNext(n), ) => *n = offset, - _ => panic!("expected a patchable jump instruction at index {op_idx}"), + _ => panic!("expected a patchable jump instruction at index {idx}"), } } - /// Emits a `Jump` that goes back to `target` (a previously recorded chunk offset). - pub fn write_jump_back(&mut self, target: usize, span: Span) -> usize { - let offset = - -isize::try_from(self.len() - target + 1).expect("loop too large to jump back"); - self.write(OpCode::Jump(offset), span) - } - pub fn is_empty(&self) -> bool { self.code.is_empty() } diff --git a/ndc_vm/src/compiler.rs b/ndc_vm/src/compiler.rs index b062491..bca1219 100644 --- a/ndc_vm/src/compiler.rs +++ b/ndc_vm/src/compiler.rs @@ -1,4 +1,4 @@ -use crate::chunk::{Chunk, OpCode}; +use crate::chunk::{Chunk, JumpOffset, OpCode}; use crate::value::{CompiledFunction, Function}; use crate::{Object, Value}; use ndc_core::{StaticType, TypeSignature}; @@ -139,16 +139,20 @@ impl Compiler { self.compile_expr(*left)?; match operator { LogicalOperator::And => { - let end_jump = self.chunk.write(OpCode::JumpIfFalse(0), left_span); + let end_jump = self + .chunk + .write(OpCode::JumpIfFalse(JumpOffset::ZERO), left_span); self.chunk.write(OpCode::Pop, Span::synthetic()); self.compile_expr(*right)?; - self.chunk.patch_jump(end_jump); + self.patch_jump(end_jump); } LogicalOperator::Or => { - let end_jump = self.chunk.write(OpCode::JumpIfTrue(0), left_span); + let end_jump = self + .chunk + .write(OpCode::JumpIfTrue(JumpOffset::ZERO), left_span); self.chunk.write(OpCode::Pop, Span::synthetic()); self.compile_expr(*right)?; - self.chunk.patch_jump(end_jump); + self.patch_jump(end_jump); } } } @@ -420,14 +424,14 @@ impl Compiler { self.chunk.write(OpCode::Return, span); } Expression::Break => { - let idx = self.chunk.write(OpCode::Jump(0), span); // will be backpatched + let idx = self.chunk.write(OpCode::Jump(JumpOffset::ZERO), span); // will be backpatched self.current_loop_context_mut() .ok_or(CompileError::unexpected_break(span))? .break_instructions .push(idx); } Expression::Continue => { - self.chunk.write_jump_back( + self.write_jump_back( self.current_loop_context() .ok_or(CompileError::unexpected_continue(span))? .start, @@ -583,6 +587,23 @@ impl Compiler { ResolvedVar::Global { .. } => unreachable!("globals are native, never assigned"), }; } + + /// Backpatches a forward jump at `op_idx` to land just after the most recently + /// written instruction. + fn patch_jump(&mut self, op_idx: usize) { + let offset = + isize::try_from(self.chunk.len() - op_idx - 1).expect("jump too large to patch"); + self.chunk.set_jump_offset(op_idx, JumpOffset::new(offset)); + } + + /// Emits a `Jump` that goes back to `target` (a previously recorded chunk offset). + fn write_jump_back(&mut self, target: usize, span: Span) -> usize { + let offset = + -isize::try_from(self.chunk.len() - target + 1).expect("loop too large to jump back"); + self.chunk + .write(OpCode::Jump(JumpOffset::new(offset)), span) + } + fn compile_block( &mut self, statements: Vec, @@ -617,23 +638,29 @@ impl Compiler { ) -> Result<(), CompileError> { let condition_span = condition.span; self.compile_expr(condition)?; - let conditional_jump_idx = self.chunk.write(OpCode::JumpIfFalse(0), condition_span); + let conditional_jump_idx = self + .chunk + .write(OpCode::JumpIfFalse(JumpOffset::ZERO), condition_span); self.chunk.write(OpCode::Pop, Span::synthetic()); self.compile_expr(on_true)?; if let Some(on_false) = on_false { - let jump_to_end = self.chunk.write(OpCode::Jump(0), Span::synthetic()); - self.chunk.patch_jump(conditional_jump_idx); + let jump_to_end = self + .chunk + .write(OpCode::Jump(JumpOffset::ZERO), Span::synthetic()); + self.patch_jump(conditional_jump_idx); self.chunk.write(OpCode::Pop, Span::synthetic()); self.compile_expr(on_false)?; - self.chunk.patch_jump(jump_to_end); + self.patch_jump(jump_to_end); } else { // No else branch — push unit so the if-expression always produces a value. - let jump_to_end = self.chunk.write(OpCode::Jump(0), Span::synthetic()); - self.chunk.patch_jump(conditional_jump_idx); + let jump_to_end = self + .chunk + .write(OpCode::Jump(JumpOffset::ZERO), Span::synthetic()); + self.patch_jump(conditional_jump_idx); self.chunk.write(OpCode::Pop, Span::synthetic()); let idx = self.chunk.add_constant(Value::unit()); self.chunk.write(OpCode::Constant(idx), Span::synthetic()); - self.chunk.patch_jump(jump_to_end); + self.patch_jump(jump_to_end); } Ok(()) @@ -648,17 +675,19 @@ impl Compiler { let condition_span = condition.span; let loop_start = self.new_loop_context(); self.compile_expr(condition)?; - let conditional_jump_idx = self.chunk.write(OpCode::JumpIfFalse(0), condition_span); + let conditional_jump_idx = self + .chunk + .write(OpCode::JumpIfFalse(JumpOffset::ZERO), condition_span); self.chunk.write(OpCode::Pop, Span::synthetic()); self.compile_expr(loop_body)?; self.chunk.write(OpCode::Pop, Span::synthetic()); - self.chunk.write_jump_back(loop_start, Span::synthetic()); - self.chunk.patch_jump(conditional_jump_idx); + self.write_jump_back(loop_start, Span::synthetic()); + self.patch_jump(conditional_jump_idx); self.chunk.write(OpCode::Pop, Span::synthetic()); let break_instructions = std::mem::take(&mut self.current_loop_context_mut().unwrap().break_instructions); for instruction in break_instructions { - self.chunk.patch_jump(instruction) + self.patch_jump(instruction) } self.end_loop_context(); Ok(()) @@ -832,7 +861,7 @@ impl Compiler { self.chunk.write(OpCode::GetIterator, sequence.span); let loop_start = self.new_loop_context(); - let iter_next = self.chunk.write(OpCode::IterNext(0), span); + let iter_next = self.chunk.write(OpCode::IterNext(JumpOffset::ZERO), span); self.compile_declare_lvalue(l_value.clone(), span)?; self.compile_for_iterations(rest, span, compile_leaf)?; @@ -843,15 +872,15 @@ impl Compiler { self.chunk.write(OpCode::CloseUpvalue(slot), span); } - self.chunk.write_jump_back(loop_start, span); + self.write_jump_back(loop_start, span); // Both IterNext-done and break jump to the iterator Pop - self.chunk.patch_jump(iter_next); + self.patch_jump(iter_next); let break_instructions = std::mem::take( &mut self.current_loop_context_mut().unwrap().break_instructions, ); for instruction in break_instructions { - self.chunk.patch_jump(instruction); + self.patch_jump(instruction); } self.end_loop_context(); @@ -860,13 +889,15 @@ impl Compiler { } ForIteration::Guard(condition) => { self.compile_expr(condition.clone())?; - let skip_jump = self.chunk.write(OpCode::JumpIfFalse(0), span); + let skip_jump = self + .chunk + .write(OpCode::JumpIfFalse(JumpOffset::ZERO), span); self.chunk.write(OpCode::Pop, Span::synthetic()); self.compile_for_iterations(rest, span, compile_leaf)?; - let end_jump = self.chunk.write(OpCode::Jump(0), span); - self.chunk.patch_jump(skip_jump); + let end_jump = self.chunk.write(OpCode::Jump(JumpOffset::ZERO), span); + self.patch_jump(skip_jump); self.chunk.write(OpCode::Pop, Span::synthetic()); - self.chunk.patch_jump(end_jump); + self.patch_jump(end_jump); } } diff --git a/ndc_vm/src/vm.rs b/ndc_vm/src/vm.rs index d406813..3e16568 100644 --- a/ndc_vm/src/vm.rs +++ b/ndc_vm/src/vm.rs @@ -217,7 +217,7 @@ impl Vm { let top = self.stack.last().expect("stack underflow"); match top { Value::Bool(false) => { - frame.ip = frame.ip.wrapping_add_signed(offset); + frame.ip = offset.apply(frame.ip); } Value::Bool(true) => {} value => { @@ -234,7 +234,7 @@ impl Vm { let top = self.stack.last().expect("stack underflow"); match top { Value::Bool(true) => { - frame.ip = frame.ip.wrapping_add_signed(offset); + frame.ip = offset.apply(frame.ip); } Value::Bool(false) => {} value => { @@ -247,8 +247,7 @@ impl Vm { } } OpCode::Jump(offset) => { - let offset = *offset; - frame.ip = frame.ip.wrapping_add_signed(offset); + frame.ip = offset.apply(frame.ip); } OpCode::Pop => { self.stack.pop(); @@ -434,7 +433,7 @@ impl Vm { self.stack.push(value); } None => { - frame.ip = frame.ip.wrapping_add_signed(offset); + frame.ip = offset.apply(frame.ip); } } } diff --git a/tests/compiler/tests/compiler.rs b/tests/compiler/tests/compiler.rs index 2f979ce..779c25a 100644 --- a/tests/compiler/tests/compiler.rs +++ b/tests/compiler/tests/compiler.rs @@ -1,5 +1,6 @@ use ndc_lexer::{Lexer, SourceId}; use ndc_parser::Parser; +use ndc_vm::chunk::JumpOffset; use ndc_vm::chunk::OpCode; use ndc_vm::chunk::OpCode::*; use ndc_vm::compiler::Compiler; @@ -53,10 +54,10 @@ fn test_if_without_else() { compile("if true { 1 }"), [ Constant(0), - JumpIfFalse(3), + JumpIfFalse(JumpOffset::new(3)), Pop, Constant(1), - Jump(2), + Jump(JumpOffset::new(2)), Pop, Constant(2), Halt @@ -80,10 +81,10 @@ fn test_if_with_else() { compile("if true { 1 } else { 2 }"), [ Constant(0), - JumpIfFalse(3), + JumpIfFalse(JumpOffset::new(3)), Pop, Constant(1), - Jump(2), + Jump(JumpOffset::new(2)), Pop, Constant(2), Halt @@ -104,7 +105,13 @@ fn test_if_with_else() { fn test_and() { assert_eq!( compile("true and false"), - [Constant(0), JumpIfFalse(2), Pop, Constant(1), Halt] + [ + Constant(0), + JumpIfFalse(JumpOffset::new(2)), + Pop, + Constant(1), + Halt + ] ); } @@ -121,7 +128,13 @@ fn test_and() { fn test_or() { assert_eq!( compile("true or false"), - [Constant(0), JumpIfTrue(2), Pop, Constant(1), Halt] + [ + Constant(0), + JumpIfTrue(JumpOffset::new(2)), + Pop, + Constant(1), + Halt + ] ); } @@ -186,10 +199,10 @@ fn test_if_with_statement_else() { compile("if true { 3 } else { 3; }"), [ Constant(0), - JumpIfFalse(3), + JumpIfFalse(JumpOffset::new(3)), Pop, Constant(1), - Jump(4), + Jump(JumpOffset::new(4)), Pop, Constant(2), Pop, @@ -221,12 +234,12 @@ fn test_if_with_statement_branches() { compile("if true { 3; } else { 3; }"), [ Constant(0), - JumpIfFalse(5), + JumpIfFalse(JumpOffset::new(5)), Pop, Constant(1), Pop, Constant(2), - Jump(4), + Jump(JumpOffset::new(4)), Pop, Constant(3), Pop, @@ -252,11 +265,11 @@ fn test_while() { compile("while true { 1 }"), [ Constant(0), - JumpIfFalse(4), + JumpIfFalse(JumpOffset::new(4)), Pop, Constant(1), Pop, - Jump(-6), + Jump(JumpOffset::new(-6)), Pop, Halt ]