diff --git a/src/analyzer/pil_analyzer.rs b/src/analyzer/pil_analyzer.rs index a4da615b0..aa3b8fdb7 100644 --- a/src/analyzer/pil_analyzer.rs +++ b/src/analyzer/pil_analyzer.rs @@ -5,7 +5,7 @@ use std::path::{Path, PathBuf}; use crate::number::{abstract_to_degree, DegreeType}; use crate::parser::ast; pub use crate::parser::ast::{BinaryOperator, UnaryOperator}; -use crate::{line_utils, parser}; +use crate::{parser, utils}; use super::*; @@ -100,7 +100,7 @@ impl PILContext { let old_line_starts = std::mem::take(&mut self.line_starts); // TOOD make this work for other line endings - self.line_starts = line_utils::compute_line_starts(contents); + self.line_starts = utils::compute_line_starts(contents); self.current_file = path.to_path_buf(); let pil_file = parser::parse(Some(path.to_str().unwrap()), contents).unwrap_or_else(|err| { @@ -181,7 +181,7 @@ impl PILContext { fn to_source_ref(&self, start: usize) -> SourceRef { let file = self.current_file.file_name().unwrap().to_str().unwrap(); SourceRef { - line: line_utils::offset_to_line(start, &self.line_starts), + line: utils::offset_to_line(start, &self.line_starts), file: file.to_string(), } } diff --git a/src/commit_evaluator/eval_error.rs b/src/commit_evaluator/eval_error.rs new file mode 100644 index 000000000..69757e144 --- /dev/null +++ b/src/commit_evaluator/eval_error.rs @@ -0,0 +1,59 @@ +use std::fmt::{Display, Formatter, Result}; + +#[derive(Clone, Debug)] +pub enum EvalError { + /// Previous value of witness column not known when trying to derive a value in the next row. + PreviousValueUnknown(String), + Generic(String), + Multiple(Vec), +} + +pub fn combine(left: EvalError, right: EvalError) -> EvalError { + match (left, right) { + (EvalError::Multiple(l), EvalError::Multiple(r)) => { + EvalError::Multiple(l.into_iter().chain(r).collect()) + } + (m @ EvalError::Multiple(_), other) | (other, m @ EvalError::Multiple(_)) => { + combine(m, EvalError::Multiple(vec![other])) + } + (l, r) => EvalError::Multiple(vec![l, r]), + } +} + +impl From for EvalError { + fn from(value: String) -> Self { + EvalError::Generic(value) + } +} + +impl Display for EvalError { + fn fmt(&self, f: &mut Formatter<'_>) -> Result { + match self { + EvalError::Generic(reason) => write!(f, "{reason}"), + EvalError::PreviousValueUnknown(names) => write!( + f, + "Previous value of the following column(s) is not (yet) known: {names}.", + ), + EvalError::Multiple(errors) => { + let (previous_unknown, mut others) = errors.iter().fold( + (vec![], vec![]), + |(mut previous_unknown, mut others), err| { + if let EvalError::PreviousValueUnknown(n) = err { + previous_unknown.push(n.clone()); + } else { + others.push(format!("{err}")); + } + (previous_unknown, others) + }, + ); + if !previous_unknown.is_empty() { + others.push(format!( + "{}", + EvalError::PreviousValueUnknown(previous_unknown.join(", ")) + )); + } + write!(f, "{}", others.join("\n")) + } + } + } +} diff --git a/src/commit_evaluator/mod.rs b/src/commit_evaluator/mod.rs index d2c0c69b6..b05f4ac11 100644 --- a/src/commit_evaluator/mod.rs +++ b/src/commit_evaluator/mod.rs @@ -6,11 +6,15 @@ use crate::analyzer::{ }; // TODO should use finite field instead of abstract number use crate::number::{abstract_to_degree, is_zero, AbstractNumberType, DegreeType}; +use crate::utils::indent; mod affine_expression; +mod eval_error; use affine_expression::AffineExpression; +use self::eval_error::EvalError; + /// Generates the committed polynomial values /// @returns the values (in source order) and the degree of the polynomials. pub fn generate<'a>( @@ -69,7 +73,7 @@ impl<'a> WitnessColumn<'a> { } } -type EvalResult = Result, String>; +type EvalResult = Result, EvalError>; struct Evaluator<'a, QueryCallback> where @@ -101,9 +105,6 @@ enum EvaluationRow { Specific(DegreeType), } -// This could have more structure in the future. -type EvalError = String; - impl<'a, QueryCallback> Evaluator<'a, QueryCallback> where QueryCallback: FnMut(&str) -> Option, @@ -160,7 +161,13 @@ where IdentityKind::Plookup => self.process_plookup(identity), _ => Ok(vec![]), } - .map_err(|err| format!("No progress on {identity}:\n {err}")); + .map_err(|err| { + format!( + "No progress on {identity}:\n{}", + indent(&format!("{err}"), " ") + ) + .into() + }); if result.is_err() { identity_failed = true; } @@ -228,7 +235,7 @@ where fn process_witness_query( &mut self, column: &&WitnessColumn, - ) -> Result, String> { + ) -> Result, EvalError> { let query = self.interpolate_query(column.query.unwrap())?; if let Some(value) = self.query_callback.as_mut().unwrap()(&query) { Ok(vec![(column.id, value)]) @@ -236,7 +243,8 @@ where Err(format!( "No query answer for {} query: {query}.", self.committed_names[column.id] - )) + ) + .into()) } } @@ -282,9 +290,9 @@ where None => { let formatted = self.format_affine_expression(&evaluated); Err(if evaluated.is_invalid() { - format!("Constraint is invalid ({formatted} != 0).") + format!("Constraint is invalid ({formatted} != 0).").into() } else { - format!("Could not solve expression {formatted} = 0.") + format!("Could not solve expression {formatted} = 0.").into() }) } } @@ -293,7 +301,7 @@ where fn process_plookup(&mut self, identity: &Identity) -> EvalResult { if identity.left.selector.is_some() || identity.right.selector.is_some() { - return Err("Selectors not yet supported.".to_string()); + return Err("Selectors not yet supported.".to_string().into()); } let left = identity .left @@ -314,7 +322,8 @@ where None => Err(format!( "First expression needs to be constant but is not: {}.", self.format_affine_expression(&v) - )), + ) + .into()), })?; let right_key = identity.right.expressions.first().unwrap(); @@ -354,7 +363,7 @@ where } } if result.is_empty() { - Err(reasons.join(", ")) + Err(reasons.into_iter().reduce(eval_error::combine).unwrap()) } else { Ok(result) } @@ -374,6 +383,7 @@ where "Constant value required: {}", self.format_affine_expression(&r) ) + .into() }) })?; @@ -391,9 +401,9 @@ where // TODO somehow also add `l` and `r` to the error message. let formatted = self.format_affine_expression(&evaluated); Err(if evaluated.is_invalid() { - format!("Constraint is invalid ({formatted} != 0).") + format!("Constraint is invalid ({formatted} != 0).").into() } else { - format!("Could not solve expression {formatted} = 0.") + format!("Could not solve expression {formatted} = 0.").into() }) } }, @@ -410,7 +420,7 @@ where } } Err(reason) => { - self.failure_reasons.push(reason); + self.failure_reasons.push(format!("{reason}")); } } } @@ -441,9 +451,7 @@ where self.current[*id] .as_ref() .map(|value| value.clone().into()) - .ok_or_else(|| { - format!("Previous value of column {} not yet known.", poly.name) - }) + .ok_or_else(|| EvalError::PreviousValueUnknown(poly.name.clone())) } else if (poly.next && row == EvaluationRow::Current) || (!poly.next && row == EvaluationRow::Next) { @@ -459,7 +467,8 @@ where Err(format!( "{}' references the next-next row when evaluating on the current row.", self.committed_names[*id] - )) + ) + .into()) } } else { // Constant polynomial (or something else) @@ -481,13 +490,19 @@ where self.evaluate_binary_operation(left, op, right, row) } Expression::UnaryOperation(op, expr) => self.evaluate_unary_operation(op, expr, row), - Expression::Tuple(_) => Err("Tuple not implemented.".to_string()), - Expression::String(_) => Err("String not implemented.".to_string()), + Expression::Tuple(_) => Err("Tuple not implemented.".to_string().into()), + Expression::String(_) => Err("String not implemented.".to_string().into()), Expression::LocalVariableReference(_) => { - Err("Local variable references not implemented.".to_string()) + Err("Local variable references not implemented." + .to_string() + .into()) + } + Expression::PublicReference(_) => { + Err("Public references not implemented.".to_string().into()) + } + Expression::FunctionCall(_, _) => { + Err("Function calls not implemented.".to_string().into()) } - Expression::PublicReference(_) => Err("Public references not implemented.".to_string()), - Expression::FunctionCall(_, _) => Err("Function calls not implemented.".to_string()), } } @@ -512,7 +527,8 @@ where "Multiplication of two non-constants: ({}) * ({})", self.format_affine_expression(&left), self.format_affine_expression(&right) - )) + ) + .into()) } } BinaryOperator::Div => { @@ -529,7 +545,8 @@ where "Division of two non-constants: ({}) / ({})", self.format_affine_expression(&left), self.format_affine_expression(&right) - )) + ) + .into()) } } BinaryOperator::Pow => { @@ -540,7 +557,8 @@ where "Pow of two non-constants: ({}) ** ({})", self.format_affine_expression(&left), self.format_affine_expression(&right) - )) + ) + .into()) } } BinaryOperator::Mod @@ -566,7 +584,7 @@ where } }, (Ok(_), Err(reason)) | (Err(reason), Ok(_)) => Err(reason), - (Err(r1), Err(r2)) => Err(format!("{r1}, {r2}")), + (Err(r1), Err(r2)) => Err(eval_error::combine(r1, r2)), } } diff --git a/src/lib.rs b/src/lib.rs index 4fd684dbb..80b15376c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,6 +4,6 @@ pub mod commit_evaluator; pub mod compiler; pub mod constant_evaluator; pub mod json_exporter; -pub mod line_utils; pub mod number; pub mod parser; +pub mod utils; diff --git a/src/parser/display.rs b/src/parser/display.rs index 05607941e..22a310a53 100644 --- a/src/parser/display.rs +++ b/src/parser/display.rs @@ -1,5 +1,7 @@ use std::fmt::{Display, Formatter, Result}; +use crate::utils::quote; + use super::ast::*; // TODO indentation @@ -78,10 +80,6 @@ impl Display for Statement { } } -fn quote(input: &str) -> String { - format!("\"{}\"", input.replace('\\', "\\\\").replace('"', "\\\"")) -} - fn format_names(names: &[PolynomialName]) -> String { names .iter() diff --git a/src/line_utils.rs b/src/utils.rs similarity index 76% rename from src/line_utils.rs rename to src/utils.rs index ac8de5609..ddae29a35 100644 --- a/src/line_utils.rs +++ b/src/utils.rs @@ -11,6 +11,18 @@ pub fn offset_to_line(offset: usize, line_starts: &[usize]) -> usize { } } +pub fn quote(input: &str) -> String { + format!("\"{}\"", input.replace('\\', "\\\\").replace('"', "\\\"")) +} + +pub fn indent(input: &str, indentation: &str) -> String { + if input.is_empty() { + String::new() + } else { + indentation.to_string() + &input.replace('\n', &format!("\n{indentation}")) + } +} + #[cfg(test)] mod test { use super::{compute_line_starts, offset_to_line};