From 7438205bfb0c6b9fcab0783872c1342c6e75397a Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 3 Nov 2022 16:23:23 -0400 Subject: [PATCH 1/5] Start handling arbitrary expressions in array sizes --- crates/noirc_frontend/src/ast/expression.rs | 63 ++++++++++++++++++--- crates/noirc_frontend/src/ast/statement.rs | 10 ++++ crates/noirc_frontend/src/parser/parser.rs | 41 +++++++------- 3 files changed, 85 insertions(+), 29 deletions(-) diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index e513fc26208..f5db31bb582 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -46,9 +46,13 @@ impl ExpressionKind { } pub fn array(contents: Vec) -> ExpressionKind { - ExpressionKind::Literal(Literal::Array(ArrayLiteral { - length: contents.len() as u128, - contents, + ExpressionKind::Literal(Literal::Array(ArrayLiteral::Standard(contents))) + } + + pub fn repeated_array(repeated_element: Expression, length: Expression) -> ExpressionKind { + ExpressionKind::Literal(Literal::Array(ArrayLiteral::Repeated { + repeated_element: Box::new(repeated_element), + length: Box::new(length), })) } @@ -174,6 +178,46 @@ impl Expression { let kind = ExpressionKind::Cast(Box::new(CastExpression { lhs, r#type })); Expression::new(kind, span) } + + pub fn contains_function_call(&self) -> bool { + match &self.kind { + ExpressionKind::Ident(_) + | ExpressionKind::Literal(_) + | ExpressionKind::Path(_) + | ExpressionKind::Error => false, + + ExpressionKind::Call(_) | ExpressionKind::MethodCall(_) => true, + + ExpressionKind::Block(block) => { + block.0.iter().any(|stmt| stmt.contains_function_call()) + } + ExpressionKind::Prefix(prefix) => prefix.rhs.contains_function_call(), + ExpressionKind::Index(index) => { + index.collection.contains_function_call() || index.index.contains_function_call() + } + ExpressionKind::Constructor(ctor) => { + ctor.fields.iter().any(|(_, expr)| expr.contains_function_call()) + } + ExpressionKind::MemberAccess(access) => access.lhs.contains_function_call(), + ExpressionKind::Cast(cast) => cast.lhs.contains_function_call(), + ExpressionKind::Infix(infix) => { + infix.lhs.contains_function_call() || infix.rhs.contains_function_call() + } + ExpressionKind::For(for_loop) => { + for_loop.start_range.contains_function_call() + || for_loop.end_range.contains_function_call() + || for_loop.block.contains_function_call() + } + ExpressionKind::If(if_expr) => { + if_expr.condition.contains_function_call() + || if_expr.consequence.contains_function_call() + || if_expr.alternative.map_or(false, |a| a.contains_function_call()) + } + ExpressionKind::Tuple(fields) => { + fields.iter().any(|field| field.contains_function_call()) + } + } + } } #[derive(Debug, PartialEq, Eq, Clone)] @@ -355,9 +399,9 @@ pub struct FunctionDefinition { } #[derive(Debug, PartialEq, Eq, Clone)] -pub struct ArrayLiteral { - pub length: u128, // XXX: Maybe allow field element, so that the user can define the length using a constant - pub contents: Vec, +pub enum ArrayLiteral { + Standard(Vec), + Repeated { repeated_element: Box, length: Box }, } #[derive(Debug, PartialEq, Eq, Clone)] @@ -444,10 +488,13 @@ impl Display for ExpressionKind { impl Display for Literal { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Literal::Array(array) => { - let contents = vecmap(&array.contents, ToString::to_string); + Literal::Array(ArrayLiteral::Standard(elements)) => { + let contents = vecmap(elements, ToString::to_string); write!(f, "[{}]", contents.join(", ")) } + Literal::Array(ArrayLiteral::Repeated { repeated_element, length }) => { + write!(f, "[{}; {}]", repeated_element, length) + } Literal::Bool(boolean) => write!(f, "{}", if *boolean { "true" } else { "false" }), Literal::Integer(integer) => write!(f, "{}", integer.to_u128()), Literal::Str(string) => write!(f, "\"{}\"", string), diff --git a/crates/noirc_frontend/src/ast/statement.rs b/crates/noirc_frontend/src/ast/statement.rs index a49e0122694..93965e97ae0 100644 --- a/crates/noirc_frontend/src/ast/statement.rs +++ b/crates/noirc_frontend/src/ast/statement.rs @@ -201,6 +201,16 @@ impl Statement { } } } + + pub fn contains_function_call(&self) -> bool { + match self { + Statement::Let(let_stmt) => let_stmt.expression.contains_function_call(), + Statement::Constrain(constrain) => constrain.0.contains_function_call(), + Statement::Assign(assign) => assign.expression.contains_function_call(), + Statement::Expression(expr) | Statement::Semi(expr) => expr.contains_function_call(), + Statement::Error => false, + } + } } #[derive(Debug, PartialEq, Eq, Clone)] diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index bed804fa2c4..b7499ba9673 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -1,5 +1,3 @@ -use std::iter::repeat; - use super::{ foldl_with_span, parameter_name_recovery, parameter_recovery, parenthesized, then_commit, then_commit_ignore, top_level_statement_recovery, ExprParser, ForRange, NoirParser, @@ -18,6 +16,7 @@ use crate::{ NoirFunction, NoirImpl, NoirStruct, Path, PathKind, Pattern, Recoverable, UnaryOp, }; +use acvm::FieldElement; use chumsky::prelude::*; use noirc_abi::AbiFEType; use noirc_errors::{CustomDiagnostic, DiagnosableError, Span, Spanned}; @@ -678,21 +677,17 @@ where P: ExprParser, { expr_parser - .then(array_length()) + .clone() + .then(just(Token::Semicolon).ignore_then(expr_parser)) .delimited_by(just(Token::LeftBracket), just(Token::RightBracket)) - .map_with_span(|(lhs, count), span| { - // Desugar the array by replicating the lhs 'count' times. TODO: This is inefficient - // for large arrays. - let name = "$array_element"; - let pattern = Pattern::Identifier(name.into()); - let decl = Statement::new_let(((pattern, UnresolvedType::Unspecified), lhs)); - - let variable = Expression::new(ExpressionKind::Ident(name.into()), span); - let elems = repeat(variable).take(count as usize).collect(); - let array = ExpressionKind::array(elems); - let array = Statement::Expression(Expression::new(array, span)); - - ExpressionKind::Block(BlockExpression(vec![decl, array])) + .validate(|(lhs, mut count), span, emit| { + if count.contains_function_call() { + let msg = "Array size expressions cannot contain function calls"; + emit(ParserError::with_reason(msg.into(), span)); + // Default the count to 1 and continue + count = Expression::new(ExpressionKind::integer(FieldElement::one()), count.span); + } + ExpressionKind::repeated_array(lhs, count) }) } @@ -966,17 +961,21 @@ mod test { } } - /// This is the standard way to declare an array #[test] fn parse_array() { let valid = vec![ "[0, 1, 2,3, 4]", "[0,1,2,3,4,]", // Trailing commas are valid syntax + "[0;5]", ]; for expr in parse_all(array_expr(expression()), valid) { - let arr_lit = expr_to_array(expr); - assert_eq!(arr_lit.length, 5); + match expr_to_array(expr) { + ArrayLiteral::Standard(elems) => assert_eq!(elems.len(), 5), + ArrayLiteral::Repeated { length, .. } => { + assert_eq!(length.kind, ExpressionKind::integer(5i128.into())) + } + } } parse_all_failing( @@ -987,10 +986,10 @@ mod test { #[test] fn parse_array_sugar() { - let valid = vec!["[0;7]", "[(1, 2); 4]"]; + let valid = vec!["[0;7]", "[(1, 2); 4]", "[0;Four]", "[2;1+3-a]"]; parse_all(array_expr(expression()), valid); - let invalid = vec!["[0;;4]", "[5; 3+2]", "[1; a]"]; + let invalid = vec!["[0;;4]", "[1; foo()]", "[1, 2; 3]"]; parse_all_failing(array_expr(expression()), invalid); } From c7e7a23ec224500302828cc0793e8a71ad86f13d Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 4 Nov 2022 13:19:01 -0400 Subject: [PATCH 2/5] Allow more expressions to be used in array sizes --- .../tests/test_data/global_consts/src/main.nr | 4 + crates/noirc_evaluator/src/interpreter.rs | 2 +- crates/noirc_evaluator/src/object/array.rs | 7 +- crates/noirc_evaluator/src/ssa/code_gen.rs | 2 +- crates/noirc_frontend/src/ast/expression.rs | 2 +- .../src/hir/resolution/errors.rs | 14 ++ .../src/hir/resolution/resolver.rs | 132 +++++++++++++++++- .../noirc_frontend/src/hir/type_check/expr.rs | 8 +- crates/noirc_frontend/src/hir_def/expr.rs | 8 +- .../src/monomorphisation/ast.rs | 1 - .../src/monomorphisation/mod.rs | 6 +- crates/noirc_frontend/src/parser/parser.rs | 21 +-- 12 files changed, 158 insertions(+), 49 deletions(-) diff --git a/crates/nargo/tests/test_data/global_consts/src/main.nr b/crates/nargo/tests/test_data/global_consts/src/main.nr index 19e34aec09c..8f6af14a2b8 100644 --- a/crates/nargo/tests/test_data/global_consts/src/main.nr +++ b/crates/nargo/tests/test_data/global_consts/src/main.nr @@ -43,6 +43,10 @@ fn main(a: [Field; M], b: [Field; M], c : pub [Field; foo::MAGIC_NUMBER]) { let arr: [Field; mysubmodule::N] = [N; 10]; constrain arr[0] == 5; constrain arr[9] == 5; + + // Example showing an array filled with (mysubmodule::N + 2) 0's + let sugared = [0; mysubmodule::N + 2]; + constrain sugared[mysubmodule::N + 1] == 0; } fn multiplyByM(x: Field) -> Field { diff --git a/crates/noirc_evaluator/src/interpreter.rs b/crates/noirc_evaluator/src/interpreter.rs index 5cabda39e87..fb2206d7a0e 100644 --- a/crates/noirc_evaluator/src/interpreter.rs +++ b/crates/noirc_evaluator/src/interpreter.rs @@ -437,7 +437,7 @@ impl<'a> Interpreter<'a> { match self.context.def_interner.expression(expr_id) { HirExpression::Literal(HirLiteral::Integer(x)) => Ok(Object::Constants(x)), HirExpression::Literal(HirLiteral::Array(arr_lit)) => { - Ok(Object::Array(Array::from(self, env, arr_lit)?)) + Ok(Object::Array(Array::from(self, env, &arr_lit)?)) } HirExpression::Ident(x) => Ok(self.evaluate_identifier(x, env)), HirExpression::Infix(infx) => { diff --git a/crates/noirc_evaluator/src/object/array.rs b/crates/noirc_evaluator/src/object/array.rs index e17e92c85a6..b2f8517881b 100644 --- a/crates/noirc_evaluator/src/object/array.rs +++ b/crates/noirc_evaluator/src/object/array.rs @@ -4,7 +4,6 @@ use crate::interpreter::Interpreter; use crate::Environment; use crate::{binary_op::maybe_equal, object::Object}; use acvm::FieldElement; -use noirc_frontend::hir_def::expr::HirArrayLiteral; use noirc_frontend::node_interner::ExprId; #[derive(Clone, Debug)] @@ -17,19 +16,19 @@ impl Array { pub fn from( evaluator: &mut Interpreter, env: &mut Environment, - arr_lit: HirArrayLiteral, + arr_lit: &[ExprId], ) -> Result { // Take each element in the array and turn it into an object // We do not check that the array is homogeneous, this is done by the type checker. // We could double check here, however with appropriate tests, it should not be needed. - let (objects, mut errs) = evaluator.expression_list_to_objects(env, &arr_lit.contents); + let (objects, mut errs) = evaluator.expression_list_to_objects(env, arr_lit); if !errs.is_empty() { // XXX Should we make this return an RunTimeError? The problem is that we do not want the OPCODES // to return RunTimeErrors, because we do not want to deal with span there return Err(errs.pop().unwrap()); } - Ok(Array { contents: objects, length: arr_lit.length }) + Ok(Array { contents: objects, length: arr_lit.len() as u128 }) } pub fn get(&self, index: u128) -> Result { if index >= self.length { diff --git a/crates/noirc_evaluator/src/ssa/code_gen.rs b/crates/noirc_evaluator/src/ssa/code_gen.rs index 09a7767d06a..aceb2c77dcf 100644 --- a/crates/noirc_evaluator/src/ssa/code_gen.rs +++ b/crates/noirc_evaluator/src/ssa/code_gen.rs @@ -475,7 +475,7 @@ impl IRGenerator { let element_type = ObjectType::from(&arr_lit.element_type); let (new_var, array_id) = - self.context.new_array("", element_type, arr_lit.length as u32, None); + self.context.new_array("", element_type, arr_lit.contents.len() as u32, None); let elements = self.codegen_expression_list(env, &arr_lit.contents); for (pos, object) in elements.into_iter().enumerate() { diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index f5db31bb582..0eb41ad648e 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -211,7 +211,7 @@ impl Expression { ExpressionKind::If(if_expr) => { if_expr.condition.contains_function_call() || if_expr.consequence.contains_function_call() - || if_expr.alternative.map_or(false, |a| a.contains_function_call()) + || if_expr.alternative.as_ref().map_or(false, |a| a.contains_function_call()) } ExpressionKind::Tuple(fields) => { fields.iter().any(|field| field.contains_function_call()) diff --git a/crates/noirc_frontend/src/hir/resolution/errors.rs b/crates/noirc_frontend/src/hir/resolution/errors.rs index 07165e198e4..b5487f6586f 100644 --- a/crates/noirc_frontend/src/hir/resolution/errors.rs +++ b/crates/noirc_frontend/src/hir/resolution/errors.rs @@ -34,6 +34,10 @@ pub enum ResolverError { ExpectedConstVariable { name: String, span: Span }, #[error("Missing expression for declared constant")] MissingRhsExpr { name: String, span: Span }, + #[error("Expression invalid in an array length context")] + InvalidArrayLengthExpr { span: Span }, + #[error("Integer too large to be evaluated in an array length context")] + IntegerTooLarge { span: Span }, } impl ResolverError { @@ -177,6 +181,16 @@ impl ResolverError { "expected expression to be stored for let statement".to_string(), span, ), + ResolverError::InvalidArrayLengthExpr { span } => Diagnostic::simple_error( + "Expression invalid in an array-length context".into(), + "Array-length expressions can only have simple integer operations and any variables used must be global constants".into(), + span, + ), + ResolverError::IntegerTooLarge { span } => Diagnostic::simple_error( + "Integer too large to be evaluated to an array-length".into(), + "Array-lengths may be a maximum size of usize::MAX, including intermediate calculations".into(), + span, + ), } } } diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index 83fdb5bb0bd..11cb0edf6b2 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -18,7 +18,7 @@ struct ResolverMeta { } use crate::hir_def::expr::{ - HirArrayLiteral, HirBinaryOp, HirBlockExpression, HirCallExpression, HirCastExpression, + HirBinaryOp, HirBlockExpression, HirCallExpression, HirCastExpression, HirConstructorExpression, HirExpression, HirForExpression, HirIdent, HirIfExpression, HirIndexExpression, HirInfixExpression, HirLiteral, HirMemberAccess, HirMethodCallExpression, HirPrefixExpression, @@ -37,8 +37,8 @@ use crate::{ Statement, UnresolvedArraySize, }; use crate::{ - Generics, LValue, NoirStruct, Path, Pattern, Shared, StructType, Type, TypeBinding, - TypeVariable, UnresolvedType, ERROR_IDENT, + ArrayLiteral, Generics, LValue, NoirStruct, Path, Pattern, Shared, StructType, Type, + TypeBinding, TypeVariable, UnresolvedType, ERROR_IDENT, }; use fm::FileId; use noirc_errors::{Location, Span, Spanned}; @@ -549,10 +549,26 @@ impl<'a> Resolver<'a> { } ExpressionKind::Literal(literal) => HirExpression::Literal(match literal { Literal::Bool(b) => HirLiteral::Bool(b), - Literal::Array(arr) => HirLiteral::Array(HirArrayLiteral { - contents: vecmap(arr.contents, |elem| self.resolve_expression(elem)), - length: arr.length, - }), + Literal::Array(ArrayLiteral::Standard(elems)) => { + HirLiteral::Array(vecmap(elems, |elem| self.resolve_expression(elem))) + } + Literal::Array(ArrayLiteral::Repeated { repeated_element, length }) => { + match self.try_eval_array_length(&length).map(|length| length.try_into()) { + Ok(Ok(length_value)) => { + let elem = self.resolve_expression(*repeated_element); + HirLiteral::Array(vec![elem; length_value]) + } + Ok(Err(_cast_err)) => { + self.push_err(ResolverError::IntegerTooLarge { span: length.span }); + HirLiteral::Array(vec![]) + } + Err(Some(error)) => { + self.push_err(error); + HirLiteral::Array(vec![]) + } + Err(None) => HirLiteral::Array(vec![]), + } + } Literal::Integer(integer) => HirLiteral::Integer(integer), Literal::Str(str) => HirLiteral::Str(str), }), @@ -835,6 +851,108 @@ impl<'a> Resolver<'a> { let hir_block = self.resolve_block(block); self.interner.push_expr(hir_block) } + + /// This function is a mini interpreter inside name resolution. + /// We should eventually get rid of it and only have 1 evaluator - the existing + /// one inside the ssa pass. Doing this would mean ssa would need to handle these + /// sugared array forms but would let users use any comptime expressions, including functions, + /// inside array lengths. + fn try_eval_array_length( + &mut self, + length: &Expression, + ) -> Result> { + let span = length.span; + match &length.kind { + ExpressionKind::Literal(Literal::Integer(int)) => { + int.try_into_u128().ok_or(Some(ResolverError::IntegerTooLarge { span })) + } + ExpressionKind::Ident(ident) => { + let ident: Ident = Spanned::from(span, ident.to_owned()).into(); + let ident = self.find_variable(&ident); + self.try_eval_array_length_ident(ident.id, span) + } + ExpressionKind::Path(path) => { + let ident = self.get_ident_from_path(path.clone()); + self.try_eval_array_length_ident(ident.id, span) + } + ExpressionKind::Prefix(operator) => { + let value = self.try_eval_array_length(&operator.rhs)?; + match operator.operator { + crate::UnaryOp::Minus => Ok(0 - value), + crate::UnaryOp::Not => Ok(!value), + } + } + ExpressionKind::Infix(operator) => { + let lhs = self.try_eval_array_length(&operator.lhs)?; + let rhs = self.try_eval_array_length(&operator.rhs)?; + match operator.operator.contents { + crate::BinaryOpKind::Add => Ok(lhs + rhs), + crate::BinaryOpKind::Subtract => Ok(lhs - rhs), + crate::BinaryOpKind::Multiply => Ok(lhs * rhs), + crate::BinaryOpKind::Divide => Ok(lhs / rhs), + crate::BinaryOpKind::ShiftRight => Ok(lhs >> rhs), + crate::BinaryOpKind::ShiftLeft => Ok(lhs << rhs), + crate::BinaryOpKind::Modulo => Ok(lhs % rhs), + crate::BinaryOpKind::And => Ok(lhs & rhs), + crate::BinaryOpKind::Or => Ok(lhs | rhs), + crate::BinaryOpKind::Xor => Ok(lhs ^ rhs), + + crate::BinaryOpKind::Equal + | crate::BinaryOpKind::NotEqual + | crate::BinaryOpKind::Less + | crate::BinaryOpKind::LessEqual + | crate::BinaryOpKind::Greater + | crate::BinaryOpKind::GreaterEqual => { + Err(Some(ResolverError::InvalidArrayLengthExpr { span })) + } + } + } + + ExpressionKind::Literal(_) + | ExpressionKind::Block(_) + | ExpressionKind::Index(_) + | ExpressionKind::Call(_) + | ExpressionKind::MethodCall(_) + | ExpressionKind::Constructor(_) + | ExpressionKind::MemberAccess(_) + | ExpressionKind::Cast(_) + | ExpressionKind::For(_) + | ExpressionKind::If(_) + | ExpressionKind::Tuple(_) => Err(Some(ResolverError::InvalidArrayLengthExpr { span })), + + ExpressionKind::Error => Err(None), + } + } + + fn try_eval_array_length_ident( + &mut self, + id: DefinitionId, + span: Span, + ) -> Result> { + if id == DefinitionId::dummy_id() { + return Err(None); // error already reported + } + + let definition = self.interner.definition(id); + match definition.rhs { + Some(rhs) if definition.is_global => self.try_eval_array_length_id(rhs), + _ => Err(Some(ResolverError::InvalidArrayLengthExpr { span })), + } + } + + fn try_eval_array_length_id(&self, rhs: ExprId) -> Result> { + let span = self.interner.expr_span(&rhs); + match self.interner.expression(&rhs) { + HirExpression::Literal(HirLiteral::Integer(int)) => { + int.try_into_u128().ok_or(Some(ResolverError::IntegerTooLarge { span })) + } + HirExpression::Literal(_) => Err(Some(ResolverError::InvalidArrayLengthExpr { span })), + other => unreachable!( + "Expected global to be initialized to a literal, but found {:?}", + other + ), + } + } } // XXX: These tests repeat a lot of code diff --git a/crates/noirc_frontend/src/hir/type_check/expr.rs b/crates/noirc_frontend/src/hir/type_check/expr.rs index 598a5f5d5b2..63086a4f556 100644 --- a/crates/noirc_frontend/src/hir/type_check/expr.rs +++ b/crates/noirc_frontend/src/hir/type_check/expr.rs @@ -32,24 +32,24 @@ pub(crate) fn type_check_expression( HirLiteral::Array(arr) => { // Type check the contents of the array let elem_types = - vecmap(&arr.contents, |arg| type_check_expression(interner, arg, errors)); + vecmap(&arr, |arg| type_check_expression(interner, arg, errors)); let first_elem_type = elem_types.get(0).cloned().unwrap_or(Type::Error); // Specify the type of the Array // Note: This assumes that the array is homogeneous, which will be checked next let arr_type = Type::Array( - Box::new(Type::ArrayLength(arr.contents.len() as u64)), + Box::new(Type::ArrayLength(arr.len() as u64)), Box::new(first_elem_type.clone()), ); // Check if the array is homogeneous for (index, elem_type) in elem_types.iter().enumerate().skip(1) { - let location = interner.expr_location(&arr.contents[index]); + let location = interner.expr_location(&arr[index]); elem_type.unify(&first_elem_type, location.span, errors, || { TypeCheckError::NonHomogeneousArray { - first_span: interner.expr_location(&arr.contents[0]).span, + first_span: interner.expr_location(&arr[0]).span, first_type: first_elem_type.to_string(), first_index: index, second_span: location.span, diff --git a/crates/noirc_frontend/src/hir_def/expr.rs b/crates/noirc_frontend/src/hir_def/expr.rs index 5933e938124..83118e417d5 100644 --- a/crates/noirc_frontend/src/hir_def/expr.rs +++ b/crates/noirc_frontend/src/hir_def/expr.rs @@ -63,7 +63,7 @@ impl HirBinaryOp { #[derive(Debug, Clone)] pub enum HirLiteral { - Array(HirArrayLiteral), + Array(Vec), Bool(bool), Integer(FieldElement), Str(String), @@ -103,12 +103,6 @@ pub struct HirCastExpression { pub r#type: Type, } -#[derive(Debug, Clone)] -pub struct HirArrayLiteral { - pub length: u128, - pub contents: Vec, -} - #[derive(Debug, Clone)] pub struct HirCallExpression { pub func_id: FuncId, diff --git a/crates/noirc_frontend/src/monomorphisation/ast.rs b/crates/noirc_frontend/src/monomorphisation/ast.rs index 1be93c22f35..53d2bb5632c 100644 --- a/crates/noirc_frontend/src/monomorphisation/ast.rs +++ b/crates/noirc_frontend/src/monomorphisation/ast.rs @@ -90,7 +90,6 @@ pub struct Cast { #[derive(Debug, Clone)] pub struct ArrayLiteral { - pub length: u128, pub contents: Vec, pub element_type: Type, } diff --git a/crates/noirc_frontend/src/monomorphisation/mod.rs b/crates/noirc_frontend/src/monomorphisation/mod.rs index 46d15f31e7a..bd8c80c446c 100644 --- a/crates/noirc_frontend/src/monomorphisation/mod.rs +++ b/crates/noirc_frontend/src/monomorphisation/mod.rs @@ -196,9 +196,9 @@ impl Monomorphiser { Literal(Integer(value, typ)) } HirExpression::Literal(HirLiteral::Array(array)) => { - let element_type = Self::convert_type(&self.interner.id_type(array.contents[0])); - let contents = vecmap(array.contents, |id| self.expr_infer(id)); - Literal(Array(ast::ArrayLiteral { length: array.length, contents, element_type })) + let element_type = Self::convert_type(&self.interner.id_type(array[0])); + let contents = vecmap(array, |id| self.expr_infer(id)); + Literal(Array(ast::ArrayLiteral { contents, element_type })) } HirExpression::Block(block) => self.block(block.0), diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index b7499ba9673..f66cb85e84c 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -16,7 +16,6 @@ use crate::{ NoirFunction, NoirImpl, NoirStruct, Path, PathKind, Pattern, Recoverable, UnaryOp, }; -use acvm::FieldElement; use chumsky::prelude::*; use noirc_abi::AbiFEType; use noirc_errors::{CustomDiagnostic, DiagnosableError, Span, Spanned}; @@ -680,15 +679,7 @@ where .clone() .then(just(Token::Semicolon).ignore_then(expr_parser)) .delimited_by(just(Token::LeftBracket), just(Token::RightBracket)) - .validate(|(lhs, mut count), span, emit| { - if count.contains_function_call() { - let msg = "Array size expressions cannot contain function calls"; - emit(ParserError::with_reason(msg.into(), span)); - // Default the count to 1 and continue - count = Expression::new(ExpressionKind::integer(FieldElement::one()), count.span); - } - ExpressionKind::repeated_array(lhs, count) - }) + .map(|(lhs, count)| ExpressionKind::repeated_array(lhs, count)) } fn expression_list

(expr_parser: P) -> impl NoirParser> @@ -818,16 +809,6 @@ fn try_field_to_u64(x: acvm::FieldElement, span: Span) -> Result impl NoirParser { - just(Token::Semicolon).ignore_then(filter_map(|span, token: Token| match token { - Token::Int(integer) => try_field_to_u64(integer, span), - _ => { - let message = "Expected an integer for the length of the array".to_string(); - Err(ParserError::with_reason(message, span)) - } - })) -} - #[cfg(test)] mod test { use noirc_errors::{CustomDiagnostic, DiagnosableError}; From 3d934bb828197b9f51077567262de49c0ca49de8 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 4 Nov 2022 13:30:55 -0400 Subject: [PATCH 3/5] Remove unused contains_function_call function --- crates/noirc_frontend/src/ast/expression.rs | 40 --------------------- crates/noirc_frontend/src/ast/statement.rs | 10 ------ 2 files changed, 50 deletions(-) diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index 0eb41ad648e..d233d281bbd 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -178,46 +178,6 @@ impl Expression { let kind = ExpressionKind::Cast(Box::new(CastExpression { lhs, r#type })); Expression::new(kind, span) } - - pub fn contains_function_call(&self) -> bool { - match &self.kind { - ExpressionKind::Ident(_) - | ExpressionKind::Literal(_) - | ExpressionKind::Path(_) - | ExpressionKind::Error => false, - - ExpressionKind::Call(_) | ExpressionKind::MethodCall(_) => true, - - ExpressionKind::Block(block) => { - block.0.iter().any(|stmt| stmt.contains_function_call()) - } - ExpressionKind::Prefix(prefix) => prefix.rhs.contains_function_call(), - ExpressionKind::Index(index) => { - index.collection.contains_function_call() || index.index.contains_function_call() - } - ExpressionKind::Constructor(ctor) => { - ctor.fields.iter().any(|(_, expr)| expr.contains_function_call()) - } - ExpressionKind::MemberAccess(access) => access.lhs.contains_function_call(), - ExpressionKind::Cast(cast) => cast.lhs.contains_function_call(), - ExpressionKind::Infix(infix) => { - infix.lhs.contains_function_call() || infix.rhs.contains_function_call() - } - ExpressionKind::For(for_loop) => { - for_loop.start_range.contains_function_call() - || for_loop.end_range.contains_function_call() - || for_loop.block.contains_function_call() - } - ExpressionKind::If(if_expr) => { - if_expr.condition.contains_function_call() - || if_expr.consequence.contains_function_call() - || if_expr.alternative.as_ref().map_or(false, |a| a.contains_function_call()) - } - ExpressionKind::Tuple(fields) => { - fields.iter().any(|field| field.contains_function_call()) - } - } - } } #[derive(Debug, PartialEq, Eq, Clone)] diff --git a/crates/noirc_frontend/src/ast/statement.rs b/crates/noirc_frontend/src/ast/statement.rs index 93965e97ae0..a49e0122694 100644 --- a/crates/noirc_frontend/src/ast/statement.rs +++ b/crates/noirc_frontend/src/ast/statement.rs @@ -201,16 +201,6 @@ impl Statement { } } } - - pub fn contains_function_call(&self) -> bool { - match self { - Statement::Let(let_stmt) => let_stmt.expression.contains_function_call(), - Statement::Constrain(constrain) => constrain.0.contains_function_call(), - Statement::Assign(assign) => assign.expression.contains_function_call(), - Statement::Expression(expr) | Statement::Semi(expr) => expr.contains_function_call(), - Statement::Error => false, - } - } } #[derive(Debug, PartialEq, Eq, Clone)] From 80ef5432e07c6fa337cea40831f9a40970fa6dcf Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 4 Nov 2022 14:27:27 -0400 Subject: [PATCH 4/5] Fix outdated test --- crates/noirc_frontend/src/parser/parser.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 451cfb29c44..1ceb71b0617 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -974,7 +974,7 @@ mod test { let valid = vec!["[0;7]", "[(1, 2); 4]", "[0;Four]", "[2;1+3-a]"]; parse_all(array_expr(expression()), valid); - let invalid = vec!["[0;;4]", "[1; foo()]", "[1, 2; 3]"]; + let invalid = vec!["[0;;4]", "[1, 2; 3]"]; parse_all_failing(array_expr(expression()), invalid); } From ae6192b3eb3d3439a272fe3896fcbb5428de6ce5 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 8 Nov 2022 15:32:46 -0500 Subject: [PATCH 5/5] Remove get_fixed_variable_array_length function --- .../src/hir/resolution/resolver.rs | 35 ++++++------------- 1 file changed, 11 insertions(+), 24 deletions(-) diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index 6028fea4f19..f5251bacfe3 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -342,31 +342,18 @@ impl<'a> Resolver<'a> { return Type::Error; } - if let Some(rhs_expr_id) = definition_info.rhs { - let length = self.get_fixed_variable_array_length(&rhs_expr_id); - Type::ArrayLength(length) + let error = if let Some(rhs_expr_id) = definition_info.rhs { + match self.try_eval_array_length_id(rhs_expr_id).map(|length| length.try_into()) { + Ok(Ok(length)) => return Type::ArrayLength(length), + Ok(Err(_cast_err)) => ResolverError::IntegerTooLarge { span: path.span() }, + Err(Some(error)) => error, + Err(None) => return Type::Error, + } } else { - self.push_err(ResolverError::MissingRhsExpr { - name: path.as_string(), - span: path.span(), - }); - Type::Error - } - } - - fn get_fixed_variable_array_length(&self, expr_id: &ExprId) -> u64 { - let expr = self.interner.expression(expr_id); - match expr { - HirExpression::Literal(literal) => match literal { - HirLiteral::Integer(field_element) => { - field_element.try_to_u64().expect("field element does not fit into u128") - } - _ => { - panic!("literal used in fixed variable array length must be an integer literal") - } - }, - _ => panic!("expression in global statement is not a literal"), - } + ResolverError::MissingRhsExpr { name: path.as_string(), span: path.span() } + }; + self.push_err(error); + Type::Error } fn get_ident_from_path(&mut self, path: Path) -> HirIdent {