From 06638574f712398750475be4e02c870d3aaa0753 Mon Sep 17 00:00:00 2001
From: _ <_@_>
Date: Sat, 30 Sep 2023 23:41:12 -0500
Subject: [PATCH] :loud_sound: improve error message for this error with the
 missing math lib

---
 src/instruction.rs |  2 +-
 src/loader.rs      |  2 --
 src/main.rs        | 12 +++++-----
 src/state.rs       | 41 ++++++++++++++++++++++++---------
 src/tests.rs       | 56 +++++++++++++++++++++++++++++++++++++---------
 5 files changed, 84 insertions(+), 29 deletions(-)

diff --git a/src/instruction.rs b/src/instruction.rs
index 3cca3c1..fb76c0e 100644
--- a/src/instruction.rs
+++ b/src/instruction.rs
@@ -1,4 +1,4 @@
-#[derive (Debug, PartialEq)]
+#[derive (Clone, Copy, Debug, PartialEq)]
 pub enum Instruction {
 	Add (u8, u8, u8),
 	AddI (u8, u8, i8),
diff --git a/src/loader.rs b/src/loader.rs
index 1611244..8b1ead7 100644
--- a/src/loader.rs
+++ b/src/loader.rs
@@ -221,8 +221,6 @@ pub fn parse_block <R: Read> (rdr: &mut R, blocks: &mut Vec <Block>)
 {
 	// Ignore things I haven't implemented yet
 	
-	use crate::value::Value;
-	
 	parse_string (rdr)?; // function name
 	parse_int (rdr).unwrap ();    // start line in source code
 	parse_int (rdr).unwrap ();    // last line in source code
diff --git a/src/main.rs b/src/main.rs
index d344c7f..7037b29 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -8,7 +8,7 @@ mod value;
 #[cfg (test)]
 mod tests;
 
-fn main () {
+fn main () -> Result <(), state::StepError> {
 	use state::State;
 	
 	let mut list_bytecode = false;
@@ -86,17 +86,17 @@ fn main () {
 			
 			match input.as_str ().trim_end () {
 				"c" => in_break = false,
-				"q" => return,
+				"q" => return Ok (()),
 				"registers" => {
 					dbg! (&vm.registers);
 					continue;
 				}
 				"s" => {
-					match vm.step () {
+					match vm.step ()? {
 						None => (),
 						Some (state::StepOutput::ChunkReturned (x)) => {
 							dbg! (x);
-							return;
+							return Ok (());
 						},
 					}
 					continue;
@@ -105,11 +105,11 @@ fn main () {
 			}
 		}
 		
-		match vm.step () {
+		match vm.step ()? {
 			None => (),
 			Some (state::StepOutput::ChunkReturned (x)) => {
 				dbg! (x);
-				return;
+				return Ok (());
 			},
 		}
 	}
diff --git a/src/state.rs b/src/state.rs
index c01390d..ec4efbe 100644
--- a/src/state.rs
+++ b/src/state.rs
@@ -79,6 +79,13 @@ pub enum StepOutput {
 	ChunkReturned (Vec <Value>),
 }
 
+#[derive (Debug)]
+pub struct StepError {
+	frame: StackFrame,
+	inst:  Instruction,
+	msg: &'static str,
+}
+
 impl <'a> State <'a> {
 	pub fn new (chunk: &'a Chunk, upvalues: &'a [Value]) -> Self {
 		Self {
@@ -140,7 +147,17 @@ impl <'a> State <'a> {
 		self.top - self.stack.last ().unwrap ().register_offset
 	}
 	
-	pub fn step (&mut self) -> Option <StepOutput> {
+	fn make_step_error (&self, msg: &'static str, inst: &Instruction) -> StepError
+	{
+		StepError {
+			frame: self.stack.last ().unwrap ().clone (),
+			inst: inst.clone (),
+			msg,
+		}
+	}
+	
+	pub fn step (&mut self) -> Result <Option <StepOutput>, StepError> 
+	{
 		let chunk = self.chunk;
 		self.step_count += 1;
 		
@@ -161,6 +178,10 @@ impl <'a> State <'a> {
 		// let r = &mut self.registers [frame.register_offset..];
 		let k = &block.constants;
 		
+		let make_step_error = |msg| {
+			Err (self.make_step_error (msg, instruction))
+		};
+		
 		match instruction {
 			Instruction::Add (a, b, c) => {
 				let v_b = self.reg (*b);
@@ -230,7 +251,7 @@ impl <'a> State <'a> {
 						}
 						
 						// Skip the PC increment at the bottom of the loop
-						return None;
+						return Ok (None);
 					},
 					Value::RsFunc (x) => {
 						let current_frame = self.stack.last ().unwrap ();
@@ -341,9 +362,9 @@ impl <'a> State <'a> {
 			},
 			Instruction::GetField (a, b, c) => {
 				let t = match self.reg (*b) {
-					Value::Nil => panic! ("R[B] must not be nil {}:{}", frame.block_idx, frame.program_counter),
+					Value::Nil => make_step_error ("R[B] must not be nil")?,
 					Value::Table (t) => t,
-					_ => panic! ("R[B] must be a table"),
+					_ => make_step_error ("R[B] must be a table")?,
 				};
 				
 				let key = match &k [usize::from (*c)] {
@@ -573,7 +594,7 @@ impl <'a> State <'a> {
 				}
 				else {
 					// Return from the entire program
-					return Some (StepOutput::ChunkReturned (self.registers [a..(a + b - 1)].to_vec()));
+					return Ok (Some (StepOutput::ChunkReturned (self.registers [a..(a + b - 1)].to_vec())));
 				}
 			},
 			Instruction::Return0 => {
@@ -697,7 +718,7 @@ impl <'a> State <'a> {
 				frame.block_idx = closure.idx;
 				
 				// Skip the PC increment
-				return None;
+				return Ok (None);
 			},
 			Instruction::Test (a, _k) => {
 				if self.reg (*a).is_truthy() {
@@ -727,11 +748,11 @@ impl <'a> State <'a> {
 			frame.program_counter = next_pc;
 		}
 		
-		None
+		Ok (None)
 	}
 	
 	pub fn execute_chunk (&mut self, breakpoints: &[Breakpoint]) 
-	-> Vec <Value> {
+	-> Result <Vec <Value>, StepError> {
 		let max_iters = 2000;
 		
 		for _ in 0..max_iters {
@@ -739,9 +760,9 @@ impl <'a> State <'a> {
 				dbg! (&self);
 			}
 			
-			match self.step () {
+			match self.step ()? {
 				None => (),
-				Some (StepOutput::ChunkReturned (x)) => return x,
+				Some (StepOutput::ChunkReturned (x)) => return Ok (x),
 			}
 		}
 		
diff --git a/src/tests.rs b/src/tests.rs
index cb67431..6ff8e5e 100644
--- a/src/tests.rs
+++ b/src/tests.rs
@@ -25,7 +25,7 @@ fn calculate_hash<T: Hash>(t: &T) -> u64 {
 fn run_chunk (args: &[&str], chunk: &Chunk) -> Vec <Value> {
 	let upvalues = State::upvalues_from_args (args.into_iter ().map (|s| s.to_string ()));
 	let mut vm = State::new (chunk, &upvalues);
-	vm.execute_chunk (&[])
+	vm.execute_chunk (&[]).unwrap ()
 }
 
 /// Takes arguments and Lua bytecode, loads it, runs it,
@@ -120,7 +120,7 @@ fn bools () {
 		
 		let upvalues = State::upvalues_from_args (arg.into_iter ().map (|s| s.to_string ()));
 		let mut vm = State::new (&chunk, &upvalues);
-		let actual = vm.execute_chunk (&[]);
+		let actual = vm.execute_chunk (&[]).unwrap ();
 		assert_eq! (actual, expected);
 	}
 }
@@ -176,7 +176,7 @@ fn floats () {
 		let expected: Vec <Value> = expected;
 		let upvalues = State::upvalues_from_args (arg.into_iter ().map (|s| s.to_string ()));
 		let mut vm = State::new (&chunk, &upvalues);
-		let actual = vm.execute_chunk (&[]);
+		let actual = vm.execute_chunk (&[]).unwrap ();
 		
 		assert_eq! (actual, expected);
 	}
@@ -198,7 +198,7 @@ fn fma () {
 		let expected: Vec <Value> = expected;
 		let upvalues = State::upvalues_from_args (arg.into_iter ().map (|s| s.to_string ()));
 		let mut vm = State::new (&chunk, &upvalues);
-		let actual = vm.execute_chunk (&[]);
+		let actual = vm.execute_chunk (&[]).unwrap ();
 		
 		assert_eq! (actual, expected);
 	}
@@ -368,14 +368,50 @@ fn value_size () {
 	// with 64-bit floats
 	// 
 	// It would be nice if LunarWaveVM is the same or better.
-	// There are some exploratory things in this test, too
+	// I'm also just checking a bunch of my assumptions about how
+	// Rust organizes and sizes different types
 	
 	use std::mem::size_of;
 	
-	assert! (size_of::<Box <()>> () <= 8);
-	assert! (size_of::<std::rc::Rc <()>> () <= 8);
+	{
+		// Make sure that Rx / Box are both pointers that hide the size
+		// of big types
+		
+		assert! (size_of::<Box <()>> () <= 8);
+		assert! (size_of::<std::rc::Rc <()>> () <= 8);
+	}
 	
-	let sz = size_of::<crate::value::Value> ();
-	let expected = 16;
-	assert! (sz <= expected, "{sz} > {expected}");
+	{
+		// Make sure LWVM's Values are 16 bytes or smaller.
+		// Because types are usually aligned to their size, f64s
+		// are supposed to be aligned to 8 bytes. So even an `Option <f64>`
+		// uses 8 bytes to say "Some" or "None".
+		// I could _maybe_ fudge this somehow but it's fine to start with.
+		
+		let sz = size_of::<crate::value::Value> ();
+		let expected = 16;
+		assert! (sz <= expected, "{sz} > {expected}");
+	}
+	
+	{
+		// All these are 8 bytes for the same reason Value is 16 bytes.
+		// Luckily Rust doesn't seem to stack the 4-byte overhead
+		// of Result and Option.
+		
+		let sz = size_of::<(i32, i32)> ();
+		let expected = 8;
+		assert! (sz == expected, "{sz} != {expected}");
+		
+		let sz = size_of::<Option <i32>> ();
+		let expected = 8;
+		assert! (sz == expected, "{sz} != {expected}");
+		
+		let sz = size_of::<Result <i32, i32>> ();
+		let expected = 8;
+		assert! (sz == expected, "{sz} != {expected}");
+		
+		let sz = size_of::<Result <Option <i32>, i32>> ();
+		let expected = 8;
+		assert! (sz == expected, "{sz} != {expected}");
+	}
 }