wasm_interp: use Result instead of panic. Focus debug info on *Wasm app*, not interp.

crates/wasm_interp/src/lib.rs

@@ -7,7 +7,7 @@ pub mod wasi;
 // Main external interface
 pub use instance::Instance;
 
-use roc_wasm_module::Value;
+use roc_wasm_module::{Value, ValueType};
 use value_stack::ValueStack;
 use wasi::WasiDispatcher;
 
@@ -56,3 +56,21 @@ impl<'a> ImportDispatcher for DefaultImportDispatcher<'a> {
         }
     }
 }
+
+#[derive(Debug, PartialEq)]
+pub(crate) enum Error {
+    ValueStackType(ValueType, ValueType),
+    ValueStackEmpty,
+}
+
+impl Error {
+    fn value_stack_type(expected: ValueType, is_float: bool, is_64: bool) -> Self {
+        let ty = match (is_float, is_64) {
+            (false, false) => ValueType::I32,
+            (false, true) => ValueType::I64,
+            (true, false) => ValueType::F32,
+            (true, true) => ValueType::F64,
+        };
+        Error::ValueStackType(expected, ty)
+    }
+}

crates/wasm_interp/src/tests/test_basics.rs

@@ -93,9 +93,9 @@ fn test_loop_help(end: i32, expected: i32) {
         &mut state.program_counter,
     );
 
-    while let Action::Continue = state.execute_next_instruction(&module) {}
+    while let Ok(Action::Continue) = state.execute_next_instruction(&module) {}
 
-    assert_eq!(state.value_stack.pop_i32(), expected);
+    assert_eq!(state.value_stack.pop_i32(), Ok(expected));
 }
 
 #[test]
@@ -161,9 +161,9 @@ fn test_if_else_help(condition: i32, expected: i32) {
         &mut state.program_counter,
     );
 
-    while let Action::Continue = state.execute_next_instruction(&module) {}
+    while let Ok(Action::Continue) = state.execute_next_instruction(&module) {}
 
-    assert_eq!(state.value_stack.pop_i32(), expected);
+    assert_eq!(state.value_stack.pop_i32(), Ok(expected));
 }
 
 #[test]
@@ -250,7 +250,7 @@ fn test_br() {
         &mut state.program_counter,
     );
 
-    while let Action::Continue = state.execute_next_instruction(&module) {}
+    while let Ok(Action::Continue) = state.execute_next_instruction(&module) {}
 
     assert_eq!(state.value_stack.pop(), Value::I32(111))
 }
@@ -348,7 +348,7 @@ fn test_br_if_help(condition: i32, expected: i32) {
         &mut state.program_counter,
     );
 
-    while let Action::Continue = state.execute_next_instruction(&module) {}
+    while let Ok(Action::Continue) = state.execute_next_instruction(&module) {}
 
     assert_eq!(state.value_stack.pop(), Value::I32(expected))
 }
@@ -452,7 +452,7 @@ fn test_br_table_help(condition: i32, expected: i32) {
         &mut state.program_counter,
     );
 
-    while let Action::Continue = state.execute_next_instruction(&module) {}
+    while let Ok(Action::Continue) = state.execute_next_instruction(&module) {}
 
     assert_eq!(state.value_stack.pop(), Value::I32(expected))
 }
@@ -664,7 +664,7 @@ fn test_call_return_with_args() {
 
     state.program_counter = func0_first_instruction as usize;
 
-    while let Action::Continue = state.execute_next_instruction(&module) {}
+    while let Ok(Action::Continue) = state.execute_next_instruction(&module) {}
 
     assert_eq!(state.value_stack.peek(), Value::I32(4));
 }
@@ -785,7 +785,7 @@ fn test_select_help(first: Value, second: Value, condition: i32, expected: Value
         &mut state.program_counter,
     );
 
-    while let Action::Continue = state.execute_next_instruction(&module) {}
+    while let Ok(Action::Continue) = state.execute_next_instruction(&module) {}
 
     assert_eq!(state.value_stack.pop(), expected);
 }
