diff --git a/compiler/noirc_evaluator/src/ssa/interpreter/errors.rs b/compiler/noirc_evaluator/src/ssa/interpreter/errors.rs new file mode 100644 index 00000000000..f0789dc1dc1 --- /dev/null +++ b/compiler/noirc_evaluator/src/ssa/interpreter/errors.rs @@ -0,0 +1,139 @@ +use crate::ssa::ir::{ + basic_block::BasicBlockId, + instruction::{BinaryOp, Intrinsic}, + value::ValueId, +}; +use acvm::FieldElement; +use thiserror::Error; + +pub(super) const MAX_SIGNED_BIT_SIZE: u32 = 64; +pub(super) const MAX_UNSIGNED_BIT_SIZE: u32 = 128; + +#[derive(Debug, Error)] +pub(crate) enum InterpreterError { + /// These errors are all the result from malformed input SSA + #[error("{0}")] + Internal(InternalError), + #[error("constrain {lhs_id} == {rhs_id} failed:\n {lhs} != {rhs}")] + ConstrainEqFailed { lhs: String, lhs_id: ValueId, rhs: String, rhs_id: ValueId }, + #[error("constrain {lhs_id} != {rhs_id} failed:\n {lhs} == {rhs}")] + ConstrainNeFailed { lhs: String, lhs_id: ValueId, rhs: String, rhs_id: ValueId }, + #[error("static_assert `{condition}` failed: {message}")] + StaticAssertFailed { condition: ValueId, message: String }, + #[error( + "Range check of {value_id} = {value} failed.\n Max bits allowed by range check = {max_bits}\n Actual bit count = {actual_bits}" + )] + RangeCheckFailed { value: String, value_id: ValueId, actual_bits: u32, max_bits: u32 }, + #[error( + "Range check of {value_id} = {value} failed.\n Max bits allowed by range check = {max_bits}\n Actual bit count = {actual_bits}\n {message}" + )] + RangeCheckFailedWithMessage { + value: String, + value_id: ValueId, + actual_bits: u32, + max_bits: u32, + message: String, + }, + /// This is not an internal error since the SSA is still valid. We're just not able to + /// interpret it since we lack the context of what the external function is. + #[error("Call to unknown foreign function {name}")] + UnknownForeignFunctionCall { name: String }, + #[error("Division by zero: `div {lhs_id}, {rhs_id}` ({lhs} / {rhs})")] + DivisionByZero { lhs_id: ValueId, lhs: String, rhs_id: ValueId, rhs: String }, + #[error("Underflow in dec_rc when decrementing reference count of `{value_id} = {value}`")] + DecRcUnderflow { value_id: ValueId, value: String }, + #[error( + "Erroneously incremented reference count of value `{value_id} = {value}` from 0 back to 1" + )] + IncRcRevive { value_id: ValueId, value: String }, + #[error("An overflow occurred while evaluating {instruction}")] + Overflow { instruction: String }, + #[error( + "if-else instruction with then condition `{then_condition_id}` and else condition `{else_condition_id}` has both branches as true. This should be impossible except for malformed SSA code" + )] + DoubleTrueIfElse { then_condition_id: ValueId, else_condition_id: ValueId }, + #[error("Tried to pop from empty slice `{slice}` in `{instruction}`")] + PoppedFromEmptySlice { slice: ValueId, instruction: &'static str }, + #[error("Unable to convert `{field_id} = {field}` to radix {radix}")] + ToRadixFailed { field_id: ValueId, field: FieldElement, radix: u32 }, +} + +/// These errors can only result from interpreting malformed SSA +#[derive(Debug, Error)] +pub(crate) enum InternalError { + #[error( + "Argument count {arguments} to block {block} does not match the expected parameter count {parameters}" + )] + BlockArgumentCountMismatch { block: BasicBlockId, arguments: usize, parameters: usize }, + #[error( + "Argument count {arguments} to `{intrinsic}` does not match the expected parameter count {parameters}" + )] + IntrinsicArgumentCountMismatch { intrinsic: Intrinsic, arguments: usize, parameters: usize }, + #[error("Block {block} is missing the terminator instruction")] + BlockMissingTerminator { block: BasicBlockId }, + #[error("Cannot call non-function value {value_id} = {value}")] + CalledNonFunction { value: String, value_id: ValueId }, + // Note that we don't need to display the value_id because displaying a reference + // value shows the original value id anyway + #[error("Reference value `{value}` passed from a constrained to an unconstrained function")] + ReferenceValueCrossedUnconstrainedBoundary { value: String }, + #[error("Reference value `{value}` loaded before it was first stored to")] + UninitializedReferenceValueLoaded { value: String }, + #[error( + "Mismatched types in binary operator: `{operator} {lhs_id}, {rhs_id}` ({operator} {lhs}, {rhs})" + )] + MismatchedTypesInBinaryOperator { + lhs_id: ValueId, + lhs: String, + operator: BinaryOp, + rhs_id: ValueId, + rhs: String, + }, + #[error("Unsupported operator `{operator}` for type `{typ}`")] + UnsupportedOperatorForType { operator: &'static str, typ: &'static str }, + #[error( + "Invalid bit size of `{bit_size}` given to truncate, maximum size allowed for unsigned values is {MAX_UNSIGNED_BIT_SIZE}" + )] + InvalidUnsignedTruncateBitSize { bit_size: u32 }, + #[error( + "Invalid bit size of `{bit_size}` given to truncate, maximum size allowed for signed values is {MAX_SIGNED_BIT_SIZE}" + )] + InvalidSignedTruncateBitSize { bit_size: u32 }, + #[error("Rhs of `{operator}` should be a u8 but found `{rhs_id} = {rhs}`")] + RhsOfBitShiftShouldBeU8 { operator: &'static str, rhs_id: ValueId, rhs: String }, + #[error( + "Expected {expected_type} value in {instruction} but instead found `{value_id} = {value}`" + )] + TypeError { + value_id: ValueId, + value: String, + expected_type: &'static str, + instruction: &'static str, + }, + #[error( + "Function {function} ({function_name}) returned {actual} argument(s) but it was expected to return {expected}" + )] + FunctionReturnedIncorrectArgCount { + function: ValueId, + function_name: String, + expected: usize, + actual: usize, + }, + #[error( + "`truncate {value_id} to 0 bits, max_bit_size: {max_bit_size}` has invalid bit size 0. This should only be possible in malformed SSA." + )] + TruncateToZeroBits { value_id: ValueId, max_bit_size: u32 }, + #[error( + "`range_check {value_id} to 0 bits` has invalid bit size 0. This should only be possible in malformed SSA." + )] + RangeCheckToZeroBits { value_id: ValueId }, + #[error("`field_less_than` can only be called in unconstrained contexts")] + FieldLessThanCalledInConstrainedContext, + #[error("Slice `{slice_id} = {slice}` contains struct/tuple elements of types `({})` and thus needs a minimum length of {} to pop 1 struct/tuple, but it is only of length {actual_length}", element_types.join(", "), element_types.len())] + NotEnoughElementsToPopSliceOfStructs { + slice_id: ValueId, + slice: String, + actual_length: usize, + element_types: Vec, + }, +} diff --git a/compiler/noirc_evaluator/src/ssa/interpreter/intrinsics.rs b/compiler/noirc_evaluator/src/ssa/interpreter/intrinsics.rs index 3eaa4e767a8..b836e7c71af 100644 --- a/compiler/noirc_evaluator/src/ssa/interpreter/intrinsics.rs +++ b/compiler/noirc_evaluator/src/ssa/interpreter/intrinsics.rs @@ -11,28 +11,29 @@ use crate::ssa::{ }, }; -use super::{IResults, Interpreter, Value}; +use super::{ArrayValue, IResult, IResults, InternalError, Interpreter, InterpreterError, Value}; impl Interpreter<'_> { pub(super) fn call_intrinsic( &mut self, intrinsic: Intrinsic, - mut args: Vec, + args: &[ValueId], results: &[ValueId], ) -> IResults { match intrinsic { Intrinsic::ArrayLen => { - assert_eq!(args.len(), 1); - let length = args[0].as_array_or_slice().unwrap().elements.borrow().len(); + check_argument_count(args, 1, intrinsic)?; + let array = self.lookup_array_or_slice(args[0], "call to array_len")?; + let length = array.elements.borrow().len(); Ok(vec![Value::Numeric(NumericValue::U32(length as u32))]) } Intrinsic::ArrayAsStrUnchecked => { - assert_eq!(args.len(), 1); - Ok(args) + check_argument_count(args, 1, intrinsic)?; + Ok(vec![self.lookup(args[0])]) } Intrinsic::AsSlice => { - assert_eq!(args.len(), 1); - let array = args[0].as_array_or_slice().unwrap(); + check_argument_count(args, 1, intrinsic)?; + let array = self.lookup_array_or_slice(args[0], "call to as_slice")?; let length = array.elements.borrow().len(); let length = Value::Numeric(NumericValue::U32(length as u32)); @@ -46,15 +47,15 @@ impl Interpreter<'_> { Ok(Vec::new()) } Intrinsic::StaticAssert => { - assert_eq!(args.len(), 2); - - let condition = args[0].as_bool().unwrap(); - if !condition { - let message = args[1].as_string().unwrap(); - panic!("static_assert failed: {message}"); + check_argument_count(args, 2, intrinsic)?; + + let condition = self.lookup_bool(args[0], "static_assert")?; + if condition { + Ok(Vec::new()) + } else { + let message = self.lookup_string(args[1], "static_assert")?; + Err(InterpreterError::StaticAssertFailed { condition: args[0], message }) } - - Ok(Vec::new()) } Intrinsic::SlicePushBack => self.slice_push_back(args), Intrinsic::SlicePushFront => self.slice_push_front(args), @@ -67,46 +68,44 @@ impl Interpreter<'_> { } // Both of these are no-ops Intrinsic::StrAsBytes | Intrinsic::AsWitness => { - assert_eq!(args.len(), 1); - let arg = args.pop().unwrap(); - Ok(vec![arg]) + check_argument_count(args, 1, intrinsic)?; + Ok(vec![self.lookup(args[0])]) } Intrinsic::ToBits(endian) => { - assert_eq!(args.len(), 1); - assert_eq!(results.len(), 1); - let field = args[0].as_field().unwrap(); - self.to_radix(endian, field, 2, results[0]) + check_argument_count(args, 1, intrinsic)?; + let field = self.lookup_field(args[0], "call to to_bits")?; + self.to_radix(endian, args[0], field, 2, results[0]) } Intrinsic::ToRadix(endian) => { - assert_eq!(args.len(), 2); - assert_eq!(results.len(), 1); - let field = args[0].as_field().unwrap(); - let radix = args[1].as_u32().unwrap(); - self.to_radix(endian, field, radix, results[0]) + check_argument_count(args, 2, intrinsic)?; + let field = self.lookup_field(args[0], "call to to_bits")?; + let radix = self.lookup_u32(args[1], "call to to_bits")?; + self.to_radix(endian, args[0], field, radix, results[0]) } Intrinsic::BlackBox(black_box_func) => { todo!("Intrinsic::BlackBox({black_box_func}) is currently unimplemented") } Intrinsic::Hint(_) => todo!("Intrinsic::Hint is currently unimplemented"), Intrinsic::IsUnconstrained => { - assert_eq!(args.len(), 0); + check_argument_count(args, 0, intrinsic)?; Ok(vec![Value::bool(self.in_unconstrained_context())]) } Intrinsic::DerivePedersenGenerators => { todo!("Intrinsic::DerivePedersenGenerators is currently unimplemented") } Intrinsic::FieldLessThan => { - assert!( - self.in_unconstrained_context(), - "FieldLessThan can only be called in unconstrained" - ); - assert_eq!(args.len(), 2); - let lhs = args[0].as_field().unwrap(); - let rhs = args[1].as_field().unwrap(); + if !self.in_unconstrained_context() { + return Err(InterpreterError::Internal( + InternalError::FieldLessThanCalledInConstrainedContext, + )); + } + check_argument_count(args, 2, intrinsic)?; + let lhs = self.lookup_field(args[0], "lhs of call to field less than")?; + let rhs = self.lookup_field(args[1], "rhs of call to field less than")?; Ok(vec![Value::bool(lhs < rhs)]) } Intrinsic::ArrayRefCount | Intrinsic::SliceRefCount => { - let array = args[0].as_array_or_slice().unwrap(); + let array = self.lookup_array_or_slice(args[0], "array/slice ref count")?; let rc = *array.rc.borrow(); Ok(vec![Value::from_constant(rc.into(), NumericType::unsigned(32))]) } @@ -116,16 +115,23 @@ impl Interpreter<'_> { fn to_radix( &self, endian: Endian, + field_id: ValueId, field: FieldElement, radix: u32, result: ValueId, ) -> IResults { - let Type::Array(_, limb_count) = self.dfg().type_of_value(result) else { - unreachable!("Expected result of to_radix/to_bytes to be an array") + let result_type = self.dfg().type_of_value(result); + let Type::Array(_, limb_count) = result_type else { + return Err(InterpreterError::Internal(InternalError::TypeError { + value_id: result, + value: result_type.to_string(), + expected_type: "array", + instruction: "call to to_radix", + })); }; let Some(limbs) = dfg::simplify::constant_to_radix(endian, field, radix, limb_count) else { - panic!("Unable to convert `{field}` to radix `{radix}`") + return Err(InterpreterError::ToRadixFailed { field_id, field, radix }); }; let elements = vecmap(limbs, |limb| Value::from_constant(limb, NumericType::unsigned(8))); @@ -133,16 +139,16 @@ impl Interpreter<'_> { } /// (length, slice, elem...) -> (length, slice) - fn slice_push_back(&self, args: Vec) -> IResults { - let length = args[0].as_u32().unwrap(); - let slice = args[1].as_array_or_slice().unwrap(); + fn slice_push_back(&self, args: &[ValueId]) -> IResults { + let length = self.lookup_u32(args[0], "call to slice_push_back")?; + let slice = self.lookup_array_or_slice(args[1], "call to slice_push_back")?; // The resulting slice should be cloned - should we check RC here to try mutating it? // It'd need to be brillig-only if so since RC is always 1 in acir. let mut new_elements = slice.elements.borrow().to_vec(); let element_types = slice.element_types.clone(); - new_elements.extend(args.into_iter().skip(2)); + new_elements.extend(args.iter().skip(2).map(|arg| self.lookup(*arg))); let new_length = Value::Numeric(NumericValue::U32(length + 1)); let new_slice = Value::slice(new_elements, element_types); @@ -150,13 +156,13 @@ impl Interpreter<'_> { } /// (length, slice, elem...) -> (length, slice) - fn slice_push_front(&self, args: Vec) -> IResults { - let length = args[0].as_u32().unwrap(); - let slice = args[1].as_array_or_slice().unwrap(); + fn slice_push_front(&self, args: &[ValueId]) -> IResults { + let length = self.lookup_u32(args[0], "call to slice_push_front")?; + let slice = self.lookup_array_or_slice(args[1], "call to slice_push_front")?; let slice_elements = slice.elements.clone(); let element_types = slice.element_types.clone(); - let mut new_elements = args.into_iter().skip(2).collect::>(); + let mut new_elements = vecmap(args.iter().skip(2), |arg| self.lookup(*arg)); new_elements.extend_from_slice(&slice_elements.borrow()); let new_length = Value::Numeric(NumericValue::U32(length + 1)); @@ -165,18 +171,18 @@ impl Interpreter<'_> { } /// (length, slice) -> (length, slice, elem...) - fn slice_pop_back(&self, args: Vec) -> IResults { - let length = args[0].as_u32().unwrap(); - let slice = args[1].as_array_or_slice().unwrap(); + fn slice_pop_back(&self, args: &[ValueId]) -> IResults { + let length = self.lookup_u32(args[0], "call to slice_pop_back")?; + let slice = self.lookup_array_or_slice(args[1], "call to slice_pop_back")?; let mut slice_elements = slice.elements.borrow().to_vec(); let element_types = slice.element_types.clone(); if slice_elements.is_empty() { - panic!("slice_pop_back: empty slice"); + let instruction = "slice_pop_back"; + return Err(InterpreterError::PoppedFromEmptySlice { slice: args[1], instruction }); } - - assert!(slice_elements.len() >= element_types.len()); + check_slice_can_pop_all_element_types(args[1], &slice)?; let mut popped_elements = vecmap(0..element_types.len(), |_| slice_elements.pop().unwrap()); popped_elements.reverse(); @@ -189,18 +195,19 @@ impl Interpreter<'_> { } /// (length, slice) -> (elem..., length, slice) - fn slice_pop_front(&self, args: Vec) -> IResults { - let length = args[0].as_u32().unwrap(); - let slice = args[1].as_array_or_slice().unwrap(); + fn slice_pop_front(&self, args: &[ValueId]) -> IResults { + let length = self.lookup_u32(args[0], "call to slice_pop_front")?; + let slice = self.lookup_array_or_slice(args[1], "call to slice_pop_front")?; let mut slice_elements = slice.elements.borrow().to_vec(); let element_types = slice.element_types.clone(); if slice_elements.is_empty() { - panic!("slice_pop_front: empty slice"); + let instruction = "slice_pop_front"; + return Err(InterpreterError::PoppedFromEmptySlice { slice: args[1], instruction }); } + check_slice_can_pop_all_element_types(args[1], &slice)?; - assert!(slice_elements.len() >= element_types.len()); let mut results = slice_elements.drain(0..element_types.len()).collect::>(); let new_length = Value::Numeric(NumericValue::U32(length - 1)); @@ -211,17 +218,17 @@ impl Interpreter<'_> { } /// (length, slice, index:u32, elem...) -> (length, slice) - fn slice_insert(&self, args: Vec) -> IResults { - let length = args[0].as_u32().unwrap(); - let slice = args[1].as_array_or_slice().unwrap(); - let index = args[2].as_u32().unwrap(); + fn slice_insert(&self, args: &[ValueId]) -> IResults { + let length = self.lookup_u32(args[0], "call to slice_insert")?; + let slice = self.lookup_array_or_slice(args[1], "call to slice_insert")?; + let index = self.lookup_u32(args[2], "call to slice_insert")?; let mut slice_elements = slice.elements.borrow().to_vec(); let element_types = slice.element_types.clone(); let mut index = index as usize * element_types.len(); - for arg in args.into_iter().skip(3) { - slice_elements.insert(index, arg); + for arg in args.iter().skip(3) { + slice_elements.insert(index, self.lookup(*arg)); index += 1; } @@ -231,18 +238,19 @@ impl Interpreter<'_> { } /// (length, slice, index:u32) -> (length, slice, elem...) - fn slice_remove(&self, args: Vec) -> IResults { - let length = args[0].as_u32().unwrap(); - let slice = args[1].as_array_or_slice().unwrap(); - let index = args[2].as_u32().unwrap(); + fn slice_remove(&self, args: &[ValueId]) -> IResults { + let length = self.lookup_u32(args[0], "call to slice_remove")?; + let slice = self.lookup_array_or_slice(args[1], "call to slice_remove")?; + let index = self.lookup_u32(args[2], "call to slice_remove")?; let mut slice_elements = slice.elements.borrow().to_vec(); let element_types = slice.element_types.clone(); if slice_elements.is_empty() { - panic!("slice_remove: empty slice"); + let instruction = "slice_remove"; + return Err(InterpreterError::PoppedFromEmptySlice { slice: args[1], instruction }); } - assert!(slice_elements.len() >= element_types.len()); + check_slice_can_pop_all_element_types(args[1], &slice)?; let index = index as usize * element_types.len(); let removed: Vec<_> = slice_elements.drain(index..index + element_types.len()).collect(); @@ -260,3 +268,33 @@ impl Interpreter<'_> { Ok(Vec::new()) } } + +fn check_argument_count( + args: &[ValueId], + expected_count: usize, + intrinsic: Intrinsic, +) -> IResult<()> { + if args.len() != expected_count { + Err(InterpreterError::Internal(InternalError::IntrinsicArgumentCountMismatch { + intrinsic, + arguments: args.len(), + parameters: expected_count, + })) + } else { + Ok(()) + } +} + +fn check_slice_can_pop_all_element_types(slice_id: ValueId, slice: &ArrayValue) -> IResult<()> { + let actual_length = slice.elements.borrow().len(); + if actual_length >= slice.element_types.len() { + Ok(()) + } else { + Err(InterpreterError::Internal(InternalError::NotEnoughElementsToPopSliceOfStructs { + slice_id, + slice: slice.to_string(), + actual_length, + element_types: vecmap(slice.element_types.iter(), ToString::to_string), + })) + } +} diff --git a/compiler/noirc_evaluator/src/ssa/interpreter/mod.rs b/compiler/noirc_evaluator/src/ssa/interpreter/mod.rs index 95c79d05e94..94ee799dac3 100644 --- a/compiler/noirc_evaluator/src/ssa/interpreter/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/interpreter/mod.rs @@ -10,13 +10,15 @@ use super::{ value::ValueId, }, }; -use crate::{errors::RuntimeError, ssa::ir::instruction::binary::truncate_field}; +use crate::ssa::ir::{instruction::binary::truncate_field, printer::display_binary}; use acvm::{AcirField, FieldElement}; +use errors::{InternalError, InterpreterError, MAX_SIGNED_BIT_SIZE, MAX_UNSIGNED_BIT_SIZE}; use fxhash::FxHashMap as HashMap; use iter_extended::vecmap; use noirc_frontend::Shared; -use value::{ArrayValue, NumericValue}; +use value::{ArrayValue, NumericValue, ReferenceValue}; +mod errors; mod intrinsics; mod tests; pub mod value; @@ -56,7 +58,7 @@ impl CallContext { } } -type IResult = Result; +type IResult = Result; type IResults = IResult>; #[allow(unused)] @@ -153,7 +155,11 @@ impl<'ssa> Interpreter<'ssa> { let block = &dfg[block_id]; if arguments.len() != block.parameters().len() { - panic!("Block argument count does not match the expected parameter count"); + return Err(internal(InternalError::BlockArgumentCountMismatch { + block: block_id, + arguments: arguments.len(), + parameters: block.parameters().len(), + })); } for (parameter, argument) in block.parameters().iter().zip(arguments) { @@ -166,7 +172,11 @@ impl<'ssa> Interpreter<'ssa> { } match block.terminator() { - None => panic!("No block terminator in block {block_id}"), + None => { + return Err(internal(InternalError::BlockMissingTerminator { + block: block_id, + })); + } Some(TerminatorInstruction::Jmp { destination, arguments: jump_args, .. }) => { block_id = *destination; arguments = self.lookup_all(jump_args); @@ -177,7 +187,7 @@ impl<'ssa> Interpreter<'ssa> { else_destination, call_stack: _, }) => { - block_id = if self.lookup(*condition).as_bool().unwrap() { + block_id = if self.lookup_bool(*condition, "jmpif condition")? { *then_destination } else { *else_destination @@ -218,6 +228,68 @@ impl<'ssa> Interpreter<'ssa> { } } + fn lookup_helper( + &self, + value_id: ValueId, + instruction: &'static str, + expected_type: &'static str, + convert: impl FnOnce(&Value) -> Option, + ) -> IResult { + let value = self.lookup(value_id); + match convert(&value) { + Some(value) => Ok(value), + None => { + let value = value.to_string(); + Err(internal(InternalError::TypeError { + value_id, + value, + expected_type, + instruction, + })) + } + } + } + + fn lookup_bool(&self, value_id: ValueId, instruction: &'static str) -> IResult { + self.lookup_helper(value_id, instruction, "bool", Value::as_bool) + } + + fn lookup_u32(&self, value_id: ValueId, instruction: &'static str) -> IResult { + self.lookup_helper(value_id, instruction, "u32", Value::as_u32) + } + + fn lookup_field(&self, value_id: ValueId, instruction: &'static str) -> IResult { + self.lookup_helper(value_id, instruction, "Field", Value::as_field) + } + + fn lookup_numeric( + &self, + value_id: ValueId, + instruction: &'static str, + ) -> IResult { + self.lookup_helper(value_id, instruction, "numeric", Value::as_numeric) + } + + fn lookup_array_or_slice( + &self, + value_id: ValueId, + instruction: &'static str, + ) -> IResult { + self.lookup_helper(value_id, instruction, "array or slice", Value::as_array_or_slice) + } + + fn lookup_string(&self, value_id: ValueId, instruction: &'static str) -> IResult { + self.lookup_helper(value_id, instruction, "string", Value::as_string) + } + + fn lookup_reference( + &self, + value_id: ValueId, + instruction: &'static str, + ) -> IResult { + self.lookup_helper(value_id, instruction, "reference", Value::as_reference) + } + fn lookup_all(&self, ids: &[ValueId]) -> Vec { vecmap(ids, |id| self.lookup(*id)) } @@ -234,53 +306,67 @@ impl<'ssa> Interpreter<'ssa> { &mut self, instruction: &Instruction, results: &[ValueId], - ) -> Result<(), RuntimeError> { + ) -> IResult<()> { match instruction { Instruction::Binary(binary) => { let result = self.interpret_binary(binary)?; self.define(results[0], result); + Ok(()) } // Cast in SSA changes the type without altering the value Instruction::Cast(value, numeric_type) => { - let field = self.lookup(*value).as_numeric().unwrap().convert_to_field(); + let field = self.lookup_numeric(*value, "cast")?.convert_to_field(); let result = Value::Numeric(NumericValue::from_constant(field, *numeric_type)); self.define(results[0], result); + Ok(()) } Instruction::Not(id) => self.interpret_not(*id, results[0]), Instruction::Truncate { value, bit_size, max_bit_size } => { - self.interpret_truncate(*value, *bit_size, *max_bit_size, results[0]); + self.interpret_truncate(*value, *bit_size, *max_bit_size, results[0]) } - Instruction::Constrain(lhs, rhs, constrain_error) => { - let lhs = self.lookup(*lhs); - let rhs = self.lookup(*rhs); + Instruction::Constrain(lhs_id, rhs_id, constrain_error) => { + let lhs = self.lookup(*lhs_id); + let rhs = self.lookup(*rhs_id); if self.side_effects_enabled() && lhs != rhs { - panic!("Constrain {lhs} == {rhs} failed!"); + let lhs = lhs.to_string(); + let rhs = rhs.to_string(); + let lhs_id = *lhs_id; + let rhs_id = *rhs_id; + return Err(InterpreterError::ConstrainEqFailed { lhs, lhs_id, rhs, rhs_id }); } + Ok(()) } - Instruction::ConstrainNotEqual(lhs, rhs, constrain_error) => { - let lhs = self.lookup(*lhs); - let rhs = self.lookup(*rhs); + Instruction::ConstrainNotEqual(lhs_id, rhs_id, constrain_error) => { + let lhs = self.lookup(*lhs_id); + let rhs = self.lookup(*rhs_id); if self.side_effects_enabled() && lhs == rhs { - panic!("Constrain {lhs} != {rhs} failed!"); + let lhs = lhs.to_string(); + let rhs = rhs.to_string(); + let lhs_id = *lhs_id; + let rhs_id = *rhs_id; + return Err(InterpreterError::ConstrainNeFailed { lhs, lhs_id, rhs, rhs_id }); } + Ok(()) } Instruction::RangeCheck { value, max_bit_size, assert_message } => { - self.interpret_range_check(*value, *max_bit_size, assert_message.as_ref()); + self.interpret_range_check(*value, *max_bit_size, assert_message.as_ref()) } - Instruction::Call { func, arguments } => { - self.interpret_call(*func, arguments, results)?; + Instruction::Call { func, arguments } => self.interpret_call(*func, arguments, results), + Instruction::Allocate => { + self.interpret_allocate(results[0]); + Ok(()) } - Instruction::Allocate => self.interpret_allocate(results[0]), Instruction::Load { address } => self.interpret_load(*address, results[0]), Instruction::Store { address, value } => self.interpret_store(*address, *value), Instruction::EnableSideEffectsIf { condition } => { - self.side_effects_enabled = self.lookup(*condition).as_bool().unwrap(); + self.side_effects_enabled = self.lookup_bool(*condition, "enable_side_effects")?; + Ok(()) } Instruction::ArrayGet { array, index } => { - self.interpret_array_get(*array, *index, results[0]); + self.interpret_array_get(*array, *index, results[0]) } Instruction::ArraySet { array, index, value, mutable } => { - self.interpret_array_set(*array, *index, *value, *mutable, results[0]); + self.interpret_array_set(*array, *index, *value, *mutable, results[0]) } Instruction::IncrementRc { value } => self.interpret_inc_rc(*value), Instruction::DecrementRc { value } => self.interpret_dec_rc(*value), @@ -294,16 +380,19 @@ impl<'ssa> Interpreter<'ssa> { ), Instruction::MakeArray { elements, typ } => { self.interpret_make_array(elements, results[0], typ); + Ok(()) } - Instruction::Noop => (), + Instruction::Noop => Ok(()), } - Ok(()) } - fn interpret_not(&mut self, id: ValueId, result: ValueId) { - let new_result = match self.lookup(id).as_numeric().unwrap() { - NumericValue::Field(field) => { - unreachable!("not: Expected integer value, found field {field}") + fn interpret_not(&mut self, id: ValueId, result: ValueId) -> IResult<()> { + let new_result = match self.lookup_numeric(id, "not instruction")? { + NumericValue::Field(_) => { + return Err(internal(InternalError::UnsupportedOperatorForType { + operator: "!", + typ: "Field", + })); } NumericValue::U1(value) => NumericValue::U1(!value), NumericValue::U8(value) => NumericValue::U8(!value), @@ -317,48 +406,54 @@ impl<'ssa> Interpreter<'ssa> { NumericValue::I64(value) => NumericValue::I64(!value), }; self.define(result, Value::Numeric(new_result)); + Ok(()) } fn interpret_truncate( &mut self, - value: ValueId, + value_id: ValueId, bit_size: u32, - _max_bit_size: u32, + max_bit_size: u32, result: ValueId, - ) { - let value = self.lookup(value).as_numeric().unwrap(); - let bit_mask = (1u128 << bit_size) - 1; - assert_ne!(bit_mask, 0); + ) -> IResult<()> { + let value = self.lookup_numeric(value_id, "truncate")?; + if bit_size == 0 { + return Err(internal(InternalError::TruncateToZeroBits { value_id, max_bit_size })); + } let truncated = match value { NumericValue::Field(value) => NumericValue::Field(truncate_field(value, bit_size)), NumericValue::U1(value) => NumericValue::U1(value), - NumericValue::U8(value) => NumericValue::U8(truncate_unsigned(value, bit_size)), - NumericValue::U16(value) => NumericValue::U16(truncate_unsigned(value, bit_size)), - NumericValue::U32(value) => NumericValue::U32(truncate_unsigned(value, bit_size)), - NumericValue::U64(value) => NumericValue::U64(truncate_unsigned(value, bit_size)), - NumericValue::U128(value) => NumericValue::U128(truncate_unsigned(value, bit_size)), - NumericValue::I8(value) => NumericValue::I8(truncate_signed(value, bit_size)), - NumericValue::I16(value) => NumericValue::I16(truncate_signed(value, bit_size)), - NumericValue::I32(value) => NumericValue::I32(truncate_signed(value, bit_size)), - NumericValue::I64(value) => NumericValue::I64(truncate_signed(value, bit_size)), + NumericValue::U8(value) => NumericValue::U8(truncate_unsigned(value, bit_size)?), + NumericValue::U16(value) => NumericValue::U16(truncate_unsigned(value, bit_size)?), + NumericValue::U32(value) => NumericValue::U32(truncate_unsigned(value, bit_size)?), + NumericValue::U64(value) => NumericValue::U64(truncate_unsigned(value, bit_size)?), + NumericValue::U128(value) => NumericValue::U128(truncate_unsigned(value, bit_size)?), + NumericValue::I8(value) => NumericValue::I8(truncate_signed(value, bit_size)?), + NumericValue::I16(value) => NumericValue::I16(truncate_signed(value, bit_size)?), + NumericValue::I32(value) => NumericValue::I32(truncate_signed(value, bit_size)?), + NumericValue::I64(value) => NumericValue::I64(truncate_signed(value, bit_size)?), }; self.define(result, Value::Numeric(truncated)); + Ok(()) } fn interpret_range_check( &mut self, - value: ValueId, + value_id: ValueId, max_bit_size: u32, error_message: Option<&String>, - ) { + ) -> IResult<()> { if !self.side_effects_enabled() { - return; + return Ok(()); + } + + if max_bit_size == 0 { + return Err(internal(InternalError::RangeCheckToZeroBits { value_id })); } - let value = self.lookup(value).as_numeric().unwrap(); - assert_ne!(max_bit_size, 0); + let value = self.lookup_numeric(value_id, "range check")?; fn bit_count(x: impl Into) -> u32 { let x = x.into(); @@ -368,7 +463,7 @@ impl<'ssa> Interpreter<'ssa> { let bit_count = match value { NumericValue::Field(value) => value.num_bits(), // max_bit_size > 0 so u1 should always pass these checks - NumericValue::U1(_) => return, + NumericValue::U1(_) => return Ok(()), NumericValue::U8(value) => bit_count(value), NumericValue::U16(value) => bit_count(value), NumericValue::U32(value) => bit_count(value), @@ -396,23 +491,33 @@ impl<'ssa> Interpreter<'ssa> { }; if bit_count > max_bit_size { + let value = value.to_string(); + let actual_bits = bit_count; + let max_bits = max_bit_size; + if let Some(message) = error_message { - panic!( - "bit count of {bit_count} exceeded max bit count of {max_bit_size}\n{message}" - ); + Err(InterpreterError::RangeCheckFailedWithMessage { + value, + value_id, + actual_bits, + max_bits, + message: message.clone(), + }) } else { - panic!("bit count of {bit_count} exceeded max bit count of {max_bit_size}"); + Err(InterpreterError::RangeCheckFailed { value, value_id, actual_bits, max_bits }) } + } else { + Ok(()) } } fn interpret_call( &mut self, - function: ValueId, + function_id: ValueId, argument_ids: &[ValueId], results: &[ValueId], ) -> IResult<()> { - let function = self.lookup(function); + let function = self.lookup(function_id); let mut arguments = vecmap(argument_ids, |argument| self.lookup(*argument)); let new_results = if self.side_effects_enabled() { @@ -424,18 +529,25 @@ impl<'ssa> Interpreter<'ssa> { if !self.in_unconstrained_context() && self.ssa.functions[&id].runtime().is_brillig() { - arguments.iter_mut().for_each(Self::reset_array_state); + for argument in arguments.iter_mut() { + Self::reset_array_state(argument)?; + } } self.call_function(id, arguments)? } Value::Intrinsic(intrinsic) => { - self.call_intrinsic(intrinsic, arguments, results)? + self.call_intrinsic(intrinsic, argument_ids, results)? } Value::ForeignFunction(name) if name == "print" => self.call_print(arguments)?, Value::ForeignFunction(name) => { - todo!("call: ForeignFunction({name}) is not yet implemented") + return Err(InterpreterError::UnknownForeignFunctionCall { name }); + } + other => { + return Err(internal(InternalError::CalledNonFunction { + value: other.to_string(), + value_id: function_id, + })); } - other => panic!("call: Expected function, found {other:?}"), } } else { vecmap(results, |result| { @@ -444,33 +556,59 @@ impl<'ssa> Interpreter<'ssa> { }) }; - assert_eq!(new_results.len(), results.len()); + if new_results.len() != results.len() { + let function_name = self.try_get_function_name(function_id); + return Err(internal(InternalError::FunctionReturnedIncorrectArgCount { + function: function_id, + function_name, + expected: results.len(), + actual: new_results.len(), + })); + } + for (result, new_result) in results.iter().zip(new_results) { self.define(*result, new_result); } Ok(()) } + /// Try to get a function's name or approximate it if it is not known + fn try_get_function_name(&self, function: ValueId) -> String { + match self.lookup(function) { + Value::Function(id) => match self.ssa.functions.get(&id) { + Some(function) => function.name().to_string(), + None => "unknown function".to_string(), + }, + Value::Intrinsic(intrinsic) => intrinsic.to_string(), + Value::ForeignFunction(name) => name, + _ => "non-function".to_string(), + } + } + /// Reset the value's `Shared` states in each array within. This is used to mimic each /// invocation of the brillig vm receiving fresh values. No matter the history of this value /// (e.g. even if they were previously returned from another brillig function) the reference /// count should always be 1 and it shouldn't alias any other arrays. - fn reset_array_state(value: &mut Value) { + fn reset_array_state(value: &mut Value) -> IResult<()> { match value { Value::Numeric(_) | Value::Function(_) | Value::Intrinsic(_) - | Value::ForeignFunction(_) => (), + | Value::ForeignFunction(_) => Ok(()), - Value::Reference(_) => panic!( - "No reference values are allowed when crossing the constrained -> unconstrained boundary" - ), + Value::Reference(value) => { + let value = value.to_string(); + Err(internal(InternalError::ReferenceValueCrossedUnconstrainedBoundary { value })) + } Value::ArrayOrSlice(array_value) => { let mut elements = array_value.elements.borrow().to_vec(); - elements.iter_mut().for_each(Self::reset_array_state); + for element in elements.iter_mut() { + Self::reset_array_state(element)?; + } array_value.elements = Shared::new(elements); array_value.rc = Shared::new(1); + Ok(()) } } } @@ -486,39 +624,42 @@ impl<'ssa> Interpreter<'ssa> { self.define(result, Value::reference(result, element_type)); } - fn interpret_load(&mut self, address: ValueId, result: ValueId) { - let address = self.lookup(address); - let address = address.as_reference().unwrap(); + fn interpret_load(&mut self, address: ValueId, result: ValueId) -> IResult<()> { + let address = self.lookup_reference(address, "load")?; let element = address.element.borrow(); let Some(value) = &*element else { - panic!( - "reference value {} is being loaded before it was stored to", - address.original_id - ); + let value = address.to_string(); + return Err(internal(InternalError::UninitializedReferenceValueLoaded { value })); }; self.define(result, value.clone()); + Ok(()) } - fn interpret_store(&mut self, address: ValueId, value: ValueId) { - let address = self.lookup(address); - let address = address.as_reference().unwrap(); + fn interpret_store(&mut self, address: ValueId, value: ValueId) -> IResult<()> { + let address = self.lookup_reference(address, "store")?; let value = self.lookup(value); *address.element.borrow_mut() = Some(value); + Ok(()) } - fn interpret_array_get(&mut self, array: ValueId, index: ValueId, result: ValueId) { + fn interpret_array_get( + &mut self, + array: ValueId, + index: ValueId, + result: ValueId, + ) -> IResult<()> { let element = if self.side_effects_enabled() { - let array = self.lookup(array); - let array = array.as_array_or_slice().unwrap(); - let index = self.lookup(index).as_u32().unwrap(); + let array = self.lookup_array_or_slice(array, "array get")?; + let index = self.lookup_u32(index, "array get index")?; array.elements.borrow()[index as usize].clone() } else { let typ = self.dfg().type_of_value(result); Value::uninitialized(&typ, result) }; self.define(result, element); + Ok(()) } fn interpret_array_set( @@ -528,11 +669,11 @@ impl<'ssa> Interpreter<'ssa> { value: ValueId, mutable: bool, result: ValueId, - ) { + ) -> IResult<()> { + let array = self.lookup_array_or_slice(array, "array set")?; + let result_array = if self.side_effects_enabled() { - let array = self.lookup(array); - let array = array.as_array_or_slice().unwrap(); - let index = self.lookup(index).as_u32().unwrap(); + let index = self.lookup_u32(index, "array set index")?; let value = self.lookup(value); let should_mutate = @@ -552,43 +693,48 @@ impl<'ssa> Interpreter<'ssa> { } } else { // Side effects are disabled, return the original array - self.lookup(array) + Value::ArrayOrSlice(array) }; self.define(result, result_array); + Ok(()) } - fn interpret_inc_rc(&self, array: ValueId) { + fn interpret_inc_rc(&self, value_id: ValueId) -> IResult<()> { if self.in_unconstrained_context() { - let array = self.lookup(array); - let array = array.as_array_or_slice().unwrap(); + let array = self.lookup_array_or_slice(value_id, "inc_rc")?; let mut rc = array.rc.borrow_mut(); - - assert_ne!(*rc, 0, "inc_rc: increment from 0 back to 1 detected"); + if *rc == 0 { + let value = array.to_string(); + return Err(InterpreterError::IncRcRevive { value_id, value }); + } *rc += 1; } + Ok(()) } - fn interpret_dec_rc(&self, array: ValueId) { + fn interpret_dec_rc(&self, value_id: ValueId) -> IResult<()> { if self.in_unconstrained_context() { - let array = self.lookup(array); - let array = array.as_array_or_slice().unwrap(); + let array = self.lookup_array_or_slice(value_id, "dec_rc")?; let mut rc = array.rc.borrow_mut(); - - assert_ne!(*rc, 0, "dec_rc: underflow detected"); + if *rc == 0 { + let value = array.to_string(); + return Err(InterpreterError::DecRcUnderflow { value_id, value }); + } *rc -= 1; } + Ok(()) } fn interpret_if_else( &mut self, - then_condition: ValueId, + then_condition_id: ValueId, then_value: ValueId, - else_condition: ValueId, + else_condition_id: ValueId, else_value: ValueId, result: ValueId, - ) { - let then_condition = self.lookup(then_condition).as_bool().unwrap(); - let else_condition = self.lookup(else_condition).as_bool().unwrap(); + ) -> IResult<()> { + let then_condition = self.lookup_bool(then_condition_id, "then condition")?; + let else_condition = self.lookup_bool(else_condition_id, "else condition")?; let then_value = self.lookup(then_value); let else_value = self.lookup(else_value); @@ -597,7 +743,12 @@ impl<'ssa> Interpreter<'ssa> { // then_condition = outer_condition & a // else_condition = outer_condition & !a // If `outer_condition` is false, both will be false. - assert!(!then_condition || !else_condition); + if then_condition && else_condition { + return Err(InterpreterError::DoubleTrueIfElse { + then_condition_id, + else_condition_id, + }); + } let new_result = if !then_condition && !else_condition { // Returning uninitialized/zero if both conditions are false to match @@ -611,6 +762,7 @@ impl<'ssa> Interpreter<'ssa> { }; self.define(result, new_result); + Ok(()) } fn interpret_make_array( @@ -633,11 +785,13 @@ impl<'ssa> Interpreter<'ssa> { } macro_rules! apply_int_binop { - ($lhs:expr, $rhs:expr, $f:expr) => {{ + ($lhs:expr, $rhs:expr, $binary:expr, $f:expr) => {{ use value::NumericValue::*; match ($lhs, $rhs) { - (Field(_), Field(_)) => panic!("Expected only integer values, found field values"), - (U1(_), U1(_)) => panic!("Expected only large integer values, found u1"), + (Field(_), Field(_)) => { + unreachable!("Expected only integer values, found field values") + } + (U1(_), U1(_)) => unreachable!("Expected only large integer values, found u1"), (U8(lhs), U8(rhs)) => U8($f(&lhs, &rhs)), (U16(lhs), U16(rhs)) => U16($f(&lhs, &rhs)), (U32(lhs), U32(rhs)) => U32($f(&lhs, &rhs)), @@ -647,38 +801,78 @@ macro_rules! apply_int_binop { (I16(lhs), I16(rhs)) => I16($f(&lhs, &rhs)), (I32(lhs), I32(rhs)) => I32($f(&lhs, &rhs)), (I64(lhs), I64(rhs)) => I64($f(&lhs, &rhs)), - (lhs, rhs) => panic!("Got mismatched types in binop: {lhs:?} and {rhs:?}"), + (lhs, rhs) => { + let binary = $binary; + return Err(internal(InternalError::MismatchedTypesInBinaryOperator { + lhs: lhs.to_string(), + rhs: rhs.to_string(), + operator: binary.operator, + lhs_id: binary.lhs, + rhs_id: binary.rhs, + })); + } } }}; } macro_rules! apply_int_binop_opt { - ($lhs:expr, $rhs:expr, $f:expr) => {{ + ($dfg:expr, $lhs:expr, $rhs:expr, $binary:expr, $f:expr) => {{ use value::NumericValue::*; - // TODO: Error if None instead of unwrapping - match ($lhs, $rhs) { - (Field(_), Field(_)) => panic!("Expected only integer values, found field values"), - (U1(_), U1(_)) => panic!("Expected only large integer values, found u1"), - (U8(lhs), U8(rhs)) => U8($f(&lhs, &rhs).unwrap()), - (U16(lhs), U16(rhs)) => U16($f(&lhs, &rhs).unwrap()), - (U32(lhs), U32(rhs)) => U32($f(&lhs, &rhs).unwrap()), - (U64(lhs), U64(rhs)) => U64($f(&lhs, &rhs).unwrap()), - (U128(lhs), U128(rhs)) => U128($f(&lhs, &rhs).unwrap()), - (I8(lhs), I8(rhs)) => I8($f(&lhs, &rhs).unwrap()), - (I16(lhs), I16(rhs)) => I16($f(&lhs, &rhs).unwrap()), - (I32(lhs), I32(rhs)) => I32($f(&lhs, &rhs).unwrap()), - (I64(lhs), I64(rhs)) => I64($f(&lhs, &rhs).unwrap()), - (lhs, rhs) => panic!("Got mismatched types in binop: {lhs:?} and {rhs:?}"), + + let lhs = $lhs; + let rhs = $rhs; + let binary = $binary; + let operator = binary.operator; + + let overflow = || { + if matches!(binary.operator, BinaryOp::Div | BinaryOp::Mod) { + let lhs_id = binary.lhs; + let rhs_id = binary.rhs; + let lhs = lhs.to_string(); + let rhs = rhs.to_string(); + InterpreterError::DivisionByZero { lhs_id, lhs, rhs_id, rhs } + } else { + let instruction = + format!("`{}` ({operator} {lhs}, {rhs})", display_binary(binary, $dfg)); + InterpreterError::Overflow { instruction } + } + }; + + match (lhs, rhs) { + (Field(_), Field(_)) => { + unreachable!("Expected only integer values, found field values") + } + (U1(_), U1(_)) => unreachable!("Expected only large integer values, found u1"), + (U8(lhs), U8(rhs)) => U8($f(&lhs, &rhs).ok_or_else(overflow)?), + (U16(lhs), U16(rhs)) => U16($f(&lhs, &rhs).ok_or_else(overflow)?), + (U32(lhs), U32(rhs)) => U32($f(&lhs, &rhs).ok_or_else(overflow)?), + (U64(lhs), U64(rhs)) => U64($f(&lhs, &rhs).ok_or_else(overflow)?), + (U128(lhs), U128(rhs)) => U128($f(&lhs, &rhs).ok_or_else(overflow)?), + (I8(lhs), I8(rhs)) => I8($f(&lhs, &rhs).ok_or_else(overflow)?), + (I16(lhs), I16(rhs)) => I16($f(&lhs, &rhs).ok_or_else(overflow)?), + (I32(lhs), I32(rhs)) => I32($f(&lhs, &rhs).ok_or_else(overflow)?), + (I64(lhs), I64(rhs)) => I64($f(&lhs, &rhs).ok_or_else(overflow)?), + (lhs, rhs) => { + return Err(internal(InternalError::MismatchedTypesInBinaryOperator { + lhs: lhs.to_string(), + rhs: rhs.to_string(), + operator, + lhs_id: binary.lhs, + rhs_id: binary.rhs, + })); + } } }}; } macro_rules! apply_int_comparison_op { - ($lhs:expr, $rhs:expr, $f:expr) => {{ + ($lhs:expr, $rhs:expr, $binary:expr, $f:expr) => {{ use NumericValue::*; match ($lhs, $rhs) { - (Field(_), Field(_)) => panic!("Expected only integer values, found field values"), - (U1(_), U1(_)) => panic!("Expected only large integer values, found u1"), + (Field(_), Field(_)) => { + unreachable!("Expected only integer values, found field values") + } + (U1(_), U1(_)) => unreachable!("Expected only large integer values, found u1"), (U8(lhs), U8(rhs)) => U1($f(&lhs, &rhs)), (U16(lhs), U16(rhs)) => U1($f(&lhs, &rhs)), (U32(lhs), U32(rhs)) => U1($f(&lhs, &rhs)), @@ -688,28 +882,37 @@ macro_rules! apply_int_comparison_op { (I16(lhs), I16(rhs)) => U1($f(&lhs, &rhs)), (I32(lhs), I32(rhs)) => U1($f(&lhs, &rhs)), (I64(lhs), I64(rhs)) => U1($f(&lhs, &rhs)), - (lhs, rhs) => panic!("Got mismatched types in binop: {lhs:?} and {rhs:?}"), + (lhs, rhs) => { + let binary = $binary; + return Err(internal(InternalError::MismatchedTypesInBinaryOperator { + lhs: lhs.to_string(), + rhs: rhs.to_string(), + operator: binary.operator, + lhs_id: binary.lhs, + rhs_id: binary.rhs, + })); + } } }}; } impl Interpreter<'_> { fn interpret_binary(&mut self, binary: &Binary) -> IResult { - // TODO: Replace unwrap with real error - let lhs = self.lookup(binary.lhs).as_numeric().unwrap(); - let rhs = self.lookup(binary.rhs).as_numeric().unwrap(); + let lhs_id = binary.lhs; + let rhs_id = binary.rhs; + let lhs = self.lookup_numeric(lhs_id, "binary op lhs")?; + let rhs = self.lookup_numeric(rhs_id, "binary op rhs")?; if lhs.get_type() != rhs.get_type() && !matches!(binary.operator, BinaryOp::Shl | BinaryOp::Shr) { - panic!( - "Type error in ({}: {}) {} ({}: {})", - binary.lhs, - lhs.get_type(), - binary.operator, - binary.rhs, - rhs.get_type() - ) + return Err(internal(InternalError::MismatchedTypesInBinaryOperator { + lhs_id, + lhs: lhs.to_string(), + operator: binary.operator, + rhs_id, + rhs: rhs.to_string(), + })); } // Disable this instruction if it is side-effectful and side effects are disabled. @@ -719,75 +922,118 @@ impl Interpreter<'_> { } if let (Some(lhs), Some(rhs)) = (lhs.as_field(), rhs.as_field()) { - return self.interpret_field_binary_op(lhs, binary.operator, rhs); + return self.interpret_field_binary_op(lhs, binary.operator, rhs, lhs_id, rhs_id); } if let (Some(lhs), Some(rhs)) = (lhs.as_bool(), rhs.as_bool()) { return self.interpret_u1_binary_op(lhs, binary.operator, rhs); } + let dfg = self.dfg(); let result = match binary.operator { BinaryOp::Add { unchecked: false } => { - apply_int_binop_opt!(lhs, rhs, num_traits::CheckedAdd::checked_add) + apply_int_binop_opt!(dfg, lhs, rhs, binary, num_traits::CheckedAdd::checked_add) } BinaryOp::Add { unchecked: true } => { - apply_int_binop!(lhs, rhs, num_traits::WrappingAdd::wrapping_add) + apply_int_binop!(lhs, rhs, binary, num_traits::WrappingAdd::wrapping_add) } BinaryOp::Sub { unchecked: false } => { - apply_int_binop_opt!(lhs, rhs, num_traits::CheckedSub::checked_sub) + apply_int_binop_opt!(dfg, lhs, rhs, binary, num_traits::CheckedSub::checked_sub) } BinaryOp::Sub { unchecked: true } => { - apply_int_binop!(lhs, rhs, num_traits::WrappingSub::wrapping_sub) + apply_int_binop!(lhs, rhs, binary, num_traits::WrappingSub::wrapping_sub) } BinaryOp::Mul { unchecked: false } => { - apply_int_binop_opt!(lhs, rhs, num_traits::CheckedMul::checked_mul) + apply_int_binop_opt!(dfg, lhs, rhs, binary, num_traits::CheckedMul::checked_mul) } BinaryOp::Mul { unchecked: true } => { - apply_int_binop!(lhs, rhs, num_traits::WrappingMul::wrapping_mul) + apply_int_binop!(lhs, rhs, binary, num_traits::WrappingMul::wrapping_mul) } BinaryOp::Div => { - apply_int_binop_opt!(lhs, rhs, num_traits::CheckedDiv::checked_div) + apply_int_binop_opt!(dfg, lhs, rhs, binary, num_traits::CheckedDiv::checked_div) } BinaryOp::Mod => { - apply_int_binop_opt!(lhs, rhs, num_traits::CheckedRem::checked_rem) + apply_int_binop_opt!(dfg, lhs, rhs, binary, num_traits::CheckedRem::checked_rem) } - BinaryOp::Eq => apply_int_comparison_op!(lhs, rhs, |a, b| a == b), - BinaryOp::Lt => apply_int_comparison_op!(lhs, rhs, |a, b| a < b), + BinaryOp::Eq => apply_int_comparison_op!(lhs, rhs, binary, |a, b| a == b), + BinaryOp::Lt => apply_int_comparison_op!(lhs, rhs, binary, |a, b| a < b), BinaryOp::And => { - apply_int_binop!(lhs, rhs, std::ops::BitAnd::bitand) + apply_int_binop!(lhs, rhs, binary, std::ops::BitAnd::bitand) } BinaryOp::Or => { - apply_int_binop!(lhs, rhs, std::ops::BitOr::bitor) + apply_int_binop!(lhs, rhs, binary, std::ops::BitOr::bitor) } BinaryOp::Xor => { - apply_int_binop!(lhs, rhs, std::ops::BitXor::bitxor) + apply_int_binop!(lhs, rhs, binary, std::ops::BitXor::bitxor) } BinaryOp::Shl => { - let rhs = rhs.as_u32().expect("Expected rhs of shl to be a u32"); - let overflow_msg = "Overflow when evaluating `shl`, `rhs` is too large"; + let Some(rhs) = rhs.as_u8() else { + let rhs = rhs.to_string(); + return Err(internal(InternalError::RhsOfBitShiftShouldBeU8 { + operator: "<<", + rhs_id, + rhs, + })); + }; + + let overflow = || { + let instruction = + format!("`{}` ({lhs} << {rhs})", display_binary(binary, self.dfg())); + InterpreterError::Overflow { instruction } + }; + + let rhs = rhs as u32; use NumericValue::*; match lhs { - Field(_) => unreachable!("<< is not implemented for Field"), - U1(_) => unreachable!("<< is not implemented for u1"), - U8(value) => U8(value.checked_shl(rhs).expect(overflow_msg)), - U16(value) => U16(value.checked_shl(rhs).expect(overflow_msg)), - U32(value) => U32(value.checked_shl(rhs).expect(overflow_msg)), - U64(value) => U64(value.checked_shl(rhs).expect(overflow_msg)), - U128(value) => U128(value.checked_shl(rhs).expect(overflow_msg)), - I8(value) => I8(value.checked_shl(rhs).expect(overflow_msg)), - I16(value) => I16(value.checked_shl(rhs).expect(overflow_msg)), - I32(value) => I32(value.checked_shl(rhs).expect(overflow_msg)), - I64(value) => I64(value.checked_shl(rhs).expect(overflow_msg)), + Field(_) => { + return Err(internal(InternalError::UnsupportedOperatorForType { + operator: "<<", + typ: "Field", + })); + } + U1(_) => { + return Err(internal(InternalError::UnsupportedOperatorForType { + operator: "<<", + typ: "u1", + })); + } + U8(value) => U8(value.checked_shl(rhs).ok_or_else(overflow)?), + U16(value) => U16(value.checked_shl(rhs).ok_or_else(overflow)?), + U32(value) => U32(value.checked_shl(rhs).ok_or_else(overflow)?), + U64(value) => U64(value.checked_shl(rhs).ok_or_else(overflow)?), + U128(value) => U128(value.checked_shl(rhs).ok_or_else(overflow)?), + I8(value) => I8(value.checked_shl(rhs).ok_or_else(overflow)?), + I16(value) => I16(value.checked_shl(rhs).ok_or_else(overflow)?), + I32(value) => I32(value.checked_shl(rhs).ok_or_else(overflow)?), + I64(value) => I64(value.checked_shl(rhs).ok_or_else(overflow)?), } } BinaryOp::Shr => { let zero = || NumericValue::zero(lhs.get_type()); - let rhs = rhs.as_u32().expect("Expected rhs of shr to be a u32"); - + let Some(rhs) = rhs.as_u8() else { + let rhs = rhs.to_string(); + return Err(internal(InternalError::RhsOfBitShiftShouldBeU8 { + operator: ">>", + rhs_id, + rhs, + })); + }; + + let rhs = rhs as u32; use NumericValue::*; match lhs { - Field(_) => unreachable!(">> is not implemented for Field"), - U1(_) => unreachable!(">> is not implemented for u1"), + Field(_) => { + return Err(internal(InternalError::UnsupportedOperatorForType { + operator: ">>", + typ: "Field", + })); + } + U1(_) => { + return Err(internal(InternalError::UnsupportedOperatorForType { + operator: ">>", + typ: "u1", + })); + } U8(value) => value.checked_shr(rhs).map(U8).unwrap_or_else(zero), U16(value) => value.checked_shr(rhs).map(U16).unwrap_or_else(zero), U32(value) => value.checked_shr(rhs).map(U32).unwrap_or_else(zero), @@ -808,26 +1054,34 @@ impl Interpreter<'_> { lhs: FieldElement, operator: BinaryOp, rhs: FieldElement, + lhs_id: ValueId, + rhs_id: ValueId, ) -> IResult { + let unsupported_operator = |operator| -> IResult { + let typ = "Field"; + Err(internal(InternalError::UnsupportedOperatorForType { operator, typ })) + }; + let result = match operator { BinaryOp::Add { unchecked: _ } => NumericValue::Field(lhs + rhs), BinaryOp::Sub { unchecked: _ } => NumericValue::Field(lhs - rhs), BinaryOp::Mul { unchecked: _ } => NumericValue::Field(lhs * rhs), BinaryOp::Div => { - // FieldElement::div returns a value with panicking on divide by zero if rhs.is_zero() { - panic!("Field division by zero"); + let lhs = lhs.to_string(); + let rhs = rhs.to_string(); + return Err(InterpreterError::DivisionByZero { lhs_id, lhs, rhs_id, rhs }); } NumericValue::Field(lhs / rhs) } - BinaryOp::Mod => panic!("Unsupported operator `%` for Field"), + BinaryOp::Mod => return unsupported_operator("%"), BinaryOp::Eq => NumericValue::U1(lhs == rhs), BinaryOp::Lt => NumericValue::U1(lhs < rhs), - BinaryOp::And => panic!("Unsupported operator `&` for Field"), - BinaryOp::Or => panic!("Unsupported operator `|` for Field"), - BinaryOp::Xor => panic!("Unsupported operator `^` for Field"), - BinaryOp::Shl => panic!("Unsupported operator `<<` for Field"), - BinaryOp::Shr => panic!("Unsupported operator `>>` for Field"), + BinaryOp::And => return unsupported_operator("&"), + BinaryOp::Or => return unsupported_operator("|"), + BinaryOp::Xor => return unsupported_operator("^"), + BinaryOp::Shl => return unsupported_operator("<<"), + BinaryOp::Shr => return unsupported_operator(">>"), }; Ok(Value::Numeric(result)) } @@ -838,45 +1092,52 @@ impl Interpreter<'_> { operator: BinaryOp, rhs: bool, ) -> IResult { + let unsupported_operator = |operator| -> IResult { + let typ = "u1"; + Err(internal(InternalError::UnsupportedOperatorForType { operator, typ })) + }; + let result = match operator { - BinaryOp::Add { unchecked: _ } => panic!("Unsupported operator `+` for u1"), - BinaryOp::Sub { unchecked: _ } => panic!("Unsupported operator `-` for u1"), + BinaryOp::Add { unchecked: _ } => return unsupported_operator("+"), + BinaryOp::Sub { unchecked: _ } => return unsupported_operator("-"), BinaryOp::Mul { unchecked: _ } => lhs & rhs, // (*) = (&) for u1 - BinaryOp::Div => panic!("Unsupported operator `/` for u1"), - BinaryOp::Mod => panic!("Unsupported operator `%` for u1"), + BinaryOp::Div => return unsupported_operator("/"), + BinaryOp::Mod => return unsupported_operator("%"), BinaryOp::Eq => lhs == rhs, // clippy complains when you do `lhs < rhs` and recommends this instead BinaryOp::Lt => !lhs & rhs, BinaryOp::And => lhs & rhs, BinaryOp::Or => lhs | rhs, BinaryOp::Xor => lhs ^ rhs, - BinaryOp::Shl => panic!("Unsupported operator `<<` for u1"), - BinaryOp::Shr => panic!("Unsupported operator `>>` for u1"), + BinaryOp::Shl => return unsupported_operator("<<"), + BinaryOp::Shr => return unsupported_operator(">>"), }; Ok(Value::Numeric(NumericValue::U1(result))) } } -fn truncate_unsigned(value: T, bit_size: u32) -> T +fn truncate_unsigned(value: T, bit_size: u32) -> IResult where u128: From, T: TryFrom, >::Error: std::fmt::Debug, { let value_u128 = u128::from(value); - let bit_mask = match bit_size.cmp(&128) { + let bit_mask = match bit_size.cmp(&MAX_UNSIGNED_BIT_SIZE) { Ordering::Less => (1u128 << bit_size) - 1, Ordering::Equal => u128::MAX, - Ordering::Greater => panic!("truncate: Invalid bit size: {bit_size}"), + Ordering::Greater => { + return Err(internal(InternalError::InvalidUnsignedTruncateBitSize { bit_size })); + } }; let result = value_u128 & bit_mask; - T::try_from(result).expect( + Ok(T::try_from(result).expect( "The truncated result should always be smaller than or equal to the original `value`", - ) + )) } -fn truncate_signed(value: T, bit_size: u32) -> T +fn truncate_signed(value: T, bit_size: u32) -> IResult where i128: From, T: TryFrom + num_traits::Bounded, @@ -886,47 +1147,53 @@ where if value_i128 < 0 { let max = 1i128 << (bit_size - 1); value_i128 += max; - assert!(bit_size <= 64, "The maximum bit size for signed integers is 64"); + if bit_size > MAX_SIGNED_BIT_SIZE { + return Err(internal(InternalError::InvalidSignedTruncateBitSize { bit_size })); + } let mask = (1i128 << bit_size) - 1; let result = (value_i128 & mask) - max; - T::try_from(result).expect( + Ok(T::try_from(result).expect( "The truncated result should always be smaller than or equal to the original `value`", - ) + )) } else { - let result = truncate_unsigned::(value_i128 as u128, bit_size) as i128; - T::try_from(result).expect( + let result = truncate_unsigned::(value_i128 as u128, bit_size)? as i128; + Ok(T::try_from(result).expect( "The truncated result should always be smaller than or equal to the original `value`", - ) + )) } } +fn internal(error: InternalError) -> InterpreterError { + InterpreterError::Internal(error) +} + #[cfg(test)] mod test { #[test] fn test_truncate_unsigned() { - assert_eq!(super::truncate_unsigned(57_u32, 8), 57); - assert_eq!(super::truncate_unsigned(257_u16, 8), 1); - assert_eq!(super::truncate_unsigned(130_u8, 7), 2); - assert_eq!(super::truncate_unsigned(u8::MAX, 8), u8::MAX); - assert_eq!(super::truncate_unsigned(u128::MAX, 128), u128::MAX); + assert_eq!(super::truncate_unsigned(57_u32, 8).unwrap(), 57); + assert_eq!(super::truncate_unsigned(257_u16, 8).unwrap(), 1); + assert_eq!(super::truncate_unsigned(130_u8, 7).unwrap(), 2); + assert_eq!(super::truncate_unsigned(u8::MAX, 8).unwrap(), u8::MAX); + assert_eq!(super::truncate_unsigned(u128::MAX, 128).unwrap(), u128::MAX); } #[test] fn test_truncate_signed() { - assert_eq!(super::truncate_signed(57_i32, 8), 57); - assert_eq!(super::truncate_signed(257_i16, 8), 1); - assert_eq!(super::truncate_signed(130_i64, 7), 2); - assert_eq!(super::truncate_signed(i16::MAX, 16), i16::MAX); - - assert_eq!(super::truncate_signed(-57_i32, 8), -57); - assert_eq!(super::truncate_signed(-1_i64, 3), -1_i64); - assert_eq!(super::truncate_signed(-258_i16, 8), -2); - assert_eq!(super::truncate_signed(-130_i16, 7), -2); - assert_eq!(super::truncate_signed(i8::MIN, 8), i8::MIN); - assert_eq!(super::truncate_signed(-8_i8, 4), -8); - assert_eq!(super::truncate_signed(-8_i8, 3), 0); - assert_eq!(super::truncate_signed(-129_i32, 8), 127); + assert_eq!(super::truncate_signed(57_i32, 8).unwrap(), 57); + assert_eq!(super::truncate_signed(257_i16, 8).unwrap(), 1); + assert_eq!(super::truncate_signed(130_i64, 7).unwrap(), 2); + assert_eq!(super::truncate_signed(i16::MAX, 16).unwrap(), i16::MAX); + + assert_eq!(super::truncate_signed(-57_i32, 8).unwrap(), -57); + assert_eq!(super::truncate_signed(-1_i64, 3).unwrap(), -1_i64); + assert_eq!(super::truncate_signed(-258_i16, 8).unwrap(), -2); + assert_eq!(super::truncate_signed(-130_i16, 7).unwrap(), -2); + assert_eq!(super::truncate_signed(i8::MIN, 8).unwrap(), i8::MIN); + assert_eq!(super::truncate_signed(-8_i8, 4).unwrap(), -8); + assert_eq!(super::truncate_signed(-8_i8, 3).unwrap(), 0); + assert_eq!(super::truncate_signed(-129_i32, 8).unwrap(), 127); } } diff --git a/compiler/noirc_evaluator/src/ssa/interpreter/tests/instructions.rs b/compiler/noirc_evaluator/src/ssa/interpreter/tests/instructions.rs index 2e33a7ff6f9..d43f9998b15 100644 --- a/compiler/noirc_evaluator/src/ssa/interpreter/tests/instructions.rs +++ b/compiler/noirc_evaluator/src/ssa/interpreter/tests/instructions.rs @@ -5,7 +5,7 @@ use noirc_frontend::Shared; use crate::ssa::{ interpreter::{ - NumericValue, Value, + InterpreterError, NumericValue, Value, tests::{expect_value, expect_values}, value::ReferenceValue, }, @@ -31,11 +31,9 @@ fn add() { assert_eq!(value, Value::Numeric(NumericValue::I32(102))); } -/// TODO: Replace panic with error #[test] -#[should_panic(expected = "called `Option::unwrap()` on a `None` value")] fn add_overflow() { - expect_error( + let error = expect_error( " acir(inline) fn main f0 { b0(): @@ -44,6 +42,7 @@ fn add_overflow() { } ", ); + assert!(matches!(error, InterpreterError::Overflow { .. })); } #[test] @@ -73,11 +72,9 @@ fn sub() { assert_eq!(value, Value::Numeric(NumericValue::I32(10000))); } -/// TODO: Replace panic with error #[test] -#[should_panic(expected = "called `Option::unwrap()` on a `None` value")] fn sub_underflow() { - expect_error( + let error = expect_error( " acir(inline) fn main f0 { b0(): @@ -86,6 +83,7 @@ fn sub_underflow() { } ", ); + assert!(matches!(error, InterpreterError::Overflow { .. })); } #[test] @@ -116,11 +114,9 @@ fn mul() { assert_eq!(value, Value::Numeric(NumericValue::U64(200))); } -/// TODO: Replace panic with error #[test] -#[should_panic(expected = "called `Option::unwrap()` on a `None` value")] fn mul_overflow() { - expect_error( + let error = expect_error( " acir(inline) fn main f0 { b0(): @@ -129,6 +125,7 @@ fn mul_overflow() { } ", ); + assert!(matches!(error, InterpreterError::Overflow { .. })); } #[test] @@ -159,11 +156,9 @@ fn div() { assert_eq!(value, Value::Numeric(NumericValue::I16(64))); } -/// TODO: Replace panic with error #[test] -#[should_panic(expected = "Field division by zero")] fn div_zero() { - expect_error( + let error = expect_error( " acir(inline) fn main f0 { b0(): @@ -172,6 +167,7 @@ fn div_zero() { } ", ); + assert!(matches!(error, InterpreterError::DivisionByZero { .. })); } #[test] @@ -188,11 +184,9 @@ fn r#mod() { assert_eq!(value, Value::Numeric(NumericValue::I64(2))); } -/// TODO: Replace panic with error #[test] -#[should_panic(expected = "called `Option::unwrap()` on a `None` value")] fn mod_zero() { - expect_error( + let error = expect_error( " acir(inline) fn main f0 { b0(): @@ -201,6 +195,7 @@ fn mod_zero() { } ", ); + assert!(matches!(error, InterpreterError::DivisionByZero { .. })); } #[test] @@ -296,27 +291,27 @@ fn shl() { " acir(inline) fn main f0 { b0(): - v0 = shl u8 3, u32 2 + v0 = shl i8 3, u8 2 return v0 } ", ); - assert_eq!(value, Value::from_constant(12_u128.into(), NumericType::unsigned(8))); + assert_eq!(value, Value::from_constant(12_u128.into(), NumericType::signed(8))); } -#[test] -#[should_panic] /// shl should overflow if the rhs is greater than the bit count +#[test] fn shl_overflow() { - expect_error( + let error = expect_error( " acir(inline) fn main f0 { b0(): - v0 = shl u8 3, u32 9 + v0 = shl u8 3, u8 9 return v0 } ", ); + assert!(matches!(error, InterpreterError::Overflow { .. })); } #[test] @@ -325,16 +320,16 @@ fn shr() { " acir(inline) fn main f0 { b0(): - v0 = shr u8 12, u32 2 - v1 = shr u8 5, u32 1 - v2 = shr u8 5, u32 4 + v0 = shr u16 12, u8 2 + v1 = shr u16 5, u8 1 + v2 = shr u16 5, u8 4 return v0, v1, v2 } ", ); - assert_eq!(values[0], Value::from_constant(3_u128.into(), NumericType::unsigned(8))); - assert_eq!(values[1], Value::from_constant(2_u128.into(), NumericType::unsigned(8))); - assert_eq!(values[2], Value::from_constant(0_u128.into(), NumericType::unsigned(8))); + assert_eq!(values[0], Value::from_constant(3_u128.into(), NumericType::unsigned(16))); + assert_eq!(values[1], Value::from_constant(2_u128.into(), NumericType::unsigned(16))); + assert_eq!(values[2], Value::from_constant(0_u128.into(), NumericType::unsigned(16))); } #[test] @@ -344,7 +339,7 @@ fn shr_overflow() { " acir(inline) fn main f0 { b0(): - v0 = shr u8 3, u32 9 + v0 = shr u8 3, u8 9 return v0 } ", @@ -477,9 +472,8 @@ fn range_check() { } #[test] -#[should_panic] fn range_check_fail() { - expect_error( + let error = expect_error( " acir(inline) fn main f0 { b0(): @@ -488,6 +482,7 @@ fn range_check_fail() { } ", ); + assert!(matches!(error, InterpreterError::RangeCheckFailed { .. })); } #[test] diff --git a/compiler/noirc_evaluator/src/ssa/interpreter/tests/mod.rs b/compiler/noirc_evaluator/src/ssa/interpreter/tests/mod.rs index 7e5cc229d58..36b27ca8cef 100644 --- a/compiler/noirc_evaluator/src/ssa/interpreter/tests/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/interpreter/tests/mod.rs @@ -4,15 +4,12 @@ use std::sync::Arc; use acvm::{AcirField, FieldElement}; -use crate::{ - errors::RuntimeError, - ssa::{ - interpreter::value::NumericValue, - ir::types::{NumericType, Type}, - }, +use crate::ssa::{ + interpreter::value::NumericValue, + ir::types::{NumericType, Type}, }; -use super::{Ssa, Value}; +use super::{InterpreterError, Ssa, Value}; mod instructions; @@ -33,7 +30,7 @@ fn expect_value(src: &str) -> Value { } #[track_caller] -fn expect_error(src: &str) -> RuntimeError { +fn expect_error(src: &str) -> InterpreterError { let ssa = Ssa::from_str(src).unwrap(); ssa.interpret(Vec::new()).unwrap_err() } diff --git a/compiler/noirc_evaluator/src/ssa/interpreter/value.rs b/compiler/noirc_evaluator/src/ssa/interpreter/value.rs index 10ad7a86c2c..bafe384c10b 100644 --- a/compiler/noirc_evaluator/src/ssa/interpreter/value.rs +++ b/compiler/noirc_evaluator/src/ssa/interpreter/value.rs @@ -111,16 +111,16 @@ impl Value { } } - pub(crate) fn as_reference(&self) -> Option<&ReferenceValue> { + pub(crate) fn as_reference(&self) -> Option { match self { - Value::Reference(value) => Some(value), + Value::Reference(value) => Some(value.clone()), _ => None, } } - pub(crate) fn as_array_or_slice(&self) -> Option<&ArrayValue> { + pub(crate) fn as_array_or_slice(&self) -> Option { match self { - Value::ArrayOrSlice(value) => Some(value), + Value::ArrayOrSlice(value) => Some(value.clone()), _ => None, } } @@ -220,9 +220,9 @@ impl NumericValue { } } - pub(crate) fn as_u32(&self) -> Option { + pub(crate) fn as_u8(&self) -> Option { match self { - NumericValue::U32(value) => Some(*value), + NumericValue::U8(value) => Some(*value), _ => None, } } diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index 76393d2b7b4..9d9d6579783 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -190,7 +190,7 @@ fn display_instruction_inner( match instruction { Instruction::Binary(binary) => { - writeln!(f, "{} {}, {}", binary.operator, show(binary.lhs), show(binary.rhs)) + writeln!(f, "{}", display_binary(binary, dfg)) } Instruction::Cast(lhs, typ) => writeln!(f, "cast {} as {typ}", show(*lhs)), Instruction::Not(rhs) => writeln!(f, "not {}", show(*rhs)), @@ -307,6 +307,10 @@ fn display_instruction_inner( } } +pub(crate) fn display_binary(binary: &super::instruction::Binary, dfg: &DataFlowGraph) -> String { + format!("{} {}, {}", binary.operator, value(dfg, binary.lhs), value(dfg, binary.rhs)) +} + fn try_byte_array_to_string(elements: &Vector, dfg: &DataFlowGraph) -> Option { let mut string = String::new(); for element in elements {