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 4df7f90e512..9685b79c030 100644 --- a/crates/nargo/tests/test_data/global_consts/src/main.nr +++ b/crates/nargo/tests/test_data/global_consts/src/main.nr @@ -44,11 +44,14 @@ fn main(a: [Field; M], b: [Field; M], c : pub [Field; foo::MAGIC_NUMBER], d: [Fi let add_from_bar_N = mysubmodule::N + foo::bar::from_bar(1); constrain 15 == add_from_bar_N; + // Example showing an array filled with (mysubmodule::N + 2) 0's + let sugared = [0; mysubmodule::N + 2]; + constrain sugared[mysubmodule::N + 1] == 0; + let arr: [Field; mysubmodule::N] = [N; 10]; constrain (arr[0] == 5) & (arr[9] == 5); foo::from_foo(d); - baz::from_baz(c); } diff --git a/crates/noirc_evaluator/src/interpreter.rs b/crates/noirc_evaluator/src/interpreter.rs index 143312fedfa..278392be512 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 e513fc26208..d233d281bbd 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), })) } @@ -355,9 +359,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 +448,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/hir/resolution/errors.rs b/crates/noirc_frontend/src/hir/resolution/errors.rs index c4c358bfd48..8c05c2e2a8c 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 { ExpectedComptimeVariable { 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 324fb2c1736..f5251bacfe3 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}; @@ -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 { @@ -549,10 +536,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 +838,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 8c49f52d18b..5ac042f13a2 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 7d411bf1c3e..f8ee984cee2 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 ab0420d5311..1ceb71b0617 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, @@ -682,22 +680,10 @@ 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])) - }) + .map(|(lhs, count)| ExpressionKind::repeated_array(lhs, count)) } fn expression_list

(expr_parser: P) -> impl NoirParser> @@ -827,16 +813,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}; @@ -970,17 +946,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( @@ -991,10 +971,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, 2; 3]"]; parse_all_failing(array_expr(expression()), invalid); }