@@ -818,9 +818,9 @@ fn test_set_get_local() {
     module.code.bytes.push(OpCode::GETLOCAL as u8);
     module.code.bytes.encode_u32(2);
 
-    state.execute_next_instruction(&module);
+    state.execute_next_instruction(&module).unwrap();
-    state.execute_next_instruction(&module);
+    state.execute_next_instruction(&module).unwrap();
-    state.execute_next_instruction(&module);
+    state.execute_next_instruction(&module).unwrap();
     assert_eq!(state.value_stack.len(), 1);
     assert_eq!(state.value_stack.pop(), Value::I32(12345));
 }
@@ -853,9 +853,9 @@ fn test_tee_get_local() {
     module.code.bytes.push(OpCode::GETLOCAL as u8);
     module.code.bytes.encode_u32(2);
 
-    state.execute_next_instruction(&module);
+    state.execute_next_instruction(&module).unwrap();
-    state.execute_next_instruction(&module);
+    state.execute_next_instruction(&module).unwrap();
-    state.execute_next_instruction(&module);
+    state.execute_next_instruction(&module).unwrap();
     assert_eq!(state.value_stack.len(), 2);
     assert_eq!(state.value_stack.pop(), Value::I32(12345));
     assert_eq!(state.value_stack.pop(), Value::I32(12345));
@@ -879,10 +879,10 @@ fn test_global() {
     module.code.bytes.push(OpCode::GETGLOBAL as u8);
     module.code.bytes.encode_u32(1);
 
-    state.execute_next_instruction(&module);
+    state.execute_next_instruction(&module).unwrap();
-    state.execute_next_instruction(&module);
+    state.execute_next_instruction(&module).unwrap();
-    state.execute_next_instruction(&module);
+    state.execute_next_instruction(&module).unwrap();
-    state.execute_next_instruction(&module);
+    state.execute_next_instruction(&module).unwrap();
     assert_eq!(state.value_stack.len(), 2);
     assert_eq!(state.value_stack.pop(), Value::I32(555));
     assert_eq!(state.value_stack.pop(), Value::I32(222));
@@ -897,7 +897,7 @@ fn test_i32const() {
     module.code.bytes.push(OpCode::I32CONST as u8);
     module.code.bytes.encode_i32(12345);
 
-    state.execute_next_instruction(&module);
+    state.execute_next_instruction(&module).unwrap();
     assert_eq!(state.value_stack.pop(), Value::I32(12345))
 }
 
@@ -910,7 +910,7 @@ fn test_i64const() {
     module.code.bytes.push(OpCode::I64CONST as u8);
     module.code.bytes.encode_i64(1234567890);
 
-    state.execute_next_instruction(&module);
+    state.execute_next_instruction(&module).unwrap();
     assert_eq!(state.value_stack.pop(), Value::I64(1234567890))
 }
 
@@ -923,7 +923,7 @@ fn test_f32const() {
     module.code.bytes.push(OpCode::F32CONST as u8);
     module.code.bytes.encode_f32(123.45);
 
-    state.execute_next_instruction(&module);
+    state.execute_next_instruction(&module).unwrap();
     assert_eq!(state.value_stack.pop(), Value::F32(123.45))
 }
 
@@ -936,6 +936,6 @@ fn test_f64const() {
     module.code.bytes.push(OpCode::F64CONST as u8);
     module.code.bytes.encode_f64(12345.67890);
 
-    state.execute_next_instruction(&module);
+    state.execute_next_instruction(&module).unwrap();
     assert_eq!(state.value_stack.pop(), Value::F64(12345.67890))
 }

crates/wasm_interp/src/tests/test_mem.rs

@@ -18,7 +18,7 @@ fn test_currentmemory() {
     module.code.bytes.push(OpCode::CURRENTMEMORY as u8);
 
     let mut state = Instance::new(&arena, pages, pc, [], DEFAULT_IMPORTS);
-    state.execute_next_instruction(&module);
+    state.execute_next_instruction(&module).unwrap();
     assert_eq!(state.value_stack.pop(), Value::I32(3))
 }
 
@@ -36,8 +36,8 @@ fn test_growmemory() {
     module.code.bytes.push(OpCode::GROWMEMORY as u8);
 
     let mut state = Instance::new(&arena, existing_pages, pc, [], DEFAULT_IMPORTS);
-    state.execute_next_instruction(&module);
+    state.execute_next_instruction(&module).unwrap();
-    state.execute_next_instruction(&module);
+    state.execute_next_instruction(&module).unwrap();
     assert_eq!(state.memory.len(), 5 * MemorySection::PAGE_SIZE as usize);
 }
 

crates/wasm_interp/src/value_stack.rs

@@ -3,6 +3,8 @@ use bumpalo::{collections::Vec, Bump};
 use roc_wasm_module::{Value, ValueType};
 use std::{fmt::Debug, mem::size_of};
 
+use crate::Error;
+
 /// Memory-efficient Struct-of-Arrays storage for the value stack.
 /// Pack the values and their types as densely as possible,
 /// to get better cache usage, at the expense of some extra logic.
@@ -15,16 +17,20 @@ pub struct ValueStack<'a> {
 macro_rules! pop_bytes {
     ($ty: ty, $bytes: expr) => {{
         const SIZE: usize = size_of::<$ty>();
-        let bytes_idx = $bytes.len() - SIZE;
+        if $bytes.len() < SIZE {
-        let mut b = [0; SIZE];
+            Err(Error::ValueStackEmpty)
-        b.copy_from_slice(&$bytes[bytes_idx..][..SIZE]);
+        } else {
-        $bytes.truncate(bytes_idx);
+            let bytes_idx = $bytes.len() - SIZE;
-        <$ty>::from_ne_bytes(b)
+            let mut b = [0; SIZE];
+            b.copy_from_slice(&$bytes[bytes_idx..][..SIZE]);
+            $bytes.truncate(bytes_idx);
+            Ok(<$ty>::from_ne_bytes(b))
+        }
     }};
 }
 
 impl<'a> ValueStack<'a> {
-    pub fn new(arena: &'a Bump) -> Self {
+    pub(crate) fn new(arena: &'a Bump) -> Self {
         ValueStack {
             bytes: Vec::with_capacity_in(1024, arena),
             is_float: BitVec::with_capacity(1024),
@@ -32,15 +38,15 @@ impl<'a> ValueStack<'a> {
         }
     }
 
-    pub fn len(&self) -> usize {
+    pub(crate) fn len(&self) -> usize {
         self.is_64.len()
     }
 
-    pub fn is_empty(&self) -> bool {
+    pub(crate) fn is_empty(&self) -> bool {
         self.is_64.is_empty()
     }
 
-    pub fn push(&mut self, value: Value) {
+    pub(crate) fn push(&mut self, value: Value) {
         match value {
             Value::I32(x) => {
                 self.bytes.extend_from_slice(&x.to_ne_bytes());
@@ -65,7 +71,7 @@ impl<'a> ValueStack<'a> {
         }
     }
 
-    pub fn pop(&mut self) -> Value {
+    pub(crate) fn pop(&mut self) -> Value {
         let is_64 = self.is_64.pop().unwrap();
         let is_float = self.is_float.pop().unwrap();
         let size = if is_64 { 8 } else { 4 };
@@ -75,7 +81,7 @@ impl<'a> ValueStack<'a> {
         value
     }
 
-    pub fn peek(&self) -> Value {
+    pub(crate) fn peek(&self) -> Value {
         let is_64 = *self.is_64.last().unwrap();
         let is_float = *self.is_float.last().unwrap();
         let size = if is_64 { 8 } else { 4 };
@@ -104,69 +110,63 @@ impl<'a> ValueStack<'a> {
     }
 
     /// Memory addresses etc
-    pub fn pop_u32(&mut self) -> u32 {
+    pub(crate) fn pop_u32(&mut self) -> Result<u32, Error> {
         match (self.is_float.pop(), self.is_64.pop()) {
             (Some(false), Some(false)) => pop_bytes!(u32, self.bytes),
-            (Some(is_float), Some(is_64)) => panic!(
+            (Some(is_float), Some(is_64)) => {
-                "Expected I32 but found {:?}",
+                Err(Error::value_stack_type(ValueType::I32, is_float, is_64))
-                type_from_flags(is_float, is_64)
+            }
-            ),
+            _ => Err(Error::ValueStackEmpty),
-            _ => panic!("Expected I32 but value stack was empty"),
         }
     }
 
-    pub fn pop_i32(&mut self) -> i32 {
+    pub(crate) fn pop_i32(&mut self) -> Result<i32, Error> {
         match (self.is_float.pop(), self.is_64.pop()) {
             (Some(false), Some(false)) => pop_bytes!(i32, self.bytes),
-            (Some(is_float), Some(is_64)) => panic!(
+            (Some(is_float), Some(is_64)) => {
-                "Expected I32 but found {:?}",
+                Err(Error::value_stack_type(ValueType::I32, is_float, is_64))
-                type_from_flags(is_float, is_64)
+            }
-            ),
+            _ => Err(Error::ValueStackEmpty),
-            _ => panic!("Expected I32 but value stack was empty"),
         }
     }
 
-    pub fn pop_u64(&mut self) -> u64 {
+    pub(crate) fn pop_u64(&mut self) -> Result<u64, Error> {
         match (self.is_float.pop(), self.is_64.pop()) {
             (Some(false), Some(true)) => pop_bytes!(u64, self.bytes),
-            (Some(is_float), Some(is_64)) => panic!(
+            (Some(is_float), Some(is_64)) => {
-                "Expected I64 but found {:?}",
+                Err(Error::value_stack_type(ValueType::I64, is_float, is_64))
-                type_from_flags(is_float, is_64)
+            }
-            ),
+            _ => Err(Error::ValueStackEmpty),
-            _ => panic!("Expected I64 but value stack was empty"),
         }
     }
 
-    pub fn pop_i64(&mut self) -> i64 {
+    pub(crate) fn pop_i64(&mut self) -> Result<i64, Error> {
         match (self.is_float.pop(), self.is_64.pop()) {
             (Some(false), Some(true)) => pop_bytes!(i64, self.bytes),
-            (Some(is_float), Some(is_64)) => panic!(
+            (Some(is_float), Some(is_64)) => {
-                "Expected I64 but found {:?}",
+                Err(Error::value_stack_type(ValueType::I64, is_float, is_64))
-                type_from_flags(is_float, is_64)
+            }
-            ),
+            _ => Err(Error::ValueStackEmpty),
-            _ => panic!("Expected I64 but value stack was empty"),
         }
     }
 
-    pub fn pop_f32(&mut self) -> f32 {
+    pub(crate) fn pop_f32(&mut self) -> Result<f32, Error> {
         match (self.is_float.pop(), self.is_64.pop()) {
             (Some(true), Some(false)) => pop_bytes!(f32, self.bytes),
-            (Some(is_float), Some(is_64)) => panic!(
+            (Some(is_float), Some(is_64)) => {
-                "Expected F32 but found {:?}",
+                Err(Error::value_stack_type(ValueType::F32, is_float, is_64))
-                type_from_flags(is_float, is_64)
+            }
-            ),
+            _ => Err(Error::ValueStackEmpty),
-            _ => panic!("Expected F32 but value stack was empty"),
         }
     }
 
-    pub fn pop_f64(&mut self) -> f64 {
+    pub(crate) fn pop_f64(&mut self) -> Result<f64, Error> {
         match (self.is_float.pop(), self.is_64.pop()) {
             (Some(true), Some(true)) => pop_bytes!(f64, self.bytes),
-            (Some(is_float), Some(is_64)) => panic!(
+            (Some(is_float), Some(is_64)) => {
-                "Expected F64 but found {:?}",
+                Err(Error::value_stack_type(ValueType::F64, is_float, is_64))
-                type_from_flags(is_float, is_64)
+            }
-            ),
+            _ => Err(Error::ValueStackEmpty),
-            _ => panic!("Expected F64 but value stack was empty"),
         }
     }
 
@@ -196,20 +196,11 @@ impl<'a> ValueStack<'a> {
         write!(f, "]")
     }
 
-    pub fn get_slice<'b>(&'b self, index: usize) -> ValueStackSlice<'a, 'b> {
+    pub(crate) fn get_slice<'b>(&'b self, index: usize) -> ValueStackSlice<'a, 'b> {
         ValueStackSlice { stack: self, index }
     }
 }
 
-fn type_from_flags(is_float: bool, is_64: bool) -> ValueType {
-    match (is_float, is_64) {
-        (false, false) => ValueType::I32,
-        (false, true) => ValueType::I64,
-        (true, false) => ValueType::F32,
-        (true, true) => ValueType::F64,
-    }
-}
-
 impl Debug for ValueStack<'_> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         self.fmt_from_index(f, 0)