diff --git a/pil-analyzer/src/expression_processor.rs b/pil-analyzer/src/expression_processor.rs index 1b22ff345e..bca7411602 100644 --- a/pil-analyzer/src/expression_processor.rs +++ b/pil-analyzer/src/expression_processor.rs @@ -5,11 +5,11 @@ use powdr_ast::{ self, asm::SymbolPath, types::Type, ArrayExpression, ArrayLiteral, BinaryOperation, BlockExpression, IfExpression, LambdaExpression, LetStatementInsideBlock, MatchArm, MatchExpression, NamedExpression, NamespacedPolynomialReference, Number, Pattern, - StatementInsideBlock, StructExpression, SymbolCategory, UnaryOperation, + SourceReference, StatementInsideBlock, StructExpression, SymbolCategory, UnaryOperation, }, }; -use powdr_parser_util::SourceRef; +use powdr_parser_util::{Error, SourceRef}; use std::{ collections::{HashMap, HashSet}, str::FromStr, @@ -41,34 +41,40 @@ impl<'a, D: AnalysisDriver> ExpressionProcessor<'a, D> { pub fn process_array_expression( &mut self, array_expression: ::powdr_ast::parsed::ArrayExpression, - ) -> ArrayExpression { + ) -> Result, Error> { match array_expression { ArrayExpression::Value(expressions) => { - let values = self.process_expressions(expressions); - ArrayExpression::Value(values) + let values = self.process_expressions(expressions)?; + Ok(ArrayExpression::Value(values)) } - ArrayExpression::RepeatedValue(expressions) => { - ArrayExpression::RepeatedValue(self.process_expressions(expressions)) - } - ArrayExpression::Concat(left, right) => ArrayExpression::Concat( - Box::new(self.process_array_expression(*left)), - Box::new(self.process_array_expression(*right)), - ), + ArrayExpression::RepeatedValue(expressions) => Ok(ArrayExpression::RepeatedValue( + self.process_expressions(expressions)?, + )), + ArrayExpression::Concat(left, right) => Ok(ArrayExpression::Concat( + Box::new(self.process_array_expression(*left)?), + Box::new(self.process_array_expression(*right)?), + )), } } - pub fn process_expressions(&mut self, exprs: Vec) -> Vec { + pub fn process_expressions( + &mut self, + exprs: Vec, + ) -> Result, Error> { exprs .into_iter() .map(|e| self.process_expression(e)) - .collect() + .collect::, _>>() } - pub fn process_expression(&mut self, expr: parsed::Expression) -> Expression { + pub fn process_expression(&mut self, expr: parsed::Expression) -> Result { use parsed::Expression as PExpression; - match expr { + Ok(match expr { PExpression::Reference(src, poly) => { - Expression::Reference(src, self.process_reference(poly)) + let reference = self + .process_reference(poly) + .map_err(|e| src.with_error(e))?; + Expression::Reference(src, reference) } PExpression::PublicReference(src, name) => Expression::PublicReference(src, name), PExpression::Number(src, Number { value: n, type_: t }) => { @@ -76,16 +82,16 @@ impl<'a, D: AnalysisDriver> ExpressionProcessor<'a, D> { } PExpression::String(src, value) => Expression::String(src, value), PExpression::Tuple(src, items) => { - Expression::Tuple(src, self.process_expressions(items)) + Expression::Tuple(src, self.process_expressions(items)?) } PExpression::ArrayLiteral(src, ArrayLiteral { items }) => Expression::ArrayLiteral( src, ArrayLiteral { - items: self.process_expressions(items), + items: self.process_expressions(items)?, }, ), PExpression::LambdaExpression(src, lambda_expression) => { - Expression::LambdaExpression(src, self.process_lambda_expression(lambda_expression)) + self.process_lambda_expression(src, lambda_expression)? } PExpression::BinaryOperation( src, @@ -97,9 +103,9 @@ impl<'a, D: AnalysisDriver> ExpressionProcessor<'a, D> { ) => Expression::BinaryOperation( src, BinaryOperation { - left: Box::new(self.process_expression(*l)), + left: Box::new(self.process_expression(*l)?), op, - right: Box::new(self.process_expression(*r)), + right: Box::new(self.process_expression(*r)?), }, ), PExpression::UnaryOperation(src, UnaryOperation { op, expr: value }) => { @@ -107,29 +113,29 @@ impl<'a, D: AnalysisDriver> ExpressionProcessor<'a, D> { src, UnaryOperation { op, - expr: Box::new(self.process_expression(*value)), + expr: Box::new(self.process_expression(*value)?), }, ) } PExpression::IndexAccess(src, index_access) => Expression::IndexAccess( src, parsed::IndexAccess { - array: Box::new(self.process_expression(*index_access.array)), - index: Box::new(self.process_expression(*index_access.index)), + array: Box::new(self.process_expression(*index_access.array)?), + index: Box::new(self.process_expression(*index_access.index)?), }, ), PExpression::FunctionCall(src, c) => Expression::FunctionCall( src, parsed::FunctionCall { - function: Box::new(self.process_expression(*c.function)), - arguments: self.process_expressions(c.arguments), + function: Box::new(self.process_expression(*c.function)?), + arguments: self.process_expressions(c.arguments)?, }, ), PExpression::MatchExpression(src, MatchExpression { scrutinee, arms }) => { Expression::MatchExpression( src, MatchExpression { - scrutinee: Box::new(self.process_expression(*scrutinee)), + scrutinee: Box::new(self.process_expression(*scrutinee)?), arms: arms .into_iter() .map(|MatchArm { pattern, value }| { @@ -137,9 +143,13 @@ impl<'a, D: AnalysisDriver> ExpressionProcessor<'a, D> { let pattern = self.process_pattern(pattern); let value = self.process_expression(value); self.reset_local_variables(vars); - MatchArm { pattern, value } + + Ok(MatchArm { + pattern: pattern?, + value: value?, + }) }) - .collect(), + .collect::, _>>()?, }, ) } @@ -153,13 +163,13 @@ impl<'a, D: AnalysisDriver> ExpressionProcessor<'a, D> { ) => Expression::IfExpression( src, IfExpression { - condition: Box::new(self.process_expression(*condition)), - body: Box::new(self.process_expression(*body)), - else_body: Box::new(self.process_expression(*else_body)), + condition: Box::new(self.process_expression(*condition)?), + body: Box::new(self.process_expression(*body)?), + else_body: Box::new(self.process_expression(*else_body)?), }, ), PExpression::BlockExpression(src, BlockExpression { statements, expr }) => { - self.process_block_expression(statements, expr, src) + self.process_block_expression(statements, expr, src)? } PExpression::FreeInput(_, _) => panic!(), PExpression::StructExpression(src, StructExpression { name, fields }) => { @@ -167,38 +177,41 @@ impl<'a, D: AnalysisDriver> ExpressionProcessor<'a, D> { .type_args .map(|args| args.into_iter().map(|t| self.process_type(t)).collect()); + let name = self + .driver + .resolve_ref(&name.path, SymbolCategory::Struct) + .map_err(|msg| src.with_error(msg))?; + Expression::StructExpression( src, StructExpression { - name: Reference::Poly(PolynomialReference { - name: self - .driver - .resolve_ref(&name.path, SymbolCategory::Struct) - .expect("TODO: Handle this up in the code"), - type_args, - }), + name: Reference::Poly(PolynomialReference { name, type_args }), fields: fields .into_iter() - .map(|named_expr| NamedExpression { - name: named_expr.name, - body: Box::new(self.process_expression(*named_expr.body)), - }) - .collect(), + .map( + |named_expr| -> Result>, Error> { + Ok(NamedExpression { + name: named_expr.name, + body: Box::new(self.process_expression(*named_expr.body)?), + }) + }, + ) + .collect::, _>>()?, }, ) } - } + }) } /// Processes a pattern, registering all variables bound in there. /// It also changes EnumPatterns consisting of a single identifier that does not resolve /// to anything into Variable patterns. - fn process_pattern(&mut self, pattern: Pattern) -> Pattern { + fn process_pattern(&mut self, pattern: Pattern) -> Result { match pattern { Pattern::CatchAll(_) | Pattern::Ellipsis(_) | Pattern::Number(_, _) - | Pattern::String(_, _) => pattern, + | Pattern::String(_, _) => Ok(pattern), Pattern::Array(source_ref, items) => { // If there is more than one Pattern::Ellipsis in items, it is an error if items @@ -207,12 +220,13 @@ impl<'a, D: AnalysisDriver> ExpressionProcessor<'a, D> { .count() > 1 { - panic!("Only one \"..\"-item allowed in array pattern"); + return Err(source_ref + .with_error("Only one \"..\"-item allowed in array pattern".to_string())); } - Pattern::Array(source_ref, self.process_pattern_vec(items)) + Ok(Pattern::Array(source_ref, self.process_pattern_vec(items)?)) } Pattern::Tuple(source_ref, items) => { - Pattern::Tuple(source_ref, self.process_pattern_vec(items)) + Ok(Pattern::Tuple(source_ref, self.process_pattern_vec(items)?)) } Pattern::Variable(source_ref, name) => self.process_variable_pattern(source_ref, name), Pattern::Enum(source_ref, name, None) => { @@ -227,39 +241,45 @@ impl<'a, D: AnalysisDriver> ExpressionProcessor<'a, D> { // It's a single identifier that does not resolve to an enum variant. self.process_variable_pattern(source_ref, identifier.clone()) } else { - panic!("Expected enum variant but got {category}: {resolved_name}"); + return Err(source_ref.with_error(format!( + "Expected enum variant but got {category}: {resolved_name}" + ))); } } else if let Some(identifier) = name.try_to_identifier() { // It's a single identifier that does not resolve to an enum variant. self.process_variable_pattern(source_ref, identifier.clone()) } else { - panic!("Symbol not found: {name}"); + return Err(source_ref.with_error(format!("Symbol not found: {name}"))); } } Pattern::Enum(source_ref, name, fields) => { let name = self .driver .resolve_value_ref(&name) - .expect("TODO: Handle this up in the code"); + .map_err(|msg| source_ref.with_error(msg))?; self.process_enum_pattern(source_ref, name, fields) } } } - fn process_pattern_vec(&mut self, patterns: Vec) -> Vec { + fn process_pattern_vec(&mut self, patterns: Vec) -> Result, Error> { patterns .into_iter() .map(|p| self.process_pattern(p)) .collect() } - fn process_variable_pattern(&mut self, source_ref: SourceRef, name: String) -> Pattern { + fn process_variable_pattern( + &mut self, + source_ref: SourceRef, + name: String, + ) -> Result { let id = self.local_variable_counter; if self.local_variables.insert(name.clone(), id).is_some() { - panic!("Variable already defined: {name}"); + return Err(source_ref.with_error(format!("Variable already defined: {name}"))); } self.local_variable_counter += 1; - Pattern::Variable(source_ref, name) + Ok(Pattern::Variable(source_ref, name)) } fn process_enum_pattern( @@ -267,56 +287,71 @@ impl<'a, D: AnalysisDriver> ExpressionProcessor<'a, D> { source_ref: SourceRef, name: String, fields: Option>, - ) -> Pattern { - Pattern::Enum( - source_ref, - SymbolPath::from_str(&name).unwrap(), - fields.map(|fields| { - fields + ) -> Result { + let fields = fields + .map(|fields_vec| { + fields_vec .into_iter() .map(|p| self.process_pattern(p)) - .collect() - }), - ) + .collect::, _>>() + }) + .transpose()?; + + Ok(Pattern::Enum( + source_ref, + SymbolPath::from_str(&name).unwrap(), + fields, + )) } - fn process_reference(&mut self, reference: NamespacedPolynomialReference) -> Reference { + fn process_reference( + &mut self, + reference: NamespacedPolynomialReference, + ) -> Result { match reference.try_to_identifier() { Some(name) if self.local_variables.contains_key(name) => { let id = self.local_variables[name]; - Reference::LocalVar(id, name.to_string()) + Ok(Reference::LocalVar(id, name.to_string())) } - _ => Reference::Poly(self.process_namespaced_polynomial_reference(reference)), + _ => Ok(Reference::Poly( + self.process_namespaced_polynomial_reference(reference)?, + )), } } pub fn process_lambda_expression( &mut self, + source_ref: SourceRef, LambdaExpression { kind, params, body, .. }: LambdaExpression, - ) -> LambdaExpression { + ) -> Result { let previous_local_vars = self.save_local_variables(); let params = params .into_iter() .map(|p| self.process_pattern(p)) - .collect::>(); + .collect::, Error>>()?; for param in ¶ms { if !param.is_irrefutable() { - panic!("Function parameters must be irrefutable, but {param} is refutable."); + return Err(source_ref.with_error(format!( + "Function parameters must be irrefutable, but {param} is refutable." + ))); } } - let body = Box::new(self.process_expression(*body)); + let body = Box::new(self.process_expression(*body)?); self.reset_local_variables(previous_local_vars); - LambdaExpression { - kind, - params, - body, - param_types: vec![], - } + Ok(Expression::LambdaExpression( + source_ref, + LambdaExpression { + kind, + params, + body, + param_types: vec![], + }, + )) } fn process_block_expression( @@ -324,56 +359,78 @@ impl<'a, D: AnalysisDriver> ExpressionProcessor<'a, D> { statements: Vec, expr: Option>, src: SourceRef, - ) -> Expression { + ) -> Result { let vars = self.save_local_variables(); let processed_statements = statements .into_iter() - .map(|statement| match statement { - StatementInsideBlock::LetStatement(LetStatementInsideBlock { pattern, ty, value }) => { - let value = value.map(|v| self.process_expression(v)); - let pattern = self.process_pattern(pattern); - let ty = ty.map(|ty| self.process_number_type(ty)); - if value.is_none() && !matches!(pattern, Pattern::Variable(_, _)) { - panic!("Let statement without value requires a single variable, but got {pattern}."); + .map(|statement| { + match statement { + StatementInsideBlock::LetStatement(LetStatementInsideBlock { + pattern, + ty, + value, + }) => { + let value = match value { + Some(v) => Some(self.process_expression(v)?), + None => None, + }; + let pattern = self.process_pattern(pattern)?; + let ty = ty.map(|ty| self.process_number_type(ty)); + + if value.is_none() && !matches!(pattern, Pattern::Variable(_, _)) { + return Err(src.with_error(format!( + "Let statement without value requires a single variable, but got {pattern}." + ))); + } + if !pattern.is_irrefutable() { + return Err(src.with_error(format!( + "Let statement requires an irrefutable pattern, but {pattern} is refutable." + ))); + } + Ok(StatementInsideBlock::LetStatement(LetStatementInsideBlock { + pattern, + ty, + value, + })) } - if !pattern.is_irrefutable() { - panic!("Let statement requires an irrefutable pattern, but {pattern} is refutable."); + StatementInsideBlock::Expression(expr) => { + Ok(StatementInsideBlock::Expression(self.process_expression(expr)?)) } - StatementInsideBlock::LetStatement(LetStatementInsideBlock { pattern, ty, value }) - } - StatementInsideBlock::Expression(expr) => { - StatementInsideBlock::Expression(self.process_expression(expr)) } }) - .collect::>(); - - let processed_expr = expr.map(|expr| Box::new(self.process_expression(*expr))); + .collect::, Error>>()?; + let processed_expr = match expr { + Some(expr) => { + let src = expr.source_reference().clone(); + match self.process_expression(*expr) { + Ok(e) => Some(Box::new(e)), + Err(e) => return Err(src.with_error(e.to_string())), + } + } + None => None, + }; self.reset_local_variables(vars); - Expression::BlockExpression( + Ok(Expression::BlockExpression( src, BlockExpression { statements: processed_statements, expr: processed_expr, }, - ) + )) } pub fn process_namespaced_polynomial_reference( &mut self, reference: NamespacedPolynomialReference, - ) -> PolynomialReference { + ) -> Result { let type_args = reference .type_args .map(|args| args.into_iter().map(|t| self.process_type(t)).collect()); - PolynomialReference { - name: self - .driver - .resolve_value_ref(&reference.path) - .expect("TODO: Handle this up in the code"), - type_args, - } + + let name = self.driver.resolve_value_ref(&reference.path)?; + Ok(PolynomialReference { name, type_args }) } fn process_type(&self, ty: Type) -> Type { diff --git a/pil-analyzer/src/pil_analyzer.rs b/pil-analyzer/src/pil_analyzer.rs index 40cbae69a1..39d1118758 100644 --- a/pil-analyzer/src/pil_analyzer.rs +++ b/pil-analyzer/src/pil_analyzer.rs @@ -523,8 +523,15 @@ impl PILAnalyzer { fn handle_namespace(&mut self, name: SymbolPath, degree: Option) { let evaluate_degree_bound = |e| { - let e = - ExpressionProcessor::new(self.driver(), &Default::default()).process_expression(e); + let e = match ExpressionProcessor::new(self.driver(), &Default::default()) + .process_expression(e) + { + Ok(e) => e, + Err(e) => { + // TODO propagate this error up + panic!("Failed to evaluate degree bound: {e}"); + } + }; u64::try_from( evaluator::evaluate_expression::( &e, diff --git a/pil-analyzer/src/statement_processor.rs b/pil-analyzer/src/statement_processor.rs index da69bd731b..02afa0286f 100644 --- a/pil-analyzer/src/statement_processor.rs +++ b/pil-analyzer/src/statement_processor.rs @@ -213,7 +213,9 @@ where } PilStatement::Expression(_, expr) => vec![PILItem::ProofItem( self.expression_processor(&Default::default()) - .process_expression(expr), + .process_expression(expr) + // TODO propagate this error up + .expect("Expression processing failed"), )], PilStatement::StructDeclaration(source, struct_declaration) => self .handle_symbol_definition( @@ -237,6 +239,11 @@ where let ty = Some(match array_size { None => base_type.into(), Some(len) => { + let len = self + .expression_processor(&Default::default()) + .process_expression(len) + // TODO propagate this error up + .expect("Failed to process length expression"); let length = untyped_evaluator::evaluate_expression_to_int(self.driver, len) .map(|length| { length @@ -461,7 +468,8 @@ where let value = FunctionValueDefinition::Expression(TypedExpression { e: self .expression_processor(&type_vars) - .process_expression(expr), + .process_expression(expr) + .expect("Failed to process expression"), type_scheme, }); @@ -476,7 +484,8 @@ where ) -> Vec { let expression = self .expression_processor(&Default::default()) - .process_array_expression(value); + .process_array_expression(value) + .expect("Failed to process array expression"); assert!(type_scheme.is_none() || type_scheme == Some(Type::Col.into())); let value = FunctionValueDefinition::Array(expression); @@ -518,8 +527,15 @@ where let name = self.driver.resolve_decl(&name); let polynomial = self .expression_processor(&Default::default()) - .process_namespaced_polynomial_reference(poly); + .process_namespaced_polynomial_reference(poly) + .expect("Failed to process polynomial reference"); + let type_vars = Default::default(); + let mut expression_processor = self.expression_processor(&type_vars); let array_index = array_index.map(|i| { + let i = expression_processor + .process_expression(i) + // TODO propagate this error up + .expect("Failed to process array index expression"); let index: u64 = untyped_evaluator::evaluate_expression_to_int(self.driver, i) .unwrap() .try_into() @@ -527,6 +543,10 @@ where assert!(index <= usize::MAX as u64); index as usize }); + + let index = expression_processor + .process_expression(index) // TODO propagate this error up + .expect("Failed to process index"); vec![PILItem::PublicDeclaration(PublicDeclaration { id, source, @@ -713,7 +733,8 @@ where name: named.name, body: Arc::new( self.expression_processor(&type_vars) - .process_expression(Arc::try_unwrap(named.body).unwrap()), + .process_expression(Arc::try_unwrap(named.body).unwrap()) + .expect("Failed to process expression inside trait"), ), }) .collect(); diff --git a/pil-analyzer/src/type_processor.rs b/pil-analyzer/src/type_processor.rs index 2c08691051..f9f98cb0d1 100644 --- a/pil-analyzer/src/type_processor.rs +++ b/pil-analyzer/src/type_processor.rs @@ -3,7 +3,10 @@ use std::{collections::HashSet, str::FromStr}; use powdr_ast::parsed::{asm::SymbolPath, types::Type, visitor::Children, Expression}; use powdr_number::BigUint; -use crate::{evaluator::EvalError, untyped_evaluator, AnalysisDriver}; +use crate::{ + evaluator::EvalError, expression_processor::ExpressionProcessor, untyped_evaluator, + AnalysisDriver, +}; /// The TypeProcessor turns parsed types into analyzed types, which means that /// it resolves local type name references, replaces named types that actually @@ -48,7 +51,10 @@ impl<'a, D: AnalysisDriver> TypeProcessor<'a, D> { // Any expression inside a type name has to be an array length, // so we expect an integer that fits u64. t.children_mut().try_for_each(|e: &mut Expression| { - let v = untyped_evaluator::evaluate_expression_to_int(self.driver, e.clone())?; + let analyzed_expr = ExpressionProcessor::new(self.driver, &Default::default()) + .process_expression(e.clone()) + .map_err(|e| EvalError::TypeError(e.message().to_string()))?; // TODO: Replace with a proper error type + let v = untyped_evaluator::evaluate_expression_to_int(self.driver, analyzed_expr)?; let v_u64: u64 = v.clone().try_into().map_err(|_| { EvalError::TypeError(format!("Number too large, expected u64, but got {v}")) })?; diff --git a/pil-analyzer/src/untyped_evaluator.rs b/pil-analyzer/src/untyped_evaluator.rs index 8dad8a7554..158e731f3d 100644 --- a/pil-analyzer/src/untyped_evaluator.rs +++ b/pil-analyzer/src/untyped_evaluator.rs @@ -1,9 +1,8 @@ -use powdr_ast::parsed; +use powdr_ast::analyzed; use powdr_number::{BigInt, GoldilocksField}; use crate::{ evaluator::{self, EvalError}, - expression_processor::ExpressionProcessor, AnalysisDriver, }; @@ -14,10 +13,10 @@ use crate::{ /// This is mainly used to evaluate array lengths in types and namespace degrees. pub fn evaluate_expression_to_int( driver: impl AnalysisDriver, - expr: parsed::Expression, + expr: analyzed::Expression, ) -> Result { evaluator::evaluate_expression::( - &ExpressionProcessor::new(driver, &Default::default()).process_expression(expr), + &expr, driver.definitions(), &Default::default(), )? diff --git a/pil-analyzer/tests/types.rs b/pil-analyzer/tests/types.rs index 84bbe26c38..c59b9193ae 100644 --- a/pil-analyzer/tests/types.rs +++ b/pil-analyzer/tests/types.rs @@ -480,7 +480,7 @@ fn enum_pattern() { } #[test] -#[should_panic = "Only one \"..\"-item allowed in array pattern"] +#[should_panic = "Only one \\\"..\\\"-item allowed in array pattern"] fn multi_ellipsis() { let input = " let t: int[] -> int = (|i| match i { [1, .., 3, ..] => 2,