diff --git a/crates/noirc_evaluator/src/errors.rs b/crates/noirc_evaluator/src/errors.rs index 2d8b02008c6..6d53668d7cb 100644 --- a/crates/noirc_evaluator/src/errors.rs +++ b/crates/noirc_evaluator/src/errors.rs @@ -1,151 +1,104 @@ +//! Noir Evaluator has two types of errors +//! +//! [RuntimeError]s that should be displayed to the user +//! +//! [InternalError]s that are used for checking internal logics of the SSA +//! +//! An Error of the former is a user Error +//! +//! An Error of the latter is an error in the implementation of the compiler +use acvm::FieldElement; use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic, Location}; use thiserror::Error; -#[derive(Debug)] -pub struct RuntimeError { - pub location: Option, - pub kind: RuntimeErrorKind, -} - -impl RuntimeError { - // XXX: In some places, we strip the span because we do not want span to - // be introduced into the binary op or low level function code, for simplicity. - // - // It's possible to have it there, but it means we will need to proliferate the code with span - // - // This does make error reporting, less specific! - pub fn remove_span(self) -> RuntimeErrorKind { - self.kind - } - - pub fn new(kind: RuntimeErrorKind, location: Option) -> RuntimeError { - RuntimeError { location, kind } - } - - // Keep one of the two location which is Some, if possible - // This is used when we optimize instructions so that we do not lose track of location - pub fn merge_location(a: Option, b: Option) -> Option { - match (a, b) { - (Some(loc), _) | (_, Some(loc)) => Some(loc), - (None, None) => None, - } - } +#[derive(Debug, PartialEq, Eq, Clone, Error)] +pub enum RuntimeError { + // We avoid showing the actual lhs and rhs since most of the time they are just 0 + // and 1 respectively. This would confuse users if a constraint such as + // assert(foo < bar) fails with "failed constraint: 0 = 1." + #[error("Failed constraint")] + FailedConstraint { lhs: FieldElement, rhs: FieldElement, location: Option }, + #[error(transparent)] + InternalError(#[from] InternalError), + #[error("Index out of bounds, array has size {index:?}, but index was {array_size:?}")] + IndexOutOfBounds { index: usize, array_size: usize, location: Option }, + #[error("All Witnesses are by default u{num_bits:?} Applying this type does not apply any constraints.\n We also currently do not allow integers of size more than {num_bits:?}, this will be handled by BigIntegers.")] + InvalidRangeConstraint { num_bits: u32, location: Option }, + #[error("Expected array index to fit into a u64")] + TypeConversion { from: String, into: String, location: Option }, + #[error("{name:?} is not initialized")] + UnInitialized { name: String, location: Option }, + #[error("Integer sized {num_bits:?} is over the max supported size of {max_num_bits:?}")] + UnsupportedIntegerSize { num_bits: u32, max_num_bits: u32, location: Option }, } -impl From for RuntimeError { - fn from(kind: RuntimeErrorKind) -> RuntimeError { - RuntimeError { location: None, kind } - } +#[derive(Debug, PartialEq, Eq, Clone, Error)] +pub enum InternalError { + #[error("ICE: Both expressions should have degree<=1")] + DegreeNotReduced { location: Option }, + #[error("Try to get element from empty array")] + EmptyArray { location: Option }, + #[error("ICE: {message:?}")] + General { message: String, location: Option }, + #[error("ICE: {name:?} missing {arg:?} arg")] + MissingArg { name: String, arg: String, location: Option }, + #[error("ICE: {name:?} should be a constant")] + NotAConstant { name: String, location: Option }, + #[error("{name:?} is not implemented yet")] + NotImplemented { name: String, location: Option }, + #[error("ICE: Undeclared AcirVar")] + UndeclaredAcirVar { location: Option }, + #[error("ICE: Expected {expected:?}, found {found:?}")] + UnExpected { expected: String, found: String, location: Option }, } impl From for FileDiagnostic { - fn from(err: RuntimeError) -> Self { - let file_id = err.location.map(|loc| loc.file).unwrap(); - FileDiagnostic { file_id, diagnostic: err.into() } + fn from(error: RuntimeError) -> Self { + match error { + RuntimeError::InternalError(ref ice_error) => match ice_error { + InternalError::DegreeNotReduced { location } + | InternalError::EmptyArray { location } + | InternalError::General { location, .. } + | InternalError::MissingArg { location, .. } + | InternalError::NotAConstant { location, .. } + | InternalError::NotImplemented { location, .. } + | InternalError::UndeclaredAcirVar { location } + | InternalError::UnExpected { location, .. } => { + let file_id = location.map(|loc| loc.file).unwrap(); + FileDiagnostic { file_id, diagnostic: error.into() } + } + }, + RuntimeError::FailedConstraint { location, .. } + | RuntimeError::IndexOutOfBounds { location, .. } + | RuntimeError::InvalidRangeConstraint { location, .. } + | RuntimeError::TypeConversion { location, .. } + | RuntimeError::UnInitialized { location, .. } + | RuntimeError::UnsupportedIntegerSize { location, .. } => { + let file_id = location.map(|loc| loc.file).unwrap(); + FileDiagnostic { file_id, diagnostic: error.into() } + } + } } } -#[derive(Error, Debug)] -pub enum RuntimeErrorKind { - // Array errors - #[error("Out of bounds")] - ArrayOutOfBounds { index: u128, bound: u128 }, - - #[error("index out of bounds: the len is {index} but the index is {bound}")] - IndexOutOfBounds { index: u32, bound: u128 }, - - #[error("cannot call {func_name} function in non main function")] - FunctionNonMainContext { func_name: String }, - - // Environment errors - #[error("Cannot find Array")] - ArrayNotFound { found_type: String, name: String }, - - #[error("Not an object")] - NotAnObject, - - #[error("Invalid id")] - InvalidId, - - #[error("Attempt to divide by zero")] - DivisionByZero, - - #[error("Failed range constraint when constraining to {0} bits")] - FailedRangeConstraint(u32), - - #[error("Unsupported integer size of {num_bits} bits. The maximum supported size is {max_num_bits} bits.")] - UnsupportedIntegerSize { num_bits: u32, max_num_bits: u32 }, - - #[error("Failed constraint")] - FailedConstraint, - - #[error( - "All Witnesses are by default u{0}. Applying this type does not apply any constraints." - )] - DefaultWitnesses(u32), - - #[error("Constraint is always false")] - ConstraintIsAlwaysFalse, - - #[error("ICE: cannot convert signed {0} bit size into field")] - CannotConvertSignedIntoField(u32), - - #[error("we do not allow private ABI inputs to be returned as public outputs")] - PrivateAbiInput, - - #[error("unimplemented")] - Unimplemented(String), - - #[error("Unsupported operation error")] - UnsupportedOp { op: String, first_type: String, second_type: String }, -} - impl From for Diagnostic { fn from(error: RuntimeError) -> Diagnostic { - let span = - if let Some(loc) = error.location { loc.span } else { noirc_errors::Span::new(0..0) }; - match &error.kind { - RuntimeErrorKind::ArrayOutOfBounds { index, bound } => Diagnostic::simple_error( - "index out of bounds".to_string(), - format!("out of bounds error, index is {index} but length is {bound}"), - span, - ), - RuntimeErrorKind::ArrayNotFound { found_type, name } => Diagnostic::simple_error( - format!("cannot find an array with name {name}"), - format!("{found_type} has type"), - span, + match error { + RuntimeError::InternalError(_) => Diagnostic::simple_error( + "Internal Consistency Evaluators Errors: \n + This is likely a bug. Consider Opening an issue at https://github.com/noir-lang/noir/issues".to_owned(), + "".to_string(), + noirc_errors::Span::new(0..0) ), - RuntimeErrorKind::NotAnObject - | RuntimeErrorKind::InvalidId - | RuntimeErrorKind::DivisionByZero - | RuntimeErrorKind::FailedRangeConstraint(_) - | RuntimeErrorKind::UnsupportedIntegerSize { .. } - | RuntimeErrorKind::FailedConstraint - | RuntimeErrorKind::DefaultWitnesses(_) - | RuntimeErrorKind::CannotConvertSignedIntoField(_) - | RuntimeErrorKind::IndexOutOfBounds { .. } - | RuntimeErrorKind::PrivateAbiInput => { - Diagnostic::simple_error("".to_owned(), error.kind.to_string(), span) + RuntimeError::FailedConstraint { location, .. } + | RuntimeError::IndexOutOfBounds { location, .. } + | RuntimeError::InvalidRangeConstraint { location, .. } + | RuntimeError::TypeConversion { location, .. } + | RuntimeError::UnInitialized { location, .. } + | RuntimeError::UnsupportedIntegerSize { location, .. } => { + let span = if let Some(loc) = location { loc.span } else { noirc_errors::Span::new(0..0) }; + Diagnostic::simple_error("".to_owned(), error.to_string(), span) } - RuntimeErrorKind::UnsupportedOp { op, first_type, second_type } => { - Diagnostic::simple_error( - "unsupported operation".to_owned(), - format!("no support for {op} with types {first_type} and {second_type}"), - span, - ) - } - RuntimeErrorKind::ConstraintIsAlwaysFalse if error.location.is_some() => { - Diagnostic::simple_error("".to_owned(), error.kind.to_string(), span) - } - RuntimeErrorKind::ConstraintIsAlwaysFalse => { - Diagnostic::from_message(&error.kind.to_string()) - } - RuntimeErrorKind::Unimplemented(message) => Diagnostic::from_message(message), - RuntimeErrorKind::FunctionNonMainContext { func_name } => Diagnostic::simple_error( - "cannot call function outside of main".to_owned(), - format!("function {func_name} can only be called in main"), - span, - ), } } } diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir.rs index 6e715002161..96800b22ad0 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir.rs @@ -1,4 +1,3 @@ pub(crate) mod acir_variable; -pub(crate) mod errors; pub(crate) mod generated_acir; pub(crate) mod sort; diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs index d953322e567..6d8178b6a2c 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs @@ -1,5 +1,6 @@ -use super::{errors::AcirGenError, generated_acir::GeneratedAcir}; +use super::generated_acir::GeneratedAcir; use crate::brillig::brillig_gen::brillig_directive; +use crate::errors::{InternalError, RuntimeError}; use crate::ssa_refactor::acir_gen::{AcirDynamicArray, AcirValue}; use crate::ssa_refactor::ir::types::Type as SsaType; use crate::ssa_refactor::ir::{instruction::Endian, types::NumericType}; @@ -9,7 +10,6 @@ use acvm::acir::{ brillig::Opcode as BrilligOpcode, circuit::brillig::{BrilligInputs, BrilligOutputs}, }; - use acvm::{ acir::{ circuit::opcodes::FunctionInput, @@ -18,7 +18,7 @@ use acvm::{ }, FieldElement, }; -use iter_extended::vecmap; +use iter_extended::{try_vecmap, vecmap}; use noirc_errors::Location; use std::collections::HashMap; use std::{borrow::Cow, hash::Hash}; @@ -182,7 +182,7 @@ impl AcirContext { &mut self, var: AcirVar, predicate: AcirVar, - ) -> Result { + ) -> Result { let var_data = &self.vars[&var]; if let AcirVarData::Const(constant) = var_data { // Note that this will return a 0 if the inverse is not available @@ -199,7 +199,7 @@ impl AcirContext { inverse_code, vec![AcirValue::Var(var, field_type.clone())], vec![field_type], - ); + )?; let inverted_var = Self::expect_one_var(results); let should_be_one = self.mul_var(inverted_var, var)?; @@ -209,7 +209,7 @@ impl AcirContext { } // Constrains `var` to be equal to the constant value `1` - pub(crate) fn assert_eq_one(&mut self, var: AcirVar) -> Result<(), AcirGenError> { + pub(crate) fn assert_eq_one(&mut self, var: AcirVar) -> Result<(), RuntimeError> { let one = self.add_constant(FieldElement::one()); self.assert_eq_var(var, one) } @@ -222,7 +222,7 @@ impl AcirContext { &mut self, var: AcirVar, predicate: AcirVar, - ) -> Result<(), AcirGenError> { + ) -> Result<(), RuntimeError> { let pred_mul_var = self.mul_var(var, predicate)?; self.assert_eq_var(pred_mul_var, predicate) } @@ -240,7 +240,7 @@ impl AcirContext { /// Returns an `AcirVar` that is `1` if `lhs` equals `rhs` and /// 0 otherwise. - pub(crate) fn eq_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result { + pub(crate) fn eq_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result { let lhs_data = &self.vars[&lhs]; let rhs_data = &self.vars[&rhs]; @@ -258,7 +258,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, typ: AcirType, - ) -> Result { + ) -> Result { let inputs = vec![AcirValue::Var(lhs, typ.clone()), AcirValue::Var(rhs, typ)]; let outputs = self.black_box_function(BlackBoxFunc::XOR, inputs)?; Ok(outputs[0]) @@ -270,7 +270,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, typ: AcirType, - ) -> Result { + ) -> Result { let inputs = vec![AcirValue::Var(lhs, typ.clone()), AcirValue::Var(rhs, typ)]; let outputs = self.black_box_function(BlackBoxFunc::AND, inputs)?; Ok(outputs[0]) @@ -282,7 +282,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, typ: AcirType, - ) -> Result { + ) -> Result { let bit_size = typ.bit_size(); if bit_size == 1 { // Operands are booleans @@ -305,7 +305,7 @@ impl AcirContext { } /// Constrains the `lhs` and `rhs` to be equal. - pub(crate) fn assert_eq_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result<(), AcirGenError> { + pub(crate) fn assert_eq_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result<(), RuntimeError> { // TODO: could use sub_var and then assert_eq_zero let lhs_data = &self.vars[&lhs]; let rhs_data = &self.vars[&rhs]; @@ -316,7 +316,7 @@ impl AcirContext { Ok(()) } else { // Constraint is always false - this program is unprovable - Err(AcirGenError::BadConstantEquality { + Err(RuntimeError::FailedConstraint { lhs: *lhs_const, rhs: *rhs_const, location: self.get_location(), @@ -338,7 +338,7 @@ impl AcirContext { rhs: AcirVar, typ: AcirType, predicate: AcirVar, - ) -> Result { + ) -> Result { let numeric_type = match typ { AcirType::NumericType(numeric_type) => numeric_type, AcirType::Array(_, _) => { @@ -365,7 +365,7 @@ impl AcirContext { /// Adds a new Variable to context whose value will /// be constrained to be the multiplication of `lhs` and `rhs` - pub(crate) fn mul_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result { + pub(crate) fn mul_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result { let lhs_data = &self.vars[&lhs]; let rhs_data = &self.vars[&rhs]; let result = match (lhs_data, rhs_data) { @@ -412,14 +412,14 @@ impl AcirContext { /// Adds a new Variable to context whose value will /// be constrained to be the subtraction of `lhs` and `rhs` - pub(crate) fn sub_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result { + pub(crate) fn sub_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result { let neg_rhs = self.neg_var(rhs); self.add_var(lhs, neg_rhs) } /// Adds a new Variable to context whose value will /// be constrained to be the addition of `lhs` and `rhs` - pub(crate) fn add_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result { + pub(crate) fn add_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result { let lhs_data = &self.vars[&lhs]; let rhs_data = &self.vars[&rhs]; let result_data = if let (AcirVarData::Const(lhs_const), AcirVarData::Const(rhs_const)) = @@ -434,7 +434,7 @@ impl AcirContext { } /// Adds a new variable that is constrained to be the logical NOT of `x`. - pub(crate) fn not_var(&mut self, x: AcirVar, typ: AcirType) -> Result { + pub(crate) fn not_var(&mut self, x: AcirVar, typ: AcirType) -> Result { let bit_size = typ.bit_size(); // Subtracting from max flips the bits let max = self.add_constant(FieldElement::from((1_u128 << bit_size) - 1)); @@ -453,7 +453,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, _typ: AcirType, - ) -> Result { + ) -> Result { let rhs_data = &self.vars[&rhs]; // Compute 2^{rhs} @@ -473,7 +473,7 @@ impl AcirContext { rhs: AcirVar, bit_size: u32, predicate: AcirVar, - ) -> Result<(AcirVar, AcirVar), AcirGenError> { + ) -> Result<(AcirVar, AcirVar), RuntimeError> { let lhs_data = &self.vars[&lhs]; let rhs_data = &self.vars[&rhs]; let predicate_data = &self.vars[&predicate]; @@ -505,7 +505,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, bit_size: u32, - ) -> Result<(AcirVar, AcirVar), AcirGenError> { + ) -> Result<(AcirVar, AcirVar), RuntimeError> { let lhs_data = &self.vars[&lhs].clone(); let rhs_data = &self.vars[&rhs].clone(); @@ -529,7 +529,7 @@ impl AcirContext { rhs: AcirVar, bit_size: u32, predicate: AcirVar, - ) -> Result { + ) -> Result { let (_, remainder) = self.euclidean_division_var(lhs, rhs, bit_size, predicate)?; Ok(remainder) } @@ -550,7 +550,7 @@ impl AcirContext { rhs: AcirVar, typ: AcirType, predicate: AcirVar, - ) -> Result { + ) -> Result { let rhs_data = &self.vars[&rhs]; // Compute 2^{rhs} @@ -565,8 +565,11 @@ impl AcirContext { /// Converts the `AcirVar` to a `Witness` if it hasn't been already, and appends it to the /// `GeneratedAcir`'s return witnesses. - pub(crate) fn return_var(&mut self, acir_var: AcirVar) { - let acir_var_data = self.vars.get(&acir_var).expect("ICE: return of undeclared AcirVar"); + pub(crate) fn return_var(&mut self, acir_var: AcirVar) -> Result<(), InternalError> { + let acir_var_data = match self.vars.get(&acir_var) { + Some(acir_var_data) => acir_var_data, + None => return Err(InternalError::UndeclaredAcirVar { location: self.get_location() }), + }; // TODO: Add caching to prevent expressions from being needlessly duplicated let witness = match acir_var_data { AcirVarData::Const(constant) => { @@ -576,6 +579,7 @@ impl AcirContext { AcirVarData::Witness(witness) => *witness, }; self.acir_ir.push_return_witness(witness); + Ok(()) } /// Constrains the `AcirVar` variable to be of type `NumericType`. @@ -583,7 +587,7 @@ impl AcirContext { &mut self, variable: AcirVar, numeric_type: &NumericType, - ) -> Result { + ) -> Result { let data = &self.vars[&variable]; match numeric_type { NumericType::Signed { bit_size } | NumericType::Unsigned { bit_size } => { @@ -606,7 +610,7 @@ impl AcirContext { lhs: AcirVar, rhs: u32, max_bit_size: u32, - ) -> Result { + ) -> Result { let lhs_data = &self.vars[&lhs]; let lhs_expr = lhs_data.to_expression(); @@ -631,7 +635,7 @@ impl AcirContext { rhs: AcirVar, bit_size: u32, predicate: AcirVar, - ) -> Result { + ) -> Result { let lhs_data = &self.vars[&lhs]; let rhs_data = &self.vars[&rhs]; @@ -658,7 +662,7 @@ impl AcirContext { rhs: AcirVar, bit_size: u32, predicate: AcirVar, - ) -> Result { + ) -> Result { // Flip the result of calling more than equal method to // compute less than. let comparison = self.more_than_eq_var(lhs, rhs, bit_size, predicate)?; @@ -673,17 +677,31 @@ impl AcirContext { &mut self, name: BlackBoxFunc, mut inputs: Vec, - ) -> Result, AcirGenError> { + ) -> Result, RuntimeError> { // Separate out any arguments that should be constants let constants = match name { BlackBoxFunc::Pedersen => { // The last argument of pedersen is the domain separator, which must be a constant - let domain_var = - inputs.pop().expect("ICE: Pedersen call requires domain separator").into_var(); - - let domain_constant = self.vars[&domain_var] - .as_constant() - .expect("ICE: Domain separator must be a constant"); + let domain_var = match inputs.pop() { + Some(domain_var) => domain_var.into_var()?, + None => { + return Err(RuntimeError::InternalError(InternalError::MissingArg { + name: "pedersen call".to_string(), + arg: "domain separator".to_string(), + location: self.get_location(), + })) + } + }; + + let domain_constant = match self.vars[&domain_var].as_constant() { + Some(domain_constant) => domain_constant, + None => { + return Err(RuntimeError::InternalError(InternalError::NotAConstant { + name: "domain separator".to_string(), + location: self.get_location(), + })) + } + }; vec![domain_constant] } @@ -694,7 +712,7 @@ impl AcirContext { let inputs = self.prepare_inputs_for_black_box_func_call(inputs)?; // Call Black box with `FunctionInput` - let outputs = self.acir_ir.call_black_box(name, inputs, constants); + let outputs = self.acir_ir.call_black_box(name, inputs, constants)?; // Convert `Witness` values which are now constrained to be the output of the // black box function call into `AcirVar`s. @@ -710,7 +728,7 @@ impl AcirContext { fn prepare_inputs_for_black_box_func_call( &mut self, inputs: Vec, - ) -> Result, AcirGenError> { + ) -> Result, RuntimeError> { let mut witnesses = Vec::new(); for input in inputs { for (input, typ) in input.flatten() { @@ -741,15 +759,26 @@ impl AcirContext { radix_var: AcirVar, limb_count_var: AcirVar, result_element_type: AcirType, - ) -> Result, AcirGenError> { - let radix = - self.vars[&radix_var].as_constant().expect("ICE: radix should be a constant").to_u128() - as u32; + ) -> Result, RuntimeError> { + let radix = match self.vars[&radix_var].as_constant() { + Some(radix) => radix.to_u128() as u32, + None => { + return Err(RuntimeError::InternalError(InternalError::NotAConstant { + name: "radix".to_string(), + location: self.get_location(), + })); + } + }; - let limb_count = self.vars[&limb_count_var] - .as_constant() - .expect("ICE: limb_size should be a constant") - .to_u128() as u32; + let limb_count = match self.vars[&limb_count_var].as_constant() { + Some(limb_count) => limb_count.to_u128() as u32, + None => { + return Err(RuntimeError::InternalError(InternalError::NotAConstant { + name: "limb_size".to_string(), + location: self.get_location(), + })); + } + }; let input_expr = &self.vars[&input_var].to_expression(); @@ -785,13 +814,13 @@ impl AcirContext { input_var: AcirVar, limb_count_var: AcirVar, result_element_type: AcirType, - ) -> Result, AcirGenError> { + ) -> Result, RuntimeError> { let two_var = self.add_constant(FieldElement::from(2_u128)); self.radix_decompose(endian, input_var, two_var, limb_count_var, result_element_type) } /// Prints the given `AcirVar`s as witnesses. - pub(crate) fn print(&mut self, input: Vec) -> Result<(), AcirGenError> { + pub(crate) fn print(&mut self, input: Vec) -> Result<(), RuntimeError> { let input = Self::flatten_values(input); let witnesses = vecmap(input, |acir_var| { @@ -850,24 +879,24 @@ impl AcirContext { code: Vec, inputs: Vec, outputs: Vec, - ) -> Vec { - let b_inputs = vecmap(inputs, |i| match i { + ) -> Result, InternalError> { + let b_inputs = try_vecmap(inputs, |i| match i { AcirValue::Var(var, _) => { - BrilligInputs::Single(self.vars[&var].to_expression().into_owned()) + Ok(BrilligInputs::Single(self.vars[&var].to_expression().into_owned())) } AcirValue::Array(vars) => { let mut var_expressions: Vec = Vec::new(); for var in vars { - self.brillig_array_input(&mut var_expressions, var); + self.brillig_array_input(&mut var_expressions, var)?; } - BrilligInputs::Array(var_expressions) + Ok(BrilligInputs::Array(var_expressions)) } AcirValue::DynamicArray(_) => { let mut var_expressions = Vec::new(); - self.brillig_array_input(&mut var_expressions, i); - BrilligInputs::Array(var_expressions) + self.brillig_array_input(&mut var_expressions, i)?; + Ok(BrilligInputs::Array(var_expressions)) } - }); + })?; let mut b_outputs = Vec::new(); let outputs_var = vecmap(outputs, |output| match output { @@ -886,17 +915,21 @@ impl AcirContext { let predicate = self.vars[&predicate].to_expression().into_owned(); self.acir_ir.brillig(Some(predicate), code, b_inputs, b_outputs); - outputs_var + Ok(outputs_var) } - fn brillig_array_input(&mut self, var_expressions: &mut Vec, input: AcirValue) { + fn brillig_array_input( + &mut self, + var_expressions: &mut Vec, + input: AcirValue, + ) -> Result<(), InternalError> { match input { AcirValue::Var(var, _) => { var_expressions.push(self.vars[&var].to_expression().into_owned()); } AcirValue::Array(vars) => { for var in vars { - self.brillig_array_input(var_expressions, var); + self.brillig_array_input(var_expressions, var)?; } } AcirValue::DynamicArray(AcirDynamicArray { block_id, len }) => { @@ -906,18 +939,19 @@ impl AcirContext { self.add_constant(FieldElement::from(i as u128)), AcirType::NumericType(NumericType::NativeField), ); - let index_var = index.into_var(); + let index_var = index.into_var()?; - let value_read_var = self.read_from_memory(block_id, &index_var); + let value_read_var = self.read_from_memory(block_id, &index_var)?; let value_read = AcirValue::Var( value_read_var, AcirType::NumericType(NumericType::NativeField), ); - self.brillig_array_input(var_expressions, value_read); + self.brillig_array_input(var_expressions, value_read)?; } } } + Ok(()) } /// Recursively create acir values for returned arrays. This is necessary because a brillig returned array can have nested arrays as elements. @@ -959,7 +993,7 @@ impl AcirContext { inputs: Vec, bit_size: u32, predicate: AcirVar, - ) -> Result, AcirGenError> { + ) -> Result, RuntimeError> { let len = inputs.len(); // Convert the inputs into expressions let inputs_expr = vecmap(inputs, |input| self.vars[&input].to_expression().into_owned()); @@ -972,7 +1006,7 @@ impl AcirContext { }); // Enforce the outputs to be a permutation of the inputs - self.acir_ir.permutation(&inputs_expr, &output_expr); + self.acir_ir.permutation(&inputs_expr, &output_expr)?; // Enforce the outputs to be sorted for i in 0..(outputs_var.len() - 1) { @@ -982,9 +1016,12 @@ impl AcirContext { Ok(outputs_var) } /// Converts an AcirVar to a Witness - fn var_to_witness(&mut self, var: AcirVar) -> Witness { - let var_data = self.vars.get(&var).expect("ICE: undeclared AcirVar"); - self.acir_ir.get_or_create_witness(&var_data.to_expression()) + fn var_to_witness(&mut self, var: AcirVar) -> Result { + let var_data = match self.vars.get(&var) { + Some(var_data) => var_data, + None => return Err(InternalError::UndeclaredAcirVar { location: self.get_location() }), + }; + Ok(self.acir_ir.get_or_create_witness(&var_data.to_expression())) } /// Constrain lhs to be less than rhs @@ -994,40 +1031,50 @@ impl AcirContext { rhs: AcirVar, bit_size: u32, predicate: AcirVar, - ) -> Result<(), AcirGenError> { + ) -> Result<(), RuntimeError> { let lhs_less_than_rhs = self.more_than_eq_var(rhs, lhs, bit_size, predicate)?; self.maybe_eq_predicate(lhs_less_than_rhs, predicate) } /// Returns a Variable that is constrained to be the result of reading /// from the memory `block_id` at the given `index`. - pub(crate) fn read_from_memory(&mut self, block_id: BlockId, index: &AcirVar) -> AcirVar { + pub(crate) fn read_from_memory( + &mut self, + block_id: BlockId, + index: &AcirVar, + ) -> Result { // Fetch the witness corresponding to the index - let index_witness = self.var_to_witness(*index); + let index_witness = self.var_to_witness(*index)?; // Create a Variable to hold the result of the read and extract the corresponding Witness let value_read_var = self.add_variable(); - let value_read_witness = self.var_to_witness(value_read_var); + let value_read_witness = self.var_to_witness(value_read_var)?; // Add the memory read operation to the list of opcodes let op = MemOp::read_at_mem_index(index_witness.into(), value_read_witness); self.acir_ir.opcodes.push(Opcode::MemoryOp { block_id, op }); - value_read_var + Ok(value_read_var) } /// Constrains the Variable `value` to be the new value located at `index` in the memory `block_id`. - pub(crate) fn write_to_memory(&mut self, block_id: BlockId, index: &AcirVar, value: &AcirVar) { + pub(crate) fn write_to_memory( + &mut self, + block_id: BlockId, + index: &AcirVar, + value: &AcirVar, + ) -> Result<(), InternalError> { // Fetch the witness corresponding to the index // - let index_witness = self.var_to_witness(*index); + let index_witness = self.var_to_witness(*index)?; // Fetch the witness corresponding to the value to be written - let value_write_witness = self.var_to_witness(*value); + let value_write_witness = self.var_to_witness(*value)?; // Add the memory write operation to the list of opcodes let op = MemOp::write_to_mem_index(index_witness.into(), value_write_witness.into()); self.acir_ir.opcodes.push(Opcode::MemoryOp { block_id, op }); + Ok(()) } /// Initializes an array in memory with the given values `optional_values`. @@ -1037,22 +1084,23 @@ impl AcirContext { block_id: BlockId, len: usize, optional_values: Option<&[AcirValue]>, - ) { + ) -> Result<(), InternalError> { // If the optional values are supplied, then we fill the initialized // array with those values. If not, then we fill it with zeros. let initialized_values = match optional_values { None => { let zero = self.add_constant(FieldElement::zero()); - let zero_witness = self.var_to_witness(zero); + let zero_witness = self.var_to_witness(zero)?; vec![zero_witness; len] } - Some(optional_values) => vecmap(optional_values, |value| { - let value = value.clone().into_var(); + Some(optional_values) => try_vecmap(optional_values, |value| { + let value = value.clone().into_var()?; self.var_to_witness(value) - }), + })?, }; self.acir_ir.opcodes.push(Opcode::MemoryInit { block_id, init: initialized_values }); + Ok(()) } } diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/errors.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/errors.rs deleted file mode 100644 index c90f98e15be..00000000000 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/errors.rs +++ /dev/null @@ -1,62 +0,0 @@ -use acvm::FieldElement; -use noirc_errors::Location; - -use crate::errors::{RuntimeError, RuntimeErrorKind}; - -#[derive(Debug, PartialEq, Eq, Clone)] -pub(crate) enum AcirGenError { - InvalidRangeConstraint { num_bits: u32, location: Option }, - IndexOutOfBounds { index: usize, array_size: usize, location: Option }, - UnsupportedIntegerSize { num_bits: u32, max_num_bits: u32, location: Option }, - BadConstantEquality { lhs: FieldElement, rhs: FieldElement, location: Option }, -} - -impl AcirGenError { - pub(crate) fn message(&self) -> String { - match self { - AcirGenError::InvalidRangeConstraint { num_bits, .. } => { - // Don't apply any constraints if the range is for the maximum number of bits or more. - format!( - "All Witnesses are by default u{num_bits} Applying this type does not apply any constraints.\n We also currently do not allow integers of size more than {num_bits}, this will be handled by BigIntegers.") - } - AcirGenError::IndexOutOfBounds { index, array_size, .. } => { - format!("Index out of bounds, array has size {array_size}, but index was {index}") - } - AcirGenError::UnsupportedIntegerSize { num_bits, max_num_bits, .. } => { - format!("Integer sized {num_bits} is over the max supported size of {max_num_bits}") - } - AcirGenError::BadConstantEquality { lhs, rhs, .. } => { - format!("{lhs} and {rhs} constrained to be equal though they never can be") - } - } - } -} - -impl From for RuntimeError { - fn from(error: AcirGenError) -> Self { - match error { - AcirGenError::InvalidRangeConstraint { num_bits, location } => { - let kind = RuntimeErrorKind::FailedRangeConstraint(num_bits); - RuntimeError::new(kind, location) - } - AcirGenError::IndexOutOfBounds { index, array_size, location } => { - let kind = RuntimeErrorKind::ArrayOutOfBounds { - index: index as u128, - bound: array_size as u128, - }; - RuntimeError::new(kind, location) - } - AcirGenError::UnsupportedIntegerSize { num_bits, max_num_bits, location } => { - let kind = RuntimeErrorKind::UnsupportedIntegerSize { num_bits, max_num_bits }; - RuntimeError::new(kind, location) - } - AcirGenError::BadConstantEquality { lhs: _, rhs: _, location } => { - // We avoid showing the actual lhs and rhs since most of the time they are just 0 - // and 1 respectively. This would confuse users if a constraint such as - // assert(foo < bar) fails with "failed constraint: 0 = 1." - let kind = RuntimeErrorKind::FailedConstraint; - RuntimeError::new(kind, location) - } - } - } -} diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs index 459458fc03e..24f001b74db 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs @@ -2,9 +2,11 @@ //! program as it is being converted from SSA form. use std::collections::HashMap; -use crate::brillig::brillig_gen::brillig_directive; +use crate::{ + brillig::brillig_gen::brillig_directive, + errors::{InternalError, RuntimeError}, +}; -use super::errors::AcirGenError; use acvm::acir::{ brillig::Opcode as BrilligOpcode, circuit::{ @@ -122,10 +124,10 @@ impl GeneratedAcir { func_name: BlackBoxFunc, mut inputs: Vec, constants: Vec, - ) -> Vec { - intrinsics_check_inputs(func_name, &inputs); + ) -> Result, InternalError> { + intrinsics_check_inputs(func_name, &inputs)?; - let output_count = black_box_expected_output_size(func_name); + let output_count = black_box_expected_output_size(func_name)?; let outputs = vecmap(0..output_count, |_| self.next_witness_index()); // clone is needed since outputs is moved when used in blackbox function. @@ -182,18 +184,30 @@ impl GeneratedAcir { outputs: (outputs[0], outputs[1]), }, BlackBoxFunc::Keccak256 => { - let var_message_size = inputs.pop().expect("ICE: Missing message_size arg"); + let var_message_size = match inputs.pop() { + Some(var_message_size) => var_message_size, + None => { + return Err(InternalError::MissingArg { + name: "".to_string(), + arg: "message_size".to_string(), + location: self.current_location, + }); + } + }; BlackBoxFuncCall::Keccak256VariableLength { inputs, var_message_size, outputs } } // TODO(#1570): Generate ACIR for recursive aggregation BlackBoxFunc::RecursiveAggregation => { - panic!("ICE: Cannot generate ACIR for recursive aggregation") + return Err(InternalError::NotImplemented { + name: "recursive aggregation".to_string(), + location: None, + }) } }; self.opcodes.push(AcirOpcode::BlackBoxFuncCall(black_box_func_call)); - outputs_clone + Ok(outputs_clone) } /// Takes an input expression and returns witnesses that are constrained to be limbs @@ -206,7 +220,7 @@ impl GeneratedAcir { radix: u32, limb_count: u32, bit_size: u32, - ) -> Result, AcirGenError> { + ) -> Result, RuntimeError> { let radix_big = BigUint::from(radix); assert_eq!( BigUint::from(2u128).pow(bit_size), @@ -320,13 +334,13 @@ impl GeneratedAcir { lhs: &Expression, rhs: &Expression, max_bit_size: u32, - ) -> Result<(Expression, Expression), AcirGenError> { + ) -> Result<(Expression, Expression), RuntimeError> { // 2^{max_bit size-1} let max_power_of_two = FieldElement::from(2_i128).pow(&FieldElement::from(max_bit_size as i128 - 1)); // Get the sign bit of rhs by computing rhs / max_power_of_two - let (rhs_leading, _) = self.euclidean_division( + let (rhs_leading_witness, _) = self.euclidean_division( rhs, &max_power_of_two.into(), max_bit_size, @@ -334,7 +348,7 @@ impl GeneratedAcir { )?; // Get the sign bit of lhs by computing lhs / max_power_of_two - let (lhs_leading, _) = self.euclidean_division( + let (lhs_leading_witness, _) = self.euclidean_division( lhs, &max_power_of_two.into(), max_bit_size, @@ -342,8 +356,8 @@ impl GeneratedAcir { )?; // Signed to unsigned: - let unsigned_lhs = self.two_complement(lhs, lhs_leading, max_bit_size); - let unsigned_rhs = self.two_complement(rhs, rhs_leading, max_bit_size); + let unsigned_lhs = self.two_complement(lhs, lhs_leading_witness, max_bit_size); + let unsigned_rhs = self.two_complement(rhs, rhs_leading_witness, max_bit_size); let unsigned_l_witness = self.get_or_create_witness(&unsigned_lhs); let unsigned_r_witness = self.get_or_create_witness(&unsigned_rhs); @@ -357,13 +371,16 @@ impl GeneratedAcir { // Unsigned to signed: derive q and r from q1,r1 and the signs of lhs and rhs // Quotient sign is lhs sign * rhs sign, whose resulting sign bit is the XOR of the sign bits - let q_sign = (&Expression::from(lhs_leading) + &Expression::from(rhs_leading)).add_mul( - -FieldElement::from(2_i128), - &(&Expression::from(lhs_leading) * &Expression::from(rhs_leading)).unwrap(), - ); + let sign_sum = + &Expression::from(lhs_leading_witness) + &Expression::from(rhs_leading_witness); + let sign_prod = (&Expression::from(lhs_leading_witness) + * &Expression::from(rhs_leading_witness)) + .expect("Product of two witnesses so result is degree 2"); + let q_sign = sign_sum.add_mul(-FieldElement::from(2_i128), &sign_prod); + let q_sign_witness = self.get_or_create_witness(&q_sign); let quotient = self.two_complement(&q1.into(), q_sign_witness, max_bit_size); - let remainder = self.two_complement(&r1.into(), lhs_leading, max_bit_size); + let remainder = self.two_complement(&r1.into(), lhs_leading_witness, max_bit_size); Ok((quotient, remainder)) } @@ -377,7 +394,7 @@ impl GeneratedAcir { rhs: &Expression, max_bit_size: u32, predicate: &Expression, - ) -> Result<(Witness, Witness), AcirGenError> { + ) -> Result<(Witness, Witness), RuntimeError> { // lhs = rhs * q + r // // If predicate is zero, `q_witness` and `r_witness` will be 0 @@ -435,7 +452,7 @@ impl GeneratedAcir { rhs: &Expression, offset: &Expression, bits: u32, - ) -> Result<(), AcirGenError> { + ) -> Result<(), RuntimeError> { const fn num_bits() -> usize { std::mem::size_of::() * 8 } @@ -635,11 +652,11 @@ impl GeneratedAcir { &mut self, witness: Witness, num_bits: u32, - ) -> Result<(), AcirGenError> { + ) -> Result<(), RuntimeError> { // We class this as an error because users should instead // do `as Field`. if num_bits >= FieldElement::max_num_bits() { - return Err(AcirGenError::InvalidRangeConstraint { + return Err(RuntimeError::InvalidRangeConstraint { num_bits: FieldElement::max_num_bits(), location: self.current_location, }); @@ -663,7 +680,7 @@ impl GeneratedAcir { predicate: Option, q_max_bits: u32, r_max_bits: u32, - ) -> Result<(Witness, Witness), AcirGenError> { + ) -> Result<(Witness, Witness), RuntimeError> { let q_witness = self.next_witness_index(); let r_witness = self.next_witness_index(); @@ -691,7 +708,7 @@ impl GeneratedAcir { b: &Expression, max_bits: u32, predicate: Expression, - ) -> Result { + ) -> Result { // Ensure that 2^{max_bits + 1} is less than the field size // // TODO: perhaps this should be a user error, instead of an assert @@ -760,7 +777,11 @@ impl GeneratedAcir { /// /// n.b. A sorting network is a predetermined set of switches, /// the control bits indicate the configuration of each switch: false for pass-through and true for cross-over - pub(crate) fn permutation(&mut self, in_expr: &[Expression], out_expr: &[Expression]) { + pub(crate) fn permutation( + &mut self, + in_expr: &[Expression], + out_expr: &[Expression], + ) -> Result<(), RuntimeError> { let mut bits_len = 0; for i in 0..in_expr.len() { bits_len += ((i + 1) as f32).log2().ceil() as u32; @@ -774,77 +795,80 @@ impl GeneratedAcir { bits: bits.clone(), sort_by: vec![0], })); - let (_, b) = self.permutation_layer(in_expr, &bits, false); + let (_, b) = self.permutation_layer(in_expr, &bits, false)?; // Constrain the network output to out_expr for (b, o) in b.iter().zip(out_expr) { self.push_opcode(AcirOpcode::Arithmetic(b - o)); } + Ok(()) } } /// This function will return the number of inputs that a blackbox function /// expects. Returning `None` if there is no expectation. -fn black_box_func_expected_input_size(name: BlackBoxFunc) -> Option { +fn black_box_func_expected_input_size(name: BlackBoxFunc) -> Result, InternalError> { match name { // Bitwise opcodes will take in 2 parameters - BlackBoxFunc::AND | BlackBoxFunc::XOR => Some(2), + BlackBoxFunc::AND | BlackBoxFunc::XOR => Ok(Some(2)), // All of the hash/cipher methods will take in a // variable number of inputs. BlackBoxFunc::Keccak256 | BlackBoxFunc::SHA256 | BlackBoxFunc::Blake2s | BlackBoxFunc::Pedersen - | BlackBoxFunc::HashToField128Security => None, + | BlackBoxFunc::HashToField128Security => Ok(None), // Can only apply a range constraint to one // witness at a time. - BlackBoxFunc::RANGE => Some(1), + BlackBoxFunc::RANGE => Ok(Some(1)), // Signature verification algorithms will take in a variable // number of inputs, since the message/hashed-message can vary in size. BlackBoxFunc::SchnorrVerify | BlackBoxFunc::EcdsaSecp256k1 - | BlackBoxFunc::EcdsaSecp256r1 => None, + | BlackBoxFunc::EcdsaSecp256r1 => Ok(None), // Inputs for fixed based scalar multiplication // is just a scalar - BlackBoxFunc::FixedBaseScalarMul => Some(1), + BlackBoxFunc::FixedBaseScalarMul => Ok(Some(1)), // TODO(#1570): Generate ACIR for recursive aggregation // RecursiveAggregation has variable inputs and we could return `None` here, - // but as it is not fully implemented we panic for now - BlackBoxFunc::RecursiveAggregation => { - panic!("ICE: Cannot generate ACIR for recursive aggregation") - } + // but as it is not fully implemented we return an ICE error for now + BlackBoxFunc::RecursiveAggregation => Err(InternalError::NotImplemented { + name: "recursive aggregation".to_string(), + location: None, + }), } } /// This function will return the number of outputs that a blackbox function /// expects. Returning `None` if there is no expectation. -fn black_box_expected_output_size(name: BlackBoxFunc) -> u32 { +fn black_box_expected_output_size(name: BlackBoxFunc) -> Result { match name { // Bitwise opcodes will return 1 parameter which is the output // or the operation. - BlackBoxFunc::AND | BlackBoxFunc::XOR => 1, + BlackBoxFunc::AND | BlackBoxFunc::XOR => Ok(1), // 32 byte hash algorithms - BlackBoxFunc::Keccak256 | BlackBoxFunc::SHA256 | BlackBoxFunc::Blake2s => 32, + BlackBoxFunc::Keccak256 | BlackBoxFunc::SHA256 | BlackBoxFunc::Blake2s => Ok(32), // Hash to field returns a field element - BlackBoxFunc::HashToField128Security => 1, + BlackBoxFunc::HashToField128Security => Ok(1), // Pedersen returns a point - BlackBoxFunc::Pedersen => 2, + BlackBoxFunc::Pedersen => Ok(2), // Can only apply a range constraint to one // witness at a time. - BlackBoxFunc::RANGE => 0, + BlackBoxFunc::RANGE => Ok(0), // Signature verification algorithms will return a boolean BlackBoxFunc::SchnorrVerify | BlackBoxFunc::EcdsaSecp256k1 - | BlackBoxFunc::EcdsaSecp256r1 => 1, + | BlackBoxFunc::EcdsaSecp256r1 => Ok(1), // Output of fixed based scalar mul over the embedded curve // will be 2 field elements representing the point. - BlackBoxFunc::FixedBaseScalarMul => 2, + BlackBoxFunc::FixedBaseScalarMul => Ok(2), // TODO(#1570): Generate ACIR for recursive aggregation - BlackBoxFunc::RecursiveAggregation => { - panic!("ICE: Cannot generate ACIR for recursive aggregation") - } + BlackBoxFunc::RecursiveAggregation => Err(InternalError::NotImplemented { + name: "recursive aggregation".to_string(), + location: None, + }), } } @@ -863,12 +887,16 @@ fn black_box_expected_output_size(name: BlackBoxFunc) -> u32 { /// #[foreign(sha256)] /// fn sha256(_input : [u8; N]) -> [u8; 32] {} /// `` -fn intrinsics_check_inputs(name: BlackBoxFunc, inputs: &[FunctionInput]) { - let expected_num_inputs = match black_box_func_expected_input_size(name) { +fn intrinsics_check_inputs( + name: BlackBoxFunc, + inputs: &[FunctionInput], +) -> Result<(), InternalError> { + let expected_num_inputs = match black_box_func_expected_input_size(name)? { Some(expected_num_inputs) => expected_num_inputs, - None => return, + None => return Ok(()), }; let got_num_inputs = inputs.len(); assert_eq!(expected_num_inputs,inputs.len(),"Tried to call black box function {name} with {got_num_inputs} inputs, but this function's definition requires {expected_num_inputs} inputs"); + Ok(()) } diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/sort.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/sort.rs index 622bf24ba65..42a6a5f1a4a 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/sort.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/sort.rs @@ -1,3 +1,5 @@ +use crate::errors::InternalError; + use super::generated_acir::GeneratedAcir; use acvm::acir::native_types::{Expression, Witness}; @@ -13,10 +15,10 @@ impl GeneratedAcir { in_expr: &[Expression], bits: &[Witness], generate_witness: bool, - ) -> (Vec, Vec) { + ) -> Result<(Vec, Vec), InternalError> { let n = in_expr.len(); if n == 1 { - return (Vec::new(), in_expr.to_vec()); + return Ok((Vec::new(), in_expr.to_vec())); } let n1 = n / 2; @@ -46,14 +48,17 @@ impl GeneratedAcir { in_sub2.push(&in_expr[2 * i + 1] - &intermediate); } if n % 2 == 1 { - in_sub2.push(in_expr.last().unwrap().clone()); + in_sub2.push(match in_expr.last() { + Some(in_expr) => in_expr.clone(), + None => return Err(InternalError::EmptyArray { location: self.current_location }), + }); } let mut out_expr = Vec::new(); // compute results for the sub networks let bits1 = if generate_witness { bits } else { &bits[n1 + (n - 1) / 2..] }; - let (w1, b1) = self.permutation_layer(&in_sub1, bits1, generate_witness); + let (w1, b1) = self.permutation_layer(&in_sub1, bits1, generate_witness)?; let bits2 = if generate_witness { bits } else { &bits[n1 + (n - 1) / 2 + w1.len()..] }; - let (w2, b2) = self.permutation_layer(&in_sub2, bits2, generate_witness); + let (w2, b2) = self.permutation_layer(&in_sub2, bits2, generate_witness)?; // apply the output switches for i in 0..(n - 1) / 2 { let c = if generate_witness { self.next_witness_index() } else { bits[n1 + i] }; @@ -63,11 +68,17 @@ impl GeneratedAcir { out_expr.push(&b2[i] - &intermediate); } if n % 2 == 0 { - out_expr.push(b1.last().unwrap().clone()); + out_expr.push(match b1.last() { + Some(b1) => b1.clone(), + None => return Err(InternalError::EmptyArray { location: self.current_location }), + }); } - out_expr.push(b2.last().unwrap().clone()); + out_expr.push(match b2.last() { + Some(b2) => b2.clone(), + None => return Err(InternalError::EmptyArray { location: self.current_location }), + }); conf.extend(w1); conf.extend(w2); - (conf, out_expr) + Ok((conf, out_expr)) } } diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs index ad10bed96f9..1fce4cd76ad 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -1,19 +1,11 @@ //! This file holds the pass to convert from Noir's SSA IR to ACIR. +mod acir_ir; use std::collections::{HashMap, HashSet}; use std::fmt::Debug; use std::ops::RangeInclusive; -use crate::brillig::brillig_ir::BrilligContext; -use crate::{ - brillig::{brillig_gen::brillig_fn::FunctionContext as BrilligFunctionContext, Brillig}, - errors::RuntimeError, -}; - -use self::acir_ir::{ - acir_variable::{AcirContext, AcirType, AcirVar}, - errors::AcirGenError, -}; +use self::acir_ir::acir_variable::{AcirContext, AcirType, AcirVar}; use super::{ ir::{ dfg::DataFlowGraph, @@ -27,17 +19,17 @@ use super::{ }, ssa_gen::Ssa, }; +use crate::brillig::brillig_ir::BrilligContext; +use crate::brillig::{brillig_gen::brillig_fn::FunctionContext as BrilligFunctionContext, Brillig}; +use crate::errors::{InternalError, RuntimeError}; +pub(crate) use acir_ir::generated_acir::GeneratedAcir; use acvm::{ acir::{brillig::Opcode, circuit::opcodes::BlockId, native_types::Expression}, FieldElement, }; use iter_extended::{try_vecmap, vecmap}; - -pub(crate) use acir_ir::generated_acir::GeneratedAcir; use noirc_abi::AbiDistinctness; -mod acir_ir; - /// Context struct for the acir generation pass. /// May be similar to the Evaluator struct in the current SSA IR. struct Context { @@ -87,12 +79,13 @@ pub(crate) enum AcirValue { } impl AcirValue { - fn into_var(self) -> AcirVar { + fn into_var(self) -> Result { match self { - AcirValue::Var(var, _) => var, - AcirValue::DynamicArray(_) | AcirValue::Array(_) => { - panic!("Called AcirValue::into_var on an array") - } + AcirValue::Var(var, _) => Ok(var), + AcirValue::DynamicArray(_) | AcirValue::Array(_) => Err(InternalError::General { + message: "Called AcirValue::into_var on an array".to_string(), + location: None, + }), } } @@ -156,7 +149,7 @@ impl Context { ssa: Ssa, brillig: Brillig, allow_log_ops: bool, - ) -> Result { + ) -> Result { let main_func = ssa.main(); match main_func.runtime() { RuntimeType::Acir => self.convert_acir_main(main_func, &ssa, brillig, allow_log_ops), @@ -170,7 +163,7 @@ impl Context { ssa: &Ssa, brillig: Brillig, allow_log_ops: bool, - ) -> Result { + ) -> Result { let dfg = &main_func.dfg; let entry_block = &dfg[main_func.entry_block()]; let input_witness = self.convert_ssa_block_params(entry_block.parameters(), dfg)?; @@ -179,7 +172,7 @@ impl Context { self.convert_ssa_instruction(*instruction_id, dfg, ssa, &brillig, allow_log_ops)?; } - self.convert_ssa_return(entry_block.unwrap_terminator(), dfg); + self.convert_ssa_return(entry_block.unwrap_terminator(), dfg)?; Ok(self.acir_context.finish(input_witness.collect())) } @@ -188,7 +181,7 @@ impl Context { mut self, main_func: &Function, brillig: Brillig, - ) -> Result { + ) -> Result { let dfg = &main_func.dfg; let inputs = try_vecmap(dfg[main_func.entry_block()].parameters(), |param_id| { @@ -200,10 +193,14 @@ impl Context { let outputs: Vec = vecmap(main_func.returns(), |result_id| dfg.type_of_value(*result_id).into()); - let code = self.gen_brillig_for(main_func, &brillig); + let code = self.gen_brillig_for(main_func, &brillig)?; - let output_values = - self.acir_context.brillig(self.current_side_effects_enabled_var, code, inputs, outputs); + let output_values = self.acir_context.brillig( + self.current_side_effects_enabled_var, + code, + inputs, + outputs, + )?; let output_vars: Vec<_> = output_values .iter() .flat_map(|value| value.clone().flatten()) @@ -211,7 +208,7 @@ impl Context { .collect(); for acir_var in output_vars { - self.acir_context.return_var(acir_var); + self.acir_context.return_var(acir_var)?; } Ok(self.acir_context.finish(witness_inputs)) @@ -222,7 +219,7 @@ impl Context { &mut self, params: &[ValueId], dfg: &DataFlowGraph, - ) -> Result, AcirGenError> { + ) -> Result, RuntimeError> { // The first witness (if any) is the next one let start_witness = self.acir_context.current_witness_index().0 + 1; for param_id in params { @@ -233,7 +230,7 @@ impl Context { AcirValue::Array(values) => { let block_id = BlockId(param_id.to_usize() as u32); let v = vecmap(values, |v| v.clone()); - self.initialize_array(block_id, values.len(), Some(&v)); + self.initialize_array(block_id, values.len(), Some(&v))?; } AcirValue::DynamicArray(_) => unreachable!( "The dynamic array type is created in Acir gen and therefore cannot be a block parameter" @@ -245,15 +242,15 @@ impl Context { Ok(start_witness..=end_witness) } - fn convert_ssa_block_param(&mut self, param_type: &Type) -> Result { + fn convert_ssa_block_param(&mut self, param_type: &Type) -> Result { self.create_value_from_type(param_type, &mut |this, typ| this.add_numeric_input_var(&typ)) } fn create_value_from_type( &mut self, param_type: &Type, - make_var: &mut impl FnMut(&mut Self, NumericType) -> Result, - ) -> Result { + make_var: &mut impl FnMut(&mut Self, NumericType) -> Result, + ) -> Result { match param_type { Type::Numeric(numeric_type) => { let typ = AcirType::new(*numeric_type); @@ -282,7 +279,7 @@ impl Context { fn add_numeric_input_var( &mut self, numeric_type: &NumericType, - ) -> Result { + ) -> Result { let acir_var = self.acir_context.add_variable(); if matches!(numeric_type, NumericType::Signed { .. } | NumericType::Unsigned { .. }) { self.acir_context.range_constrain_var(acir_var, numeric_type)?; @@ -298,7 +295,7 @@ impl Context { ssa: &Ssa, brillig: &Brillig, allow_log_ops: bool, - ) -> Result<(), AcirGenError> { + ) -> Result<(), RuntimeError> { let instruction = &dfg[instruction_id]; self.acir_context.set_location(dfg.get_location(&instruction_id)); match instruction { @@ -307,7 +304,7 @@ impl Context { self.define_result_var(dfg, instruction_id, result_acir_var); } Instruction::Constrain(value_id) => { - let constrain_condition = self.convert_numeric_value(*value_id, dfg); + let constrain_condition = self.convert_numeric_value(*value_id, dfg)?; self.acir_context.assert_eq_one(constrain_condition)?; } Instruction::Cast(value_id, typ) => { @@ -326,11 +323,11 @@ impl Context { RuntimeType::Brillig => { let inputs = vecmap(arguments, |arg| self.convert_value(*arg, dfg)); - let code = self.gen_brillig_for(func, brillig); + let code = self.gen_brillig_for(func, brillig)?; let outputs: Vec = vecmap(result_ids, |result_id| dfg.type_of_value(*result_id).into()); - let output_values = self.acir_context.brillig(self.current_side_effects_enabled_var, code, inputs, outputs); + let output_values = self.acir_context.brillig(self.current_side_effects_enabled_var, code, inputs, outputs)?; // Compiler sanity check assert_eq!(result_ids.len(), output_values.len(), "ICE: The number of Brillig output values should match the result ids in SSA"); @@ -378,7 +375,7 @@ impl Context { self.define_result_var(dfg, instruction_id, result_acir_var); } Instruction::EnableSideEffects { condition } => { - let acir_var = self.convert_numeric_value(*condition, dfg); + let acir_var = self.convert_numeric_value(*condition, dfg)?; self.current_side_effects_enabled_var = acir_var; } Instruction::ArrayGet { array, index } => { @@ -401,7 +398,11 @@ impl Context { Ok(()) } - fn gen_brillig_for(&self, func: &Function, brillig: &Brillig) -> Vec { + fn gen_brillig_for( + &self, + func: &Function, + brillig: &Brillig, + ) -> Result, InternalError> { // Create the entry point artifact let mut entry_point = BrilligContext::new_entry_point_artifact( BrilligFunctionContext::parameters(func), @@ -410,13 +411,20 @@ impl Context { ); // Link the entry point with all dependencies while let Some(unresolved_fn_label) = entry_point.first_unresolved_function_call() { - let artifact = &brillig - .find_by_function_label(unresolved_fn_label.clone()) - .unwrap_or_else(|| panic!("Cannot find linked fn {unresolved_fn_label}")); + let artifact = &brillig.find_by_function_label(unresolved_fn_label.clone()); + let artifact = match artifact { + Some(artifact) => artifact, + None => { + return Err(InternalError::General { + message: format!("Cannot find linked fn {unresolved_fn_label}"), + location: None, + }) + } + }; entry_point.link_with(artifact); } // Generate the final bytecode - entry_point.finish() + Ok(entry_point.finish()) } /// Handles an ArrayGet or ArraySet instruction. @@ -429,23 +437,37 @@ impl Context { index: ValueId, store_value: Option, dfg: &DataFlowGraph, - ) -> Result<(), AcirGenError> { + ) -> Result<(), RuntimeError> { let index_const = dfg.get_numeric_constant(index); match self.convert_value(array, dfg) { - AcirValue::Var(acir_var, _) => panic!("Expected an array value, found: {acir_var:?}"), + AcirValue::Var(acir_var, _) => { + return Err(RuntimeError::InternalError(InternalError::UnExpected { + expected: "an array value".to_string(), + found: format!("{acir_var:?}"), + location: self.acir_context.get_location(), + })) + } AcirValue::Array(array) => { if let Some(index_const) = index_const { let array_size = array.len(); - let index = - index_const.try_to_u64().expect("Expected array index to fit into a u64") - as usize; + let index = match index_const.try_to_u64() { + Some(index_const) => index_const as usize, + None => { + let location = self.acir_context.get_location(); + return Err(RuntimeError::TypeConversion { + from: "array index".to_string(), + into: "u64".to_string(), + location, + }); + } + }; if index >= array_size { // Ignore the error if side effects are disabled. if self.acir_context.is_constant_one(&self.current_side_effects_enabled_var) { let location = self.acir_context.get_location(); - return Err(AcirGenError::IndexOutOfBounds { + return Err(RuntimeError::IndexOutOfBounds { index, array_size, location, @@ -474,9 +496,9 @@ impl Context { } if let Some(store) = store_value { - self.array_set(instruction, array, index, store, dfg); + self.array_set(instruction, array, index, store, dfg)?; } else { - self.array_get(instruction, array, index, dfg); + self.array_get(instruction, array, index, dfg)?; } Ok(()) @@ -489,7 +511,7 @@ impl Context { array: ValueId, index: ValueId, dfg: &DataFlowGraph, - ) { + ) -> Result<(), RuntimeError> { let array = dfg.resolve(array); let block_id = BlockId(array.to_usize() as u32); if !self.initialized_arrays.contains(&block_id) { @@ -497,14 +519,19 @@ impl Context { Value::Array { array, .. } => { let values: Vec = array.iter().map(|i| self.convert_value(*i, dfg)).collect(); - self.initialize_array(block_id, array.len(), Some(&values)); + self.initialize_array(block_id, array.len(), Some(&values))?; + } + _ => { + return Err(RuntimeError::UnInitialized { + name: "array".to_string(), + location: self.acir_context.get_location(), + }) } - _ => panic!("reading uninitialized array"), } } - let index_var = self.convert_value(index, dfg).into_var(); - let read = self.acir_context.read_from_memory(block_id, &index_var); + let index_var = self.convert_value(index, dfg).into_var()?; + let read = self.acir_context.read_from_memory(block_id, &index_var)?; let typ = match dfg.type_of_value(array) { Type::Array(typ, _) => { if typ.len() != 1 { @@ -518,6 +545,7 @@ impl Context { }; let typ = AcirType::from(typ); self.define_result(dfg, instruction, AcirValue::Var(read, typ)); + Ok(()) } /// Copy the array and generates a write opcode on the new array @@ -530,7 +558,7 @@ impl Context { index: ValueId, store_value: ValueId, dfg: &DataFlowGraph, - ) { + ) -> Result<(), InternalError> { // Fetch the internal SSA ID for the array let array = dfg.resolve(array); let array_ssa_id = array.to_usize() as u32; @@ -554,9 +582,14 @@ impl Context { Value::Array { array, .. } => { let values: Vec = array.iter().map(|i| self.convert_value(*i, dfg)).collect(); - self.initialize_array(block_id, array.len(), Some(&values)); + self.initialize_array(block_id, array.len(), Some(&values))?; + } + _ => { + return Err(InternalError::General { + message: format!("Array {array} should be initialized"), + location: self.acir_context.get_location(), + }) } - _ => panic!("Array {} should be initialized", array), } } @@ -570,7 +603,7 @@ impl Context { let result_block_id = BlockId(result_array_id); // Initialize the new array with zero values - self.initialize_array(result_block_id, len, None); + self.initialize_array(result_block_id, len, None)?; // Copy the values from the old array into the newly created zeroed array for i in 0..len { @@ -578,26 +611,33 @@ impl Context { self.acir_context.add_constant(FieldElement::from(i as u128)), AcirType::NumericType(NumericType::NativeField), ); - let var = index.into_var(); - let read = self.acir_context.read_from_memory(block_id, &var); - self.acir_context.write_to_memory(result_block_id, &var, &read); + let var = index.into_var()?; + let read = self.acir_context.read_from_memory(block_id, &var)?; + self.acir_context.write_to_memory(result_block_id, &var, &read)?; } // Write the new value into the new array at the specified index - let index_var = self.convert_value(index, dfg).into_var(); - let value_var = self.convert_value(store_value, dfg).into_var(); - self.acir_context.write_to_memory(result_block_id, &index_var, &value_var); + let index_var = self.convert_value(index, dfg).into_var()?; + let value_var = self.convert_value(store_value, dfg).into_var()?; + self.acir_context.write_to_memory(result_block_id, &index_var, &value_var)?; let result_value = AcirValue::DynamicArray(AcirDynamicArray { block_id: result_block_id, len }); self.define_result(dfg, instruction, result_value); + Ok(()) } /// Initializes an array with the given values and caches the fact that we /// have initialized this array. - fn initialize_array(&mut self, array: BlockId, len: usize, values: Option<&[AcirValue]>) { - self.acir_context.initialize_array(array, len, values); + fn initialize_array( + &mut self, + array: BlockId, + len: usize, + values: Option<&[AcirValue]>, + ) -> Result<(), InternalError> { + self.acir_context.initialize_array(array, len, values)?; self.initialized_arrays.insert(array); + Ok(()) } /// Remember the result of an instruction returning a single value @@ -624,7 +664,11 @@ impl Context { } /// Converts an SSA terminator's return values into their ACIR representations - fn convert_ssa_return(&mut self, terminator: &TerminatorInstruction, dfg: &DataFlowGraph) { + fn convert_ssa_return( + &mut self, + terminator: &TerminatorInstruction, + dfg: &DataFlowGraph, + ) -> Result<(), InternalError> { let return_values = match terminator { TerminatorInstruction::Return { return_values } => return_values, _ => unreachable!("ICE: Program must have a singular return"), @@ -634,8 +678,9 @@ impl Context { // will expand the array if there is one. let return_acir_vars = self.flatten_value_list(return_values, dfg); for acir_var in return_acir_vars { - self.acir_context.return_var(acir_var); + self.acir_context.return_var(acir_var)?; } + Ok(()) } /// Gets the cached `AcirVar` that was converted from the corresponding `ValueId`. If it does @@ -679,11 +724,25 @@ impl Context { acir_value } - fn convert_numeric_value(&mut self, value_id: ValueId, dfg: &DataFlowGraph) -> AcirVar { + fn convert_numeric_value( + &mut self, + value_id: ValueId, + dfg: &DataFlowGraph, + ) -> Result { match self.convert_value(value_id, dfg) { - AcirValue::Var(acir_var, _) => acir_var, - AcirValue::Array(array) => panic!("Expected a numeric value, found: {array:?}"), - AcirValue::DynamicArray(_) => panic!("Expected a numeric value, found an array"), + AcirValue::Var(acir_var, _) => Ok(acir_var), + AcirValue::Array(array) => { + return Err(InternalError::UnExpected { + expected: "a numeric value".to_string(), + found: format!("{array:?}"), + location: self.acir_context.get_location(), + }) + } + AcirValue::DynamicArray(_) => Err(InternalError::UnExpected { + expected: "a numeric value".to_string(), + found: "an array".to_string(), + location: self.acir_context.get_location(), + }), } } @@ -692,9 +751,9 @@ impl Context { &mut self, binary: &Binary, dfg: &DataFlowGraph, - ) -> Result { - let lhs = self.convert_numeric_value(binary.lhs, dfg); - let rhs = self.convert_numeric_value(binary.rhs, dfg); + ) -> Result { + let lhs = self.convert_numeric_value(binary.lhs, dfg)?; + let rhs = self.convert_numeric_value(binary.rhs, dfg)?; let binary_type = self.type_of_binary_operation(binary, dfg); match &binary_type { @@ -705,7 +764,7 @@ impl Context { // truncation technique: result % 2^bit_size to be valid. let max_integer_bit_size = FieldElement::max_num_bits() / 2; if *bit_size > max_integer_bit_size { - return Err(AcirGenError::UnsupportedIntegerSize { + return Err(RuntimeError::UnsupportedIntegerSize { num_bits: *bit_size, max_num_bits: max_integer_bit_size, location: self.acir_context.get_location(), @@ -813,7 +872,7 @@ impl Context { value_id: &ValueId, typ: &Type, dfg: &DataFlowGraph, - ) -> Result { + ) -> Result { let (variable, incoming_type) = match self.convert_value(*value_id, dfg) { AcirValue::Var(variable, typ) => (variable, typ), AcirValue::DynamicArray(_) | AcirValue::Array(_) => { @@ -851,8 +910,8 @@ impl Context { bit_size: u32, max_bit_size: u32, dfg: &DataFlowGraph, - ) -> Result { - let mut var = self.convert_numeric_value(value_id, dfg); + ) -> Result { + let mut var = self.convert_numeric_value(value_id, dfg)?; let truncation_target = match &dfg[value_id] { Value::Instruction { instruction, .. } => &dfg[*instruction], _ => unreachable!("ICE: Truncates are only ever applied to the result of a binary op"), @@ -879,7 +938,7 @@ impl Context { dfg: &DataFlowGraph, allow_log_ops: bool, result_ids: &[ValueId], - ) -> Result, AcirGenError> { + ) -> Result, RuntimeError> { match intrinsic { Intrinsic::BlackBox(black_box) => { let inputs = vecmap(arguments, |arg| self.convert_value(*arg, dfg)); @@ -889,16 +948,16 @@ impl Context { Ok(Self::convert_vars_to_values(vars, dfg, result_ids)) } Intrinsic::ToRadix(endian) => { - let field = self.convert_value(arguments[0], dfg).into_var(); - let radix = self.convert_value(arguments[1], dfg).into_var(); - let limb_size = self.convert_value(arguments[2], dfg).into_var(); + let field = self.convert_value(arguments[0], dfg).into_var()?; + let radix = self.convert_value(arguments[1], dfg).into_var()?; + let limb_size = self.convert_value(arguments[2], dfg).into_var()?; let result_type = Self::array_element_type(dfg, result_ids[0]); self.acir_context.radix_decompose(endian, field, radix, limb_size, result_type) } Intrinsic::ToBits(endian) => { - let field = self.convert_value(arguments[0], dfg).into_var(); - let bit_size = self.convert_value(arguments[1], dfg).into_var(); + let field = self.convert_value(arguments[0], dfg).into_var()?; + let bit_size = self.convert_value(arguments[1], dfg).into_var()?; let result_type = Self::array_element_type(dfg, result_ids[0]); self.acir_context.bit_decompose(endian, field, bit_size, result_type) @@ -1020,7 +1079,7 @@ impl Context { } /// Creates a default, meaningless value meant only to be a valid value of the given type. - fn create_default_value(&mut self, param_type: &Type) -> Result { + fn create_default_value(&mut self, param_type: &Type) -> Result { self.create_value_from_type(param_type, &mut |this, _| { Ok(this.acir_context.add_constant(FieldElement::zero())) })