From e0b7b16b29fd5dc636506d117bfe52d40adecc3a Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Thu, 27 Jul 2023 17:32:13 +0100 Subject: [PATCH 01/14] implement acir gen errors --- crates/noirc_evaluator/src/errors.rs | 343 +++++++++++------- crates/noirc_evaluator/src/ssa_refactor.rs | 2 +- .../src/ssa_refactor/acir_gen/acir_ir.rs | 1 - .../acir_gen/acir_ir/acir_variable.rs | 210 ++++++----- .../ssa_refactor/acir_gen/acir_ir/errors.rs | 62 ---- .../acir_gen/acir_ir/generated_acir.rs | 138 ++++--- .../src/ssa_refactor/acir_gen/acir_ir/sort.rs | 16 +- .../src/ssa_refactor/acir_gen/mod.rs | 228 +++++++----- 8 files changed, 580 insertions(+), 420 deletions(-) delete mode 100644 crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/errors.rs diff --git a/crates/noirc_evaluator/src/errors.rs b/crates/noirc_evaluator/src/errors.rs index 2d8b02008c6..3f9ca2b8c6f 100644 --- a/crates/noirc_evaluator/src/errors.rs +++ b/crates/noirc_evaluator/src/errors.rs @@ -1,151 +1,228 @@ +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)] +// 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, +// } +// } +// } + +// impl From for RuntimeError { +// fn from(kind: RuntimeErrorKind) -> RuntimeError { +// RuntimeError { location: None, kind } +// } +// } + +// 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() } +// } +// } + +// #[derive(Error, Debug)] +// pub enum RuntimeErrorKind { +// // Array errors +// #[error("Out of bounds")] +// ArrayOutOfBounds { index: u128, 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( +// "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 }, + +// #[error(transparent)] +// AcirGenError(#[from] AcirGenError), +// } + +// 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, +// ), +// RuntimeErrorKind::NotAnObject +// | RuntimeErrorKind::InvalidId +// | RuntimeErrorKind::DivisionByZero +// | RuntimeErrorKind::AcirGenError(_) +// | RuntimeErrorKind::DefaultWitnesses(_) +// | RuntimeErrorKind::CannotConvertSignedIntoField(_) +// | RuntimeErrorKind::PrivateAbiInput => { +// Diagnostic::simple_error("".to_owned(), error.kind.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, +// ), +// } +// } +// } + +#[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)] + ICEError(#[from] ICEError), + #[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 ICEError { + #[error("ICE: Both expressions are reduced to be degree<=1")] + DegreeNotReduced { 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 implmented 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::ICEError(ref ice_error) => match ice_error { + ICEError::DegreeNotReduced { location } + | ICEError::General { location, .. } + | ICEError::MissingArg { location, .. } + | ICEError::NotAConstant { location, .. } + | ICEError::NotImplemented { location, .. } + | ICEError::UndeclaredAcirVar { location } + | ICEError::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::ICEError(_) => Diagnostic::simple_error( + "Internal Consistency Evaluators Errors \n This is likely a bug. Consider Openning 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.rs b/crates/noirc_evaluator/src/ssa_refactor.rs index 67a908209ae..34370eb5084 100644 --- a/crates/noirc_evaluator/src/ssa_refactor.rs +++ b/crates/noirc_evaluator/src/ssa_refactor.rs @@ -19,7 +19,7 @@ use noirc_frontend::monomorphization::ast::Program; use self::{abi_gen::gen_abi, acir_gen::GeneratedAcir, ir::function::RuntimeType, ssa_gen::Ssa}; mod abi_gen; -mod acir_gen; +pub(crate) mod acir_gen; pub mod ir; mod opt; mod ssa_builder; 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 2fdfb0bd10f..e631bf1ebb0 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::{ICEError, 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}; @@ -162,7 +162,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 @@ -179,7 +179,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)?; @@ -189,7 +189,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) } @@ -202,7 +202,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) } @@ -220,7 +220,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]; @@ -238,7 +238,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]) @@ -250,7 +250,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]) @@ -262,7 +262,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, typ: AcirType, - ) -> Result { + ) -> Result { let bit_size = typ.bit_size(); if bit_size == 1 { // Operands are booleans @@ -285,7 +285,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]; @@ -296,7 +296,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(), @@ -318,7 +318,7 @@ impl AcirContext { rhs: AcirVar, typ: AcirType, predicate: AcirVar, - ) -> Result { + ) -> Result { let numeric_type = match typ { AcirType::NumericType(numeric_type) => numeric_type, AcirType::Array(_, _) => { @@ -345,7 +345,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) { @@ -392,14 +392,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)) = @@ -414,7 +414,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)); @@ -433,7 +433,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, _typ: AcirType, - ) -> Result { + ) -> Result { let rhs_data = &self.vars[&rhs]; // Compute 2^{rhs} @@ -453,7 +453,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]; @@ -485,7 +485,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(); @@ -509,7 +509,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) } @@ -530,7 +530,7 @@ impl AcirContext { rhs: AcirVar, typ: AcirType, predicate: AcirVar, - ) -> Result { + ) -> Result { let rhs_data = &self.vars[&rhs]; // Compute 2^{rhs} @@ -545,8 +545,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<(), ICEError> { + let acir_var_data = match self.vars.get(&acir_var) { + Some(acir_var_data) => acir_var_data, + None => return Err(ICEError::UndeclaredAcirVar { location: self.get_location() }), + }; // TODO: Add caching to prevent expressions from being needlessly duplicated let witness = match acir_var_data { AcirVarData::Const(constant) => { @@ -556,6 +559,7 @@ impl AcirContext { AcirVarData::Witness(witness) => *witness, }; self.acir_ir.push_return_witness(witness); + Ok(()) } /// Constrains the `AcirVar` variable to be of type `NumericType`. @@ -563,7 +567,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 } => { @@ -584,7 +588,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(); @@ -609,7 +613,7 @@ impl AcirContext { rhs: AcirVar, bit_size: u32, predicate: AcirVar, - ) -> Result { + ) -> Result { let lhs_data = &self.vars[&lhs]; let rhs_data = &self.vars[&rhs]; @@ -636,7 +640,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)?; @@ -651,17 +655,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(domian_var) => domian_var.into_var()?, + None => { + return Err(RuntimeError::ICEError(ICEError::MissingArg { + name: "pedersen call".to_string(), + arg: "domain seperator".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::ICEError(ICEError::NotAConstant { + name: "domain separator".to_string(), + location: self.get_location(), + })) + } + }; vec![domain_constant] } @@ -672,7 +690,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. @@ -688,7 +706,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() { @@ -719,15 +737,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::ICEError(ICEError::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::ICEError(ICEError::NotAConstant { + name: "limb_size".to_string(), + location: self.get_location(), + })); + } + }; let input_expr = &self.vars[&input_var].to_expression(); @@ -763,13 +792,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| { @@ -827,24 +856,24 @@ impl AcirContext { code: Vec, inputs: Vec, outputs: Vec, - ) -> Vec { - let b_inputs = vecmap(inputs, |i| match i { + ) -> Result, ICEError> { + 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 { @@ -863,17 +892,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<(), ICEError> { 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 }) => { @@ -883,18 +916,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. @@ -936,7 +970,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()); @@ -949,7 +983,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) { @@ -959,9 +993,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(ICEError::UndeclaredAcirVar { location: self.get_location() }), + }; + Ok(self.acir_ir.get_or_create_witness(&var_data.to_expression())) } /// Constrain lhs to be less than rhs @@ -971,40 +1008,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<(), ICEError> { // 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`. @@ -1014,22 +1061,23 @@ impl AcirContext { block_id: BlockId, len: usize, optional_values: Option<&[AcirValue]>, - ) { + ) -> Result<(), ICEError> { // 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(); - self.var_to_witness(value) - }), + Some(optional_values) => try_vecmap(optional_values, |value| { + let value = value.clone().into_var()?; + Ok(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 18c7216a6fa..0947a7e35ea 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::{ICEError, RuntimeError}, +}; -use super::errors::AcirGenError; use acvm::acir::{ brillig::Opcode as BrilligOpcode, circuit::{ @@ -119,10 +121,10 @@ impl GeneratedAcir { func_name: BlackBoxFunc, mut inputs: Vec, constants: Vec, - ) -> Vec { - intrinsics_check_inputs(func_name, &inputs); + ) -> Result, ICEError> { + 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. @@ -179,18 +181,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(ICEError::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(ICEError::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 @@ -203,7 +217,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), @@ -263,15 +277,26 @@ impl GeneratedAcir { /// /// (1) is because an [`Expression`] can hold at most a degree-2 univariate polynomial /// which is what you get when you multiply two degree-1 univariate polynomials. - pub(crate) fn mul_with_witness(&mut self, lhs: &Expression, rhs: &Expression) -> Expression { + pub(crate) fn mul_with_witness( + &mut self, + lhs: &Expression, + rhs: &Expression, + ) -> Result { use std::borrow::Cow; let lhs_is_linear = lhs.is_linear(); let rhs_is_linear = rhs.is_linear(); // Case 1: Both expressions have at most a total degree of 1 in each term if lhs_is_linear && rhs_is_linear { - return (lhs * rhs) - .expect("one of the expressions is a constant and so this should not fail"); + match lhs * rhs { + Some(expr) => return Ok(expr), + None => { + return Err(ICEError::NotAConstant { + name: "one of the expressions".to_string(), + location: self.current_location, + }) + } + } } // Case 2: One or both of the sides needs to be reduced to a degree-1 univariate polynomial @@ -284,8 +309,10 @@ impl GeneratedAcir { // If the lhs and rhs are the same, then we do not need to reduce // rhs, we only need to square the lhs. if lhs == rhs { - return (&*lhs_reduced * &*lhs_reduced) - .expect("Both expressions are reduced to be degree<=1"); + match &*lhs_reduced * &*lhs_reduced { + Some(expr) => return Ok(expr), + None => return Err(ICEError::DegreeNotReduced { location: self.current_location }), + } }; let rhs_reduced = if rhs_is_linear { @@ -294,7 +321,10 @@ impl GeneratedAcir { Cow::Owned(self.get_or_create_witness(rhs).into()) }; - (&*lhs_reduced * &*rhs_reduced).expect("Both expressions are reduced to be degree<=1") + match &*lhs_reduced * &*rhs_reduced { + Some(expr) => Ok(expr), + None => Err(ICEError::DegreeNotReduced { location: self.current_location }), + } } /// Signed division lhs / rhs @@ -310,7 +340,7 @@ 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)); @@ -367,7 +397,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 @@ -399,9 +429,9 @@ impl GeneratedAcir { // When the predicate is 0, the equation always passes. // When the predicate is 1, the euclidean division needs to be // true. - let rhs_constraint = &self.mul_with_witness(rhs, &q_witness.into()) + r_witness; - let div_euclidean = &self.mul_with_witness(lhs, predicate) - - &self.mul_with_witness(&rhs_constraint, predicate); + let rhs_constraint = &self.mul_with_witness(rhs, &q_witness.into())? + r_witness; + let div_euclidean = &self.mul_with_witness(lhs, predicate)? + - &self.mul_with_witness(&rhs_constraint, predicate)?; self.push_opcode(AcirOpcode::Arithmetic(div_euclidean)); @@ -425,7 +455,7 @@ impl GeneratedAcir { rhs: &Expression, offset: &Expression, bits: u32, - ) -> Result<(), AcirGenError> { + ) -> Result<(), RuntimeError> { const fn num_bits() -> usize { std::mem::size_of::() * 8 } @@ -625,11 +655,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, }); @@ -653,7 +683,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(); @@ -681,7 +711,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 @@ -750,7 +780,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; @@ -764,77 +798,82 @@ 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, ICEError> { 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 + // but as it is not fully implemented we return an ICE error for now BlackBoxFunc::RecursiveAggregation => { - panic!("ICE: Cannot generate ACIR for recursive aggregation") + return Err(ICEError::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(ICEError::NotImplemented { + name: "recursive aggregation".to_string(), + location: None, + }), } } @@ -853,12 +892,13 @@ 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<(), ICEError> { + 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..958995dd827 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::ICEError; + 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), ICEError> { 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; @@ -39,7 +41,7 @@ impl GeneratedAcir { let intermediate = self.mul_with_witness( &Expression::from(conf[i]), &(&in_expr[2 * i + 1] - &in_expr[2 * i]), - ); + )?; //b1=a1+q in_sub1.push(&intermediate + &in_expr[2 * i]); //b2=a2-q @@ -51,14 +53,14 @@ impl GeneratedAcir { 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] }; conf.push(c); - let intermediate = self.mul_with_witness(&Expression::from(c), &(&b2[i] - &b1[i])); + let intermediate = self.mul_with_witness(&Expression::from(c), &(&b2[i] - &b1[i]))?; out_expr.push(&intermediate + &b1[i]); out_expr.push(&b2[i] - &intermediate); } @@ -68,6 +70,6 @@ impl GeneratedAcir { out_expr.push(b2.last().unwrap().clone()); 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 b0ade9419fe..54027a174fb 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -1,18 +1,10 @@ //! 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 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, @@ -26,17 +18,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::{ICEError, 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 { @@ -86,11 +78,11 @@ pub(crate) enum AcirValue { } impl AcirValue { - fn into_var(self) -> AcirVar { + fn into_var(self) -> Result { match self { - AcirValue::Var(var, _) => var, + AcirValue::Var(var, _) => Ok(var), AcirValue::DynamicArray(_) | AcirValue::Array(_) => { - panic!("Called AcirValue::into_var on an array") + return Err(ICEError::General { message: "".to_string(), location: None }) } } } @@ -155,7 +147,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), @@ -169,7 +161,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()]; @@ -179,7 +171,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()) } @@ -188,7 +180,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| { @@ -199,10 +191,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()) @@ -210,7 +206,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()) @@ -221,7 +217,7 @@ impl Context { &mut self, params: &[ValueId], dfg: &DataFlowGraph, - ) -> Result<(), AcirGenError> { + ) -> Result<(), RuntimeError> { for param_id in params { let typ = dfg.type_of_value(*param_id); let value = self.convert_ssa_block_param(&typ)?; @@ -230,7 +226,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" @@ -241,15 +237,15 @@ impl Context { Ok(()) } - 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); @@ -278,7 +274,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)?; @@ -294,7 +290,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 { @@ -303,7 +299,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) => { @@ -322,11 +318,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"); @@ -374,7 +370,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 } => { @@ -397,7 +393,7 @@ impl Context { Ok(()) } - fn gen_brillig_for(&self, func: &Function, brillig: &Brillig) -> Vec { + fn gen_brillig_for(&self, func: &Function, brillig: &Brillig) -> Result, ICEError> { // Create the entry point artifact let mut entry_point = BrilligContext::new_entry_point_artifact( BrilligFunctionContext::parameters(func), @@ -406,13 +402,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(ICEError::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. @@ -425,23 +428,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::ICEError(ICEError::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, @@ -470,9 +487,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(()) @@ -485,7 +502,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) { @@ -493,14 +510,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 { @@ -514,6 +536,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 @@ -526,7 +549,7 @@ impl Context { index: ValueId, store_value: ValueId, dfg: &DataFlowGraph, - ) { + ) -> Result<(), ICEError> { // Fetch the internal SSA ID for the array let array = dfg.resolve(array); let array_ssa_id = array.to_usize() as u32; @@ -550,9 +573,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(ICEError::General { + message: format!("Array {array} should be initialized"), + location: self.acir_context.get_location(), + }) } - _ => panic!("Array {} should be initialized", array), } } @@ -566,7 +594,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 { @@ -574,26 +602,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<(), ICEError> { + self.acir_context.initialize_array(array, len, values)?; self.initialized_arrays.insert(array); + Ok(()) } /// Remember the result of an instruction returning a single value @@ -620,7 +655,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<(), ICEError> { let return_values = match terminator { TerminatorInstruction::Return { return_values } => return_values, _ => unreachable!("ICE: Program must have a singular return"), @@ -630,8 +669,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 @@ -675,11 +715,27 @@ 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(ICEError::UnExpected { + expected: "a numeric value".to_string(), + found: format!("{array:?}"), + location: self.acir_context.get_location(), + }) + } + AcirValue::DynamicArray(_) => { + return Err(ICEError::UnExpected { + expected: "a numeric value".to_string(), + found: "an array".to_string(), + location: self.acir_context.get_location(), + }) + } } } @@ -688,9 +744,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 { @@ -701,7 +757,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(), @@ -809,7 +865,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(_) => { @@ -847,8 +903,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"), @@ -875,7 +931,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)); @@ -885,16 +941,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) @@ -1016,7 +1072,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())) }) From feb950cd20bfc35c194ee57478c0ef8e59f74c87 Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Thu, 27 Jul 2023 17:46:04 +0100 Subject: [PATCH 02/14] comment --- crates/noirc_evaluator/src/errors.rs | 147 ++---------------- .../acir_gen/acir_ir/acir_variable.rs | 2 +- .../acir_gen/acir_ir/generated_acir.rs | 10 +- .../src/ssa_refactor/acir_gen/mod.rs | 14 +- 4 files changed, 21 insertions(+), 152 deletions(-) diff --git a/crates/noirc_evaluator/src/errors.rs b/crates/noirc_evaluator/src/errors.rs index 3f9ca2b8c6f..3fe51d2578d 100644 --- a/crates/noirc_evaluator/src/errors.rs +++ b/crates/noirc_evaluator/src/errors.rs @@ -1,144 +1,17 @@ +//! Noir Evaluator has two types of errors +//! +//! [RuntimeError] that should be displayed to the user +//! +//! Internal Consistency Evaluators Errors [ICEError] +//! 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, -// } -// } -// } - -// impl From for RuntimeError { -// fn from(kind: RuntimeErrorKind) -> RuntimeError { -// RuntimeError { location: None, kind } -// } -// } - -// 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() } -// } -// } - -// #[derive(Error, Debug)] -// pub enum RuntimeErrorKind { -// // Array errors -// #[error("Out of bounds")] -// ArrayOutOfBounds { index: u128, 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( -// "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 }, - -// #[error(transparent)] -// AcirGenError(#[from] AcirGenError), -// } - -// 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, -// ), -// RuntimeErrorKind::NotAnObject -// | RuntimeErrorKind::InvalidId -// | RuntimeErrorKind::DivisionByZero -// | RuntimeErrorKind::AcirGenError(_) -// | RuntimeErrorKind::DefaultWitnesses(_) -// | RuntimeErrorKind::CannotConvertSignedIntoField(_) -// | RuntimeErrorKind::PrivateAbiInput => { -// Diagnostic::simple_error("".to_owned(), error.kind.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, -// ), -// } -// } -// } - #[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 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 e631bf1ebb0..ac34b989775 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 @@ -1072,7 +1072,7 @@ impl AcirContext { } Some(optional_values) => try_vecmap(optional_values, |value| { let value = value.clone().into_var()?; - Ok(self.var_to_witness(value)?) + self.var_to_witness(value) })?, }; 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 0947a7e35ea..42deb60bb88 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 @@ -837,12 +837,10 @@ fn black_box_func_expected_input_size(name: BlackBoxFunc) -> Result { - return Err(ICEError::NotImplemented { - name: "recursive aggregation".to_string(), - location: None, - }) - } + BlackBoxFunc::RecursiveAggregation => Err(ICEError::NotImplemented { + name: "recursive aggregation".to_string(), + location: None, + }), } } 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 54027a174fb..0d9c57f7b11 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -82,7 +82,7 @@ impl AcirValue { match self { AcirValue::Var(var, _) => Ok(var), AcirValue::DynamicArray(_) | AcirValue::Array(_) => { - return Err(ICEError::General { message: "".to_string(), location: None }) + Err(ICEError::General { message: "".to_string(), location: None }) } } } @@ -729,13 +729,11 @@ impl Context { location: self.acir_context.get_location(), }) } - AcirValue::DynamicArray(_) => { - return Err(ICEError::UnExpected { - expected: "a numeric value".to_string(), - found: "an array".to_string(), - location: self.acir_context.get_location(), - }) - } + AcirValue::DynamicArray(_) => Err(ICEError::UnExpected { + expected: "a numeric value".to_string(), + found: "an array".to_string(), + location: self.acir_context.get_location(), + }), } } From d36e824a4fffcac35adc3586de6e0f7a4de99a2d Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Thu, 27 Jul 2023 18:00:51 +0100 Subject: [PATCH 03/14] remove unwrap --- crates/noirc_evaluator/src/errors.rs | 5 +++- .../acir_gen/acir_ir/generated_acir.rs | 26 +++++++++++++------ .../src/ssa_refactor/acir_gen/acir_ir/sort.rs | 15 ++++++++--- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/crates/noirc_evaluator/src/errors.rs b/crates/noirc_evaluator/src/errors.rs index 3fe51d2578d..c86a504678b 100644 --- a/crates/noirc_evaluator/src/errors.rs +++ b/crates/noirc_evaluator/src/errors.rs @@ -35,7 +35,7 @@ pub enum RuntimeError { #[derive(Debug, PartialEq, Eq, Clone, Error)] pub enum ICEError { - #[error("ICE: Both expressions are reduced to be degree<=1")] + #[error("ICE: Both expressions should have degree<=1")] DegreeNotReduced { location: Option }, #[error("ICE: {message:?}")] General { message: String, location: Option }, @@ -45,6 +45,8 @@ pub enum ICEError { NotAConstant { name: String, location: Option }, #[error("{name:?} is not implmented yet")] NotImplemented { name: String, location: Option }, + #[error("Try to get element from empty array")] + UmptyArray { location: Option }, #[error("ICE: Undeclared AcirVar")] UndeclaredAcirVar { location: Option }, #[error("ICE: Expected {expected:?}, found {found:?}")] @@ -60,6 +62,7 @@ impl From for FileDiagnostic { | ICEError::MissingArg { location, .. } | ICEError::NotAConstant { location, .. } | ICEError::NotImplemented { location, .. } + | ICEError::UmptyArray { location } | ICEError::UndeclaredAcirVar { location } | ICEError::UnExpected { location, .. } => { let file_id = location.map(|loc| loc.file).unwrap(); 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 42deb60bb88..bbf99a74b16 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 @@ -259,11 +259,14 @@ impl GeneratedAcir { lhs: &Expression, leading: Witness, max_bit_size: u32, - ) -> Expression { + ) -> Result { let max_power_of_two = FieldElement::from(2_i128).pow(&FieldElement::from(max_bit_size as i128 - 1)); - let inter = &(&Expression::from_field(max_power_of_two) - lhs) * &leading.into(); - lhs.add_mul(FieldElement::from(2_i128), &inter.unwrap()) + let inter = match &(&Expression::from_field(max_power_of_two) - lhs) * &leading.into() { + Some(inter) => inter, + None => return Err(ICEError::DegreeNotReduced { location: self.current_location }), + }; + Ok(lhs.add_mul(FieldElement::from(2_i128), &inter)) } /// Returns an expression which represents `lhs * rhs` @@ -362,8 +365,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, max_bit_size)?; + let unsigned_rhs = self.two_complement(rhs, rhs_leading, 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); @@ -379,11 +382,18 @@ impl GeneratedAcir { // 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(), + match &(&Expression::from(lhs_leading) * &Expression::from(rhs_leading)) { + Some(expr) => expr, + None => { + return Err(RuntimeError::ICEError(ICEError::DegreeNotReduced { + location: self.current_location, + })) + } + }, ); 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 quotient = self.two_complement(&q1.into(), q_sign_witness, max_bit_size)?; + let remainder = self.two_complement(&r1.into(), lhs_leading, max_bit_size)?; Ok((quotient, remainder)) } 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 958995dd827..9fbc1e975a6 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 @@ -48,7 +48,10 @@ 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(ICEError::UmptyArray { location: self.current_location }), + }); } let mut out_expr = Vec::new(); // compute results for the sub networks @@ -65,9 +68,15 @@ 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(ICEError::UmptyArray { location: self.current_location }), + }); } - out_expr.push(b2.last().unwrap().clone()); + out_expr.push(match b2.last() { + Some(b2) => b2.clone(), + None => return Err(ICEError::UmptyArray { location: self.current_location }), + }); conf.extend(w1); conf.extend(w2); Ok((conf, out_expr)) From 7d7890a753748fa0f9b43491ad718a96b509ae8f Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Thu, 27 Jul 2023 18:04:13 +0100 Subject: [PATCH 04/14] rename to internal error --- crates/noirc_evaluator/src/errors.rs | 26 ++++++------- .../acir_gen/acir_ir/acir_variable.rs | 28 ++++++------- .../acir_gen/acir_ir/generated_acir.rs | 39 +++++++++++-------- .../src/ssa_refactor/acir_gen/acir_ir/sort.rs | 10 ++--- .../src/ssa_refactor/acir_gen/mod.rs | 30 +++++++------- 5 files changed, 72 insertions(+), 61 deletions(-) diff --git a/crates/noirc_evaluator/src/errors.rs b/crates/noirc_evaluator/src/errors.rs index c86a504678b..2a7c8f8f30d 100644 --- a/crates/noirc_evaluator/src/errors.rs +++ b/crates/noirc_evaluator/src/errors.rs @@ -2,7 +2,7 @@ //! //! [RuntimeError] that should be displayed to the user //! -//! Internal Consistency Evaluators Errors [ICEError] +//! Internal Errors [InternalError] //! that are used for checking internal logics of the SSA //! //! An Error of the former is a user Error @@ -20,7 +20,7 @@ pub enum RuntimeError { #[error("Failed constraint")] FailedConstraint { lhs: FieldElement, rhs: FieldElement, location: Option }, #[error(transparent)] - ICEError(#[from] ICEError), + 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.")] @@ -34,7 +34,7 @@ pub enum RuntimeError { } #[derive(Debug, PartialEq, Eq, Clone, Error)] -pub enum ICEError { +pub enum InternalError { #[error("ICE: Both expressions should have degree<=1")] DegreeNotReduced { location: Option }, #[error("ICE: {message:?}")] @@ -56,15 +56,15 @@ pub enum ICEError { impl From for FileDiagnostic { fn from(error: RuntimeError) -> Self { match error { - RuntimeError::ICEError(ref ice_error) => match ice_error { - ICEError::DegreeNotReduced { location } - | ICEError::General { location, .. } - | ICEError::MissingArg { location, .. } - | ICEError::NotAConstant { location, .. } - | ICEError::NotImplemented { location, .. } - | ICEError::UmptyArray { location } - | ICEError::UndeclaredAcirVar { location } - | ICEError::UnExpected { location, .. } => { + RuntimeError::InternalError(ref ice_error) => match ice_error { + InternalError::DegreeNotReduced { location } + | InternalError::General { location, .. } + | InternalError::MissingArg { location, .. } + | InternalError::NotAConstant { location, .. } + | InternalError::NotImplemented { location, .. } + | InternalError::UmptyArray { location } + | InternalError::UndeclaredAcirVar { location } + | InternalError::UnExpected { location, .. } => { let file_id = location.map(|loc| loc.file).unwrap(); FileDiagnostic { file_id, diagnostic: error.into() } } @@ -85,7 +85,7 @@ impl From for FileDiagnostic { impl From for Diagnostic { fn from(error: RuntimeError) -> Diagnostic { match error { - RuntimeError::ICEError(_) => Diagnostic::simple_error( + RuntimeError::InternalError(_) => Diagnostic::simple_error( "Internal Consistency Evaluators Errors \n This is likely a bug. Consider Openning an issue at https://github.com/noir-lang/noir/issues".to_owned(), "".to_string(), noirc_errors::Span::new(0..0) 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 ac34b989775..187619ff07e 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,6 +1,6 @@ use super::generated_acir::GeneratedAcir; use crate::brillig::brillig_gen::brillig_directive; -use crate::errors::{ICEError, RuntimeError}; +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}; @@ -545,10 +545,10 @@ 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) -> Result<(), ICEError> { + 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(ICEError::UndeclaredAcirVar { location: self.get_location() }), + 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 { @@ -663,7 +663,7 @@ impl AcirContext { let domain_var = match inputs.pop() { Some(domian_var) => domian_var.into_var()?, None => { - return Err(RuntimeError::ICEError(ICEError::MissingArg { + return Err(RuntimeError::InternalError(InternalError::MissingArg { name: "pedersen call".to_string(), arg: "domain seperator".to_string(), location: self.get_location(), @@ -674,7 +674,7 @@ impl AcirContext { let domain_constant = match self.vars[&domain_var].as_constant() { Some(domain_constant) => domain_constant, None => { - return Err(RuntimeError::ICEError(ICEError::NotAConstant { + return Err(RuntimeError::InternalError(InternalError::NotAConstant { name: "domain separator".to_string(), location: self.get_location(), })) @@ -741,7 +741,7 @@ impl AcirContext { let radix = match self.vars[&radix_var].as_constant() { Some(radix) => radix.to_u128() as u32, None => { - return Err(RuntimeError::ICEError(ICEError::NotAConstant { + return Err(RuntimeError::InternalError(InternalError::NotAConstant { name: "radix".to_string(), location: self.get_location(), })); @@ -751,7 +751,7 @@ impl AcirContext { let limb_count = match self.vars[&limb_count_var].as_constant() { Some(limb_count) => limb_count.to_u128() as u32, None => { - return Err(RuntimeError::ICEError(ICEError::NotAConstant { + return Err(RuntimeError::InternalError(InternalError::NotAConstant { name: "limb_size".to_string(), location: self.get_location(), })); @@ -856,7 +856,7 @@ impl AcirContext { code: Vec, inputs: Vec, outputs: Vec, - ) -> Result, ICEError> { + ) -> Result, InternalError> { let b_inputs = try_vecmap(inputs, |i| match i { AcirValue::Var(var, _) => { Ok(BrilligInputs::Single(self.vars[&var].to_expression().into_owned())) @@ -899,7 +899,7 @@ impl AcirContext { &mut self, var_expressions: &mut Vec, input: AcirValue, - ) -> Result<(), ICEError> { + ) -> Result<(), InternalError> { match input { AcirValue::Var(var, _) => { var_expressions.push(self.vars[&var].to_expression().into_owned()); @@ -993,10 +993,10 @@ impl AcirContext { Ok(outputs_var) } /// Converts an AcirVar to a Witness - fn var_to_witness(&mut self, var: AcirVar) -> Result { + 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(ICEError::UndeclaredAcirVar { location: self.get_location() }), + None => return Err(InternalError::UndeclaredAcirVar { location: self.get_location() }), }; Ok(self.acir_ir.get_or_create_witness(&var_data.to_expression())) } @@ -1019,7 +1019,7 @@ impl AcirContext { &mut self, block_id: BlockId, index: &AcirVar, - ) -> Result { + ) -> Result { // Fetch the witness corresponding to the index let index_witness = self.var_to_witness(*index)?; @@ -1040,7 +1040,7 @@ impl AcirContext { block_id: BlockId, index: &AcirVar, value: &AcirVar, - ) -> Result<(), ICEError> { + ) -> Result<(), InternalError> { // Fetch the witness corresponding to the index // let index_witness = self.var_to_witness(*index)?; @@ -1061,7 +1061,7 @@ impl AcirContext { block_id: BlockId, len: usize, optional_values: Option<&[AcirValue]>, - ) -> Result<(), ICEError> { + ) -> 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 { 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 bbf99a74b16..52c0accd867 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 @@ -4,7 +4,7 @@ use std::collections::HashMap; use crate::{ brillig::brillig_gen::brillig_directive, - errors::{ICEError, RuntimeError}, + errors::{InternalError, RuntimeError}, }; use acvm::acir::{ @@ -121,7 +121,7 @@ impl GeneratedAcir { func_name: BlackBoxFunc, mut inputs: Vec, constants: Vec, - ) -> Result, ICEError> { + ) -> Result, InternalError> { intrinsics_check_inputs(func_name, &inputs)?; let output_count = black_box_expected_output_size(func_name)?; @@ -184,7 +184,7 @@ impl GeneratedAcir { let var_message_size = match inputs.pop() { Some(var_message_size) => var_message_size, None => { - return Err(ICEError::MissingArg { + return Err(InternalError::MissingArg { name: "".to_string(), arg: "message_size".to_string(), location: self.current_location, @@ -195,7 +195,7 @@ impl GeneratedAcir { } // TODO(#1570): Generate ACIR for recursive aggregation BlackBoxFunc::RecursiveAggregation => { - return Err(ICEError::NotImplemented { + return Err(InternalError::NotImplemented { name: "recursive aggregation".to_string(), location: None, }) @@ -259,12 +259,14 @@ impl GeneratedAcir { lhs: &Expression, leading: Witness, max_bit_size: u32, - ) -> Result { + ) -> Result { let max_power_of_two = FieldElement::from(2_i128).pow(&FieldElement::from(max_bit_size as i128 - 1)); let inter = match &(&Expression::from_field(max_power_of_two) - lhs) * &leading.into() { Some(inter) => inter, - None => return Err(ICEError::DegreeNotReduced { location: self.current_location }), + None => { + return Err(InternalError::DegreeNotReduced { location: self.current_location }) + } }; Ok(lhs.add_mul(FieldElement::from(2_i128), &inter)) } @@ -284,7 +286,7 @@ impl GeneratedAcir { &mut self, lhs: &Expression, rhs: &Expression, - ) -> Result { + ) -> Result { use std::borrow::Cow; let lhs_is_linear = lhs.is_linear(); let rhs_is_linear = rhs.is_linear(); @@ -294,7 +296,7 @@ impl GeneratedAcir { match lhs * rhs { Some(expr) => return Ok(expr), None => { - return Err(ICEError::NotAConstant { + return Err(InternalError::NotAConstant { name: "one of the expressions".to_string(), location: self.current_location, }) @@ -314,7 +316,9 @@ impl GeneratedAcir { if lhs == rhs { match &*lhs_reduced * &*lhs_reduced { Some(expr) => return Ok(expr), - None => return Err(ICEError::DegreeNotReduced { location: self.current_location }), + None => { + return Err(InternalError::DegreeNotReduced { location: self.current_location }) + } } }; @@ -326,7 +330,7 @@ impl GeneratedAcir { match &*lhs_reduced * &*rhs_reduced { Some(expr) => Ok(expr), - None => Err(ICEError::DegreeNotReduced { location: self.current_location }), + None => Err(InternalError::DegreeNotReduced { location: self.current_location }), } } @@ -385,7 +389,7 @@ impl GeneratedAcir { match &(&Expression::from(lhs_leading) * &Expression::from(rhs_leading)) { Some(expr) => expr, None => { - return Err(RuntimeError::ICEError(ICEError::DegreeNotReduced { + return Err(RuntimeError::InternalError(InternalError::DegreeNotReduced { location: self.current_location, })) } @@ -820,7 +824,7 @@ impl GeneratedAcir { /// 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) -> Result, ICEError> { +fn black_box_func_expected_input_size(name: BlackBoxFunc) -> Result, InternalError> { match name { // Bitwise opcodes will take in 2 parameters BlackBoxFunc::AND | BlackBoxFunc::XOR => Ok(Some(2)), @@ -847,7 +851,7 @@ fn black_box_func_expected_input_size(name: BlackBoxFunc) -> Result Err(ICEError::NotImplemented { + BlackBoxFunc::RecursiveAggregation => Err(InternalError::NotImplemented { name: "recursive aggregation".to_string(), location: None, }), @@ -856,7 +860,7 @@ fn black_box_func_expected_input_size(name: BlackBoxFunc) -> Result Result { +fn black_box_expected_output_size(name: BlackBoxFunc) -> Result { match name { // Bitwise opcodes will return 1 parameter which is the output // or the operation. @@ -878,7 +882,7 @@ fn black_box_expected_output_size(name: BlackBoxFunc) -> Result { // will be 2 field elements representing the point. BlackBoxFunc::FixedBaseScalarMul => Ok(2), // TODO(#1570): Generate ACIR for recursive aggregation - BlackBoxFunc::RecursiveAggregation => Err(ICEError::NotImplemented { + BlackBoxFunc::RecursiveAggregation => Err(InternalError::NotImplemented { name: "recursive aggregation".to_string(), location: None, }), @@ -900,7 +904,10 @@ fn black_box_expected_output_size(name: BlackBoxFunc) -> Result { /// #[foreign(sha256)] /// fn sha256(_input : [u8; N]) -> [u8; 32] {} /// `` -fn intrinsics_check_inputs(name: BlackBoxFunc, inputs: &[FunctionInput]) -> Result<(), ICEError> { +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 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 9fbc1e975a6..2f0328377e3 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,4 +1,4 @@ -use crate::errors::ICEError; +use crate::errors::InternalError; use super::generated_acir::GeneratedAcir; use acvm::acir::native_types::{Expression, Witness}; @@ -15,7 +15,7 @@ impl GeneratedAcir { in_expr: &[Expression], bits: &[Witness], generate_witness: bool, - ) -> Result<(Vec, Vec), ICEError> { + ) -> Result<(Vec, Vec), InternalError> { let n = in_expr.len(); if n == 1 { return Ok((Vec::new(), in_expr.to_vec())); @@ -50,7 +50,7 @@ impl GeneratedAcir { if n % 2 == 1 { in_sub2.push(match in_expr.last() { Some(in_expr) => in_expr.clone(), - None => return Err(ICEError::UmptyArray { location: self.current_location }), + None => return Err(InternalError::UmptyArray { location: self.current_location }), }); } let mut out_expr = Vec::new(); @@ -70,12 +70,12 @@ impl GeneratedAcir { if n % 2 == 0 { out_expr.push(match b1.last() { Some(b1) => b1.clone(), - None => return Err(ICEError::UmptyArray { location: self.current_location }), + None => return Err(InternalError::UmptyArray { location: self.current_location }), }); } out_expr.push(match b2.last() { Some(b2) => b2.clone(), - None => return Err(ICEError::UmptyArray { location: self.current_location }), + None => return Err(InternalError::UmptyArray { location: self.current_location }), }); conf.extend(w1); conf.extend(w2); 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 0d9c57f7b11..da1cb5167e3 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -20,7 +20,7 @@ use super::{ }; use crate::brillig::brillig_ir::BrilligContext; use crate::brillig::{brillig_gen::brillig_fn::FunctionContext as BrilligFunctionContext, Brillig}; -use crate::errors::{ICEError, RuntimeError}; +use crate::errors::{InternalError, RuntimeError}; pub(crate) use acir_ir::generated_acir::GeneratedAcir; use acvm::{ acir::{brillig::Opcode, circuit::opcodes::BlockId, native_types::Expression}, @@ -78,11 +78,11 @@ pub(crate) enum AcirValue { } impl AcirValue { - fn into_var(self) -> Result { + fn into_var(self) -> Result { match self { AcirValue::Var(var, _) => Ok(var), AcirValue::DynamicArray(_) | AcirValue::Array(_) => { - Err(ICEError::General { message: "".to_string(), location: None }) + Err(InternalError::General { message: "".to_string(), location: None }) } } } @@ -393,7 +393,11 @@ impl Context { Ok(()) } - fn gen_brillig_for(&self, func: &Function, brillig: &Brillig) -> Result, ICEError> { + 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), @@ -406,7 +410,7 @@ impl Context { let artifact = match artifact { Some(artifact) => artifact, None => { - return Err(ICEError::General { + return Err(InternalError::General { message: format!("Cannot find linked fn {unresolved_fn_label}"), location: None, }) @@ -433,7 +437,7 @@ impl Context { match self.convert_value(array, dfg) { AcirValue::Var(acir_var, _) => { - return Err(RuntimeError::ICEError(ICEError::UnExpected { + return Err(RuntimeError::InternalError(InternalError::UnExpected { expected: "an array value".to_string(), found: format!("{acir_var:?}"), location: self.acir_context.get_location(), @@ -549,7 +553,7 @@ impl Context { index: ValueId, store_value: ValueId, dfg: &DataFlowGraph, - ) -> Result<(), ICEError> { + ) -> Result<(), InternalError> { // Fetch the internal SSA ID for the array let array = dfg.resolve(array); let array_ssa_id = array.to_usize() as u32; @@ -576,7 +580,7 @@ impl Context { self.initialize_array(block_id, array.len(), Some(&values))?; } _ => { - return Err(ICEError::General { + return Err(InternalError::General { message: format!("Array {array} should be initialized"), location: self.acir_context.get_location(), }) @@ -625,7 +629,7 @@ impl Context { array: BlockId, len: usize, values: Option<&[AcirValue]>, - ) -> Result<(), ICEError> { + ) -> Result<(), InternalError> { self.acir_context.initialize_array(array, len, values)?; self.initialized_arrays.insert(array); Ok(()) @@ -659,7 +663,7 @@ impl Context { &mut self, terminator: &TerminatorInstruction, dfg: &DataFlowGraph, - ) -> Result<(), ICEError> { + ) -> Result<(), InternalError> { let return_values = match terminator { TerminatorInstruction::Return { return_values } => return_values, _ => unreachable!("ICE: Program must have a singular return"), @@ -719,17 +723,17 @@ impl Context { &mut self, value_id: ValueId, dfg: &DataFlowGraph, - ) -> Result { + ) -> Result { match self.convert_value(value_id, dfg) { AcirValue::Var(acir_var, _) => Ok(acir_var), AcirValue::Array(array) => { - return Err(ICEError::UnExpected { + return Err(InternalError::UnExpected { expected: "a numeric value".to_string(), found: format!("{array:?}"), location: self.acir_context.get_location(), }) } - AcirValue::DynamicArray(_) => Err(ICEError::UnExpected { + AcirValue::DynamicArray(_) => Err(InternalError::UnExpected { expected: "a numeric value".to_string(), found: "an array".to_string(), location: self.acir_context.get_location(), From d487445cdcaf2fa27f8ee24f48a5bc02669482fa Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Thu, 27 Jul 2023 18:04:59 +0100 Subject: [PATCH 05/14] comment --- crates/noirc_evaluator/src/errors.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/noirc_evaluator/src/errors.rs b/crates/noirc_evaluator/src/errors.rs index 2a7c8f8f30d..7dcfae4f73a 100644 --- a/crates/noirc_evaluator/src/errors.rs +++ b/crates/noirc_evaluator/src/errors.rs @@ -1,9 +1,8 @@ //! Noir Evaluator has two types of errors //! -//! [RuntimeError] that should be displayed to the user +//! [RuntimeError]s that should be displayed to the user //! -//! Internal Errors [InternalError] -//! that are used for checking internal logics of the SSA +//! [InternalError]s that are used for checking internal logics of the SSA //! //! An Error of the former is a user Error //! From ff110cd1f5021d35071c0004b53c862653927896 Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Thu, 27 Jul 2023 18:10:40 +0100 Subject: [PATCH 06/14] comment --- crates/noirc_evaluator/src/errors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/noirc_evaluator/src/errors.rs b/crates/noirc_evaluator/src/errors.rs index 7dcfae4f73a..b34fd257a75 100644 --- a/crates/noirc_evaluator/src/errors.rs +++ b/crates/noirc_evaluator/src/errors.rs @@ -4,7 +4,7 @@ //! //! [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 former is an user Error //! //! An Error of the latter is an error in the implementation of the compiler use acvm::FieldElement; From b81476d201303a2dcd83bf788f8b08bbc75ccb4c Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Thu, 27 Jul 2023 18:12:04 +0100 Subject: [PATCH 07/14] . --- crates/noirc_evaluator/src/ssa_refactor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor.rs b/crates/noirc_evaluator/src/ssa_refactor.rs index 34370eb5084..67a908209ae 100644 --- a/crates/noirc_evaluator/src/ssa_refactor.rs +++ b/crates/noirc_evaluator/src/ssa_refactor.rs @@ -19,7 +19,7 @@ use noirc_frontend::monomorphization::ast::Program; use self::{abi_gen::gen_abi, acir_gen::GeneratedAcir, ir::function::RuntimeType, ssa_gen::Ssa}; mod abi_gen; -pub(crate) mod acir_gen; +mod acir_gen; pub mod ir; mod opt; mod ssa_builder; From 2f530c1c613c42fd21aacb06f4bbd4c37ee500ef Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Thu, 27 Jul 2023 18:14:29 +0100 Subject: [PATCH 08/14] . --- crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 da1cb5167e3..5bb7a94c449 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -81,9 +81,10 @@ impl AcirValue { fn into_var(self) -> Result { match self { AcirValue::Var(var, _) => Ok(var), - AcirValue::DynamicArray(_) | AcirValue::Array(_) => { - Err(InternalError::General { message: "".to_string(), location: None }) - } + AcirValue::DynamicArray(_) | AcirValue::Array(_) => Err(InternalError::General { + message: "Called AcirValue::into_var on an array".to_string(), + location: None, + }), } } From 0b363d838e0d0b5c16044383537f640fec42f4da Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Thu, 27 Jul 2023 18:23:09 +0100 Subject: [PATCH 09/14] . --- crates/noirc_evaluator/src/errors.rs | 8 ++++---- .../src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs | 4 ++-- .../src/ssa_refactor/acir_gen/acir_ir/sort.rs | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/noirc_evaluator/src/errors.rs b/crates/noirc_evaluator/src/errors.rs index b34fd257a75..3d9fa13cd22 100644 --- a/crates/noirc_evaluator/src/errors.rs +++ b/crates/noirc_evaluator/src/errors.rs @@ -36,16 +36,16 @@ pub enum RuntimeError { 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 implmented yet")] + #[error("{name:?} is not implemented yet")] NotImplemented { name: String, location: Option }, - #[error("Try to get element from empty array")] - UmptyArray { location: Option }, #[error("ICE: Undeclared AcirVar")] UndeclaredAcirVar { location: Option }, #[error("ICE: Expected {expected:?}, found {found:?}")] @@ -57,11 +57,11 @@ impl From for FileDiagnostic { 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::UmptyArray { location } | InternalError::UndeclaredAcirVar { location } | InternalError::UnExpected { location, .. } => { let file_id = location.map(|loc| loc.file).unwrap(); 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 187619ff07e..3f981ee8039 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 @@ -661,11 +661,11 @@ impl AcirContext { BlackBoxFunc::Pedersen => { // The last argument of pedersen is the domain separator, which must be a constant let domain_var = match inputs.pop() { - Some(domian_var) => domian_var.into_var()?, + Some(domain_var) => domain_var.into_var()?, None => { return Err(RuntimeError::InternalError(InternalError::MissingArg { name: "pedersen call".to_string(), - arg: "domain seperator".to_string(), + arg: "domain separator".to_string(), location: self.get_location(), })) } 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 2f0328377e3..cbc7bdc0e82 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 @@ -50,7 +50,7 @@ impl GeneratedAcir { if n % 2 == 1 { in_sub2.push(match in_expr.last() { Some(in_expr) => in_expr.clone(), - None => return Err(InternalError::UmptyArray { location: self.current_location }), + None => return Err(InternalError::EmptyArray { location: self.current_location }), }); } let mut out_expr = Vec::new(); @@ -70,12 +70,12 @@ impl GeneratedAcir { if n % 2 == 0 { out_expr.push(match b1.last() { Some(b1) => b1.clone(), - None => return Err(InternalError::UmptyArray { location: self.current_location }), + None => return Err(InternalError::EmptyArray { location: self.current_location }), }); } out_expr.push(match b2.last() { Some(b2) => b2.clone(), - None => return Err(InternalError::UmptyArray { location: self.current_location }), + None => return Err(InternalError::EmptyArray { location: self.current_location }), }); conf.extend(w1); conf.extend(w2); From 24e3c35ddfb7516948cae15fa376ffb735eed56c Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Fri, 28 Jul 2023 15:26:43 +0100 Subject: [PATCH 10/14] review --- crates/noirc_evaluator/src/errors.rs | 3 +- .../acir_gen/acir_ir/generated_acir.rs | 36 +++++-------------- .../src/ssa_refactor/acir_gen/acir_ir/sort.rs | 4 +-- 3 files changed, 13 insertions(+), 30 deletions(-) diff --git a/crates/noirc_evaluator/src/errors.rs b/crates/noirc_evaluator/src/errors.rs index 3d9fa13cd22..d05ec9ede4b 100644 --- a/crates/noirc_evaluator/src/errors.rs +++ b/crates/noirc_evaluator/src/errors.rs @@ -85,7 +85,8 @@ impl From for Diagnostic { fn from(error: RuntimeError) -> Diagnostic { match error { RuntimeError::InternalError(_) => Diagnostic::simple_error( - "Internal Consistency Evaluators Errors \n This is likely a bug. Consider Openning an issue at https://github.com/noir-lang/noir/issues".to_owned(), + "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) ), 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 52c0accd867..0aab6afee41 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 @@ -282,26 +282,15 @@ impl GeneratedAcir { /// /// (1) is because an [`Expression`] can hold at most a degree-2 univariate polynomial /// which is what you get when you multiply two degree-1 univariate polynomials. - pub(crate) fn mul_with_witness( - &mut self, - lhs: &Expression, - rhs: &Expression, - ) -> Result { + pub(crate) fn mul_with_witness(&mut self, lhs: &Expression, rhs: &Expression) -> Expression { use std::borrow::Cow; let lhs_is_linear = lhs.is_linear(); let rhs_is_linear = rhs.is_linear(); // Case 1: Both expressions have at most a total degree of 1 in each term if lhs_is_linear && rhs_is_linear { - match lhs * rhs { - Some(expr) => return Ok(expr), - None => { - return Err(InternalError::NotAConstant { - name: "one of the expressions".to_string(), - location: self.current_location, - }) - } - } + return (lhs * rhs) + .expect("one of the expressions is a constant and so this should not fail"); } // Case 2: One or both of the sides needs to be reduced to a degree-1 univariate polynomial @@ -314,12 +303,8 @@ impl GeneratedAcir { // If the lhs and rhs are the same, then we do not need to reduce // rhs, we only need to square the lhs. if lhs == rhs { - match &*lhs_reduced * &*lhs_reduced { - Some(expr) => return Ok(expr), - None => { - return Err(InternalError::DegreeNotReduced { location: self.current_location }) - } - } + return (&*lhs_reduced * &*lhs_reduced) + .expect("Both expressions are reduced to be degree<=1"); }; let rhs_reduced = if rhs_is_linear { @@ -328,10 +313,7 @@ impl GeneratedAcir { Cow::Owned(self.get_or_create_witness(rhs).into()) }; - match &*lhs_reduced * &*rhs_reduced { - Some(expr) => Ok(expr), - None => Err(InternalError::DegreeNotReduced { location: self.current_location }), - } + (&*lhs_reduced * &*rhs_reduced).expect("Both expressions are reduced to be degree<=1") } /// Signed division lhs / rhs @@ -443,9 +425,9 @@ impl GeneratedAcir { // When the predicate is 0, the equation always passes. // When the predicate is 1, the euclidean division needs to be // true. - let rhs_constraint = &self.mul_with_witness(rhs, &q_witness.into())? + r_witness; - let div_euclidean = &self.mul_with_witness(lhs, predicate)? - - &self.mul_with_witness(&rhs_constraint, predicate)?; + let rhs_constraint = &self.mul_with_witness(rhs, &q_witness.into()) + r_witness; + let div_euclidean = &self.mul_with_witness(lhs, predicate) + - &self.mul_with_witness(&rhs_constraint, predicate); self.push_opcode(AcirOpcode::Arithmetic(div_euclidean)); 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 cbc7bdc0e82..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 @@ -41,7 +41,7 @@ impl GeneratedAcir { let intermediate = self.mul_with_witness( &Expression::from(conf[i]), &(&in_expr[2 * i + 1] - &in_expr[2 * i]), - )?; + ); //b1=a1+q in_sub1.push(&intermediate + &in_expr[2 * i]); //b2=a2-q @@ -63,7 +63,7 @@ impl GeneratedAcir { for i in 0..(n - 1) / 2 { let c = if generate_witness { self.next_witness_index() } else { bits[n1 + i] }; conf.push(c); - let intermediate = self.mul_with_witness(&Expression::from(c), &(&b2[i] - &b1[i]))?; + let intermediate = self.mul_with_witness(&Expression::from(c), &(&b2[i] - &b1[i])); out_expr.push(&intermediate + &b1[i]); out_expr.push(&b2[i] - &intermediate); } From 83dc70941a0e617db14dace5222dfa1be943124f Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Mon, 31 Jul 2023 08:38:05 +0100 Subject: [PATCH 11/14] . --- .../src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 9f84c383e9e..b42cd64d84f 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 @@ -266,7 +266,7 @@ impl GeneratedAcir { let intermediate = self.mul_with_witness(&(&Expression::from(max_power_of_two) - lhs), &leading.into()); - lhs.add_mul(FieldElement::from(2_i128), &intermediate) + Ok(lhs.add_mul(FieldElement::from(2_i128), &intermediate)) } /// Returns an expression which represents `lhs * rhs` From 46b6201494628a66e171abfa880cf8f251cf5827 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Mon, 31 Jul 2023 07:37:29 +0000 Subject: [PATCH 12/14] chore: fix merge conflict --- .../ssa_refactor/acir_gen/acir_ir/generated_acir.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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 b42cd64d84f..a5d66fe2363 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 @@ -259,14 +259,14 @@ impl GeneratedAcir { lhs: &Expression, leading: Witness, max_bit_size: u32, - ) -> Result { + ) -> Expression { let max_power_of_two = FieldElement::from(2_i128).pow(&FieldElement::from(max_bit_size as i128 - 1)); let intermediate = self.mul_with_witness(&(&Expression::from(max_power_of_two) - lhs), &leading.into()); - Ok(lhs.add_mul(FieldElement::from(2_i128), &intermediate)) + lhs.add_mul(FieldElement::from(2_i128), &intermediate) } /// Returns an expression which represents `lhs * rhs` @@ -353,8 +353,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, max_bit_size); + let unsigned_rhs = self.two_complement(rhs, rhs_leading, 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); @@ -380,8 +380,8 @@ impl GeneratedAcir { }, ); 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 quotient = self.two_complement(&q1.into(), q_sign_witness, max_bit_size); + let remainder = self.two_complement(&r1.into(), lhs_leading, max_bit_size); Ok((quotient, remainder)) } From 033abbe20583793827dc4188bc18841455bb16b6 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Mon, 31 Jul 2023 07:42:32 +0000 Subject: [PATCH 13/14] chore: make multiplication of 2 witnesses more explicit --- .../acir_gen/acir_ir/generated_acir.rs | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) 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 a5d66fe2363..30b85cb9bae 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 @@ -337,7 +337,7 @@ impl GeneratedAcir { 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, @@ -345,7 +345,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, @@ -353,8 +353,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); @@ -368,20 +368,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), - match &(&Expression::from(lhs_leading) * &Expression::from(rhs_leading)) { - Some(expr) => expr, - None => { - return Err(RuntimeError::InternalError(InternalError::DegreeNotReduced { - location: self.current_location, - })) - } - }, - ); + 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)) } From 088dc34bc7178e4d93ca4f5de8d2c0bf6fb5a43b Mon Sep 17 00:00:00 2001 From: jfecher Date: Mon, 31 Jul 2023 08:50:11 -0500 Subject: [PATCH 14/14] Update crates/noirc_evaluator/src/errors.rs --- crates/noirc_evaluator/src/errors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/noirc_evaluator/src/errors.rs b/crates/noirc_evaluator/src/errors.rs index d05ec9ede4b..6d53668d7cb 100644 --- a/crates/noirc_evaluator/src/errors.rs +++ b/crates/noirc_evaluator/src/errors.rs @@ -4,7 +4,7 @@ //! //! [InternalError]s that are used for checking internal logics of the SSA //! -//! An Error of the former is an user Error +//! 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;