From 6951dfb07174f6386bab60939b449f9f5aec98d3 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 6 Mar 2024 17:05:58 -0500 Subject: [PATCH 01/28] wip separating out array and slice types in the AST --- .../noirc_evaluator/src/ssa/ssa_gen/mod.rs | 14 ++- compiler/noirc_frontend/src/ast/expression.rs | 19 ++++ compiler/noirc_frontend/src/ast/mod.rs | 9 +- .../src/hir/resolution/resolver.rs | 66 ++++++------ .../src/hir/type_check/errors.rs | 5 +- .../noirc_frontend/src/hir/type_check/expr.rs | 101 +++++++++++------- compiler/noirc_frontend/src/hir_def/expr.rs | 1 + compiler/noirc_frontend/src/hir_def/types.rs | 91 +++++++--------- .../src/monomorphization/ast.rs | 1 + .../src/monomorphization/mod.rs | 35 ++++-- .../src/monomorphization/printer.rs | 5 + compiler/noirc_frontend/src/node_interner.rs | 3 +- compiler/noirc_frontend/src/parser/parser.rs | 67 +++++++++++- .../noirc_frontend/src/parser/parser/types.rs | 13 ++- compiler/noirc_printable_type/src/lib.rs | 38 +++++-- tooling/nargo/src/artifacts/debug_vars.rs | 18 ++-- tooling/nargo_fmt/src/rewrite/array.rs | 9 +- tooling/nargo_fmt/src/rewrite/expr.rs | 11 +- tooling/nargo_fmt/src/rewrite/typ.rs | 12 +-- tooling/noirc_abi/src/lib.rs | 2 +- 20 files changed, 353 insertions(+), 167 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index d95295ae3c9..39013fa6e12 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -194,6 +194,18 @@ impl<'a> FunctionContext<'a> { ast::Type::Array(_, _) => { self.codegen_array_checked(elements, typ[0].clone())? } + _ => unreachable!( + "ICE: unexpected array literal type, got {}", + array.typ + ), + }) + } + ast::Literal::Slice(array) => { + let elements = + try_vecmap(&array.contents, |element| self.codegen_expression(element))?; + + let typ = Self::convert_type(&array.typ).flatten(); + Ok(match array.typ { ast::Type::Slice(_) => { let slice_length = self.builder.length_constant(array.contents.len() as u128); @@ -202,7 +214,7 @@ impl<'a> FunctionContext<'a> { Tree::Branch(vec![slice_length.into(), slice_contents]) } _ => unreachable!( - "ICE: array literal type must be an array or a slice, but got {}", + "ICE: unexpected slice literal type, got {}", array.typ ), }) diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index 2a252633a29..4110287c480 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -70,6 +70,17 @@ impl ExpressionKind { })) } + pub fn slice(contents: Vec) -> ExpressionKind { + ExpressionKind::Literal(Literal::Slice(ArrayLiteral::Standard(contents))) + } + + pub fn repeated_slice(repeated_element: Expression, length: Expression) -> ExpressionKind { + ExpressionKind::Literal(Literal::Slice(ArrayLiteral::Repeated { + repeated_element: Box::new(repeated_element), + length: Box::new(length), + })) + } + pub fn integer(contents: FieldElement) -> ExpressionKind { ExpressionKind::Literal(Literal::Integer(contents, false)) } @@ -319,6 +330,7 @@ impl UnaryOp { #[derive(Debug, PartialEq, Eq, Clone)] pub enum Literal { Array(ArrayLiteral), + Slice(ArrayLiteral), Bool(bool), Integer(FieldElement, /*sign*/ bool), // false for positive integer and true for negative Str(String), @@ -515,6 +527,13 @@ impl Display for Literal { Literal::Array(ArrayLiteral::Repeated { repeated_element, length }) => { write!(f, "[{repeated_element}; {length}]") } + Literal::Slice(ArrayLiteral::Standard(elements)) => { + let contents = vecmap(elements, ToString::to_string); + write!(f, "&[{}]", contents.join(", ")) + } + Literal::Slice(ArrayLiteral::Repeated { repeated_element, length }) => { + write!(f, "&[{repeated_element}; {length}]") + } Literal::Bool(boolean) => write!(f, "{}", if *boolean { "true" } else { "false" }), Literal::Integer(integer, sign) => { if *sign { diff --git a/compiler/noirc_frontend/src/ast/mod.rs b/compiler/noirc_frontend/src/ast/mod.rs index 29edbaca594..4406fdfb5df 100644 --- a/compiler/noirc_frontend/src/ast/mod.rs +++ b/compiler/noirc_frontend/src/ast/mod.rs @@ -83,7 +83,8 @@ impl core::fmt::Display for IntegerBitSize { #[derive(Debug, PartialEq, Eq, Clone, Hash)] pub enum UnresolvedTypeData { FieldElement, - Array(Option, Box), // [4]Witness = Array(4, Witness) + Array(UnresolvedTypeExpression, Box), // [4]Witness = Array(4, Witness) + Slice(Box), Integer(Signedness, IntegerBitSize), // u32 = Integer(unsigned, ThirtyTwo) Bool, Expression(UnresolvedTypeExpression), @@ -151,10 +152,8 @@ impl std::fmt::Display for UnresolvedTypeData { use UnresolvedTypeData::*; match self { FieldElement => write!(f, "Field"), - Array(len, typ) => match len { - None => write!(f, "[{typ}]"), - Some(len) => write!(f, "[{typ}; {len}]"), - }, + Array(len, typ) => write!(f, "[{typ}; {len}]"), + Slice(typ) => write!(f, "[{typ}]"), Integer(sign, num_bits) => match sign { Signedness::Signed => write!(f, "i{num_bits}"), Signedness::Unsigned => write!(f, "u{num_bits}"), diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 7f9e48353a7..96fcc1a43d4 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -468,13 +468,13 @@ impl<'a> Resolver<'a> { FieldElement => Type::FieldElement, Array(size, elem) => { let elem = Box::new(self.resolve_type_inner(*elem, new_variables)); - let size = if size.is_none() { - Type::NotConstant - } else { - self.resolve_array_size(size, new_variables) - }; + let size = self.resolve_array_size(Some(size), new_variables); Type::Array(Box::new(size), elem) } + Slice(elem) => { + let elem = Box::new(self.resolve_type_inner(*elem, new_variables)); + Type::Slice(elem) + } Expression(expr) => self.convert_expression_type(expr), Integer(sign, bits) => Type::Integer(sign, bits), Bool => Type::Bool, @@ -1090,7 +1090,6 @@ impl<'a> Resolver<'a> { | Type::TypeVariable(_, _) | Type::Constant(_) | Type::NamedGeneric(_, _) - | Type::NotConstant | Type::TraitAsType(..) | Type::Forall(_, _) => (), @@ -1101,6 +1100,10 @@ impl<'a> Resolver<'a> { Self::find_numeric_generics_in_type(element_type, found); } + Type::Slice(element_type) => { + Self::find_numeric_generics_in_type(element_type, found); + } + Type::Tuple(fields) => { for field in fields { Self::find_numeric_generics_in_type(field, found); @@ -1392,28 +1395,35 @@ impl<'a> Resolver<'a> { } } + fn resolve_array_literal(&mut self, array_literal: ArrayLiteral) -> HirArrayLiteral { + match array_literal { + ArrayLiteral::Standard(elements) => { + let elements = vecmap(elements, |elem| self.resolve_expression(elem)); + HirArrayLiteral::Standard(elements) + }, + ArrayLiteral::Repeated { repeated_element, length } => { + let span = length.span; + let length = UnresolvedTypeExpression::from_expr(*length, span).unwrap_or_else( + |error| { + self.errors.push(ResolverError::ParserError(Box::new(error))); + UnresolvedTypeExpression::Constant(0, span) + }, + ); + + let length = self.convert_expression_type(length); + let repeated_element = self.resolve_expression(*repeated_element); + + HirArrayLiteral::Repeated { repeated_element, length } + }, + } + } + pub fn resolve_expression(&mut self, expr: Expression) -> ExprId { let hir_expr = match expr.kind { ExpressionKind::Literal(literal) => HirExpression::Literal(match literal { Literal::Bool(b) => HirLiteral::Bool(b), - Literal::Array(ArrayLiteral::Standard(elements)) => { - let elements = vecmap(elements, |elem| self.resolve_expression(elem)); - HirLiteral::Array(HirArrayLiteral::Standard(elements)) - } - Literal::Array(ArrayLiteral::Repeated { repeated_element, length }) => { - let span = length.span; - let length = UnresolvedTypeExpression::from_expr(*length, span).unwrap_or_else( - |error| { - self.errors.push(ResolverError::ParserError(Box::new(error))); - UnresolvedTypeExpression::Constant(0, span) - }, - ); - - let length = self.convert_expression_type(length); - let repeated_element = self.resolve_expression(*repeated_element); - - HirLiteral::Array(HirArrayLiteral::Repeated { repeated_element, length }) - } + Literal::Array(array_literal) => HirLiteral::Array(self.resolve_array_literal(array_literal)), + Literal::Slice(array_literal) => HirLiteral::Slice(self.resolve_array_literal(array_literal)), Literal::Integer(integer, sign) => HirLiteral::Integer(integer, sign), Literal::Str(str) => HirLiteral::Str(str), Literal::RawStr(str, _) => HirLiteral::Str(str), @@ -2015,18 +2025,14 @@ impl<'a> Resolver<'a> { | UnresolvedTypeData::Function(_, _, _) | UnresolvedTypeData::FormatString(_, _) | UnresolvedTypeData::TraitAsType(..) + | UnresolvedTypeData::Slice(_) | UnresolvedTypeData::Unspecified => { let span = typ.span.expect("Function parameters should always have spans"); self.push_err(ResolverError::InvalidTypeForEntryPoint { span }); } UnresolvedTypeData::Array(length, element) => { - if let Some(length) = length { - self.verify_type_expression_valid_for_program_input(length); - } else { - let span = typ.span.expect("Function parameters should always have spans"); - self.push_err(ResolverError::InvalidTypeForEntryPoint { span }); - } + self.verify_type_expression_valid_for_program_input(length); self.verify_type_valid_for_program_input(element); } UnresolvedTypeData::Expression(expression) => { diff --git a/compiler/noirc_frontend/src/hir/type_check/errors.rs b/compiler/noirc_frontend/src/hir/type_check/errors.rs index 96d30100d8b..c1eabc76b5c 100644 --- a/compiler/noirc_frontend/src/hir/type_check/errors.rs +++ b/compiler/noirc_frontend/src/hir/type_check/errors.rs @@ -122,6 +122,8 @@ pub enum TypeCheckError { ConstrainedReferenceToUnconstrained { span: Span }, #[error("Slices cannot be returned from an unconstrained runtime to a constrained runtime")] UnconstrainedSliceReturnToConstrained { span: Span }, + #[error("Slices must have constant length")] + NonConstantSliceLength { span: Span }, } impl TypeCheckError { @@ -211,7 +213,8 @@ impl From for Diagnostic { | TypeCheckError::OverflowingAssignment { span, .. } | TypeCheckError::FieldModulo { span } | TypeCheckError::ConstrainedReferenceToUnconstrained { span } - | TypeCheckError::UnconstrainedSliceReturnToConstrained { span } => { + | TypeCheckError::UnconstrainedSliceReturnToConstrained { span } + | TypeCheckError::NonConstantSliceLength { span } => { Diagnostic::simple_error(error.to_string(), String::new(), span) } TypeCheckError::PublicReturnType { typ, span } => Diagnostic::simple_error( diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 7b854e58fca..900571cd972 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -48,6 +48,51 @@ impl<'interner> TypeChecker<'interner> { false } + fn check_hir_array_literal(&mut self, hir_array_literal: HirArrayLiteral) -> (Result, u64>, Box) { + match hir_array_literal { + HirArrayLiteral::Standard(arr) => { + let elem_types = vecmap(&arr, |arg| self.check_expression(arg)); + + let first_elem_type = elem_types + .first() + .cloned() + .unwrap_or_else(|| self.interner.next_type_variable()); + + // Check if the array is homogeneous + for (index, elem_type) in elem_types.iter().enumerate().skip(1) { + let location = self.interner.expr_location(&arr[index]); + + elem_type.unify(&first_elem_type, &mut self.errors, || { + TypeCheckError::NonHomogeneousArray { + first_span: self.interner.expr_location(&arr[0]).span, + first_type: first_elem_type.to_string(), + first_index: index, + second_span: location.span, + second_type: elem_type.to_string(), + second_index: index + 1, + } + .add_context("elements in an array must have the same type") + }); + } + + ( + Err(arr.len() as u64), + Box::new(first_elem_type.clone()) + ) + }, + HirArrayLiteral::Repeated { repeated_element, length } => { + let elem_type = self.check_expression(&repeated_element); + let length = match length { + Type::Constant(length) => { + Err(length) + } + other => Ok(Box::new(other)), + }; + (length, Box::new(elem_type)) + }, + } + } + /// Infers a type for a given expression, and return this type. /// As a side-effect, this function will also remember this type in the NodeInterner /// for the given expr_id key. @@ -61,48 +106,22 @@ impl<'interner> TypeChecker<'interner> { HirExpression::Ident(ident) => self.check_ident(ident, expr_id), HirExpression::Literal(literal) => { match literal { - HirLiteral::Array(HirArrayLiteral::Standard(arr)) => { - let elem_types = vecmap(&arr, |arg| self.check_expression(arg)); - - let first_elem_type = elem_types - .first() - .cloned() - .unwrap_or_else(|| self.interner.next_type_variable()); - - let arr_type = Type::Array( - Box::new(Type::constant_variable(arr.len() as u64, self.interner)), - 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 = self.interner.expr_location(&arr[index]); - - elem_type.unify(&first_elem_type, &mut self.errors, || { - TypeCheckError::NonHomogeneousArray { - first_span: self.interner.expr_location(&arr[0]).span, - first_type: first_elem_type.to_string(), - first_index: index, - second_span: location.span, - second_type: elem_type.to_string(), - second_index: index + 1, - } - .add_context("elements in an array must have the same type") - }); + HirLiteral::Array(hir_array_literal) => { + let (length, elem_type) = self.check_hir_array_literal(hir_array_literal); + Type::Array(elem_type, length.unwrap_or_else(|constant| Box::new(Type::constant_variable(constant, self.interner)))) + }, + HirLiteral::Slice(hir_array_literal) => { + let (length_type, elem_type) = self.check_hir_array_literal(hir_array_literal); + match length_type { + Ok(_non_constant) => { + self.errors.push(TypeCheckError::NonConstantSliceLength { + span: self.interner.expr_span(expr_id), + }); + Type::Error + }, + Err(_length) => Type::Slice(elem_type), } - - arr_type - } - HirLiteral::Array(HirArrayLiteral::Repeated { repeated_element, length }) => { - let elem_type = self.check_expression(&repeated_element); - let length = match length { - Type::Constant(length) => { - Type::constant_variable(length, self.interner) - } - other => other, - }; - Type::Array(Box::new(length), Box::new(elem_type)) - } + }, HirLiteral::Bool(_) => Type::Bool, HirLiteral::Integer(_, _) => Type::polymorphic_integer_or_field(self.interner), HirLiteral::Str(string) => { diff --git a/compiler/noirc_frontend/src/hir_def/expr.rs b/compiler/noirc_frontend/src/hir_def/expr.rs index b4c590de491..6a1b4385f0d 100644 --- a/compiler/noirc_frontend/src/hir_def/expr.rs +++ b/compiler/noirc_frontend/src/hir_def/expr.rs @@ -99,6 +99,7 @@ impl HirBinaryOp { #[derive(Debug, Clone)] pub enum HirLiteral { Array(HirArrayLiteral), + Slice(HirArrayLiteral), Bool(bool), Integer(FieldElement, bool), //true for negative integer and false for positive Str(String), diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index b70aa43701c..6e6b24296cb 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -27,6 +27,9 @@ pub enum Type { /// is either a type variable of some kind or a Type::Constant. Array(Box, Box), + /// Slice(E) is a slice of elements of type E. + Slice(Box), + /// A primitive integer type with the given sign and bit count. /// E.g. `u32` would be `Integer(Unsigned, ThirtyTwo)` Integer(Signedness, IntegerBitSize), @@ -98,11 +101,6 @@ pub enum Type { /// bind to an integer without special checks to bind it to a non-type. Constant(u64), - /// The type of a slice is an array of size NotConstant. - /// The size of an array literal is resolved to this if it ever uses operations - /// involving slices. - NotConstant, - /// The result of some type error. Remembering type errors as their own type variant lets /// us avoid issuing repeat type errors for the same item. For example, a lambda with /// an invalid type would otherwise issue a new error each time it is called @@ -150,19 +148,16 @@ impl Type { | Type::MutableReference(_) | Type::Forall(_, _) | Type::Constant(_) - | Type::NotConstant + | Type::Slice(_) | Type::Error => unreachable!("This type cannot exist as a parameter to main"), } } pub(crate) fn is_nested_slice(&self) -> bool { match self { - Type::Array(size, elem) => { - if let Type::NotConstant = size.as_ref() { - elem.as_ref().contains_slice() - } else { - false - } + Type::Slice(_) => true, + Type::Array(_, elem) => { + elem.as_ref().contains_slice() } _ => false, } @@ -170,7 +165,7 @@ impl Type { pub(crate) fn contains_slice(&self) -> bool { match self { - Type::Array(size, _) => matches!(size.as_ref(), Type::NotConstant), + Type::Slice(_) => true, Type::Struct(struct_typ, generics) => { let fields = struct_typ.borrow().get_fields(generics); for field in fields.iter() { @@ -457,7 +452,7 @@ pub enum TypeVariableKind { /// that can only be bound to Type::Integer, or other polymorphic integers. Integer, - /// A potentially constant array size. This will only bind to itself, Type::NotConstant, or + /// A potentially constant array size. This will only bind to itself or /// Type::Constant(n) with a matching size. This defaults to Type::Constant(n) if still unbound /// during monomorphization. Constant(u64), @@ -635,14 +630,15 @@ impl Type { | Type::TypeVariable(_, _) | Type::Constant(_) | Type::NamedGeneric(_, _) - | Type::NotConstant | Type::Forall(_, _) | Type::TraitAsType(..) => false, Type::Array(length, elem) => { elem.contains_numeric_typevar(target_id) || named_generic_id_matches_target(length) } - + Type::Slice(elem) => { + elem.contains_numeric_typevar(target_id) + } Type::Tuple(fields) => { fields.iter().any(|field| field.contains_numeric_typevar(target_id)) } @@ -700,8 +696,8 @@ impl Type { | Type::Function(_, _, _) | Type::MutableReference(_) | Type::Forall(_, _) - | Type::TraitAsType(..) - | Type::NotConstant => false, + | Type::Slice(_) + | Type::TraitAsType(..) => false, // This function is called during name resolution before we've verified aliases // are not cyclic. As a result, it wouldn't be safe to check this alias' definition @@ -770,11 +766,10 @@ impl std::fmt::Display for Type { write!(f, "Field") } Type::Array(len, typ) => { - if matches!(len.follow_bindings(), Type::NotConstant) { - write!(f, "[{typ}]") - } else { - write!(f, "[{typ}; {len}]") - } + write!(f, "[{typ}; {len}]") + } + Type::Slice(typ) => { + write!(f, "[{typ}]") } Type::Integer(sign, num_bits) => match sign { Signedness::Signed => write!(f, "i{num_bits}"), @@ -864,7 +859,6 @@ impl std::fmt::Display for Type { Type::MutableReference(element) => { write!(f, "&mut {element}") } - Type::NotConstant => write!(f, "_"), } } } @@ -920,10 +914,6 @@ impl Type { bindings.insert(target_id, (var.clone(), this)); Ok(()) } - Type::NotConstant => { - bindings.insert(target_id, (var.clone(), Type::NotConstant)); - Ok(()) - } // A TypeVariable is less specific than a MaybeConstant, so we bind // to the other type variable instead. Type::TypeVariable(new_var, kind) => { @@ -951,14 +941,8 @@ impl Type { bindings.insert(*new_target_id, (new_var.clone(), clone)); Ok(()) } - // The lengths don't match, but neither are set in stone so we can - // just set them both to NotConstant. See issue 2370 - TypeVariableKind::Constant(_) => { - // *length != target_length - bindings.insert(target_id, (var.clone(), Type::NotConstant)); - bindings.insert(*new_target_id, (new_var.clone(), Type::NotConstant)); - Ok(()) - } + // *length != target_length + TypeVariableKind::Constant(_) => Err(UnificationError), TypeVariableKind::IntegerOrField => Err(UnificationError), TypeVariableKind::Integer => Err(UnificationError), }, @@ -1312,12 +1296,11 @@ impl Type { let this = self.follow_bindings(); let target = target.follow_bindings(); - if let (Type::Array(size1, element1), Type::Array(size2, element2)) = (&this, &target) { - let size1 = size1.follow_bindings(); - let size2 = size2.follow_bindings(); + if let (Type::Array(size, element1), Type::Slice(element2)) = (&this, &target) { + let length = size.follow_bindings(); // If we have an array and our target is a slice - if matches!(size1, Type::Constant(_)) && matches!(size2, Type::NotConstant) { + if matches!(length, Type::Constant(_)) { // Still have to ensure the element types match. // Don't need to issue an error here if not, it will be done in unify_with_coercions let mut bindings = TypeBindings::new(); @@ -1493,6 +1476,10 @@ impl Type { let element = element.substitute_helper(type_bindings, substitute_bound_typevars); Type::Array(Box::new(size), Box::new(element)) } + Type::Slice(element) => { + let element = element.substitute_helper(type_bindings, substitute_bound_typevars); + Type::Slice(Box::new(element)) + } Type::String(size) => { let size = size.substitute_helper(type_bindings, substitute_bound_typevars); Type::String(Box::new(size)) @@ -1552,7 +1539,6 @@ impl Type { | Type::Constant(_) | Type::TraitAsType(..) | Type::Error - | Type::NotConstant | Type::Unit => self.clone(), } } @@ -1561,6 +1547,7 @@ impl Type { pub fn occurs(&self, target_id: TypeVariableId) -> bool { match self { Type::Array(len, elem) => len.occurs(target_id) || elem.occurs(target_id), + Type::Slice(elem) => elem.occurs(target_id), Type::String(len) => len.occurs(target_id), Type::FmtString(len, fields) => { let len_occurs = len.occurs(target_id); @@ -1593,7 +1580,6 @@ impl Type { | Type::Constant(_) | Type::TraitAsType(..) | Type::Error - | Type::NotConstant | Type::Unit => false, } } @@ -1610,6 +1596,9 @@ impl Type { Array(size, elem) => { Array(Box::new(size.follow_bindings()), Box::new(elem.follow_bindings())) } + Slice(elem) => { + Slice(Box::new(elem.follow_bindings())) + } String(size) => String(Box::new(size.follow_bindings())), FmtString(size, args) => { let size = Box::new(size.follow_bindings()); @@ -1650,8 +1639,7 @@ impl Type { | Bool | Constant(_) | Unit - | Error - | NotConstant => self.clone(), + | Error => self.clone(), } } @@ -1731,6 +1719,10 @@ impl From<&Type> for PrintableType { let typ = typ.as_ref(); PrintableType::Array { length, typ: Box::new(typ.into()) } } + Type::Slice(typ) => { + let typ = typ.as_ref(); + PrintableType::Slice{ typ: Box::new(typ.into()) } + } Type::Integer(sign, bit_width) => match sign { Signedness::Unsigned => { PrintableType::UnsignedInteger { width: (*bit_width).into() } @@ -1774,7 +1766,6 @@ impl From<&Type> for PrintableType { Type::MutableReference(typ) => { PrintableType::MutableReference { typ: Box::new(typ.as_ref().into()) } } - Type::NotConstant => unreachable!(), } } } @@ -1786,11 +1777,10 @@ impl std::fmt::Debug for Type { write!(f, "Field") } Type::Array(len, typ) => { - if matches!(len.follow_bindings(), Type::NotConstant) { - write!(f, "[{typ:?}]") - } else { - write!(f, "[{typ:?}; {len:?}]") - } + write!(f, "[{typ:?}; {len:?}]") + } + Type::Slice(typ) => { + write!(f, "[{typ:?}]") } Type::Integer(sign, num_bits) => match sign { Signedness::Signed => write!(f, "i{num_bits}"), @@ -1860,7 +1850,6 @@ impl std::fmt::Debug for Type { Type::MutableReference(element) => { write!(f, "&mut {element:?}") } - Type::NotConstant => write!(f, "NotConstant"), } } } diff --git a/compiler/noirc_frontend/src/monomorphization/ast.rs b/compiler/noirc_frontend/src/monomorphization/ast.rs index e4e619d5d92..978430bc068 100644 --- a/compiler/noirc_frontend/src/monomorphization/ast.rs +++ b/compiler/noirc_frontend/src/monomorphization/ast.rs @@ -87,6 +87,7 @@ pub struct For { #[derive(Debug, Clone, Hash)] pub enum Literal { Array(ArrayLiteral), + Slice(ArrayLiteral), Integer(FieldElement, Type, Location), Bool(bool), Str(String), diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index ce880401d77..a62a041ecd3 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -417,9 +417,15 @@ impl<'interner> Monomorphizer<'interner> { } } HirExpression::Literal(HirLiteral::Array(array)) => match array { - HirArrayLiteral::Standard(array) => self.standard_array(expr, array)?, + HirArrayLiteral::Standard(array) => self.standard_array(expr, array, false)?, HirArrayLiteral::Repeated { repeated_element, length } => { - self.repeated_array(expr, repeated_element, length)? + self.repeated_array(expr, repeated_element, length, false)? + } + }, + HirExpression::Literal(HirLiteral::Slice(array)) => match array { + HirArrayLiteral::Standard(array) => self.standard_array(expr, array, true)?, + HirArrayLiteral::Repeated { repeated_element, length } => { + self.repeated_array(expr, repeated_element, length, true)? } }, HirExpression::Literal(HirLiteral::Unit) => ast::Expression::Block(vec![]), @@ -518,10 +524,15 @@ impl<'interner> Monomorphizer<'interner> { &mut self, array: node_interner::ExprId, array_elements: Vec, + is_slice: bool, ) -> Result { let typ = self.convert_type(&self.interner.id_type(array)); let contents = try_vecmap(array_elements, |id| self.expr(id))?; - Ok(ast::Expression::Literal(ast::Literal::Array(ast::ArrayLiteral { contents, typ }))) + if is_slice { + Ok(ast::Expression::Literal(ast::Literal::Slice(ast::ArrayLiteral { contents, typ }))) + } else { + Ok(ast::Expression::Literal(ast::Literal::Array(ast::ArrayLiteral { contents, typ }))) + } } fn repeated_array( @@ -529,6 +540,7 @@ impl<'interner> Monomorphizer<'interner> { array: node_interner::ExprId, repeated_element: node_interner::ExprId, length: HirType, + is_slice: bool, ) -> Result { let typ = self.convert_type(&self.interner.id_type(array)); @@ -538,7 +550,11 @@ impl<'interner> Monomorphizer<'interner> { })?; let contents = try_vecmap(0..length, |_| self.expr(repeated_element))?; - Ok(ast::Expression::Literal(ast::Literal::Array(ast::ArrayLiteral { contents, typ }))) + if is_slice { + Ok(ast::Expression::Literal(ast::Literal::Slice(ast::ArrayLiteral { contents, typ }))) + } else { + Ok(ast::Expression::Literal(ast::Literal::Array(ast::ArrayLiteral { contents, typ }))) + } } fn index( @@ -858,6 +874,10 @@ impl<'interner> Monomorphizer<'interner> { ast::Type::Slice(element) } } + HirType::Slice(element) => { + let element = Box::new(self.convert_type(element.as_ref())); + ast::Type::Slice(element) + } HirType::TraitAsType(..) => { unreachable!("All TraitAsType should be replaced before calling convert_type"); } @@ -933,7 +953,6 @@ impl<'interner> Monomorphizer<'interner> { HirType::Forall(_, _) | HirType::Constant(_) - | HirType::NotConstant | HirType::Error => { unreachable!("Unexpected type {} found", typ) } @@ -1149,11 +1168,7 @@ impl<'interner> Monomorphizer<'interner> { fn append_printable_type_info_inner(typ: &Type, arguments: &mut Vec) { // Disallow printing slices and mutable references for consistency, // since they cannot be passed from ACIR into Brillig - if let HirType::Array(size, _) = typ { - if let HirType::NotConstant = **size { - unreachable!("println and format strings do not support slices. Convert the slice to an array before passing it to println"); - } - } else if matches!(typ, HirType::MutableReference(_)) { + if matches!(typ, HirType::MutableReference(_)) { unreachable!("println and format strings do not support mutable references."); } diff --git a/compiler/noirc_frontend/src/monomorphization/printer.rs b/compiler/noirc_frontend/src/monomorphization/printer.rs index 7aec2193494..2531dc6574e 100644 --- a/compiler/noirc_frontend/src/monomorphization/printer.rs +++ b/compiler/noirc_frontend/src/monomorphization/printer.rs @@ -95,6 +95,11 @@ impl AstPrinter { self.print_comma_separated(&array.contents, f)?; write!(f, "]") } + super::ast::Literal::Slice(array) => { + write!(f, "&[")?; + self.print_comma_separated(&array.contents, f)?; + write!(f, "]") + } super::ast::Literal::Integer(x, _, _) => x.fmt(f), super::ast::Literal::Bool(x) => x.fmt(f), super::ast::Literal::Str(s) => s.fmt(f), diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 5de43e59254..1ee8610ccf2 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -1681,6 +1681,7 @@ enum TypeMethodKey { /// accept only fields or integers, it is just that their names may not clash. FieldOrInt, Array, + Slice, Bool, String, FmtString, @@ -1696,6 +1697,7 @@ fn get_type_method_key(typ: &Type) -> Option { match &typ { Type::FieldElement => Some(FieldOrInt), Type::Array(_, _) => Some(Array), + Type::Slice(_) => Some(Slice), Type::Integer(_, _) => Some(FieldOrInt), Type::TypeVariable(_, TypeVariableKind::IntegerOrField) => Some(FieldOrInt), Type::TypeVariable(_, TypeVariableKind::Integer) => Some(FieldOrInt), @@ -1714,7 +1716,6 @@ fn get_type_method_key(typ: &Type) -> Option { | Type::Forall(_, _) | Type::Constant(_) | Type::Error - | Type::NotConstant | Type::Struct(_, _) | Type::TraitAsType(..) => None, } diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 75f4a6359bf..ec6d613b85f 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -563,6 +563,7 @@ fn parse_type_inner( format_string_type(recursive_type_parser.clone()), named_type(recursive_type_parser.clone()), named_trait(recursive_type_parser.clone()), + slice_type(recursive_type_parser.clone()), array_type(recursive_type_parser.clone()), parenthesized_type(recursive_type_parser.clone()), tuple_type(recursive_type_parser.clone()), @@ -699,13 +700,22 @@ fn generic_type_args( fn array_type(type_parser: impl NoirParser) -> impl NoirParser { just(Token::LeftBracket) .ignore_then(type_parser) - .then(just(Token::Semicolon).ignore_then(type_expression()).or_not()) + .then(just(Token::Semicolon).ignore_then(type_expression())) .then_ignore(just(Token::RightBracket)) .map_with_span(|(element_type, size), span| { UnresolvedTypeData::Array(size, Box::new(element_type)).with_span(span) }) } +fn slice_type(type_parser: impl NoirParser) -> impl NoirParser { + just(Token::LeftBracket) + .ignore_then(type_parser) + .then_ignore(just(Token::RightBracket)) + .map_with_span(|element_type, span| { + UnresolvedTypeData::Slice(Box::new(element_type)).with_span(span) + }) +} + fn type_expression() -> impl NoirParser { recursive(|expr| { expression_with_precedence( @@ -1067,6 +1077,14 @@ where .map(|(lhs, count)| ExpressionKind::repeated_array(lhs, count)) } +/// &[array_expr] +fn slice_expr

(expr_parser: P) -> impl NoirParser +where + P: ExprParser, +{ + just(Token::Ampersand).ignore_then(array_expr(expr_parser)) +} + fn expression_list

(expr_parser: P) -> impl NoirParser> where P: ExprParser, @@ -1090,6 +1108,7 @@ where { choice(( if_expr(expr_no_constructors, statement.clone()), + slice_expr(expr_parser.clone()), array_expr(expr_parser.clone()), if allow_constructors { constructor(expr_parser.clone()).boxed() @@ -1168,7 +1187,7 @@ where mod test { use super::test_helpers::*; use super::*; - use crate::{ArrayLiteral, Literal}; + use crate::{ArrayLiteral, SliceLiteral, Literal}; #[test] fn parse_infix() { @@ -1281,6 +1300,50 @@ mod test { parse_all_failing(array_expr(expression()), invalid); } + fn expr_to_slice(expr: ExpressionKind) -> SliceLiteral { + let lit = match expr { + ExpressionKind::Literal(literal) => literal, + _ => unreachable!("expected a literal"), + }; + + match lit { + Literal::Slice(arr) => arr, + _ => unreachable!("expected an slice"), + } + } + + #[test] + fn parse_slice() { + 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(slice_expr(expression()), valid) { + match expr_to_slice(expr) { + SliceLiteral::Standard(elements) => assert_eq!(elements.len(), 5), + SliceLiteral::Repeated { length, .. } => { + assert_eq!(length.kind, ExpressionKind::integer(5i128.into())); + } + } + } + + parse_all_failing( + slice_expr(expression()), + vec!["0,1,2,3,4]", "&[[0,1,2,3,4]", "&[0,1,2,,]", "&[0,1,2,3,4"], + ); + } + + #[test] + fn parse_slice_sugar() { + let valid = vec!["&[0;7]", "&[(1, 2); 4]", "&[0;Four]", "&[2;1+3-a]"]; + parse_all(slice_expr(expression()), valid); + + let invalid = vec!["&[0;;4]", "&[1, 2; 3]"]; + parse_all_failing(slice_expr(expression()), invalid); + } + #[test] fn parse_block() { parse_with(block(fresh_statement()), "{ [0,1,2,3,4] }").unwrap(); diff --git a/compiler/noirc_frontend/src/parser/parser/types.rs b/compiler/noirc_frontend/src/parser/parser/types.rs index 572397d6527..d8a63d161b9 100644 --- a/compiler/noirc_frontend/src/parser/parser/types.rs +++ b/compiler/noirc_frontend/src/parser/parser/types.rs @@ -88,13 +88,24 @@ pub(super) fn array_type( ) -> impl NoirParser { just(Token::LeftBracket) .ignore_then(type_parser) - .then(just(Token::Semicolon).ignore_then(type_expression()).or_not()) + .then(just(Token::Semicolon).ignore_then(type_expression())) .then_ignore(just(Token::RightBracket)) .map_with_span(|(element_type, size), span| { UnresolvedTypeData::Array(size, Box::new(element_type)).with_span(span) }) } +pub(super) fn slice_type( + type_parser: impl NoirParser, +) -> impl NoirParser { + just(Token::LeftBracket) + .ignore_then(type_parser) + .then_ignore(just(Token::RightBracket)) + .map_with_span(|element_type, span| { + UnresolvedTypeData::Slice(Box::new(element_type)).with_span(span) + }) +} + pub(super) fn type_expression() -> impl NoirParser { recursive(|expr| { expression_with_precedence( diff --git a/compiler/noirc_printable_type/src/lib.rs b/compiler/noirc_printable_type/src/lib.rs index 24f4f275a14..9abc993606b 100644 --- a/compiler/noirc_printable_type/src/lib.rs +++ b/compiler/noirc_printable_type/src/lib.rs @@ -15,6 +15,10 @@ pub enum PrintableType { #[serde(rename = "type")] typ: Box, }, + Slice { + #[serde(rename = "type")] + typ: Box, + }, Tuple { types: Vec, }, @@ -48,7 +52,10 @@ pub enum PrintableType { pub enum PrintableValue { Field(FieldElement), String(String), - Vec(Vec), + Vec { + array_elements: Vec, + is_slice: bool, + }, Struct(BTreeMap), Other, } @@ -182,9 +189,12 @@ fn to_string(value: &PrintableValue, typ: &PrintableType) -> Option { (_, PrintableType::MutableReference { .. }) => { output.push_str("<>"); } - (PrintableValue::Vec(vector), PrintableType::Array { typ, .. }) => { + (PrintableValue::Vec { array_elements, is_slice }, PrintableType::Array { typ, .. }) => { + if *is_slice { + output.push('&') + } output.push('['); - let mut values = vector.iter().peekable(); + let mut values = array_elements.iter().peekable(); while let Some(value) = values.next() { output.push_str(&format!( "{}", @@ -219,9 +229,9 @@ fn to_string(value: &PrintableValue, typ: &PrintableType) -> Option { output.push_str(" }"); } - (PrintableValue::Vec(values), PrintableType::Tuple { types }) => { + (PrintableValue::Vec { array_elements, .. }, PrintableType::Tuple { types }) => { output.push('('); - let mut elems = values.iter().zip(types).peekable(); + let mut elems = array_elements.iter().zip(types).peekable(); while let Some((value, typ)) = elems.next() { output.push_str( &PrintableValueDisplay::Plain(value.clone(), typ.clone()).to_string(), @@ -320,7 +330,7 @@ pub fn decode_value( array_elements.push(decode_value(field_iterator, typ)); } - PrintableValue::Vec(array_elements) + PrintableValue::Vec { array_elements, is_slice: false } } PrintableType::Array { length: Some(length), typ } => { let length = *length as usize; @@ -329,10 +339,22 @@ pub fn decode_value( array_elements.push(decode_value(field_iterator, typ)); } - PrintableValue::Vec(array_elements) + PrintableValue::Vec { array_elements, is_slice: false } + } + PrintableType::Slice { typ } => { + let length = field_iterator + .next() + .expect("not enough data to decode variable array length") + .to_u128() as usize; + let mut array_elements = Vec::with_capacity(length); + for _ in 0..length { + array_elements.push(decode_value(field_iterator, typ)); + } + + PrintableValue::Vec { array_elements, is_slice: true } } PrintableType::Tuple { types } => { - PrintableValue::Vec(vecmap(types, |typ| decode_value(field_iterator, typ))) + PrintableValue::Vec { array_elements: vecmap(types, |typ| decode_value(field_iterator, typ)), is_slice: false } } PrintableType::String { length } => { let field_elements: Vec = field_iterator.take(*length as usize).collect(); diff --git a/tooling/nargo/src/artifacts/debug_vars.rs b/tooling/nargo/src/artifacts/debug_vars.rs index 20f2637f7d6..0921dbd3e7c 100644 --- a/tooling/nargo/src/artifacts/debug_vars.rs +++ b/tooling/nargo/src/artifacts/debug_vars.rs @@ -62,16 +62,21 @@ impl DebugVars { .unwrap_or_else(|| panic!("type unavailable for type id {cursor_type_id:?}")); for index in indexes.iter() { (cursor, cursor_type) = match (cursor, cursor_type) { - (PrintableValue::Vec(array), PrintableType::Array { length, typ }) => { + (PrintableValue::Vec { array_elements, is_slice }, PrintableType::Array { length, typ }) => { + assert!(!*is_slice, "slice has array type"); if let Some(len) = length { if *index as u64 >= *len { panic!("unexpected field index past array length") } - if *len != array.len() as u64 { + if *len != array_elements.len() as u64 { panic!("type/array length mismatch") } } - (array.get_mut(*index as usize).unwrap(), &*Box::leak(typ.clone())) + (array_elements.get_mut(*index as usize).unwrap(), &*Box::leak(typ.clone())) + } + (PrintableValue::Vec { array_elements, is_slice }, PrintableType::Slice { typ }) => { + assert!(*is_slice, "array has slice type"); + (array_elements.get_mut(*index as usize).unwrap(), &*Box::leak(typ.clone())) } ( PrintableValue::Struct(field_map), @@ -83,18 +88,19 @@ impl DebugVars { let (key, typ) = fields.get(*index as usize).unwrap(); (field_map.get_mut(key).unwrap(), typ) } - (PrintableValue::Vec(array), PrintableType::Tuple { types }) => { + (PrintableValue::Vec { array_elements, is_slice }, PrintableType::Tuple { types }) => { + assert!(!*is_slice, "slice has tuple type"); if *index >= types.len() as u32 { panic!( "unexpected field index ({index}) past tuple length ({})", types.len() ); } - if types.len() != array.len() { + if types.len() != array_elements.len() { panic!("type/array length mismatch") } let typ = types.get(*index as usize).unwrap(); - (array.get_mut(*index as usize).unwrap(), typ) + (array_elements.get_mut(*index as usize).unwrap(), typ) } _ => { panic!("unexpected assign field of {cursor_type:?} type"); diff --git a/tooling/nargo_fmt/src/rewrite/array.rs b/tooling/nargo_fmt/src/rewrite/array.rs index 77e5e756f19..4499760c1a4 100644 --- a/tooling/nargo_fmt/src/rewrite/array.rs +++ b/tooling/nargo_fmt/src/rewrite/array.rs @@ -6,7 +6,7 @@ use crate::{ visitor::{expr::NewlineMode, FmtVisitor}, }; -pub(crate) fn rewrite(mut visitor: FmtVisitor, array: Vec, array_span: Span) -> String { +pub(crate) fn rewrite(mut visitor: FmtVisitor, array: Vec, array_span: Span, is_slice: bool) -> String { let pattern: &[_] = &[' ', '\t']; visitor.indent.block_indent(visitor.config); @@ -75,8 +75,13 @@ pub(crate) fn rewrite(mut visitor: FmtVisitor, array: Vec, array_spa } } + let open_bracket = if is_slice { + "&[" + } else { + "[" + }; crate::visitor::expr::wrap_exprs( - "[", + open_bracket, "]", items_str.trim().into(), nested_indent, diff --git a/tooling/nargo_fmt/src/rewrite/expr.rs b/tooling/nargo_fmt/src/rewrite/expr.rs index 32d104f559b..ffb8dea5a9d 100644 --- a/tooling/nargo_fmt/src/rewrite/expr.rs +++ b/tooling/nargo_fmt/src/rewrite/expr.rs @@ -122,7 +122,16 @@ pub(crate) fn rewrite( format!("[{repeated}; {length}]") } Literal::Array(ArrayLiteral::Standard(exprs)) => { - super::array(visitor.fork(), exprs, span) + super::array(visitor.fork(), exprs, span, false) + } + Literal::Slice(ArrayLiteral::Repeated { repeated_element, length }) => { + let repeated = rewrite_sub_expr(visitor, shape, *repeated_element); + let length = rewrite_sub_expr(visitor, shape, *length); + + format!("&[{repeated}; {length}]") + } + Literal::Slice(ArrayLiteral::Standard(exprs)) => { + super::array(visitor.fork(), exprs, span, true) } Literal::Unit => "()".to_string(), }, diff --git a/tooling/nargo_fmt/src/rewrite/typ.rs b/tooling/nargo_fmt/src/rewrite/typ.rs index aaa77b0bea5..922337cdb74 100644 --- a/tooling/nargo_fmt/src/rewrite/typ.rs +++ b/tooling/nargo_fmt/src/rewrite/typ.rs @@ -9,12 +9,12 @@ pub(crate) fn rewrite(visitor: &FmtVisitor, _shape: Shape, typ: UnresolvedType) match typ.typ { UnresolvedTypeData::Array(length, element) => { let typ = rewrite(visitor, _shape, *element); - if let Some(length) = length { - let length = visitor.slice(length.span()); - format!("[{typ}; {length}]") - } else { - format!("[{typ}]") - } + let length = visitor.slice(length.span()); + format!("[{typ}; {length}]") + } + UnresolvedTypeData::Slice(element) => { + let typ = rewrite(visitor, _shape, *element); + format!("[{typ}]") } UnresolvedTypeData::Parenthesized(typ) => { let typ = rewrite(visitor, _shape, *typ); diff --git a/tooling/noirc_abi/src/lib.rs b/tooling/noirc_abi/src/lib.rs index 26feab65d83..26edb7a2af6 100644 --- a/tooling/noirc_abi/src/lib.rs +++ b/tooling/noirc_abi/src/lib.rs @@ -178,7 +178,7 @@ impl AbiType { | Type::TypeVariable(_, _) | Type::NamedGeneric(..) | Type::Forall(..) - | Type::NotConstant + | Type::Slice(_) | Type::Function(_, _, _) => unreachable!("Type cannot be used in the abi"), Type::FmtString(_, _) => unreachable!("format strings cannot be used in the abi"), Type::MutableReference(_) => unreachable!("&mut cannot be used in the abi"), From 2f93bedbfa777607032f702bf1758e53e472204b Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 7 Mar 2024 12:53:01 -0600 Subject: [PATCH 02/28] Update ordering of Type::Array construction in type_check/expr.rs --- compiler/noirc_frontend/src/hir/type_check/expr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 900571cd972..7c143ff433c 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -108,7 +108,7 @@ impl<'interner> TypeChecker<'interner> { match literal { HirLiteral::Array(hir_array_literal) => { let (length, elem_type) = self.check_hir_array_literal(hir_array_literal); - Type::Array(elem_type, length.unwrap_or_else(|constant| Box::new(Type::constant_variable(constant, self.interner)))) + Type::Array(length.unwrap_or_else(|constant| Box::new(Type::constant_variable(constant, self.interner))), elem_type) }, HirLiteral::Slice(hir_array_literal) => { let (length_type, elem_type) = self.check_hir_array_literal(hir_array_literal); From 654732a27392269f9166799280ff4910ce5ba460 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Fri, 8 Mar 2024 13:31:26 -0500 Subject: [PATCH 03/28] wip debugging and updating tests: fix parser and parser tests for ampersand, updating stdlib, fixed nested slice for new type, found missing unification case, added more specific error for missing array length, debugging slice parser, added noirc_frontend test for slices, debugging stdlib failing to build --- .../src/hir/def_collector/dc_crate.rs | 2 +- .../src/hir/def_collector/errors.rs | 4 +-- compiler/noirc_frontend/src/hir_def/types.rs | 6 +++- .../src/monomorphization/mod.rs | 15 ++++---- compiler/noirc_frontend/src/parser/parser.rs | 35 +++++++++++++++---- compiler/noirc_frontend/src/tests.rs | 15 ++++++++ noir_stdlib/src/bigint.nr | 8 ++--- 7 files changed, 64 insertions(+), 21 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 7f36af5b30e..6074095714b 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -136,7 +136,7 @@ pub struct DefCollector { pub(crate) type ImplMap = HashMap<(UnresolvedType, LocalModuleId), Vec<(UnresolvedGenerics, Span, UnresolvedFunctions)>>; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub enum CompilationError { ParseError(ParserError), DefinitionError(DefCollectorErrorKind), diff --git a/compiler/noirc_frontend/src/hir/def_collector/errors.rs b/compiler/noirc_frontend/src/hir/def_collector/errors.rs index de45be48c4e..2ffc78d800e 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/errors.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/errors.rs @@ -23,7 +23,7 @@ pub enum DuplicateType { TraitAssociatedFunction, } -#[derive(Error, Debug, Clone)] +#[derive(Error, Debug, Clone, PartialEq)] pub enum DefCollectorErrorKind { #[error("duplicate {typ} found in namespace")] Duplicate { typ: DuplicateType, first_def: Ident, second_def: Ident }, @@ -78,7 +78,7 @@ pub enum DefCollectorErrorKind { } /// An error struct that macro processors can return. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct MacroError { pub primary_message: String, pub secondary_message: Option, diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 6e6b24296cb..3f7eba598db 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -155,7 +155,7 @@ impl Type { pub(crate) fn is_nested_slice(&self) -> bool { match self { - Type::Slice(_) => true, + Type::Slice(elem) => elem.as_ref().contains_slice(), Type::Array(_, elem) => { elem.as_ref().contains_slice() } @@ -1152,6 +1152,10 @@ impl Type { elem_a.try_unify(elem_b, bindings) } + (Slice(elem_a), Slice(elem_b)) => { + elem_a.try_unify(elem_b, bindings) + } + (String(len_a), String(len_b)) => len_a.try_unify(len_b, bindings), (FmtString(len_a, elements_a), FmtString(len_b, elements_b)) => { diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index a62a041ecd3..5a8af4c699f 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -867,12 +867,11 @@ impl<'interner> Monomorphizer<'interner> { HirType::Unit => ast::Type::Unit, HirType::Array(length, element) => { let element = Box::new(self.convert_type(element.as_ref())); - - if let Some(length) = length.evaluate_to_u64() { - ast::Type::Array(length, element) - } else { - ast::Type::Slice(element) - } + // TODO: convert to MonomorphizationError + let length = length.evaluate_to_u64().unwrap_or_else(|| { + panic!("Length of generic array could not be determined.") + }); + ast::Type::Array(length, element) } HirType::Slice(element) => { let element = Box::new(self.convert_type(element.as_ref())); @@ -1236,6 +1235,10 @@ impl<'interner> Monomorphizer<'interner> { location: Location, ) -> ast::Expression { use ast::*; + + // TODO: remove + println!("modulus_array_literal: {:?}, {:?}, {:?}", bytes, arr_elem_bits, location); + let int_type = Type::Integer(crate::Signedness::Unsigned, arr_elem_bits); let bytes_as_expr = vecmap(bytes, |byte| { diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index ec6d613b85f..30948267384 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -1077,12 +1077,33 @@ where .map(|(lhs, count)| ExpressionKind::repeated_array(lhs, count)) } -/// &[array_expr] fn slice_expr

(expr_parser: P) -> impl NoirParser where P: ExprParser, { - just(Token::Ampersand).ignore_then(array_expr(expr_parser)) + just(Token::Ampersand).ignore_then(standard_slice(expr_parser.clone()).or(slice_sugar(expr_parser))) +} + +/// &[a, b, c, ...] +fn standard_slice

(expr_parser: P) -> impl NoirParser +where + P: ExprParser, +{ + expression_list(expr_parser) + .delimited_by(just(Token::LeftBracket), just(Token::RightBracket)) + .validate(|elements, _span, _emit| ExpressionKind::slice(elements)) +} + +/// &[a; N] +fn slice_sugar

(expr_parser: P) -> impl NoirParser +where + P: ExprParser, +{ + expr_parser + .clone() + .then(just(Token::Semicolon).ignore_then(expr_parser)) + .delimited_by(just(Token::LeftBracket), just(Token::RightBracket)) + .map(|(lhs, count)| ExpressionKind::repeated_slice(lhs, count)) } fn expression_list

(expr_parser: P) -> impl NoirParser> @@ -1187,7 +1208,7 @@ where mod test { use super::test_helpers::*; use super::*; - use crate::{ArrayLiteral, SliceLiteral, Literal}; + use crate::{ArrayLiteral, Literal}; #[test] fn parse_infix() { @@ -1300,7 +1321,7 @@ mod test { parse_all_failing(array_expr(expression()), invalid); } - fn expr_to_slice(expr: ExpressionKind) -> SliceLiteral { + fn expr_to_slice(expr: ExpressionKind) -> ArrayLiteral { let lit = match expr { ExpressionKind::Literal(literal) => literal, _ => unreachable!("expected a literal"), @@ -1308,7 +1329,7 @@ mod test { match lit { Literal::Slice(arr) => arr, - _ => unreachable!("expected an slice"), + _ => unreachable!("expected a slice: {:?}", lit), } } @@ -1322,8 +1343,8 @@ mod test { for expr in parse_all(slice_expr(expression()), valid) { match expr_to_slice(expr) { - SliceLiteral::Standard(elements) => assert_eq!(elements.len(), 5), - SliceLiteral::Repeated { length, .. } => { + ArrayLiteral::Standard(elements) => assert_eq!(elements.len(), 5), + ArrayLiteral::Repeated { length, .. } => { assert_eq!(length.kind, ExpressionKind::integer(5i128.into())); } } diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index c661cc92eef..bf2081355ad 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -904,6 +904,21 @@ mod test { assert!(get_program_errors(src).is_empty()); } + #[test] + fn resolve_literal_slice() { + let src = r#" + fn const_0(_x : [Field]) -> Field { + 0 + } + fn main(x : Field) { + let y: [Field] = &[1, 2, 3]; + assert(const_0(y) == x); + } + "#; + // assert!(get_program_errors(src).is_empty()); + assert!(get_program_errors(src) == vec![], "{:?}", get_program_errors(src)); + } + #[test] fn multiple_resolution_errors() { let src = r#" diff --git a/noir_stdlib/src/bigint.nr b/noir_stdlib/src/bigint.nr index 98237a54779..23fb627e453 100644 --- a/noir_stdlib/src/bigint.nr +++ b/noir_stdlib/src/bigint.nr @@ -9,10 +9,10 @@ global secpk1_fr = [0x41, 0x41, 0x36, 0xD0, 0x8C, 0x5E, 0xD2, 0xBF, 0x3B, 0xA0, 0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF]; global secpk1_fq = [0x2F, 0xFC, 0xFF, 0xFF, 0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF]; -global secpr1_fq = [0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, 0xFF, 0xFF, 0xFF]; -global secpr1_fr = [0x51, 0x25, 0x63, 0xFC, 0xC2, 0xCA, 0xB9, 0xF3, 0x84, 0x9E, 0x17, 0xA7, 0xAD, 0xFA, 0xE6, 0xBC, - 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00,0xFF, 0xFF, 0xFF, 0xFF]; +global secpr1_fq: [u8] = &[0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, 0xFF, 0xFF, 0xFF]; +global secpr1_fr: [u8] = &[0x51, 0x25, 0x63, 0xFC, 0xC2, 0xCA, 0xB9, 0xF3, 0x84, 0x9E, 0x17, 0xA7, 0xAD, 0xFA, 0xE6, 0xBC, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00,0xFF, 0xFF, 0xFF, 0xFF]; struct BigInt { pointer: u32, From 23d3a725017efb37c1f919ddfdd772715a0e36fc Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Fri, 8 Mar 2024 15:09:58 -0500 Subject: [PATCH 04/28] wip debugging: test slice index, got slice index working, slice-specific black box functions, stdlib building --- .../noirc_frontend/src/hir/type_check/expr.rs | 1 + compiler/noirc_frontend/src/tests.rs | 14 ++++- noir_stdlib/src/array.nr | 2 +- noir_stdlib/src/bigint.nr | 22 ++++---- noir_stdlib/src/collections/vec.nr | 2 +- noir_stdlib/src/hash.nr | 51 +++++++++++++++++-- noir_stdlib/src/hash/pedersen.nr | 6 +-- noir_stdlib/src/slice.nr | 3 ++ 8 files changed, 80 insertions(+), 21 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 7c143ff433c..cc7bfa09a13 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -572,6 +572,7 @@ impl<'interner> TypeChecker<'interner> { // XXX: We can check the array bounds here also, but it may be better to constant fold first // and have ConstId instead of ExprId for constants Type::Array(_, base_type) => *base_type, + Type::Slice(base_type) => *base_type, Type::Error => Type::Error, typ => { let span = self.interner.expr_span(&new_lhs); diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index bf2081355ad..5ae566509f9 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -915,8 +915,18 @@ mod test { assert(const_0(y) == x); } "#; - // assert!(get_program_errors(src).is_empty()); - assert!(get_program_errors(src) == vec![], "{:?}", get_program_errors(src)); + assert!(get_program_errors(src).is_empty()); + } + + #[test] + fn index_literal_slice() { + let src = r#" + fn main(x : Field) { + let y: [Field] = &[1, 2, 3]; + assert(y[0] == x); + } + "#; + assert!(get_program_errors(src).is_empty()); } #[test] diff --git a/noir_stdlib/src/array.nr b/noir_stdlib/src/array.nr index 3da4b649174..b5d1572537e 100644 --- a/noir_stdlib/src/array.nr +++ b/noir_stdlib/src/array.nr @@ -54,7 +54,7 @@ impl [T; N] { // Converts an array into a slice. pub fn as_slice(self) -> [T] { - let mut slice = []; + let mut slice = &[]; for elem in self { slice = slice.push_back(elem); } diff --git a/noir_stdlib/src/bigint.nr b/noir_stdlib/src/bigint.nr index 23fb627e453..3afa50e5b6e 100644 --- a/noir_stdlib/src/bigint.nr +++ b/noir_stdlib/src/bigint.nr @@ -1,17 +1,17 @@ use crate::ops::{Add, Sub, Mul, Div}; use crate::cmp::Eq; -global bn254_fq = [0x47, 0xFD, 0x7C, 0xD8, 0x16, 0x8C, 0x20, 0x3C, 0x8d, 0xca, 0x71, 0x68, 0x91, 0x6a, 0x81, 0x97, - 0x5d, 0x58, 0x81, 0x81, 0xb6, 0x45, 0x50, 0xb8, 0x29, 0xa0, 0x31, 0xe1, 0x72, 0x4e, 0x64, 0x30]; -global bn254_fr = [0x01, 0x00, 0x00, 0x00, 0x3F, 0x59, 0x1F, 0x43, 0x09, 0x97, 0xB9, 0x79, 0x48, 0xE8, 0x33, 0x28, - 0x5D, 0x58, 0x81, 0x81, 0xB6, 0x45, 0x50, 0xB8, 0x29, 0xA0, 0x31, 0xE1, 0x72, 0x4E, 0x64, 0x30]; -global secpk1_fr = [0x41, 0x41, 0x36, 0xD0, 0x8C, 0x5E, 0xD2, 0xBF, 0x3B, 0xA0, 0x48, 0xAF, 0xE6, 0xDC, 0xAE, 0xBA, - 0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF]; -global secpk1_fq = [0x2F, 0xFC, 0xFF, 0xFF, 0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, - 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF]; -global secpr1_fq: [u8] = &[0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, 0xFF, 0xFF, 0xFF]; -global secpr1_fr: [u8] = &[0x51, 0x25, 0x63, 0xFC, 0xC2, 0xCA, 0xB9, 0xF3, 0x84, 0x9E, 0x17, 0xA7, 0xAD, 0xFA, 0xE6, 0xBC, +global bn254_fq = &[0x47, 0xFD, 0x7C, 0xD8, 0x16, 0x8C, 0x20, 0x3C, 0x8d, 0xca, 0x71, 0x68, 0x91, 0x6a, 0x81, 0x97, + 0x5d, 0x58, 0x81, 0x81, 0xb6, 0x45, 0x50, 0xb8, 0x29, 0xa0, 0x31, 0xe1, 0x72, 0x4e, 0x64, 0x30]; +global bn254_fr = &[0x01, 0x00, 0x00, 0x00, 0x3F, 0x59, 0x1F, 0x43, 0x09, 0x97, 0xB9, 0x79, 0x48, 0xE8, 0x33, 0x28, + 0x5D, 0x58, 0x81, 0x81, 0xB6, 0x45, 0x50, 0xB8, 0x29, 0xA0, 0x31, 0xE1, 0x72, 0x4E, 0x64, 0x30]; +global secpk1_fr = &[0x41, 0x41, 0x36, 0xD0, 0x8C, 0x5E, 0xD2, 0xBF, 0x3B, 0xA0, 0x48, 0xAF, 0xE6, 0xDC, 0xAE, 0xBA, + 0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF]; +global secpk1_fq = &[0x2F, 0xFC, 0xFF, 0xFF, 0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF]; +global secpr1_fq = &[0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, 0xFF, 0xFF, 0xFF]; +global secpr1_fr = &[0x51, 0x25, 0x63, 0xFC, 0xC2, 0xCA, 0xB9, 0xF3, 0x84, 0x9E, 0x17, 0xA7, 0xAD, 0xFA, 0xE6, 0xBC, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00,0xFF, 0xFF, 0xFF, 0xFF]; struct BigInt { diff --git a/noir_stdlib/src/collections/vec.nr b/noir_stdlib/src/collections/vec.nr index deec98185ff..56354438c89 100644 --- a/noir_stdlib/src/collections/vec.nr +++ b/noir_stdlib/src/collections/vec.nr @@ -5,7 +5,7 @@ struct Vec { // A separate type is technically not needed but helps differentiate which operations are mutable. impl Vec { pub fn new() -> Self { - Self { slice: [] } + Self { slice: &[] } } // Create a Vec containing each element from the given slice. diff --git a/noir_stdlib/src/hash.nr b/noir_stdlib/src/hash.nr index fcf21436197..701525e00c1 100644 --- a/noir_stdlib/src/hash.nr +++ b/noir_stdlib/src/hash.nr @@ -11,18 +11,36 @@ pub fn sha256(input: [u8; N]) -> [u8; 32] // docs:end:sha256 {} +#[foreign(sha256)] +// docs:start:sha256_slice +pub fn sha256_slice(input: [u8]) -> [u8; 32] +// docs:end:sha256_slice +{} + #[foreign(blake2s)] // docs:start:blake2s pub fn blake2s(input: [u8; N]) -> [u8; 32] // docs:end:blake2s {} +#[foreign(blake2s)] +// docs:start:blake2s_slice +pub fn blake2s_slice(input: [u8]) -> [u8; 32] +// docs:end:blake2s_slice +{} + #[foreign(blake3)] // docs:start:blake3 pub fn blake3(input: [u8; N]) -> [u8; 32] // docs:end:blake3 {} +#[foreign(blake3)] +// docs:start:blake3_slice +pub fn blake3_slice(input: [u8]) -> [u8; 32] +// docs:end:blake3_slice +{} + // docs:start:pedersen_commitment struct PedersenPoint { x : Field, @@ -43,6 +61,14 @@ pub fn pedersen_commitment_with_separator(input: [Field; N], separator: u32) PedersenPoint { x: values[0], y: values[1] } } +#[foreign(pedersen_commitment)] +pub fn __pedersen_commitment_with_separator_slice(input: [Field], separator: u32) -> [Field; 2] {} + +pub fn pedersen_commitment_with_separator_slice(input: [Field], separator: u32) -> PedersenPoint { + let values = __pedersen_commitment_with_separator_slice(input, separator); + PedersenPoint { x: values[0], y: values[1] } +} + // docs:start:pedersen_hash pub fn pedersen_hash(input: [Field; N]) -> Field // docs:end:pedersen_hash @@ -50,11 +76,21 @@ pub fn pedersen_hash(input: [Field; N]) -> Field pedersen_hash_with_separator(input, 0) } +// docs:start:pedersen_hash_slice +pub fn pedersen_hash_slice(input: [Field]) -> Field +// docs:end:pedersen_hash_slice +{ + pedersen_hash_with_separator_slice(input, 0) +} + #[foreign(pedersen_hash)] pub fn pedersen_hash_with_separator(input: [Field; N], separator: u32) -> Field {} +#[foreign(pedersen_hash)] +pub fn pedersen_hash_with_separator_slice(input: [Field], separator: u32) -> Field {} + pub fn hash_to_field(input: [Field; N]) -> Field { - let mut inputs_as_bytes = []; + let mut inputs_as_bytes = &[]; for i in 0..N { let input_bytes = input[i].to_le_bytes(32); @@ -63,7 +99,7 @@ pub fn hash_to_field(input: [Field; N]) -> Field { } } - let hashed_input = blake2s(inputs_as_bytes); + let hashed_input = blake2s_slice(inputs_as_bytes); crate::field::bytes32_to_field(hashed_input) } @@ -73,9 +109,18 @@ pub fn keccak256(input: [u8; N], message_size: u32) -> [u8; 32] // docs:end:keccak256 {} +#[foreign(keccak256)] +// docs:start:keccak256_slice +pub fn keccak256_slice(input: [u8], message_size: u32) -> [u8; 32] +// docs:end:keccak256_slice +{} + #[foreign(poseidon2_permutation)] pub fn poseidon2_permutation(_input: [Field; N], _state_length: u32) -> [Field; N] {} +#[foreign(poseidon2_permutation)] +pub fn poseidon2_permutation_slice(_input: [Field], _state_length: u32) -> [Field] {} + #[foreign(sha256_compression)] pub fn sha256_compression(_input: [u32; 16], _state: [u32; 8]) -> [u32; 8] {} @@ -123,7 +168,7 @@ where // TODO: add implementations for the remainder of primitive types. impl Hash for Field{ fn hash(self, state: &mut H) where H: Hasher{ - let input: [Field] = [self]; + let input: [Field] = &[self]; H::write(state, input); } } diff --git a/noir_stdlib/src/hash/pedersen.nr b/noir_stdlib/src/hash/pedersen.nr index ace6851099d..ad21e728945 100644 --- a/noir_stdlib/src/hash/pedersen.nr +++ b/noir_stdlib/src/hash/pedersen.nr @@ -1,4 +1,4 @@ -use crate::hash::{Hasher, pedersen_hash}; +use crate::hash::{Hasher, pedersen_hash_slice}; use crate::default::Default; struct PedersenHasher{ @@ -7,7 +7,7 @@ struct PedersenHasher{ impl Hasher for PedersenHasher { fn finish(self) -> Field { - pedersen_hash(self._state) + pedersen_hash_slice(self._state) } fn write(&mut self, input: [Field]){ @@ -18,7 +18,7 @@ impl Hasher for PedersenHasher { impl Default for PedersenHasher{ fn default() -> Self{ PedersenHasher{ - _state: [] + _state: &[] } } } diff --git a/noir_stdlib/src/slice.nr b/noir_stdlib/src/slice.nr index ea8d09d14ce..164b4f96cf6 100644 --- a/noir_stdlib/src/slice.nr +++ b/noir_stdlib/src/slice.nr @@ -1,4 +1,7 @@ impl [T] { + #[builtin(array_len)] + pub fn len(self) -> u64 {} + /// Push a new element to the end of the slice, returning a /// new slice with a length one greater than the /// original unmodified slice. From c683028c2218a0dba14a9c5ec598a595c7a8de3c Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Sun, 10 Mar 2024 22:15:36 -0400 Subject: [PATCH 05/28] got tests passing, update stdlib methods to take slices, fix missing type check case, updating test_programs, defaulting unknown array length to zero, cleanup debugging println, add utf8 decoding and serde error to execution json decoding error message, cargo clippy/fmt --- .../noirc_evaluator/src/ssa/ssa_gen/mod.rs | 10 +-- .../src/hir/resolution/resolver.rs | 19 +++-- .../noirc_frontend/src/hir/type_check/expr.rs | 79 ++++++++++--------- .../noirc_frontend/src/hir/type_check/stmt.rs | 1 + compiler/noirc_frontend/src/hir_def/types.rs | 30 +++---- .../src/monomorphization/mod.rs | 11 +-- compiler/noirc_frontend/src/parser/parser.rs | 3 +- compiler/noirc_printable_type/src/lib.rs | 12 ++- noir_stdlib/src/ecdsa_secp256k1.nr | 4 +- noir_stdlib/src/ecdsa_secp256r1.nr | 4 +- noir_stdlib/src/hash.nr | 71 +++-------------- noir_stdlib/src/hash/pedersen.nr | 4 +- noir_stdlib/src/merkle.nr | 2 +- noir_stdlib/src/schnorr.nr | 4 +- noir_stdlib/src/sha512.nr | 2 +- .../intrinsic_die/src/main.nr | 2 +- .../compile_success_empty/vectors/src/main.nr | 2 +- test_programs/execution_success/6/src/main.nr | 4 +- test_programs/execution_success/7/src/main.nr | 2 +- .../array_dynamic_blackbox_input/src/main.nr | 2 +- .../src/main.nr | 2 +- .../execution_success/bigint/src/main.nr | 4 +- .../execution_success/blake3/src/main.nr | 2 +- .../brillig_blake2s/src/main.nr | 2 +- .../brillig_blake3/src/main.nr | 2 +- .../brillig_cow_regression/src/main.nr | 4 +- .../brillig_ecdsa_secp256k1/src/main.nr | 2 +- .../brillig_ecdsa_secp256r1/src/main.nr | 2 +- .../brillig_hash_to_field/src/main.nr | 2 +- .../brillig_keccak/src/main.nr | 4 +- .../brillig_pedersen/src/main.nr | 8 +- .../brillig_schnorr/src/main.nr | 2 +- .../brillig_sha256/src/main.nr | 2 +- .../brillig_slices/src/main.nr | 8 +- .../conditional_1/src/main.nr | 2 +- .../src/main.nr | 4 +- .../ecdsa_secp256k1/src/main.nr | 4 +- .../ecdsa_secp256r1/src/main.nr | 2 +- .../hash_to_field/src/main.nr | 2 +- .../execution_success/import/src/main.nr | 2 +- .../execution_success/keccak256/src/main.nr | 8 +- .../nested_array_in_slice/src/main.nr | 2 +- .../pedersen_check/src/main.nr | 8 +- .../pedersen_commitment/src/main.nr | 2 +- .../pedersen_hash/src/main.nr | 2 +- .../regression_4202/src/main.nr | 2 +- .../execution_success/schnorr/src/main.nr | 2 +- .../execution_success/sha256/src/main.nr | 2 +- .../execution_success/sha2_byte/src/main.nr | 2 +- .../simple_shield/src/main.nr | 6 +- .../slice_dynamic_index/src/main.nr | 2 +- .../execution_success/slices/src/main.nr | 33 ++++---- .../execution_success/strings/src/main.nr | 4 +- .../should_fail_with_matches/src/main.nr | 4 +- tooling/nargo/src/artifacts/debug_vars.rs | 15 +++- tooling/nargo_cli/build.rs | 4 +- tooling/nargo_fmt/src/rewrite/array.rs | 13 +-- 57 files changed, 195 insertions(+), 246 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 39013fa6e12..6d4c01ab0b2 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -194,10 +194,7 @@ impl<'a> FunctionContext<'a> { ast::Type::Array(_, _) => { self.codegen_array_checked(elements, typ[0].clone())? } - _ => unreachable!( - "ICE: unexpected array literal type, got {}", - array.typ - ), + _ => unreachable!("ICE: unexpected array literal type, got {}", array.typ), }) } ast::Literal::Slice(array) => { @@ -213,10 +210,7 @@ impl<'a> FunctionContext<'a> { self.codegen_array_checked(elements, typ[1].clone())?; Tree::Branch(vec![slice_length.into(), slice_contents]) } - _ => unreachable!( - "ICE: unexpected slice literal type, got {}", - array.typ - ), + _ => unreachable!("ICE: unexpected slice literal type, got {}", array.typ), }) } ast::Literal::Integer(value, typ, location) => { diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 96fcc1a43d4..3928235dfcd 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -1400,21 +1400,20 @@ impl<'a> Resolver<'a> { ArrayLiteral::Standard(elements) => { let elements = vecmap(elements, |elem| self.resolve_expression(elem)); HirArrayLiteral::Standard(elements) - }, + } ArrayLiteral::Repeated { repeated_element, length } => { let span = length.span; - let length = UnresolvedTypeExpression::from_expr(*length, span).unwrap_or_else( - |error| { + let length = + UnresolvedTypeExpression::from_expr(*length, span).unwrap_or_else(|error| { self.errors.push(ResolverError::ParserError(Box::new(error))); UnresolvedTypeExpression::Constant(0, span) - }, - ); + }); let length = self.convert_expression_type(length); let repeated_element = self.resolve_expression(*repeated_element); HirArrayLiteral::Repeated { repeated_element, length } - }, + } } } @@ -1422,8 +1421,12 @@ impl<'a> Resolver<'a> { let hir_expr = match expr.kind { ExpressionKind::Literal(literal) => HirExpression::Literal(match literal { Literal::Bool(b) => HirLiteral::Bool(b), - Literal::Array(array_literal) => HirLiteral::Array(self.resolve_array_literal(array_literal)), - Literal::Slice(array_literal) => HirLiteral::Slice(self.resolve_array_literal(array_literal)), + Literal::Array(array_literal) => { + HirLiteral::Array(self.resolve_array_literal(array_literal)) + } + Literal::Slice(array_literal) => { + HirLiteral::Slice(self.resolve_array_literal(array_literal)) + } Literal::Integer(integer, sign) => HirLiteral::Integer(integer, sign), Literal::Str(str) => HirLiteral::Str(str), Literal::RawStr(str, _) => HirLiteral::Str(str), diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index cc7bfa09a13..a3c31366886 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -48,7 +48,10 @@ impl<'interner> TypeChecker<'interner> { false } - fn check_hir_array_literal(&mut self, hir_array_literal: HirArrayLiteral) -> (Result, u64>, Box) { + fn check_hir_array_literal( + &mut self, + hir_array_literal: HirArrayLiteral, + ) -> (Result, u64>, Box) { match hir_array_literal { HirArrayLiteral::Standard(arr) => { let elem_types = vecmap(&arr, |arg| self.check_expression(arg)); @@ -75,21 +78,16 @@ impl<'interner> TypeChecker<'interner> { }); } - ( - Err(arr.len() as u64), - Box::new(first_elem_type.clone()) - ) - }, + (Err(arr.len() as u64), Box::new(first_elem_type.clone())) + } HirArrayLiteral::Repeated { repeated_element, length } => { let elem_type = self.check_expression(&repeated_element); let length = match length { - Type::Constant(length) => { - Err(length) - } + Type::Constant(length) => Err(length), other => Ok(Box::new(other)), }; (length, Box::new(elem_type)) - }, + } } } @@ -104,38 +102,41 @@ impl<'interner> TypeChecker<'interner> { pub(crate) fn check_expression(&mut self, expr_id: &ExprId) -> Type { let typ = match self.interner.expression(expr_id) { HirExpression::Ident(ident) => self.check_ident(ident, expr_id), - HirExpression::Literal(literal) => { - match literal { - HirLiteral::Array(hir_array_literal) => { - let (length, elem_type) = self.check_hir_array_literal(hir_array_literal); - Type::Array(length.unwrap_or_else(|constant| Box::new(Type::constant_variable(constant, self.interner))), elem_type) - }, - HirLiteral::Slice(hir_array_literal) => { - let (length_type, elem_type) = self.check_hir_array_literal(hir_array_literal); - match length_type { - Ok(_non_constant) => { - self.errors.push(TypeCheckError::NonConstantSliceLength { - span: self.interner.expr_span(expr_id), - }); - Type::Error - }, - Err(_length) => Type::Slice(elem_type), + HirExpression::Literal(literal) => match literal { + HirLiteral::Array(hir_array_literal) => { + let (length, elem_type) = self.check_hir_array_literal(hir_array_literal); + Type::Array( + length.unwrap_or_else(|constant| { + Box::new(Type::constant_variable(constant, self.interner)) + }), + elem_type, + ) + } + HirLiteral::Slice(hir_array_literal) => { + let (length_type, elem_type) = self.check_hir_array_literal(hir_array_literal); + match length_type { + Ok(_non_constant) => { + self.errors.push(TypeCheckError::NonConstantSliceLength { + span: self.interner.expr_span(expr_id), + }); + Type::Error } - }, - HirLiteral::Bool(_) => Type::Bool, - HirLiteral::Integer(_, _) => Type::polymorphic_integer_or_field(self.interner), - HirLiteral::Str(string) => { - let len = Type::Constant(string.len() as u64); - Type::String(Box::new(len)) + Err(_length) => Type::Slice(elem_type), } - HirLiteral::FmtStr(string, idents) => { - let len = Type::Constant(string.len() as u64); - let types = vecmap(&idents, |elem| self.check_expression(elem)); - Type::FmtString(Box::new(len), Box::new(Type::Tuple(types))) - } - HirLiteral::Unit => Type::Unit, } - } + HirLiteral::Bool(_) => Type::Bool, + HirLiteral::Integer(_, _) => Type::polymorphic_integer_or_field(self.interner), + HirLiteral::Str(string) => { + let len = Type::Constant(string.len() as u64); + Type::String(Box::new(len)) + } + HirLiteral::FmtStr(string, idents) => { + let len = Type::Constant(string.len() as u64); + let types = vecmap(&idents, |elem| self.check_expression(elem)); + Type::FmtString(Box::new(len), Box::new(Type::Tuple(types))) + } + HirLiteral::Unit => Type::Unit, + }, HirExpression::Infix(infix_expr) => { // The type of the infix expression must be looked up from a type table let lhs_type = self.check_expression(&infix_expr.lhs); diff --git a/compiler/noirc_frontend/src/hir/type_check/stmt.rs b/compiler/noirc_frontend/src/hir/type_check/stmt.rs index 358bea86922..2fb0522623c 100644 --- a/compiler/noirc_frontend/src/hir/type_check/stmt.rs +++ b/compiler/noirc_frontend/src/hir/type_check/stmt.rs @@ -256,6 +256,7 @@ impl<'interner> TypeChecker<'interner> { let typ = match lvalue_type.follow_bindings() { Type::Array(_, elem_type) => *elem_type, + Type::Slice(elem_type) => *elem_type, Type::Error => Type::Error, other => { // TODO: Need a better span here diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 3f7eba598db..05fe706192a 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -156,9 +156,7 @@ impl Type { pub(crate) fn is_nested_slice(&self) -> bool { match self { Type::Slice(elem) => elem.as_ref().contains_slice(), - Type::Array(_, elem) => { - elem.as_ref().contains_slice() - } + Type::Array(_, elem) => elem.as_ref().contains_slice(), _ => false, } } @@ -636,9 +634,7 @@ impl Type { Type::Array(length, elem) => { elem.contains_numeric_typevar(target_id) || named_generic_id_matches_target(length) } - Type::Slice(elem) => { - elem.contains_numeric_typevar(target_id) - } + Type::Slice(elem) => elem.contains_numeric_typevar(target_id), Type::Tuple(fields) => { fields.iter().any(|field| field.contains_numeric_typevar(target_id)) } @@ -1152,9 +1148,7 @@ impl Type { elem_a.try_unify(elem_b, bindings) } - (Slice(elem_a), Slice(elem_b)) => { - elem_a.try_unify(elem_b, bindings) - } + (Slice(elem_a), Slice(elem_b)) => elem_a.try_unify(elem_b, bindings), (String(len_a), String(len_b)) => len_a.try_unify(len_b, bindings), @@ -1304,7 +1298,7 @@ impl Type { let length = size.follow_bindings(); // If we have an array and our target is a slice - if matches!(length, Type::Constant(_)) { + if matches!(length, Type::Constant(_)) { // Still have to ensure the element types match. // Don't need to issue an error here if not, it will be done in unify_with_coercions let mut bindings = TypeBindings::new(); @@ -1600,9 +1594,7 @@ impl Type { Array(size, elem) => { Array(Box::new(size.follow_bindings()), Box::new(elem.follow_bindings())) } - Slice(elem) => { - Slice(Box::new(elem.follow_bindings())) - } + Slice(elem) => Slice(Box::new(elem.follow_bindings())), String(size) => String(Box::new(size.follow_bindings())), FmtString(size, args) => { let size = Box::new(size.follow_bindings()); @@ -1637,13 +1629,9 @@ impl Type { // Expect that this function should only be called on instantiated types Forall(..) => unreachable!(), - TraitAsType(..) - | FieldElement - | Integer(_, _) - | Bool - | Constant(_) - | Unit - | Error => self.clone(), + TraitAsType(..) | FieldElement | Integer(_, _) | Bool | Constant(_) | Unit | Error => { + self.clone() + } } } @@ -1725,7 +1713,7 @@ impl From<&Type> for PrintableType { } Type::Slice(typ) => { let typ = typ.as_ref(); - PrintableType::Slice{ typ: Box::new(typ.into()) } + PrintableType::Slice { typ: Box::new(typ.into()) } } Type::Integer(sign, bit_width) => match sign { Signedness::Unsigned => { diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 5a8af4c699f..d28f9056f1f 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -868,9 +868,7 @@ impl<'interner> Monomorphizer<'interner> { HirType::Array(length, element) => { let element = Box::new(self.convert_type(element.as_ref())); // TODO: convert to MonomorphizationError - let length = length.evaluate_to_u64().unwrap_or_else(|| { - panic!("Length of generic array could not be determined.") - }); + let length = length.evaluate_to_u64().unwrap_or(0); ast::Type::Array(length, element) } HirType::Slice(element) => { @@ -950,9 +948,7 @@ impl<'interner> Monomorphizer<'interner> { ast::Type::MutableReference(Box::new(element)) } - HirType::Forall(_, _) - | HirType::Constant(_) - | HirType::Error => { + HirType::Forall(_, _) | HirType::Constant(_) | HirType::Error => { unreachable!("Unexpected type {} found", typ) } } @@ -1236,9 +1232,6 @@ impl<'interner> Monomorphizer<'interner> { ) -> ast::Expression { use ast::*; - // TODO: remove - println!("modulus_array_literal: {:?}, {:?}, {:?}", bytes, arr_elem_bits, location); - let int_type = Type::Integer(crate::Signedness::Unsigned, arr_elem_bits); let bytes_as_expr = vecmap(bytes, |byte| { diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 30948267384..86df57f9f7b 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -1081,7 +1081,8 @@ fn slice_expr

(expr_parser: P) -> impl NoirParser where P: ExprParser, { - just(Token::Ampersand).ignore_then(standard_slice(expr_parser.clone()).or(slice_sugar(expr_parser))) + just(Token::Ampersand) + .ignore_then(standard_slice(expr_parser.clone()).or(slice_sugar(expr_parser))) } /// &[a, b, c, ...] diff --git a/compiler/noirc_printable_type/src/lib.rs b/compiler/noirc_printable_type/src/lib.rs index 9abc993606b..44877b5d990 100644 --- a/compiler/noirc_printable_type/src/lib.rs +++ b/compiler/noirc_printable_type/src/lib.rs @@ -52,10 +52,7 @@ pub enum PrintableType { pub enum PrintableValue { Field(FieldElement), String(String), - Vec { - array_elements: Vec, - is_slice: bool, - }, + Vec { array_elements: Vec, is_slice: bool }, Struct(BTreeMap), Other, } @@ -353,9 +350,10 @@ pub fn decode_value( PrintableValue::Vec { array_elements, is_slice: true } } - PrintableType::Tuple { types } => { - PrintableValue::Vec { array_elements: vecmap(types, |typ| decode_value(field_iterator, typ)), is_slice: false } - } + PrintableType::Tuple { types } => PrintableValue::Vec { + array_elements: vecmap(types, |typ| decode_value(field_iterator, typ)), + is_slice: false, + }, PrintableType::String { length } => { let field_elements: Vec = field_iterator.take(*length as usize).collect(); diff --git a/noir_stdlib/src/ecdsa_secp256k1.nr b/noir_stdlib/src/ecdsa_secp256k1.nr index b72a1acd041..79510dc0e5e 100644 --- a/noir_stdlib/src/ecdsa_secp256k1.nr +++ b/noir_stdlib/src/ecdsa_secp256k1.nr @@ -1,10 +1,10 @@ #[foreign(ecdsa_secp256k1)] // docs:start:ecdsa_secp256k1 -pub fn verify_signature( +pub fn verify_signature( public_key_x: [u8; 32], public_key_y: [u8; 32], signature: [u8; 64], - message_hash: [u8; N] + message_hash: [u8] ) -> bool // docs:end:ecdsa_secp256k1 {} diff --git a/noir_stdlib/src/ecdsa_secp256r1.nr b/noir_stdlib/src/ecdsa_secp256r1.nr index ef92bf24ae4..8085693bde2 100644 --- a/noir_stdlib/src/ecdsa_secp256r1.nr +++ b/noir_stdlib/src/ecdsa_secp256r1.nr @@ -1,10 +1,10 @@ #[foreign(ecdsa_secp256r1)] // docs:start:ecdsa_secp256r1 -pub fn verify_signature( +pub fn verify_signature( public_key_x: [u8; 32], public_key_y: [u8; 32], signature: [u8; 64], - message_hash: [u8; N] + message_hash: [u8] ) -> bool // docs:end:ecdsa_secp256r1 {} diff --git a/noir_stdlib/src/hash.nr b/noir_stdlib/src/hash.nr index 701525e00c1..da5cc3f63cf 100644 --- a/noir_stdlib/src/hash.nr +++ b/noir_stdlib/src/hash.nr @@ -7,120 +7,75 @@ use crate::default::Default; #[foreign(sha256)] // docs:start:sha256 -pub fn sha256(input: [u8; N]) -> [u8; 32] +pub fn sha256(input: [u8]) -> [u8; 32] // docs:end:sha256 {} -#[foreign(sha256)] -// docs:start:sha256_slice -pub fn sha256_slice(input: [u8]) -> [u8; 32] -// docs:end:sha256_slice -{} - #[foreign(blake2s)] // docs:start:blake2s -pub fn blake2s(input: [u8; N]) -> [u8; 32] +pub fn blake2s(input: [u8]) -> [u8; 32] // docs:end:blake2s {} -#[foreign(blake2s)] -// docs:start:blake2s_slice -pub fn blake2s_slice(input: [u8]) -> [u8; 32] -// docs:end:blake2s_slice -{} - #[foreign(blake3)] // docs:start:blake3 -pub fn blake3(input: [u8; N]) -> [u8; 32] +pub fn blake3(input: [u8]) -> [u8; 32] // docs:end:blake3 {} -#[foreign(blake3)] -// docs:start:blake3_slice -pub fn blake3_slice(input: [u8]) -> [u8; 32] -// docs:end:blake3_slice -{} - // docs:start:pedersen_commitment struct PedersenPoint { x : Field, y : Field, } -pub fn pedersen_commitment(input: [Field; N]) -> PedersenPoint +pub fn pedersen_commitment(input: [Field]) -> PedersenPoint // docs:end:pedersen_commitment { pedersen_commitment_with_separator(input, 0) } #[foreign(pedersen_commitment)] -pub fn __pedersen_commitment_with_separator(input: [Field; N], separator: u32) -> [Field; 2] {} +pub fn __pedersen_commitment_with_separator(input: [Field], separator: u32) -> [Field; 2] {} -pub fn pedersen_commitment_with_separator(input: [Field; N], separator: u32) -> PedersenPoint { +pub fn pedersen_commitment_with_separator(input: [Field], separator: u32) -> PedersenPoint { let values = __pedersen_commitment_with_separator(input, separator); PedersenPoint { x: values[0], y: values[1] } } -#[foreign(pedersen_commitment)] -pub fn __pedersen_commitment_with_separator_slice(input: [Field], separator: u32) -> [Field; 2] {} - -pub fn pedersen_commitment_with_separator_slice(input: [Field], separator: u32) -> PedersenPoint { - let values = __pedersen_commitment_with_separator_slice(input, separator); - PedersenPoint { x: values[0], y: values[1] } -} - // docs:start:pedersen_hash -pub fn pedersen_hash(input: [Field; N]) -> Field +pub fn pedersen_hash(input: [Field]) -> Field // docs:end:pedersen_hash { pedersen_hash_with_separator(input, 0) } -// docs:start:pedersen_hash_slice -pub fn pedersen_hash_slice(input: [Field]) -> Field -// docs:end:pedersen_hash_slice -{ - pedersen_hash_with_separator_slice(input, 0) -} - -#[foreign(pedersen_hash)] -pub fn pedersen_hash_with_separator(input: [Field; N], separator: u32) -> Field {} - #[foreign(pedersen_hash)] -pub fn pedersen_hash_with_separator_slice(input: [Field], separator: u32) -> Field {} +pub fn pedersen_hash_with_separator(input: [Field], separator: u32) -> Field {} -pub fn hash_to_field(input: [Field; N]) -> Field { +pub fn hash_to_field(inputs: [Field]) -> Field { let mut inputs_as_bytes = &[]; - for i in 0..N { - let input_bytes = input[i].to_le_bytes(32); + for input in inputs { + let input_bytes = input.to_le_bytes(32); for i in 0..32 { inputs_as_bytes = inputs_as_bytes.push_back(input_bytes[i]); } } - let hashed_input = blake2s_slice(inputs_as_bytes); + let hashed_input = blake2s(inputs_as_bytes); crate::field::bytes32_to_field(hashed_input) } #[foreign(keccak256)] // docs:start:keccak256 -pub fn keccak256(input: [u8; N], message_size: u32) -> [u8; 32] +pub fn keccak256(input: [u8], message_size: u32) -> [u8; 32] // docs:end:keccak256 {} -#[foreign(keccak256)] -// docs:start:keccak256_slice -pub fn keccak256_slice(input: [u8], message_size: u32) -> [u8; 32] -// docs:end:keccak256_slice -{} - #[foreign(poseidon2_permutation)] pub fn poseidon2_permutation(_input: [Field; N], _state_length: u32) -> [Field; N] {} -#[foreign(poseidon2_permutation)] -pub fn poseidon2_permutation_slice(_input: [Field], _state_length: u32) -> [Field] {} - #[foreign(sha256_compression)] pub fn sha256_compression(_input: [u32; 16], _state: [u32; 8]) -> [u32; 8] {} diff --git a/noir_stdlib/src/hash/pedersen.nr b/noir_stdlib/src/hash/pedersen.nr index ad21e728945..09e436a32ae 100644 --- a/noir_stdlib/src/hash/pedersen.nr +++ b/noir_stdlib/src/hash/pedersen.nr @@ -1,4 +1,4 @@ -use crate::hash::{Hasher, pedersen_hash_slice}; +use crate::hash::{Hasher, pedersen_hash}; use crate::default::Default; struct PedersenHasher{ @@ -7,7 +7,7 @@ struct PedersenHasher{ impl Hasher for PedersenHasher { fn finish(self) -> Field { - pedersen_hash_slice(self._state) + pedersen_hash(self._state) } fn write(&mut self, input: [Field]){ diff --git a/noir_stdlib/src/merkle.nr b/noir_stdlib/src/merkle.nr index 9b15fe7313d..ba65b1adae5 100644 --- a/noir_stdlib/src/merkle.nr +++ b/noir_stdlib/src/merkle.nr @@ -13,7 +13,7 @@ pub fn compute_merkle_root(leaf: Field, index: Field, hash_path: [Field; N]) } else { (current, hash_path[i]) }; - current = crate::hash::pedersen_hash([hash_left, hash_right]); + current = crate::hash::pedersen_hash([hash_left, hash_right].as_slice()); } current } diff --git a/noir_stdlib/src/schnorr.nr b/noir_stdlib/src/schnorr.nr index 757963d40d7..024f47e63e8 100644 --- a/noir_stdlib/src/schnorr.nr +++ b/noir_stdlib/src/schnorr.nr @@ -1,10 +1,10 @@ #[foreign(schnorr_verify)] // docs:start:schnorr_verify -pub fn verify_signature( +pub fn verify_signature( public_key_x: Field, public_key_y: Field, signature: [u8; 64], - message: [u8; N] + message: [u8] ) -> bool // docs:end:schnorr_verify {} diff --git a/noir_stdlib/src/sha512.nr b/noir_stdlib/src/sha512.nr index 4dfe78308e2..39009bd0651 100644 --- a/noir_stdlib/src/sha512.nr +++ b/noir_stdlib/src/sha512.nr @@ -85,7 +85,7 @@ fn msg_u8_to_u64(msg: [u8; 128]) -> [u64; 16] { msg64 } // SHA-512 hash function -pub fn digest(msg: [u8; N]) -> [u8; 64] { +pub fn digest(msg: [u8]) -> [u8; 64] { let mut msg_block: [u8; 128] = [0; 128]; // noir-fmt:ignore let mut h: [u64; 8] = [7640891576956012808, 13503953896175478587, 4354685564936845355, 11912009170470909681, 5840696475078001361, 11170449401992604703, 2270897969802886507, 6620516959819538809]; // Intermediate hash, starting with the canonical initial value diff --git a/test_programs/compile_success_empty/intrinsic_die/src/main.nr b/test_programs/compile_success_empty/intrinsic_die/src/main.nr index 8cac707dfea..8963073859a 100644 --- a/test_programs/compile_success_empty/intrinsic_die/src/main.nr +++ b/test_programs/compile_success_empty/intrinsic_die/src/main.nr @@ -1,6 +1,6 @@ use dep::std; // This test checks that we perform dead-instruction-elimination on intrinsic functions. fn main(x: Field) { - let hash = std::hash::pedersen_commitment([x]); + let hash = std::hash::pedersen_commitment([x].as_slice()); let _p1 = std::scalar_mul::fixed_base_embedded_curve(x, 0); } diff --git a/test_programs/compile_success_empty/vectors/src/main.nr b/test_programs/compile_success_empty/vectors/src/main.nr index 28187a4f619..d105ceed180 100644 --- a/test_programs/compile_success_empty/vectors/src/main.nr +++ b/test_programs/compile_success_empty/vectors/src/main.nr @@ -26,7 +26,7 @@ fn main(x: Field, y: pub Field) { assert(vector.get(3) == 3); assert(vector.len() == 4); - let mut inputs_vector = Vec::from_slice([x, y]); + let mut inputs_vector = Vec::from_slice(&[x, y]); assert(inputs_vector.get(0) == x); assert(inputs_vector.get(1) == y); } diff --git a/test_programs/execution_success/6/src/main.nr b/test_programs/execution_success/6/src/main.nr index 5ecb809e68b..294d406637a 100644 --- a/test_programs/execution_success/6/src/main.nr +++ b/test_programs/execution_success/6/src/main.nr @@ -8,9 +8,9 @@ use dep::std; fn main(x: [u8; 5], result: pub [u8; 32]) { - let mut digest = std::hash::sha256(x); + let mut digest = std::hash::sha256(x.as_slice()); digest[0] = 5 as u8; - digest = std::hash::sha256(x); + digest = std::hash::sha256(x.as_slice()); assert(digest == result); let y = [12, 45, 78, 41]; diff --git a/test_programs/execution_success/7/src/main.nr b/test_programs/execution_success/7/src/main.nr index a6bba978644..996a3b24f24 100644 --- a/test_programs/execution_success/7/src/main.nr +++ b/test_programs/execution_success/7/src/main.nr @@ -5,6 +5,6 @@ use dep::std; fn main(x: [u8; 5], result: [u8; 32]) { - let digest = std::hash::blake2s(x); + let digest = std::hash::blake2s(x.as_slice()); assert(digest == result); } diff --git a/test_programs/execution_success/array_dynamic_blackbox_input/src/main.nr b/test_programs/execution_success/array_dynamic_blackbox_input/src/main.nr index 4cbf1bd8e6d..a936f1d5c4a 100644 --- a/test_programs/execution_success/array_dynamic_blackbox_input/src/main.nr +++ b/test_programs/execution_success/array_dynamic_blackbox_input/src/main.nr @@ -18,7 +18,7 @@ fn compute_root(leaf: [u8; 32], path: [u8; 64], _index: u32, root: [u8; 32]) { hash_input[j + b] = path[offset + j]; } - current = dep::std::hash::sha256(hash_input); + current = dep::std::hash::sha256(hash_input.as_slice()); index = index >> 1; } diff --git a/test_programs/execution_success/array_dynamic_nested_blackbox_input/src/main.nr b/test_programs/execution_success/array_dynamic_nested_blackbox_input/src/main.nr index 8faaf69dfc8..7c92890e950 100644 --- a/test_programs/execution_success/array_dynamic_nested_blackbox_input/src/main.nr +++ b/test_programs/execution_success/array_dynamic_nested_blackbox_input/src/main.nr @@ -15,6 +15,6 @@ fn main(mut x: [Foo; 3], y: pub Field, hash_result: pub [u8; 32]) { // Make sure that we are passing a dynamic array to the black box function call // by setting the array using a dynamic index here hash_input[y - 1] = 0; - let hash = dep::std::hash::sha256(hash_input); + let hash = dep::std::hash::sha256(hash_input.as_slice()); assert_eq(hash, hash_result); } diff --git a/test_programs/execution_success/bigint/src/main.nr b/test_programs/execution_success/bigint/src/main.nr index b93fec370e5..9ca612baf63 100644 --- a/test_programs/execution_success/bigint/src/main.nr +++ b/test_programs/execution_success/bigint/src/main.nr @@ -1,8 +1,8 @@ use dep::std::bigint; fn main(mut x: [u8; 5], y: [u8; 5]) { - let a = bigint::Secpk1Fq::from_le_bytes([x[0], x[1], x[2], x[3], x[4]]); - let b = bigint::Secpk1Fq::from_le_bytes([y[0], y[1], y[2], y[3], y[4]]); + let a = bigint::Secpk1Fq::from_le_bytes(&[x[0], x[1], x[2], x[3], x[4]]); + let b = bigint::Secpk1Fq::from_le_bytes(&[y[0], y[1], y[2], y[3], y[4]]); let a_bytes = a.to_le_bytes(); let b_bytes = b.to_le_bytes(); for i in 0..5 { diff --git a/test_programs/execution_success/blake3/src/main.nr b/test_programs/execution_success/blake3/src/main.nr index 3bfea6c5f95..7a9810b6abc 100644 --- a/test_programs/execution_success/blake3/src/main.nr +++ b/test_programs/execution_success/blake3/src/main.nr @@ -1,6 +1,6 @@ use dep::std; fn main(x: [u8; 5], result: [u8; 32]) { - let digest = std::hash::blake3(x); + let digest = std::hash::blake3(x.as_slice()); assert(digest == result); } diff --git a/test_programs/execution_success/brillig_blake2s/src/main.nr b/test_programs/execution_success/brillig_blake2s/src/main.nr index 5bd52666ae9..b358fa8236b 100644 --- a/test_programs/execution_success/brillig_blake2s/src/main.nr +++ b/test_programs/execution_success/brillig_blake2s/src/main.nr @@ -7,5 +7,5 @@ fn main(x: [u8; 5], result: [u8; 32]) { } unconstrained fn blake2s(x: [u8; 5]) -> [u8; 32] { - std::hash::blake2s(x) + std::hash::blake2s(x.as_slice()) } diff --git a/test_programs/execution_success/brillig_blake3/src/main.nr b/test_programs/execution_success/brillig_blake3/src/main.nr index 05a5b31f936..30dc2fb46a2 100644 --- a/test_programs/execution_success/brillig_blake3/src/main.nr +++ b/test_programs/execution_success/brillig_blake3/src/main.nr @@ -1,6 +1,6 @@ use dep::std; unconstrained fn main(x: [u8; 5], result: [u8; 32]) { - let digest = std::hash::blake3(x); + let digest = std::hash::blake3(x.as_slice()); assert(digest == result); } diff --git a/test_programs/execution_success/brillig_cow_regression/src/main.nr b/test_programs/execution_success/brillig_cow_regression/src/main.nr index ba51548d9dd..e2a8ff2fb35 100644 --- a/test_programs/execution_success/brillig_cow_regression/src/main.nr +++ b/test_programs/execution_success/brillig_cow_regression/src/main.nr @@ -25,7 +25,7 @@ struct NewContractData { impl NewContractData { fn hash(self) -> Field { - dep::std::hash::pedersen_hash([self.contract_address, self.portal_contract_address]) + dep::std::hash::pedersen_hash(&[self.contract_address, self.portal_contract_address]) } } @@ -173,6 +173,6 @@ unconstrained fn main(kernel_data: DataToHash) -> pub [Field; NUM_FIELDS_PER_SHA } } - let sha_digest = dep::std::hash::sha256(hash_input_flattened); + let sha_digest = dep::std::hash::sha256(hash_input_flattened.as_slice()); U256::from_bytes32(sha_digest).to_u128_limbs() } diff --git a/test_programs/execution_success/brillig_ecdsa_secp256k1/src/main.nr b/test_programs/execution_success/brillig_ecdsa_secp256k1/src/main.nr index 5d84d885567..64af1132aff 100644 --- a/test_programs/execution_success/brillig_ecdsa_secp256k1/src/main.nr +++ b/test_programs/execution_success/brillig_ecdsa_secp256k1/src/main.nr @@ -12,5 +12,5 @@ unconstrained fn ecdsa( pub_key_y: [u8; 32], signature: [u8; 64] ) -> bool { - std::ecdsa_secp256k1::verify_signature(pub_key_x, pub_key_y, signature, hashed_message) + std::ecdsa_secp256k1::verify_signature(pub_key_x, pub_key_y, signature, hashed_message.as_slice()) } diff --git a/test_programs/execution_success/brillig_ecdsa_secp256r1/src/main.nr b/test_programs/execution_success/brillig_ecdsa_secp256r1/src/main.nr index 9da07f531aa..c96261858fb 100644 --- a/test_programs/execution_success/brillig_ecdsa_secp256r1/src/main.nr +++ b/test_programs/execution_success/brillig_ecdsa_secp256r1/src/main.nr @@ -12,5 +12,5 @@ unconstrained fn ecdsa( pub_key_y: [u8; 32], signature: [u8; 64] ) -> bool { - std::ecdsa_secp256r1::verify_signature(pub_key_x, pub_key_y, signature, hashed_message) + std::ecdsa_secp256r1::verify_signature(pub_key_x, pub_key_y, signature, hashed_message.as_slice()) } diff --git a/test_programs/execution_success/brillig_hash_to_field/src/main.nr b/test_programs/execution_success/brillig_hash_to_field/src/main.nr index 4b4177a521e..53ed85b3ddd 100644 --- a/test_programs/execution_success/brillig_hash_to_field/src/main.nr +++ b/test_programs/execution_success/brillig_hash_to_field/src/main.nr @@ -7,5 +7,5 @@ fn main(input: Field) -> pub Field { } unconstrained fn hash_to_field(input: Field) -> Field { - std::hash::hash_to_field([input]) + std::hash::hash_to_field(&[input]) } diff --git a/test_programs/execution_success/brillig_keccak/src/main.nr b/test_programs/execution_success/brillig_keccak/src/main.nr index 1e9b65a6eb4..641ba485094 100644 --- a/test_programs/execution_success/brillig_keccak/src/main.nr +++ b/test_programs/execution_success/brillig_keccak/src/main.nr @@ -7,7 +7,7 @@ fn main(x: Field, result: [u8; 32]) { // The padding is taken care of by the program let digest = keccak256([x as u8], 1); assert(digest == result); - //#1399: variable meesage size + //#1399: variable message size let message_size = 4; let hash_a = keccak256([1, 2, 3, 4], message_size); let hash_b = keccak256([1, 2, 3, 4, 0, 0, 0, 0], message_size); @@ -21,5 +21,5 @@ fn main(x: Field, result: [u8; 32]) { } unconstrained fn keccak256(data: [u8; N], msg_len: u32) -> [u8; 32] { - std::hash::keccak256(data, msg_len) + std::hash::keccak256(data.as_slice(), msg_len) } diff --git a/test_programs/execution_success/brillig_pedersen/src/main.nr b/test_programs/execution_success/brillig_pedersen/src/main.nr index 2379818c454..3a23a7b3de6 100644 --- a/test_programs/execution_success/brillig_pedersen/src/main.nr +++ b/test_programs/execution_success/brillig_pedersen/src/main.nr @@ -1,11 +1,11 @@ use dep::std; unconstrained fn main(x: Field, y: Field, salt: Field, out_x: Field, out_y: Field, out_hash: Field) { - let res = std::hash::pedersen_commitment_with_separator([x, y], 0); + let res = std::hash::pedersen_commitment_with_separator(&[x, y], 0); assert(res.x == out_x); assert(res.y == out_y); - let res_hash = std::hash::pedersen_hash_with_separator([x, y], 0); + let res_hash = std::hash::pedersen_hash_with_separator(&[x, y], 0); assert_eq(res_hash, out_hash); assert(res_hash != res.x); @@ -16,7 +16,7 @@ unconstrained fn main(x: Field, y: Field, salt: Field, out_x: Field, out_y: Fiel state = state * 8 + raw_data[i]; } state += salt; - let hash = std::hash::pedersen_commitment_with_separator([state], 0); - assert(std::hash::pedersen_commitment_with_separator([43], 0).x == hash.x); + let hash = std::hash::pedersen_commitment_with_separator(&[state], 0); + assert(std::hash::pedersen_commitment_with_separator(&[43], 0).x == hash.x); } diff --git a/test_programs/execution_success/brillig_schnorr/src/main.nr b/test_programs/execution_success/brillig_schnorr/src/main.nr index 4cc79ae7e07..4c7ec16887e 100644 --- a/test_programs/execution_success/brillig_schnorr/src/main.nr +++ b/test_programs/execution_success/brillig_schnorr/src/main.nr @@ -20,6 +20,6 @@ unconstrained fn main( let valid_signature = std::schnorr::verify_signature(pub_key_x, pub_key_y, signature, message_field_bytes); assert(valid_signature); // Check that passing an array as the message is valid - let valid_signature = std::schnorr::verify_signature(pub_key_x, pub_key_y, signature, message); + let valid_signature = std::schnorr::verify_signature(pub_key_x, pub_key_y, signature, message.as_slice()); assert(valid_signature); } diff --git a/test_programs/execution_success/brillig_sha256/src/main.nr b/test_programs/execution_success/brillig_sha256/src/main.nr index e76109df9c3..953bfe05644 100644 --- a/test_programs/execution_success/brillig_sha256/src/main.nr +++ b/test_programs/execution_success/brillig_sha256/src/main.nr @@ -9,6 +9,6 @@ fn main(x: Field, result: [u8; 32]) { unconstrained fn sha256(x: Field) -> [u8; 32] { // We use the `as` keyword here to denote the fact that we want to take just the first byte from the x Field // The padding is taken care of by the program - std::hash::sha256([x as u8]) + std::hash::sha256(&[x as u8]) } diff --git a/test_programs/execution_success/brillig_slices/src/main.nr b/test_programs/execution_success/brillig_slices/src/main.nr index 847c41de25c..2cf1850f151 100644 --- a/test_programs/execution_success/brillig_slices/src/main.nr +++ b/test_programs/execution_success/brillig_slices/src/main.nr @@ -1,6 +1,6 @@ use dep::std::slice; unconstrained fn main(x: Field, y: Field) { - let mut slice: [Field] = [y, x]; + let mut slice: [Field] = &[y, x]; assert(slice.len() == 2); slice = slice.push_back(7); @@ -108,7 +108,7 @@ unconstrained fn merge_slices_else(x: Field) { } // Test returning a merged slice without a mutation unconstrained fn merge_slices_return(x: Field, y: Field) -> [Field] { - let slice = [0; 2]; + let slice = &[0; 2]; if x != y { if x != 20 { slice.push_back(y) } else { slice } } else { @@ -117,7 +117,7 @@ unconstrained fn merge_slices_return(x: Field, y: Field) -> [Field] { } // Test mutating a slice inside of an if statement unconstrained fn merge_slices_mutate(x: Field, y: Field) -> [Field] { - let mut slice = [0; 2]; + let mut slice = &[0; 2]; if x != y { slice = slice.push_back(y); slice = slice.push_back(x); @@ -128,7 +128,7 @@ unconstrained fn merge_slices_mutate(x: Field, y: Field) -> [Field] { } // Test mutating a slice inside of a loop in an if statement unconstrained fn merge_slices_mutate_in_loop(x: Field, y: Field) -> [Field] { - let mut slice = [0; 2]; + let mut slice = &[0; 2]; if x != y { for i in 0..5 { slice = slice.push_back(i as Field); diff --git a/test_programs/execution_success/conditional_1/src/main.nr b/test_programs/execution_success/conditional_1/src/main.nr index 5064c82bce9..f49c24579ba 100644 --- a/test_programs/execution_success/conditional_1/src/main.nr +++ b/test_programs/execution_success/conditional_1/src/main.nr @@ -55,7 +55,7 @@ fn main(a: u32, mut c: [u32; 4], x: [u8; 5], result: pub [u8; 32]) { let mut y = 0; if a == 0 { - let digest = std::hash::sha256(x); + let digest = std::hash::sha256(x.as_slice()); y = digest[0]; } else { y = 5; diff --git a/test_programs/execution_success/conditional_regression_short_circuit/src/main.nr b/test_programs/execution_success/conditional_regression_short_circuit/src/main.nr index d260fa49dc3..7eeefae16e9 100644 --- a/test_programs/execution_success/conditional_regression_short_circuit/src/main.nr +++ b/test_programs/execution_success/conditional_regression_short_circuit/src/main.nr @@ -26,9 +26,9 @@ fn bar(x: Field) { } fn call_intrinsic(x: [u8; 5], result: [u8; 32]) { - let mut digest = std::hash::sha256(x); + let mut digest = std::hash::sha256(x.as_slice()); digest[0] = 5 as u8; - digest = std::hash::sha256(x); + digest = std::hash::sha256(x.as_slice()); assert(digest == result); } diff --git a/test_programs/execution_success/ecdsa_secp256k1/src/main.nr b/test_programs/execution_success/ecdsa_secp256k1/src/main.nr index ac0359e4bb8..d9ffef69644 100644 --- a/test_programs/execution_success/ecdsa_secp256k1/src/main.nr +++ b/test_programs/execution_success/ecdsa_secp256k1/src/main.nr @@ -8,9 +8,9 @@ fn main( signature: [u8; 64] ) { // Hash the message, since secp256k1 expects a hashed_message - let expected = std::hash::sha256(message); + let expected = std::hash::sha256(message.as_slice()); assert(hashed_message == expected); - let valid_signature = std::ecdsa_secp256k1::verify_signature(pub_key_x, pub_key_y, signature, hashed_message); + let valid_signature = std::ecdsa_secp256k1::verify_signature(pub_key_x, pub_key_y, signature, hashed_message.as_slice()); assert(valid_signature); } diff --git a/test_programs/execution_success/ecdsa_secp256r1/src/main.nr b/test_programs/execution_success/ecdsa_secp256r1/src/main.nr index c64e390d652..dda6d688a60 100644 --- a/test_programs/execution_success/ecdsa_secp256r1/src/main.nr +++ b/test_programs/execution_success/ecdsa_secp256r1/src/main.nr @@ -1,6 +1,6 @@ use dep::std; fn main(hashed_message: [u8; 32], pub_key_x: [u8; 32], pub_key_y: [u8; 32], signature: [u8; 64]) { - let valid_signature = std::ecdsa_secp256r1::verify_signature(pub_key_x, pub_key_y, signature, hashed_message); + let valid_signature = std::ecdsa_secp256r1::verify_signature(pub_key_x, pub_key_y, signature, hashed_message.as_slice()); assert(valid_signature); } diff --git a/test_programs/execution_success/hash_to_field/src/main.nr b/test_programs/execution_success/hash_to_field/src/main.nr index 5af1c5af55e..242b5ecbc18 100644 --- a/test_programs/execution_success/hash_to_field/src/main.nr +++ b/test_programs/execution_success/hash_to_field/src/main.nr @@ -1,5 +1,5 @@ use dep::std; fn main(input: Field) -> pub Field { - std::hash::hash_to_field([input]) + std::hash::hash_to_field(&[input]) } diff --git a/test_programs/execution_success/import/src/main.nr b/test_programs/execution_success/import/src/main.nr index 7dcc16fed16..85e7b0c2dfa 100644 --- a/test_programs/execution_success/import/src/main.nr +++ b/test_programs/execution_success/import/src/main.nr @@ -2,7 +2,7 @@ mod import; use crate::import::hello; fn main(x: Field, y: Field) { - let _k = dep::std::hash::pedersen_commitment([x]); + let _k = dep::std::hash::pedersen_commitment(&[x]); let _l = hello(x); assert(x != import::hello(y)); diff --git a/test_programs/execution_success/keccak256/src/main.nr b/test_programs/execution_success/keccak256/src/main.nr index eb401fe614c..24f1695c67d 100644 --- a/test_programs/execution_success/keccak256/src/main.nr +++ b/test_programs/execution_success/keccak256/src/main.nr @@ -4,18 +4,18 @@ use dep::std; fn main(x: Field, result: [u8; 32]) { // We use the `as` keyword here to denote the fact that we want to take just the first byte from the x Field // The padding is taken care of by the program - let digest = std::hash::keccak256([x as u8], 1); + let digest = std::hash::keccak256(&[x as u8], 1); assert(digest == result); //#1399: variable message size let message_size = 4; - let hash_a = std::hash::keccak256([1, 2, 3, 4], message_size); - let hash_b = std::hash::keccak256([1, 2, 3, 4, 0, 0, 0, 0], message_size); + let hash_a = std::hash::keccak256(&[1, 2, 3, 4], message_size); + let hash_b = std::hash::keccak256(&[1, 2, 3, 4, 0, 0, 0, 0], message_size); assert(hash_a == hash_b); let message_size_big = 8; - let hash_c = std::hash::keccak256([1, 2, 3, 4, 0, 0, 0, 0], message_size_big); + let hash_c = std::hash::keccak256(&[1, 2, 3, 4, 0, 0, 0, 0], message_size_big); assert(hash_a != hash_c); } diff --git a/test_programs/execution_success/nested_array_in_slice/src/main.nr b/test_programs/execution_success/nested_array_in_slice/src/main.nr index a3007d5d0dc..0890115e95a 100644 --- a/test_programs/execution_success/nested_array_in_slice/src/main.nr +++ b/test_programs/execution_success/nested_array_in_slice/src/main.nr @@ -13,7 +13,7 @@ fn main(y: Field) { let foo_two = Foo { a: 4, b: [5, 6, 21], bar: Bar { inner: [103, 104, 105] } }; let foo_three = Foo { a: 7, b: [8, 9, 22], bar: Bar { inner: [106, 107, 108] } }; let foo_four = Foo { a: 10, b: [11, 12, 23], bar: Bar { inner: [109, 110, 111] } }; - let mut x = [foo_one]; + let mut x = &[foo_one]; x = x.push_back(foo_two); x = x.push_back(foo_three); x = x.push_back(foo_four); diff --git a/test_programs/execution_success/pedersen_check/src/main.nr b/test_programs/execution_success/pedersen_check/src/main.nr index 90ef218249b..16e3841ea25 100644 --- a/test_programs/execution_success/pedersen_check/src/main.nr +++ b/test_programs/execution_success/pedersen_check/src/main.nr @@ -1,11 +1,11 @@ use dep::std; fn main(x: Field, y: Field, salt: Field, out_x: Field, out_y: Field, out_hash: Field) { - let res = std::hash::pedersen_commitment([x, y]); + let res = std::hash::pedersen_commitment(&[x, y]); assert(res.x == out_x); assert(res.y == out_y); - let res_hash = std::hash::pedersen_hash_with_separator([x, y], 0); + let res_hash = std::hash::pedersen_hash_with_separator(&[x, y], 0); assert_eq(res_hash, out_hash); assert(res_hash != res.x); @@ -16,7 +16,7 @@ fn main(x: Field, y: Field, salt: Field, out_x: Field, out_y: Field, out_hash: F state = state * 8 + raw_data[i]; } state += salt; - let hash = std::hash::pedersen_commitment([state]); - assert(std::hash::pedersen_commitment([43]).x == hash.x); + let hash = std::hash::pedersen_commitment(&[state]); + assert(std::hash::pedersen_commitment(&[43]).x == hash.x); } diff --git a/test_programs/execution_success/pedersen_commitment/src/main.nr b/test_programs/execution_success/pedersen_commitment/src/main.nr index 83cbe20851d..4d07cbffbe6 100644 --- a/test_programs/execution_success/pedersen_commitment/src/main.nr +++ b/test_programs/execution_success/pedersen_commitment/src/main.nr @@ -2,7 +2,7 @@ use dep::std; fn main(x: Field, y: Field, expected_commitment: std::hash::PedersenPoint) { - let commitment = std::hash::pedersen_commitment([x, y]); + let commitment = std::hash::pedersen_commitment(&[x, y]); assert_eq(commitment.x, expected_commitment.x); assert_eq(commitment.y, expected_commitment.y); } diff --git a/test_programs/execution_success/pedersen_hash/src/main.nr b/test_programs/execution_success/pedersen_hash/src/main.nr index 20c7de12d6c..d97495ce3b7 100644 --- a/test_programs/execution_success/pedersen_hash/src/main.nr +++ b/test_programs/execution_success/pedersen_hash/src/main.nr @@ -2,7 +2,7 @@ use dep::std; fn main(x: Field, y: Field, expected_hash: Field) { - let hash = std::hash::pedersen_hash([x, y]); + let hash = std::hash::pedersen_hash(&[x, y]); assert_eq(hash, expected_hash); } // docs:end:pedersen-hash diff --git a/test_programs/execution_success/regression_4202/src/main.nr b/test_programs/execution_success/regression_4202/src/main.nr index 37d2ee4578d..faa14f03912 100644 --- a/test_programs/execution_success/regression_4202/src/main.nr +++ b/test_programs/execution_success/regression_4202/src/main.nr @@ -1,5 +1,5 @@ fn main(input: [u32; 4]) { - let mut slice1: [u32] = [1, 2, 3, 4]; + let mut slice1: [u32] = &[1, 2, 3, 4]; if slice1[0] == 3 { slice1[1] = 4; } diff --git a/test_programs/execution_success/schnorr/src/main.nr b/test_programs/execution_success/schnorr/src/main.nr index 107af152625..4e4e602595c 100644 --- a/test_programs/execution_success/schnorr/src/main.nr +++ b/test_programs/execution_success/schnorr/src/main.nr @@ -20,6 +20,6 @@ fn main( let valid_signature = std::schnorr::verify_signature(pub_key_x, pub_key_y, signature, message_field_bytes); assert(valid_signature); // Check that passing an array as the message is valid - let valid_signature = std::schnorr::verify_signature(pub_key_x, pub_key_y, signature, message); + let valid_signature = std::schnorr::verify_signature(pub_key_x, pub_key_y, signature, message.as_slice()); assert(valid_signature); } diff --git a/test_programs/execution_success/sha256/src/main.nr b/test_programs/execution_success/sha256/src/main.nr index fd5340e2384..5bdd04ff9f4 100644 --- a/test_programs/execution_success/sha256/src/main.nr +++ b/test_programs/execution_success/sha256/src/main.nr @@ -14,6 +14,6 @@ use dep::std; fn main(x: Field, result: [u8; 32]) { // We use the `as` keyword here to denote the fact that we want to take just the first byte from the x Field // The padding is taken care of by the program - let digest = std::hash::sha256([x as u8]); + let digest = std::hash::sha256(&[x as u8]); assert(digest == result); } diff --git a/test_programs/execution_success/sha2_byte/src/main.nr b/test_programs/execution_success/sha2_byte/src/main.nr index fa8ddfbdf69..0a24a9dc98e 100644 --- a/test_programs/execution_success/sha2_byte/src/main.nr +++ b/test_programs/execution_success/sha2_byte/src/main.nr @@ -5,6 +5,6 @@ fn main(x: Field, result256: [u8; 32], result512: [u8; 64]) { let digest256 = std::sha256::digest([x as u8]); assert(digest256 == result256); - let digest512 = std::sha512::digest([x as u8]); + let digest512 = std::sha512::digest(&[x as u8]); assert(digest512 == result512); } diff --git a/test_programs/execution_success/simple_shield/src/main.nr b/test_programs/execution_success/simple_shield/src/main.nr index c46d3b4594c..e33326ab749 100644 --- a/test_programs/execution_success/simple_shield/src/main.nr +++ b/test_programs/execution_success/simple_shield/src/main.nr @@ -17,11 +17,11 @@ fn main( let pubkey_x = pubkey[0]; let pubkey_y = pubkey[1]; // Compute input note commitment - let note_commitment = std::hash::pedersen_commitment([pubkey_x, pubkey_y]); + let note_commitment = std::hash::pedersen_commitment(&[pubkey_x, pubkey_y]); // Compute input note nullifier - let nullifier = std::hash::pedersen_commitment([note_commitment.x, index, priv_key]); + let nullifier = std::hash::pedersen_commitment(&[note_commitment.x, index, priv_key]); // Compute output note nullifier - let receiver_note_commitment = std::hash::pedersen_commitment([to_pubkey_x, to_pubkey_y]); + let receiver_note_commitment = std::hash::pedersen_commitment(&[to_pubkey_x, to_pubkey_y]); // Check that the input note nullifier is in the root assert(note_root == std::merkle::compute_merkle_root(note_commitment.x, index, note_hash_path)); diff --git a/test_programs/execution_success/slice_dynamic_index/src/main.nr b/test_programs/execution_success/slice_dynamic_index/src/main.nr index 41fc9a645c1..9b97c0721bb 100644 --- a/test_programs/execution_success/slice_dynamic_index/src/main.nr +++ b/test_programs/execution_success/slice_dynamic_index/src/main.nr @@ -4,7 +4,7 @@ fn main(x: Field) { } fn regression_dynamic_slice_index(x: Field, y: Field) { - let mut slice = []; + let mut slice = &[]; for i in 0..5 { slice = slice.push_back(i as Field); } diff --git a/test_programs/execution_success/slices/src/main.nr b/test_programs/execution_success/slices/src/main.nr index eca42a660c4..0d253c8b2b2 100644 --- a/test_programs/execution_success/slices/src/main.nr +++ b/test_programs/execution_success/slices/src/main.nr @@ -2,7 +2,7 @@ use dep::std::slice; use dep::std; fn main(x: Field, y: pub Field) { - let mut slice = [0; 2]; + let mut slice = &[0; 2]; assert(slice[0] == 0); assert(slice[0] != 1); slice[0] = x; @@ -13,7 +13,7 @@ fn main(x: Field, y: pub Field) { assert(slice_plus_10[2] != 8); assert(slice_plus_10.len() == 3); - let mut new_slice = []; + let mut new_slice = &[]; for i in 0..5 { new_slice = new_slice.push_back(i); } @@ -41,7 +41,7 @@ fn main(x: Field, y: pub Field) { assert(remove_slice[3] == 3); assert(remove_slice.len() == 4); - let append = [1, 2].append([3, 4, 5]); + let append = &[1, 2].append(&[3, 4, 5]); assert(append.len() == 5); assert(append[0] == 1); assert(append[4] == 5); @@ -51,9 +51,10 @@ fn main(x: Field, y: pub Field) { regression_merge_slices(x, y); regression_2370(); } + // Ensure that slices of struct/tuple values work. fn regression_2083() { - let y = [(1, 2)]; + let y = &[(1, 2)]; let y = y.push_back((3, 4)); // [(1, 2), (3, 4)] let y = y.push_back((5, 6)); // [(1, 2), (3, 4), (5, 6)] assert(y[2].1 == 6); @@ -82,6 +83,7 @@ fn regression_2083() { assert(x.0 == 5); assert(x.1 == 6); } + // The parameters to this function must come from witness values (inputs to main) fn regression_merge_slices(x: Field, y: Field) { merge_slices_if(x, y); @@ -142,18 +144,20 @@ fn merge_slices_else(x: Field) { assert(slice[2] == 5); assert(slice.len() == 3); } + // Test returning a merged slice without a mutation fn merge_slices_return(x: Field, y: Field) -> [Field] { - let slice = [0; 2]; + let slice = &[0; 2]; if x != y { if x != 20 { slice.push_back(y) } else { slice } } else { slice } } + // Test mutating a slice inside of an if statement fn merge_slices_mutate(x: Field, y: Field) -> [Field] { - let mut slice = [0; 2]; + let mut slice = &[0; 2]; if x != y { slice = slice.push_back(y); slice = slice.push_back(x); @@ -162,9 +166,10 @@ fn merge_slices_mutate(x: Field, y: Field) -> [Field] { } slice } + // Test mutating a slice inside of a loop in an if statement fn merge_slices_mutate_in_loop(x: Field, y: Field) -> [Field] { - let mut slice = [0; 2]; + let mut slice = &[0; 2]; if x != y { for i in 0..5 { slice = slice.push_back(i as Field); @@ -176,7 +181,7 @@ fn merge_slices_mutate_in_loop(x: Field, y: Field) -> [Field] { } fn merge_slices_mutate_two_ifs(x: Field, y: Field) -> [Field] { - let mut slice = [0; 2]; + let mut slice = &[0; 2]; if x != y { slice = slice.push_back(y); slice = slice.push_back(x); @@ -195,7 +200,7 @@ fn merge_slices_mutate_two_ifs(x: Field, y: Field) -> [Field] { } fn merge_slices_mutate_between_ifs(x: Field, y: Field) -> [Field] { - let mut slice = [0; 2]; + let mut slice = &[0; 2]; if x != y { slice = slice.push_back(y); slice = slice.push_back(x); @@ -221,7 +226,7 @@ fn merge_slices_mutate_between_ifs(x: Field, y: Field) -> [Field] { } fn merge_slices_push_then_pop(x: Field, y: Field) { - let mut slice = [0; 2]; + let mut slice = &[0; 2]; if x != y { slice = slice.push_back(y); slice = slice.push_back(x); @@ -245,7 +250,7 @@ fn merge_slices_push_then_pop(x: Field, y: Field) { } fn merge_slices_push_then_insert(x: Field, y: Field) -> [Field] { - let mut slice = [0; 2]; + let mut slice = &[0; 2]; if x != y { slice = slice.push_back(y); slice = slice.push_back(x); @@ -268,7 +273,7 @@ fn merge_slices_push_then_insert(x: Field, y: Field) -> [Field] { } fn merge_slices_remove_between_ifs(x: Field, y: Field) -> [Field] { - let mut slice = [0; 2]; + let mut slice = &[0; 2]; if x != y { slice = slice.push_back(y); slice = slice.push_back(x); @@ -294,6 +299,6 @@ fn merge_slices_remove_between_ifs(x: Field, y: Field) -> [Field] { // Previously, we'd get a type error when trying to assign an array of a different size to // an existing array variable. Now, we infer the variable must be a slice. fn regression_2370() { - let mut slice = []; - slice = [1, 2, 3]; + let mut slice = &[]; + slice = &[1, 2, 3]; } diff --git a/test_programs/execution_success/strings/src/main.nr b/test_programs/execution_success/strings/src/main.nr index cff229d368a..6f039fef285 100644 --- a/test_programs/execution_success/strings/src/main.nr +++ b/test_programs/execution_success/strings/src/main.nr @@ -27,7 +27,7 @@ fn main(message: pub str<11>, y: Field, hex_as_string: str<4>, hex_as_field: Fie std::print(bad_message); assert(message != bad_message); - let hash = std::hash::pedersen_commitment([x]); + let hash = std::hash::pedersen_commitment(&[x]); std::println(hash); std::print(hash); @@ -61,7 +61,7 @@ fn test_prints_array() { std::println(array); - let hash = std::hash::pedersen_commitment(array); + let hash = std::hash::pedersen_commitment(array.as_slice()); std::println(hash); } diff --git a/test_programs/noir_test_success/should_fail_with_matches/src/main.nr b/test_programs/noir_test_success/should_fail_with_matches/src/main.nr index d2b7d155a32..5eda9db4d1a 100644 --- a/test_programs/noir_test_success/should_fail_with_matches/src/main.nr +++ b/test_programs/noir_test_success/should_fail_with_matches/src/main.nr @@ -10,10 +10,10 @@ fn test_should_fail_without_match() { #[test(should_fail_with = "Not equal")] fn test_should_fail_with_runtime_match() { - assert_eq(dep::std::hash::pedersen_commitment([27]).x, 0, "Not equal"); + assert_eq(dep::std::hash::pedersen_commitment(&[27]).x, 0, "Not equal"); } #[test(should_fail)] fn test_should_fail_without_runtime_match() { - assert_eq(dep::std::hash::pedersen_commitment([27]).x, 0); + assert_eq(dep::std::hash::pedersen_commitment(&[27]).x, 0); } diff --git a/tooling/nargo/src/artifacts/debug_vars.rs b/tooling/nargo/src/artifacts/debug_vars.rs index 0921dbd3e7c..86cb4256635 100644 --- a/tooling/nargo/src/artifacts/debug_vars.rs +++ b/tooling/nargo/src/artifacts/debug_vars.rs @@ -62,7 +62,10 @@ impl DebugVars { .unwrap_or_else(|| panic!("type unavailable for type id {cursor_type_id:?}")); for index in indexes.iter() { (cursor, cursor_type) = match (cursor, cursor_type) { - (PrintableValue::Vec { array_elements, is_slice }, PrintableType::Array { length, typ }) => { + ( + PrintableValue::Vec { array_elements, is_slice }, + PrintableType::Array { length, typ }, + ) => { assert!(!*is_slice, "slice has array type"); if let Some(len) = length { if *index as u64 >= *len { @@ -74,7 +77,10 @@ impl DebugVars { } (array_elements.get_mut(*index as usize).unwrap(), &*Box::leak(typ.clone())) } - (PrintableValue::Vec { array_elements, is_slice }, PrintableType::Slice { typ }) => { + ( + PrintableValue::Vec { array_elements, is_slice }, + PrintableType::Slice { typ }, + ) => { assert!(*is_slice, "array has slice type"); (array_elements.get_mut(*index as usize).unwrap(), &*Box::leak(typ.clone())) } @@ -88,7 +94,10 @@ impl DebugVars { let (key, typ) = fields.get(*index as usize).unwrap(); (field_map.get_mut(key).unwrap(), typ) } - (PrintableValue::Vec { array_elements, is_slice }, PrintableType::Tuple { types }) => { + ( + PrintableValue::Vec { array_elements, is_slice }, + PrintableType::Tuple { types }, + ) => { assert!(!*is_slice, "slice has tuple type"); if *index >= types.len() as u32 { panic!( diff --git a/tooling/nargo_cli/build.rs b/tooling/nargo_cli/build.rs index 1ca12b75dfb..0a40810bdab 100644 --- a/tooling/nargo_cli/build.rs +++ b/tooling/nargo_cli/build.rs @@ -203,8 +203,8 @@ fn compile_success_empty_{test_name}() {{ }} // `compile_success_empty` tests should be able to compile down to an empty circuit. - let json: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap_or_else(|_| {{ - panic!("JSON was not well-formatted {{:?}}",output.stdout) + let json: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap_or_else(|e| {{ + panic!("JSON was not well-formatted {{:?}}\n\n{{:?}}", e, std::str::from_utf8(&output.stdout)) }}); let num_opcodes = &json["programs"][0]["acir_opcodes"]; assert_eq!(num_opcodes.as_u64().expect("number of opcodes should fit in a u64"), 0); diff --git a/tooling/nargo_fmt/src/rewrite/array.rs b/tooling/nargo_fmt/src/rewrite/array.rs index 4499760c1a4..db7dc4701b7 100644 --- a/tooling/nargo_fmt/src/rewrite/array.rs +++ b/tooling/nargo_fmt/src/rewrite/array.rs @@ -6,7 +6,12 @@ use crate::{ visitor::{expr::NewlineMode, FmtVisitor}, }; -pub(crate) fn rewrite(mut visitor: FmtVisitor, array: Vec, array_span: Span, is_slice: bool) -> String { +pub(crate) fn rewrite( + mut visitor: FmtVisitor, + array: Vec, + array_span: Span, + is_slice: bool, +) -> String { let pattern: &[_] = &[' ', '\t']; visitor.indent.block_indent(visitor.config); @@ -75,11 +80,7 @@ pub(crate) fn rewrite(mut visitor: FmtVisitor, array: Vec, array_spa } } - let open_bracket = if is_slice { - "&[" - } else { - "[" - }; + let open_bracket = if is_slice { "&[" } else { "[" }; crate::visitor::expr::wrap_exprs( open_bracket, "]", From 64738057a6178440f3ab7f633ee4bb803565dec1 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Sun, 10 Mar 2024 22:52:25 -0400 Subject: [PATCH 06/28] update poseidon/mimic hash slice literals, add eq/ord/default instances for slices, fix slice literal in regression test --- noir_stdlib/src/cmp.nr | 30 +++++++++++++++++++ noir_stdlib/src/default.nr | 6 ++++ noir_stdlib/src/hash/mimc.nr | 2 +- noir_stdlib/src/hash/poseidon.nr | 2 +- noir_stdlib/src/hash/poseidon2.nr | 2 +- .../execution_success/slices/src/main.nr | 2 +- 6 files changed, 40 insertions(+), 4 deletions(-) diff --git a/noir_stdlib/src/cmp.nr b/noir_stdlib/src/cmp.nr index 38316e5d6a8..ed89cf89f7f 100644 --- a/noir_stdlib/src/cmp.nr +++ b/noir_stdlib/src/cmp.nr @@ -28,6 +28,16 @@ impl Eq for [T; N] where T: Eq { } } +impl Eq for [T] where T: Eq { + fn eq(self, other: [T]) -> bool { + let mut result = true; + for i in 0 .. self.len() { + result &= self[i].eq(other[i]); + } + result + } +} + impl Eq for str { fn eq(self, other: str) -> bool { let self_bytes = self.as_bytes(); @@ -213,6 +223,26 @@ impl Ord for [T; N] where T: Ord { } } +impl Ord for [T] where T: Ord { + // The first non-equal element of both arrays determines + // the ordering for the whole array. + fn cmp(self, other: [T]) -> Ordering { + let mut result = Ordering::equal(); + for i in 0 .. self.len() { + if result == Ordering::equal() { + let result_i = self[i].cmp(other[i]); + + if result_i == Ordering::less() { + result = result_i; + } else if result_i == Ordering::greater() { + result = result_i; + } + } + } + result + } +} + impl Ord for (A, B) where A: Ord, B: Ord { fn cmp(self, other: (A, B)) -> Ordering { let result = self.0.cmp(other.0); diff --git a/noir_stdlib/src/default.nr b/noir_stdlib/src/default.nr index 32c4f3f3b48..bd2f1ce0cd2 100644 --- a/noir_stdlib/src/default.nr +++ b/noir_stdlib/src/default.nr @@ -23,6 +23,12 @@ impl Default for [T; N] where T: Default { } } +impl Default for [T] { + fn default() -> [T] { + &[] + } +} + impl Default for (A, B) where A: Default, B: Default { fn default() -> (A, B) { (A::default(), B::default()) diff --git a/noir_stdlib/src/hash/mimc.nr b/noir_stdlib/src/hash/mimc.nr index db8a32d7909..1fb53701013 100644 --- a/noir_stdlib/src/hash/mimc.nr +++ b/noir_stdlib/src/hash/mimc.nr @@ -152,7 +152,7 @@ impl Hasher for MimcHasher { impl Default for MimcHasher{ fn default() -> Self{ MimcHasher{ - _state: [], + _state: &[], _len: 0, } } diff --git a/noir_stdlib/src/hash/poseidon.nr b/noir_stdlib/src/hash/poseidon.nr index 7f99ad36316..85a0802f630 100644 --- a/noir_stdlib/src/hash/poseidon.nr +++ b/noir_stdlib/src/hash/poseidon.nr @@ -171,7 +171,7 @@ impl Hasher for PoseidonHasher { impl Default for PoseidonHasher{ fn default() -> Self{ PoseidonHasher{ - _state: [], + _state: &[], _len: 0, } } diff --git a/noir_stdlib/src/hash/poseidon2.nr b/noir_stdlib/src/hash/poseidon2.nr index 52229f18dbd..5b97d809896 100644 --- a/noir_stdlib/src/hash/poseidon2.nr +++ b/noir_stdlib/src/hash/poseidon2.nr @@ -139,7 +139,7 @@ impl Hasher for Poseidon2Hasher { impl Default for Poseidon2Hasher{ fn default() -> Self{ Poseidon2Hasher{ - _state: [], + _state: &[], _len: 0, } } diff --git a/test_programs/execution_success/slices/src/main.nr b/test_programs/execution_success/slices/src/main.nr index 6c293ba2571..b20e3478898 100644 --- a/test_programs/execution_success/slices/src/main.nr +++ b/test_programs/execution_success/slices/src/main.nr @@ -332,6 +332,6 @@ fn regression_slice_call_result(x: Field, y: Field) { } fn regression_4506() { - let slice: [Field] = [1, 2, 3]; + let slice: [Field] = &[1, 2, 3]; assert(slice == slice); } From 4f66228ce97360767da77d30f5b19c1dae732511 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Sun, 10 Mar 2024 23:57:41 -0400 Subject: [PATCH 07/28] added missing element_type_at_index case for slices, fixed all but one debugger error (timeout in brillig_cow_regression) --- compiler/noirc_frontend/src/monomorphization/debug.rs | 3 ++- tooling/debugger/tests/debug.rs | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/monomorphization/debug.rs b/compiler/noirc_frontend/src/monomorphization/debug.rs index cf4e0ab792e..9c682ac119b 100644 --- a/compiler/noirc_frontend/src/monomorphization/debug.rs +++ b/compiler/noirc_frontend/src/monomorphization/debug.rs @@ -192,11 +192,12 @@ impl<'interner> Monomorphizer<'interner> { fn element_type_at_index(ptype: &PrintableType, i: usize) -> &PrintableType { match ptype { PrintableType::Array { length: _length, typ } => typ.as_ref(), + PrintableType::Slice { typ } => typ.as_ref(), PrintableType::Tuple { types } => &types[i], PrintableType::Struct { name: _name, fields } => &fields[i].1, PrintableType::String { length: _length } => &PrintableType::UnsignedInteger { width: 8 }, _ => { - panic!["expected type with sub-fields, found terminal type"] + panic!("expected type with sub-fields, found terminal type: {:?}", ptype) } } } diff --git a/tooling/debugger/tests/debug.rs b/tooling/debugger/tests/debug.rs index 143ee7987f8..4de1b25f48a 100644 --- a/tooling/debugger/tests/debug.rs +++ b/tooling/debugger/tests/debug.rs @@ -12,7 +12,9 @@ mod tests { let nargo_bin = cargo_bin("nargo").into_os_string().into_string().expect("Cannot parse nargo path"); - let mut dbg_session = spawn_bash(Some(15000)).expect("Could not start bash session"); + let timeout_seconds = 120; + let mut dbg_session = + spawn_bash(Some(timeout_seconds * 1000)).expect("Could not start bash session"); // Set backend to `/dev/null` to force an error if nargo tries to speak to a backend. dbg_session From cd2807d02c946931d969cf7699812d63642894cb Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 11 Mar 2024 00:06:26 -0400 Subject: [PATCH 08/28] update slice literal in pedersen_hash wasm example --- compiler/wasm/test/fixtures/deps/lib-c/src/module/foo.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/wasm/test/fixtures/deps/lib-c/src/module/foo.nr b/compiler/wasm/test/fixtures/deps/lib-c/src/module/foo.nr index e0c82fb1960..877f932b038 100644 --- a/compiler/wasm/test/fixtures/deps/lib-c/src/module/foo.nr +++ b/compiler/wasm/test/fixtures/deps/lib-c/src/module/foo.nr @@ -1,3 +1,3 @@ pub fn bar(param: Field) -> Field { - dep::std::hash::pedersen_hash([param]) + dep::std::hash::pedersen_hash(&[param]) } From 7a50955933b0b118e9877fb5b85ebb50c9318829 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 11 Mar 2024 09:41:50 -0400 Subject: [PATCH 09/28] remove redundant match on array vs slice --- compiler/noirc_frontend/src/hir_def/types.rs | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 63d156c408e..fc23d8beb3f 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1295,18 +1295,13 @@ impl Type { let target = target.follow_bindings(); if let (Type::Array(size, element1), Type::Slice(element2)) = (&this, &target) { - let length = size.follow_bindings(); - - // If we have an array and our target is a slice - if matches!(length, Type::Constant(_)) { - // Still have to ensure the element types match. - // Don't need to issue an error here if not, it will be done in unify_with_coercions - let mut bindings = TypeBindings::new(); - if element1.try_unify(element2, &mut bindings).is_ok() { - convert_array_expression_to_slice(expression, this, target, interner); - Self::apply_type_bindings(bindings); - return true; - } + // Still have to ensure the element types match. + // Don't need to issue an error here if not, it will be done in unify_with_coercions + let mut bindings = TypeBindings::new(); + if element1.try_unify(element2, &mut bindings).is_ok() { + convert_array_expression_to_slice(expression, this, target, interner); + Self::apply_type_bindings(bindings); + return true; } } false From 4adc6915ab8096e3fbe953cd1a30781aaac853a7 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 11 Mar 2024 09:51:30 -0400 Subject: [PATCH 10/28] remove unused variable, flip result type order --- .../noirc_frontend/src/hir/type_check/expr.rs | 19 ++++++++++--------- compiler/noirc_frontend/src/hir_def/types.rs | 2 +- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 5bae9a0b3ef..856bb2fed23 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -51,7 +51,7 @@ impl<'interner> TypeChecker<'interner> { fn check_hir_array_literal( &mut self, hir_array_literal: HirArrayLiteral, - ) -> (Result, u64>, Box) { + ) -> (Result>, Box) { match hir_array_literal { HirArrayLiteral::Standard(arr) => { let elem_types = vecmap(&arr, |arg| self.check_expression(arg)); @@ -78,13 +78,13 @@ impl<'interner> TypeChecker<'interner> { }); } - (Err(arr.len() as u64), Box::new(first_elem_type.clone())) + (Ok(arr.len() as u64), Box::new(first_elem_type.clone())) } HirArrayLiteral::Repeated { repeated_element, length } => { let elem_type = self.check_expression(&repeated_element); let length = match length { - Type::Constant(length) => Err(length), - other => Ok(Box::new(other)), + Type::Constant(length) => Ok(length), + other => Err(Box::new(other)), }; (length, Box::new(elem_type)) } @@ -106,22 +106,23 @@ impl<'interner> TypeChecker<'interner> { HirLiteral::Array(hir_array_literal) => { let (length, elem_type) = self.check_hir_array_literal(hir_array_literal); Type::Array( - length.unwrap_or_else(|constant| { - Box::new(Type::constant_variable(constant, self.interner)) - }), + length.map_or_else( + |typ| typ, + |constant| Box::new(Type::constant_variable(constant, self.interner)), + ), elem_type, ) } HirLiteral::Slice(hir_array_literal) => { let (length_type, elem_type) = self.check_hir_array_literal(hir_array_literal); match length_type { - Ok(_non_constant) => { + Ok(_length) => Type::Slice(elem_type), + Err(_non_constant) => { self.errors.push(TypeCheckError::NonConstantSliceLength { span: self.interner.expr_span(expr_id), }); Type::Error } - Err(_length) => Type::Slice(elem_type), } } HirLiteral::Bool(_) => Type::Bool, diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index fc23d8beb3f..309671aaf0f 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1294,7 +1294,7 @@ impl Type { let this = self.follow_bindings(); let target = target.follow_bindings(); - if let (Type::Array(size, element1), Type::Slice(element2)) = (&this, &target) { + if let (Type::Array(_size, element1), Type::Slice(element2)) = (&this, &target) { // Still have to ensure the element types match. // Don't need to issue an error here if not, it will be done in unify_with_coercions let mut bindings = TypeBindings::new(); From d986bfc5aa8282f70a8c3584e17d483b68b00a21 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 11 Mar 2024 14:07:58 -0400 Subject: [PATCH 11/28] add as_slice builtin function, add execution test --- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 58 +++++++++++++++++++ .../noirc_evaluator/src/ssa/ir/instruction.rs | 4 ++ .../src/ssa/ir/instruction/call.rs | 10 ++++ noir_stdlib/src/array.nr | 10 +--- .../array_to_slice/Nargo.toml | 7 +++ .../array_to_slice/Prover.toml | 2 + .../array_to_slice/src/main.nr | 23 ++++++++ 7 files changed, 106 insertions(+), 8 deletions(-) create mode 100644 test_programs/execution_success/array_to_slice/Nargo.toml create mode 100644 test_programs/execution_success/array_to_slice/Prover.toml create mode 100644 test_programs/execution_success/array_to_slice/src/main.nr diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 8d4d0668534..9519e7565ba 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1663,6 +1663,64 @@ impl Context { }; Ok(vec![AcirValue::Var(self.acir_context.add_constant(len), AcirType::field())]) } + Intrinsic::AsSlice => { + let elements = match self.convert_value(arguments[0], dfg) { + AcirValue::Array(values) => values, + _ => unreachable!("Non-array passed to as_slice method"), + }; + + let slice_length = self.acir_context.add_constant(elements.len()); + let (slice_contents, slice_typ, _) = + self.check_array_is_initialized(arguments[0], dfg)?; + assert!(!slice_typ.is_nested_slice(), "ICE: Nested slice used in ACIR generation"); + + let slice = self.convert_value(slice_contents, dfg); + let mut new_elem_size = Self::flattened_value_size(&slice); + + let mut new_slice = elements.clone(); + self.slice_intrinsic_input(&mut new_slice, slice)?; + + let new_slice_val = AcirValue::Array(new_slice); + let result_block_id = self.block_id(&result_ids[1]); + self.initialize_array(result_block_id, new_elem_size, Some(new_slice_val.clone()))?; + // The previous slice length represents the index we want to write into. + // Write the elements we wish to push back directly. + // The slice's underlying array value should already be filled with dummy data + // to enable this write to be within bounds. + // The dummy data is either attached during SSA gen or in this match case for non-nested slices. + // These values can then be accessed due to the increased dynamic slice length. + let mut var_index = slice_length; + for elem in elements { + new_elem_size += Self::flattened_value_size(&elem); + self.array_set_value(&elem, result_block_id, &mut var_index)?; + } + + let element_type_sizes = if !can_omit_element_sizes_array(&slice_typ) { + Some(self.init_element_type_sizes_array( + &slice_typ, + slice_contents, + Some(&new_slice_val), + dfg, + )?) + } else { + None + }; + + let value_types = new_slice_val.flat_numeric_types(); + assert_eq!( + value_types.len(), + new_elem_size, + "ICE: Value types array must match new slice size" + ); + + let result = AcirValue::DynamicArray(AcirDynamicArray { + block_id: result_block_id, + len: new_elem_size, + value_types, + element_type_sizes, + }); + Ok(vec![AcirValue::Var(slice_length, AcirType::field()), result]) + } Intrinsic::SlicePushBack => { // arguments = [slice_length, slice_contents, ...elements_to_push] let slice_length = self.convert_value(arguments[0], dfg).into_var()?; diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index 0b6c7074e45..709c87ee0f1 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -37,6 +37,7 @@ pub(crate) type InstructionId = Id; #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub(crate) enum Intrinsic { ArrayLen, + AsSlice, AssertConstant, SlicePushBack, SlicePushFront, @@ -57,6 +58,7 @@ impl std::fmt::Display for Intrinsic { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Intrinsic::ArrayLen => write!(f, "array_len"), + Intrinsic::AsSlice => write!(f, "as_slice"), Intrinsic::AssertConstant => write!(f, "assert_constant"), Intrinsic::SlicePushBack => write!(f, "slice_push_back"), Intrinsic::SlicePushFront => write!(f, "slice_push_front"), @@ -89,6 +91,7 @@ impl Intrinsic { Intrinsic::ToBits(_) | Intrinsic::ToRadix(_) => true, Intrinsic::ArrayLen + | Intrinsic::AsSlice | Intrinsic::SlicePushBack | Intrinsic::SlicePushFront | Intrinsic::SlicePopBack @@ -109,6 +112,7 @@ impl Intrinsic { pub(crate) fn lookup(name: &str) -> Option { match name { "array_len" => Some(Intrinsic::ArrayLen), + "as_slice" => Some(Intrinsic::AsSlice), "assert_constant" => Some(Intrinsic::AssertConstant), "apply_range_constraint" => Some(Intrinsic::ApplyRangeConstraint), "slice_push_back" => Some(Intrinsic::SlicePushBack), diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index 9349d58c4d9..8b800e0db54 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -84,6 +84,16 @@ pub(super) fn simplify_call( SimplifyResult::None } } + Intrinsic::AsSlice => { + let slice = dfg.get_array_constant(arguments[0]); + if let Some((slice, element_type)) = slice { + let slice_length = dfg.make_constant(slice.len().into(), Type::length_type()); + let new_slice = dfg.make_array(slice, element_type); + SimplifyResult::SimplifiedToMultiple(vec![slice_length, new_slice]) + } else { + SimplifyResult::None + } + } Intrinsic::SlicePushBack => { let slice = dfg.get_array_constant(arguments[1]); if let Some((mut slice, element_type)) = slice { diff --git a/noir_stdlib/src/array.nr b/noir_stdlib/src/array.nr index 3da4b649174..fd91d7dad9b 100644 --- a/noir_stdlib/src/array.nr +++ b/noir_stdlib/src/array.nr @@ -52,14 +52,8 @@ impl [T; N] { result } - // Converts an array into a slice. - pub fn as_slice(self) -> [T] { - let mut slice = []; - for elem in self { - slice = slice.push_back(elem); - } - slice - } + #[builtin(as_slice)] + pub fn as_slice(self) -> [T] {} // Apply a function to each element of an array, returning a new array // containing the mapped elements. diff --git a/test_programs/execution_success/array_to_slice/Nargo.toml b/test_programs/execution_success/array_to_slice/Nargo.toml new file mode 100644 index 00000000000..90c67b07b2b --- /dev/null +++ b/test_programs/execution_success/array_to_slice/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "array_to_slice" +type = "bin" +authors = [""] +compiler_version = ">=0.24.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_success/array_to_slice/Prover.toml b/test_programs/execution_success/array_to_slice/Prover.toml new file mode 100644 index 00000000000..26fdbc19975 --- /dev/null +++ b/test_programs/execution_success/array_to_slice/Prover.toml @@ -0,0 +1,2 @@ +x = "0" +y = "1" diff --git a/test_programs/execution_success/array_to_slice/src/main.nr b/test_programs/execution_success/array_to_slice/src/main.nr new file mode 100644 index 00000000000..06966f201f4 --- /dev/null +++ b/test_programs/execution_success/array_to_slice/src/main.nr @@ -0,0 +1,23 @@ +// Converts an array into a slice. +fn as_slice_push(xs: [T; N]) -> [T] { + let mut slice = []; + for elem in xs { + slice = slice.push_back(elem); + } + slice +} + +fn main(x: Field, y: pub Field) { + let xs: [Field; 0] = []; + let ys: [Field; 1] = [1]; + let zs: [Field; 2] = [1, 2]; + let ws: [Field; 3] = [1; 3]; + let qs: [Field; 4] = [3, 2, 1, 0]; + + assert(x != y); + assert(xs.as_slice() == as_slice_push(xs)); + assert(ys.as_slice() == as_slice_push(ys)); + assert(zs.as_slice() == as_slice_push(zs)); + assert(ws.as_slice() == as_slice_push(ws)); + assert(qs.as_slice() == as_slice_push(qs)); +} From 462af6456db1013fada332aa56ab38debe035bde Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 11 Mar 2024 15:22:06 -0400 Subject: [PATCH 12/28] wip debugging test failures, add dynamic array case to test --- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 49 ++++++------------- .../array_to_slice/src/main.nr | 9 ++++ 2 files changed, 23 insertions(+), 35 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 9519e7565ba..55a3a3466a7 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1664,58 +1664,37 @@ impl Context { Ok(vec![AcirValue::Var(self.acir_context.add_constant(len), AcirType::field())]) } Intrinsic::AsSlice => { - let elements = match self.convert_value(arguments[0], dfg) { - AcirValue::Array(values) => values, - _ => unreachable!("Non-array passed to as_slice method"), - }; - - let slice_length = self.acir_context.add_constant(elements.len()); - let (slice_contents, slice_typ, _) = + let (slice_contents, slice_typ, block_id) = self.check_array_is_initialized(arguments[0], dfg)?; assert!(!slice_typ.is_nested_slice(), "ICE: Nested slice used in ACIR generation"); - let slice = self.convert_value(slice_contents, dfg); - let mut new_elem_size = Self::flattened_value_size(&slice); - - let mut new_slice = elements.clone(); - self.slice_intrinsic_input(&mut new_slice, slice)?; - - let new_slice_val = AcirValue::Array(new_slice); let result_block_id = self.block_id(&result_ids[1]); - self.initialize_array(result_block_id, new_elem_size, Some(new_slice_val.clone()))?; - // The previous slice length represents the index we want to write into. - // Write the elements we wish to push back directly. - // The slice's underlying array value should already be filled with dummy data - // to enable this write to be within bounds. - // The dummy data is either attached during SSA gen or in this match case for non-nested slices. - // These values can then be accessed due to the increased dynamic slice length. - let mut var_index = slice_length; - for elem in elements { - new_elem_size += Self::flattened_value_size(&elem); - self.array_set_value(&elem, result_block_id, &mut var_index)?; - } - let element_type_sizes = if !can_omit_element_sizes_array(&slice_typ) { Some(self.init_element_type_sizes_array( &slice_typ, slice_contents, - Some(&new_slice_val), + None, dfg, )?) } else { None }; - let value_types = new_slice_val.flat_numeric_types(); - assert_eq!( - value_types.len(), - new_elem_size, - "ICE: Value types array must match new slice size" - ); + // TODO: remove this and the assert + let array_len = if !slice_typ.contains_slice_element() { + slice_typ.flattened_size() + } else { + self.flattened_slice_size(slice_contents, dfg) + }; + let value_types = self.convert_value(slice_contents, dfg).flat_numeric_types(); + assert!(array_len == value_types.len(), "AsSlice: unexpected length difference: {:?} != {:?}", array_len, value_types.len()); + + let slice_length = self.acir_context.add_constant(value_types.len()); + self.copy_dynamic_array(block_id, result_block_id, value_types.len())?; let result = AcirValue::DynamicArray(AcirDynamicArray { block_id: result_block_id, - len: new_elem_size, + len: value_types.len(), value_types, element_type_sizes, }); diff --git a/test_programs/execution_success/array_to_slice/src/main.nr b/test_programs/execution_success/array_to_slice/src/main.nr index 06966f201f4..879607f8f8a 100644 --- a/test_programs/execution_success/array_to_slice/src/main.nr +++ b/test_programs/execution_success/array_to_slice/src/main.nr @@ -14,10 +14,19 @@ fn main(x: Field, y: pub Field) { let ws: [Field; 3] = [1; 3]; let qs: [Field; 4] = [3, 2, 1, 0]; + let mut dynamic: [Field; 4] = [3, 2, 1, 0]; + let dynamic_expected: [Field] = [1000, 2, 1, 0]; + dynamic[x] = 1000; + assert(x != y); assert(xs.as_slice() == as_slice_push(xs)); assert(ys.as_slice() == as_slice_push(ys)); assert(zs.as_slice() == as_slice_push(zs)); assert(ws.as_slice() == as_slice_push(ws)); assert(qs.as_slice() == as_slice_push(qs)); + assert(dynamic.as_slice()[0] == dynamic_expected[0]); + assert(dynamic.as_slice()[1] == dynamic_expected[1]); + assert(dynamic.as_slice()[2] == dynamic_expected[2]); + assert(dynamic.as_slice()[3] == dynamic_expected[3]); + assert(dynamic.as_slice().len() == 4); } From 2b749e124e31419aeb48c821577ea29c5d53d84c Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 11 Mar 2024 15:55:25 -0400 Subject: [PATCH 13/28] wip debugging error on valid index of result of as_slice --- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 34 +++++++++++-------- .../array_to_slice/src/main.nr | 3 +- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 55a3a3466a7..554a3a9d930 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1239,15 +1239,16 @@ impl Context { source: BlockId, destination: BlockId, array_len: usize, - ) -> Result<(), RuntimeError> { + ) -> Result, RuntimeError> { let init_values = try_vecmap(0..array_len, |i| { let index_var = self.acir_context.add_constant(i); let read = self.acir_context.read_from_memory(source, &index_var)?; Ok::(AcirValue::Var(read, AcirType::field())) })?; - self.initialize_array(destination, array_len, Some(AcirValue::Array(init_values.into())))?; - Ok(()) + let array: im::Vector = init_values.into(); + self.initialize_array(destination, array_len, Some(AcirValue::Array(array.clone())))?; + Ok(array) } fn get_flattened_index( @@ -1669,16 +1670,7 @@ impl Context { assert!(!slice_typ.is_nested_slice(), "ICE: Nested slice used in ACIR generation"); let result_block_id = self.block_id(&result_ids[1]); - let element_type_sizes = if !can_omit_element_sizes_array(&slice_typ) { - Some(self.init_element_type_sizes_array( - &slice_typ, - slice_contents, - None, - dfg, - )?) - } else { - None - }; + let value_types = self.convert_value(slice_contents, dfg).flat_numeric_types(); // TODO: remove this and the assert let array_len = if !slice_typ.contains_slice_element() { @@ -1686,11 +1678,23 @@ impl Context { } else { self.flattened_slice_size(slice_contents, dfg) }; - let value_types = self.convert_value(slice_contents, dfg).flat_numeric_types(); assert!(array_len == value_types.len(), "AsSlice: unexpected length difference: {:?} != {:?}", array_len, value_types.len()); let slice_length = self.acir_context.add_constant(value_types.len()); - self.copy_dynamic_array(block_id, result_block_id, value_types.len())?; + let array_elements = self.copy_dynamic_array(block_id, result_block_id, value_types.len())?; + + assert!(array_elements.len() == value_types.len(), "AsSlice: unexpected length difference (2): {:?} != {:?}", array_elements.len(), value_types.len()); + + let element_type_sizes = if !can_omit_element_sizes_array(&slice_typ) { + Some(self.init_element_type_sizes_array( + &slice_typ, + slice_contents, + Some(&AcirValue::Array(array_elements)), + dfg, + )?) + } else { + None + }; let result = AcirValue::DynamicArray(AcirDynamicArray { block_id: result_block_id, diff --git a/test_programs/execution_success/array_to_slice/src/main.nr b/test_programs/execution_success/array_to_slice/src/main.nr index 879607f8f8a..4f5594c6d11 100644 --- a/test_programs/execution_success/array_to_slice/src/main.nr +++ b/test_programs/execution_success/array_to_slice/src/main.nr @@ -15,7 +15,7 @@ fn main(x: Field, y: pub Field) { let qs: [Field; 4] = [3, 2, 1, 0]; let mut dynamic: [Field; 4] = [3, 2, 1, 0]; - let dynamic_expected: [Field] = [1000, 2, 1, 0]; + let dynamic_expected: [Field; 4] = [1000, 2, 1, 0]; dynamic[x] = 1000; assert(x != y); @@ -24,6 +24,7 @@ fn main(x: Field, y: pub Field) { assert(zs.as_slice() == as_slice_push(zs)); assert(ws.as_slice() == as_slice_push(ws)); assert(qs.as_slice() == as_slice_push(qs)); + assert(dynamic.as_slice()[0] == dynamic_expected[0]); assert(dynamic.as_slice()[1] == dynamic_expected[1]); assert(dynamic.as_slice()[2] == dynamic_expected[2]); From 3da8474a6ef82d100fdc5195ecd32284f29e0b93 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Mon, 11 Mar 2024 21:47:18 +0000 Subject: [PATCH 14/28] update can_omit_element_sizes_array --- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 554a3a9d930..1109767894d 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1025,7 +1025,8 @@ impl Context { self.array_set_value(&store_value, result_block_id, &mut var_index)?; let element_type_sizes = if !can_omit_element_sizes_array(&array_typ) { - Some(self.init_element_type_sizes_array(&array_typ, array_id, None, dfg)?) + let acir_value = self.convert_value(array_id, dfg); + Some(self.init_element_type_sizes_array(&array_typ, array_id, Some(&acir_value), dfg)?) } else { None }; @@ -1258,7 +1259,9 @@ impl Context { var_index: AcirVar, dfg: &DataFlowGraph, ) -> Result { + dbg!(!can_omit_element_sizes_array(array_typ)); if !can_omit_element_sizes_array(array_typ) { + dbg!(array_typ.clone()); let element_type_sizes = self.init_element_type_sizes_array(array_typ, array_id, None, dfg)?; @@ -1670,7 +1673,7 @@ impl Context { assert!(!slice_typ.is_nested_slice(), "ICE: Nested slice used in ACIR generation"); let result_block_id = self.block_id(&result_ids[1]); - let value_types = self.convert_value(slice_contents, dfg).flat_numeric_types(); + let acir_value = self.convert_value(slice_contents, dfg); // TODO: remove this and the assert let array_len = if !slice_typ.contains_slice_element() { @@ -1678,24 +1681,24 @@ impl Context { } else { self.flattened_slice_size(slice_contents, dfg) }; - assert!(array_len == value_types.len(), "AsSlice: unexpected length difference: {:?} != {:?}", array_len, value_types.len()); - let slice_length = self.acir_context.add_constant(value_types.len()); - let array_elements = self.copy_dynamic_array(block_id, result_block_id, value_types.len())?; - - assert!(array_elements.len() == value_types.len(), "AsSlice: unexpected length difference (2): {:?} != {:?}", array_elements.len(), value_types.len()); + let slice_length = self.acir_context.add_constant(array_len); + self.copy_dynamic_array(block_id, result_block_id, array_len)?; let element_type_sizes = if !can_omit_element_sizes_array(&slice_typ) { Some(self.init_element_type_sizes_array( &slice_typ, slice_contents, - Some(&AcirValue::Array(array_elements)), + Some(&acir_value), dfg, )?) } else { None }; + let value_types = self.convert_value(slice_contents, dfg).flat_numeric_types(); + assert!(array_len == value_types.len(), "AsSlice: unexpected length difference: {:?} != {:?}", array_len, value_types.len()); + let result = AcirValue::DynamicArray(AcirDynamicArray { block_id: result_block_id, len: value_types.len(), @@ -2309,11 +2312,9 @@ impl Context { // We can omit the element size array for arrays which don't contain arrays or slices. fn can_omit_element_sizes_array(array_typ: &Type) -> bool { - if array_typ.contains_slice_element() { - return false; - } - let Type::Array(types, _) = array_typ else { - panic!("ICE: expected array type"); + let types = match array_typ { + Type::Array(types, _) | Type::Slice(types) => types, + _ => panic!("ICE: expected array or slice type"), }; !types.iter().any(|typ| typ.contains_an_array()) From 197fd77fa7ba15ec663afc1a9c444add39025927 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 11 Mar 2024 19:22:52 -0400 Subject: [PATCH 15/28] remove dbg calls and cleanup, add brillig implementation for AsSlice, wip implementing convert_ssa_as_slice --- .../src/brillig/brillig_gen/brillig_block.rs | 30 +++++++++++++++++++ .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 18 +++++++---- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index c04d8475f08..76bd4bcae1f 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -453,6 +453,15 @@ impl<'block> BrilligBlock<'block> { self.convert_ssa_array_len(arguments[0], result_variable.address, dfg); } } + Value::Intrinsic(Intrinsic::AsSlice) => { + let result_variable = self.variables.define_single_addr_variable( + self.function_context, + self.brillig_context, + dfg.instruction_results(instruction_id)[0], + dfg, + ); + self.convert_ssa_as_slice(arguments[0], result_variable.address, dfg); + } Value::Intrinsic( Intrinsic::SlicePushBack | Intrinsic::SlicePopBack @@ -1488,6 +1497,27 @@ impl<'block> BrilligBlock<'block> { } } } + + /// Gets the "user-facing" slice from an array + /// An array of structs with two fields would be stored as an 2 * array.len() array/vector. + fn convert_ssa_as_slice( + &mut self, + array_id: ValueId, + result_register: MemoryAddress, + dfg: &DataFlowGraph, + ) { + let array_variable = self.convert_ssa_value(array_id, dfg); + match array_variable { + BrilligVariable::BrilligArray(array) => { + let slice_var = self.brillig_context.array_to_vector(&array); + + self.brillig_context.allocate_array_instruction(result_register, slice_var.size); + } + _ => { + unreachable!("ICE: Cannot convert {array_variable:?} to slice") + } + } + } } /// Returns the type of the operation considering the types of the operands diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 1109767894d..65dd8821ae1 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1026,7 +1026,12 @@ impl Context { let element_type_sizes = if !can_omit_element_sizes_array(&array_typ) { let acir_value = self.convert_value(array_id, dfg); - Some(self.init_element_type_sizes_array(&array_typ, array_id, Some(&acir_value), dfg)?) + Some(self.init_element_type_sizes_array( + &array_typ, + array_id, + Some(&acir_value), + dfg, + )?) } else { None }; @@ -1259,9 +1264,7 @@ impl Context { var_index: AcirVar, dfg: &DataFlowGraph, ) -> Result { - dbg!(!can_omit_element_sizes_array(array_typ)); if !can_omit_element_sizes_array(array_typ) { - dbg!(array_typ.clone()); let element_type_sizes = self.init_element_type_sizes_array(array_typ, array_id, None, dfg)?; @@ -1675,13 +1678,11 @@ impl Context { let result_block_id = self.block_id(&result_ids[1]); let acir_value = self.convert_value(slice_contents, dfg); - // TODO: remove this and the assert let array_len = if !slice_typ.contains_slice_element() { slice_typ.flattened_size() } else { self.flattened_slice_size(slice_contents, dfg) }; - let slice_length = self.acir_context.add_constant(array_len); self.copy_dynamic_array(block_id, result_block_id, array_len)?; @@ -1697,7 +1698,12 @@ impl Context { }; let value_types = self.convert_value(slice_contents, dfg).flat_numeric_types(); - assert!(array_len == value_types.len(), "AsSlice: unexpected length difference: {:?} != {:?}", array_len, value_types.len()); + assert!( + array_len == value_types.len(), + "AsSlice: unexpected length difference: {:?} != {:?}", + array_len, + value_types.len() + ); let result = AcirValue::DynamicArray(AcirDynamicArray { block_id: result_block_id, From a5f7d9e13266b7602496c035258b82b0d43bc183 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 12 Mar 2024 13:34:00 -0400 Subject: [PATCH 16/28] wip debugging brillig as_slice implementation, rewrote implementation to match similar ones, recreated debugger error in execution test --- .../src/brillig/brillig_gen/brillig_block.rs | 59 +++++++++++++++---- .../array_to_slice/src/main.nr | 12 ++++ 2 files changed, 61 insertions(+), 10 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 76bd4bcae1f..db7abf5011d 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -454,13 +454,8 @@ impl<'block> BrilligBlock<'block> { } } Value::Intrinsic(Intrinsic::AsSlice) => { - let result_variable = self.variables.define_single_addr_variable( - self.function_context, - self.brillig_context, - dfg.instruction_results(instruction_id)[0], - dfg, - ); - self.convert_ssa_as_slice(arguments[0], result_variable.address, dfg); + let result_ids = dfg.instruction_results(instruction_id); + self.convert_ssa_as_slice(arguments[0], result_ids[0], result_ids[1], dfg); } Value::Intrinsic( Intrinsic::SlicePushBack @@ -1307,13 +1302,18 @@ impl<'block> BrilligBlock<'block> { /// Converts an SSA `ValueId` into a `RegisterOrMemory`. Initializes if necessary. fn convert_ssa_value(&mut self, value_id: ValueId, dfg: &DataFlowGraph) -> BrilligVariable { + dbg!("convert_ssa_value id: {:?}", value_id); let value_id = dfg.resolve(value_id); + dbg!("convert_ssa_value id2: {:?}", value_id); let value = &dfg[value_id]; + dbg!("convert_ssa_value value: {:?}", value); match value { Value::Param { .. } | Value::Instruction { .. } => { // All block parameters and instruction results should have already been // converted to registers so we fetch from the cache. + + dbg!("convert_ssa_value instruction: {:?}", value); self.variables.get_allocation(self.function_context, value_id, dfg) } Value::NumericConstant { constant, typ } => { @@ -1335,11 +1335,13 @@ impl<'block> BrilligBlock<'block> { } } Value::Array { array, .. } => { + dbg!("convert_ssa_value array: {:?}", array); if let Some(variable) = self.variables.get_constant(value_id, dfg) { variable } else { let new_variable = self.variables.allocate_constant(self.brillig_context, value_id, dfg); + dbg!("convert_ssa_value new_variable: {:?}", new_variable); // Initialize the variable let pointer = match new_variable { @@ -1363,12 +1365,14 @@ impl<'block> BrilligBlock<'block> { ), }; + dbg!("convert_ssa_value pointer: {:?}", pointer); // Write the items // Allocate a register for the iterator let iterator_register = self.brillig_context.make_usize_constant(0_usize.into()); + dbg!("convert_ssa_value iterator_register: {:?}", iterator_register); for element_id in array.iter() { let element_variable = self.convert_ssa_value(*element_id, dfg); // Store the item in memory @@ -1383,6 +1387,7 @@ impl<'block> BrilligBlock<'block> { self.brillig_context.deallocate_register(iterator_register); + dbg!("convert_ssa_value new_variable 2: {:?}", new_variable); new_variable } } @@ -1503,15 +1508,49 @@ impl<'block> BrilligBlock<'block> { fn convert_ssa_as_slice( &mut self, array_id: ValueId, - result_register: MemoryAddress, + target_len: ValueId, + target_variable: ValueId, dfg: &DataFlowGraph, ) { let array_variable = self.convert_ssa_value(array_id, dfg); match array_variable { BrilligVariable::BrilligArray(array) => { - let slice_var = self.brillig_context.array_to_vector(&array); + dbg!("array: {:?}", array); + dbg!("array_id: {:?}", array_id); + dbg!("target_len: {:?}", target_len); + dbg!("target_variable: {:?}", target_variable); + dbg!("target_variable(convert): {:?}", self.convert_ssa_value(target_variable, dfg)); + + let array_size_address = self.brillig_context.allocate_register(); + self.brillig_context.usize_const(array_size_address, array.size.into()); + + let len_variable = self.convert_ssa_value(target_len, dfg); + let len_address = len_variable.extract_single_addr(); + self.brillig_context.mov_instruction(len_address.address, array_size_address); + + let result_variable = self.variables.define_variable( + self.function_context, + self.brillig_context, + target_variable, + dfg, + ); + let target_vector = result_variable.extract_vector(); + + self.brillig_context.allocate_array_instruction(target_vector.pointer, array_size_address); + // We initialize the RC of the target vector to 1 + self.brillig_context.usize_const(target_vector.rc, 1_usize.into()); + + // Now we offset the target pointer by variables_to_insert.len() + let destination_copy_pointer = self.brillig_context.allocate_register(); + self.brillig_context.copy_array_instruction( + array.pointer, + destination_copy_pointer, + array_size_address, + ); + + self.brillig_context.deallocate_register(array_size_address); + self.brillig_context.deallocate_register(destination_copy_pointer); - self.brillig_context.allocate_array_instruction(result_register, slice_var.size); } _ => { unreachable!("ICE: Cannot convert {array_variable:?} to slice") diff --git a/test_programs/execution_success/array_to_slice/src/main.nr b/test_programs/execution_success/array_to_slice/src/main.nr index 4f5594c6d11..7421d14440e 100644 --- a/test_programs/execution_success/array_to_slice/src/main.nr +++ b/test_programs/execution_success/array_to_slice/src/main.nr @@ -7,6 +7,16 @@ fn as_slice_push(xs: [T; N]) -> [T] { slice } +unconstrained fn brillig_as_slice(x: Field) -> bool { + let mut dynamic: [Field; 1] = [2]; + dynamic[x] = 999; + let brillig_expected: [Field; 1] = [999]; + let brillig_slice = dynamic.as_slice(); + + (brillig_slice[0] == brillig_expected[0]) & + (brillig_slice.len() == 1) +} + fn main(x: Field, y: pub Field) { let xs: [Field; 0] = []; let ys: [Field; 1] = [1]; @@ -30,4 +40,6 @@ fn main(x: Field, y: pub Field) { assert(dynamic.as_slice()[2] == dynamic_expected[2]); assert(dynamic.as_slice()[3] == dynamic_expected[3]); assert(dynamic.as_slice().len() == 4); + + assert(brillig_as_slice(x)); } From e8637a58b2d0bfd4ba7cf4328443eeacd07be0a3 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 12 Mar 2024 14:33:53 -0400 Subject: [PATCH 17/28] wip debugging brillig slice access error, fixed wrong initialization of brillig length variable --- .../src/brillig/brillig_gen/brillig_block.rs | 30 ++++++++----------- .../array_to_slice/src/main.nr | 22 +++++++++----- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index db7abf5011d..45ef9515d58 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -1302,18 +1302,14 @@ impl<'block> BrilligBlock<'block> { /// Converts an SSA `ValueId` into a `RegisterOrMemory`. Initializes if necessary. fn convert_ssa_value(&mut self, value_id: ValueId, dfg: &DataFlowGraph) -> BrilligVariable { - dbg!("convert_ssa_value id: {:?}", value_id); let value_id = dfg.resolve(value_id); - dbg!("convert_ssa_value id2: {:?}", value_id); let value = &dfg[value_id]; - dbg!("convert_ssa_value value: {:?}", value); match value { Value::Param { .. } | Value::Instruction { .. } => { // All block parameters and instruction results should have already been // converted to registers so we fetch from the cache. - dbg!("convert_ssa_value instruction: {:?}", value); self.variables.get_allocation(self.function_context, value_id, dfg) } Value::NumericConstant { constant, typ } => { @@ -1335,13 +1331,11 @@ impl<'block> BrilligBlock<'block> { } } Value::Array { array, .. } => { - dbg!("convert_ssa_value array: {:?}", array); if let Some(variable) = self.variables.get_constant(value_id, dfg) { variable } else { let new_variable = self.variables.allocate_constant(self.brillig_context, value_id, dfg); - dbg!("convert_ssa_value new_variable: {:?}", new_variable); // Initialize the variable let pointer = match new_variable { @@ -1365,14 +1359,12 @@ impl<'block> BrilligBlock<'block> { ), }; - dbg!("convert_ssa_value pointer: {:?}", pointer); // Write the items // Allocate a register for the iterator let iterator_register = self.brillig_context.make_usize_constant(0_usize.into()); - dbg!("convert_ssa_value iterator_register: {:?}", iterator_register); for element_id in array.iter() { let element_variable = self.convert_ssa_value(*element_id, dfg); // Store the item in memory @@ -1387,7 +1379,6 @@ impl<'block> BrilligBlock<'block> { self.brillig_context.deallocate_register(iterator_register); - dbg!("convert_ssa_value new_variable 2: {:?}", new_variable); new_variable } } @@ -1515,18 +1506,23 @@ impl<'block> BrilligBlock<'block> { let array_variable = self.convert_ssa_value(array_id, dfg); match array_variable { BrilligVariable::BrilligArray(array) => { - dbg!("array: {:?}", array); - dbg!("array_id: {:?}", array_id); - dbg!("target_len: {:?}", target_len); - dbg!("target_variable: {:?}", target_variable); - dbg!("target_variable(convert): {:?}", self.convert_ssa_value(target_variable, dfg)); + // dbg!("array: {:?}", array); + // dbg!("array_id: {:?}", array_id); + // dbg!("target_len: {:?}", target_len); + // dbg!("target_variable: {:?}", target_variable); + // dbg!("target_variable(convert): {:?}", self.convert_ssa_value(target_variable, dfg)); + dbg!("array.size: {:?}", array.size); let array_size_address = self.brillig_context.allocate_register(); self.brillig_context.usize_const(array_size_address, array.size.into()); - let len_variable = self.convert_ssa_value(target_len, dfg); - let len_address = len_variable.extract_single_addr(); - self.brillig_context.mov_instruction(len_address.address, array_size_address); + let len_variable = self.variables.define_single_addr_variable( + self.function_context, + self.brillig_context, + target_len, + dfg, + ); + self.brillig_context.mov_instruction(len_variable.address, array_size_address); let result_variable = self.variables.define_variable( self.function_context, diff --git a/test_programs/execution_success/array_to_slice/src/main.nr b/test_programs/execution_success/array_to_slice/src/main.nr index 7421d14440e..a702c85c8d3 100644 --- a/test_programs/execution_success/array_to_slice/src/main.nr +++ b/test_programs/execution_success/array_to_slice/src/main.nr @@ -7,14 +7,19 @@ fn as_slice_push(xs: [T; N]) -> [T] { slice } -unconstrained fn brillig_as_slice(x: Field) -> bool { - let mut dynamic: [Field; 1] = [2]; - dynamic[x] = 999; - let brillig_expected: [Field; 1] = [999]; +unconstrained fn brillig_as_slice(x: Field) -> Field { + let mut dynamic: [Field; 1] = [1]; + dynamic[x] = 2; + assert(dynamic[0] == 2); + print(dynamic); + print("\n"); + let brillig_slice = dynamic.as_slice(); + assert(brillig_slice.len() == 1); - (brillig_slice[0] == brillig_expected[0]) & - (brillig_slice.len() == 1) + print(brillig_slice[0]); + print("\n"); + brillig_slice[0] } fn main(x: Field, y: pub Field) { @@ -41,5 +46,8 @@ fn main(x: Field, y: pub Field) { assert(dynamic.as_slice()[3] == dynamic_expected[3]); assert(dynamic.as_slice().len() == 4); - assert(brillig_as_slice(x)); + let res = brillig_as_slice(x); + // print(res); + // print("\n"); + assert(res == 2); } From efa81dbd034538a3cb57dd7c03c6bdb3ae69b0ca Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 12 Mar 2024 14:40:02 -0400 Subject: [PATCH 18/28] wip debugging: cleanup for second pair of eyes --- .../src/brillig/brillig_gen/brillig_block.rs | 5 +- .../array_to_slice/src/main.nr | 46 +++++++++---------- 2 files changed, 24 insertions(+), 27 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 45ef9515d58..d05659aec80 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -1511,6 +1511,7 @@ impl<'block> BrilligBlock<'block> { // dbg!("target_len: {:?}", target_len); // dbg!("target_variable: {:?}", target_variable); // dbg!("target_variable(convert): {:?}", self.convert_ssa_value(target_variable, dfg)); + dbg!("array: {:?}", array); dbg!("array.size: {:?}", array.size); let array_size_address = self.brillig_context.allocate_register(); @@ -1533,10 +1534,6 @@ impl<'block> BrilligBlock<'block> { let target_vector = result_variable.extract_vector(); self.brillig_context.allocate_array_instruction(target_vector.pointer, array_size_address); - // We initialize the RC of the target vector to 1 - self.brillig_context.usize_const(target_vector.rc, 1_usize.into()); - - // Now we offset the target pointer by variables_to_insert.len() let destination_copy_pointer = self.brillig_context.allocate_register(); self.brillig_context.copy_array_instruction( array.pointer, diff --git a/test_programs/execution_success/array_to_slice/src/main.nr b/test_programs/execution_success/array_to_slice/src/main.nr index a702c85c8d3..6e814f7db5d 100644 --- a/test_programs/execution_success/array_to_slice/src/main.nr +++ b/test_programs/execution_success/array_to_slice/src/main.nr @@ -17,34 +17,34 @@ unconstrained fn brillig_as_slice(x: Field) -> Field { let brillig_slice = dynamic.as_slice(); assert(brillig_slice.len() == 1); - print(brillig_slice[0]); + print([brillig_slice[0]]); print("\n"); brillig_slice[0] } fn main(x: Field, y: pub Field) { - let xs: [Field; 0] = []; - let ys: [Field; 1] = [1]; - let zs: [Field; 2] = [1, 2]; - let ws: [Field; 3] = [1; 3]; - let qs: [Field; 4] = [3, 2, 1, 0]; - - let mut dynamic: [Field; 4] = [3, 2, 1, 0]; - let dynamic_expected: [Field; 4] = [1000, 2, 1, 0]; - dynamic[x] = 1000; - - assert(x != y); - assert(xs.as_slice() == as_slice_push(xs)); - assert(ys.as_slice() == as_slice_push(ys)); - assert(zs.as_slice() == as_slice_push(zs)); - assert(ws.as_slice() == as_slice_push(ws)); - assert(qs.as_slice() == as_slice_push(qs)); - - assert(dynamic.as_slice()[0] == dynamic_expected[0]); - assert(dynamic.as_slice()[1] == dynamic_expected[1]); - assert(dynamic.as_slice()[2] == dynamic_expected[2]); - assert(dynamic.as_slice()[3] == dynamic_expected[3]); - assert(dynamic.as_slice().len() == 4); + // let xs: [Field; 0] = []; + // let ys: [Field; 1] = [1]; + // let zs: [Field; 2] = [1, 2]; + // let ws: [Field; 3] = [1; 3]; + // let qs: [Field; 4] = [3, 2, 1, 0]; + // + // let mut dynamic: [Field; 4] = [3, 2, 1, 0]; + // let dynamic_expected: [Field; 4] = [1000, 2, 1, 0]; + // dynamic[x] = 1000; + // + // assert(x != y); + // assert(xs.as_slice() == as_slice_push(xs)); + // assert(ys.as_slice() == as_slice_push(ys)); + // assert(zs.as_slice() == as_slice_push(zs)); + // assert(ws.as_slice() == as_slice_push(ws)); + // assert(qs.as_slice() == as_slice_push(qs)); + // + // assert(dynamic.as_slice()[0] == dynamic_expected[0]); + // assert(dynamic.as_slice()[1] == dynamic_expected[1]); + // assert(dynamic.as_slice()[2] == dynamic_expected[2]); + // assert(dynamic.as_slice()[3] == dynamic_expected[3]); + // assert(dynamic.as_slice().len() == 4); let res = brillig_as_slice(x); // print(res); From 84cfbc39e721705506e59bf226483e6131ea71c5 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 12 Mar 2024 18:25:03 -0400 Subject: [PATCH 19/28] got brillig tests passing using version of convert_ssa_array_set, cleanup as_slice test --- .../src/brillig/brillig_gen/brillig_block.rs | 131 ++++++++++++------ .../array_to_slice/src/main.nr | 50 +++---- 2 files changed, 108 insertions(+), 73 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index d05659aec80..07d7d9cf444 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -454,8 +454,27 @@ impl<'block> BrilligBlock<'block> { } } Value::Intrinsic(Intrinsic::AsSlice) => { + let source_variable = self.convert_ssa_value(arguments[0], dfg); let result_ids = dfg.instruction_results(instruction_id); - self.convert_ssa_as_slice(arguments[0], result_ids[0], result_ids[1], dfg); + + // this needs to be initialized for convert_ssa_as_slice + let _len_variable = self.variables.define_variable( + self.function_context, + self.brillig_context, + result_ids[0], + dfg, + ); + let destination_variable = self.variables.define_variable( + self.function_context, + self.brillig_context, + result_ids[1], + dfg, + ); + + self.convert_ssa_as_slice( + source_variable, + destination_variable, + ); } Value::Intrinsic( Intrinsic::SlicePushBack @@ -1498,57 +1517,79 @@ impl<'block> BrilligBlock<'block> { /// An array of structs with two fields would be stored as an 2 * array.len() array/vector. fn convert_ssa_as_slice( &mut self, - array_id: ValueId, - target_len: ValueId, - target_variable: ValueId, - dfg: &DataFlowGraph, + source_variable: BrilligVariable, + destination_variable: BrilligVariable, ) { - let array_variable = self.convert_ssa_value(array_id, dfg); - match array_variable { - BrilligVariable::BrilligArray(array) => { - // dbg!("array: {:?}", array); - // dbg!("array_id: {:?}", array_id); - // dbg!("target_len: {:?}", target_len); - // dbg!("target_variable: {:?}", target_variable); - // dbg!("target_variable(convert): {:?}", self.convert_ssa_value(target_variable, dfg)); - dbg!("array: {:?}", array); - dbg!("array.size: {:?}", array.size); - - let array_size_address = self.brillig_context.allocate_register(); - self.brillig_context.usize_const(array_size_address, array.size.into()); - - let len_variable = self.variables.define_single_addr_variable( - self.function_context, - self.brillig_context, - target_len, - dfg, - ); - self.brillig_context.mov_instruction(len_variable.address, array_size_address); + let destination_pointer = match destination_variable { + BrilligVariable::BrilligArray(BrilligArray { pointer, .. }) => pointer, + BrilligVariable::BrilligVector(BrilligVector { pointer, .. }) => pointer, + _ => unreachable!("ICE: as_slice returns non-array"), + }; - let result_variable = self.variables.define_variable( - self.function_context, - self.brillig_context, - target_variable, - dfg, - ); - let target_vector = result_variable.extract_vector(); - - self.brillig_context.allocate_array_instruction(target_vector.pointer, array_size_address); - let destination_copy_pointer = self.brillig_context.allocate_register(); - self.brillig_context.copy_array_instruction( - array.pointer, - destination_copy_pointer, - array_size_address, - ); + let reference_count = match source_variable { + BrilligVariable::BrilligArray(BrilligArray { rc, .. }) + | BrilligVariable::BrilligVector(BrilligVector { rc, .. }) => rc, + _ => unreachable!("ICE: as_slice on non-array"), + }; - self.brillig_context.deallocate_register(array_size_address); - self.brillig_context.deallocate_register(destination_copy_pointer); + let (source_pointer, source_size_as_register) = match source_variable { + BrilligVariable::BrilligArray(BrilligArray { size, pointer, rc: _ }) => { + let source_size_register = self.brillig_context.allocate_register(); + self.brillig_context.usize_const(source_size_register, size.into()); + (pointer, source_size_register) + } + BrilligVariable::BrilligVector(BrilligVector { size, pointer, rc: _ }) => { + let source_size_register = self.brillig_context.allocate_register(); + self.brillig_context.mov_instruction(source_size_register, size); + (pointer, source_size_register) + } + _ => unreachable!("ICE: as_slice on non-array"), + }; + + let one = self.brillig_context.make_usize_constant(1_usize.into()); + let condition = self.brillig_context.allocate_register(); + + self.brillig_context.binary_instruction( + reference_count, + one, + condition, + BrilligBinaryOp::Field { op: BinaryFieldOp::Equals }, + ); + self.brillig_context.branch_instruction(condition, |ctx, cond| { + if cond { + // Reference count is 1, we can mutate the array directly + ctx.mov_instruction(destination_pointer, source_pointer); + } else { + // First issue a array copy to the destination + ctx.allocate_array_instruction(destination_pointer, source_size_as_register); + + ctx.copy_array_instruction( + source_pointer, + destination_pointer, + source_size_as_register, + ); } - _ => { - unreachable!("ICE: Cannot convert {array_variable:?} to slice") + }); + + match destination_variable { + BrilligVariable::BrilligArray(BrilligArray { rc: target_rc, .. }) => { + self.brillig_context.usize_const(target_rc, 1_usize.into()); } + BrilligVariable::BrilligVector(BrilligVector { + size: target_size, + rc: target_rc, + .. + }) => { + self.brillig_context.mov_instruction(target_size, source_size_as_register); + self.brillig_context.usize_const(target_rc, 1_usize.into()); + } + _ => unreachable!("ICE: as_slice on non-array"), } + + self.brillig_context.deallocate_register(source_size_as_register); + self.brillig_context.deallocate_register(one); + self.brillig_context.deallocate_register(condition); } } diff --git a/test_programs/execution_success/array_to_slice/src/main.nr b/test_programs/execution_success/array_to_slice/src/main.nr index 6e814f7db5d..8fb7aa8d818 100644 --- a/test_programs/execution_success/array_to_slice/src/main.nr +++ b/test_programs/execution_success/array_to_slice/src/main.nr @@ -11,43 +11,37 @@ unconstrained fn brillig_as_slice(x: Field) -> Field { let mut dynamic: [Field; 1] = [1]; dynamic[x] = 2; assert(dynamic[0] == 2); - print(dynamic); - print("\n"); let brillig_slice = dynamic.as_slice(); assert(brillig_slice.len() == 1); - print([brillig_slice[0]]); - print("\n"); brillig_slice[0] } fn main(x: Field, y: pub Field) { - // let xs: [Field; 0] = []; - // let ys: [Field; 1] = [1]; - // let zs: [Field; 2] = [1, 2]; - // let ws: [Field; 3] = [1; 3]; - // let qs: [Field; 4] = [3, 2, 1, 0]; - // - // let mut dynamic: [Field; 4] = [3, 2, 1, 0]; - // let dynamic_expected: [Field; 4] = [1000, 2, 1, 0]; - // dynamic[x] = 1000; - // - // assert(x != y); - // assert(xs.as_slice() == as_slice_push(xs)); - // assert(ys.as_slice() == as_slice_push(ys)); - // assert(zs.as_slice() == as_slice_push(zs)); - // assert(ws.as_slice() == as_slice_push(ws)); - // assert(qs.as_slice() == as_slice_push(qs)); - // - // assert(dynamic.as_slice()[0] == dynamic_expected[0]); - // assert(dynamic.as_slice()[1] == dynamic_expected[1]); - // assert(dynamic.as_slice()[2] == dynamic_expected[2]); - // assert(dynamic.as_slice()[3] == dynamic_expected[3]); - // assert(dynamic.as_slice().len() == 4); + let xs: [Field; 0] = []; + let ys: [Field; 1] = [1]; + let zs: [Field; 2] = [1, 2]; + let ws: [Field; 3] = [1; 3]; + let qs: [Field; 4] = [3, 2, 1, 0]; + + let mut dynamic: [Field; 4] = [3, 2, 1, 0]; + let dynamic_expected: [Field; 4] = [1000, 2, 1, 0]; + dynamic[x] = 1000; + + assert(x != y); + assert(xs.as_slice() == as_slice_push(xs)); + assert(ys.as_slice() == as_slice_push(ys)); + assert(zs.as_slice() == as_slice_push(zs)); + assert(ws.as_slice() == as_slice_push(ws)); + assert(qs.as_slice() == as_slice_push(qs)); + + assert(dynamic.as_slice()[0] == dynamic_expected[0]); + assert(dynamic.as_slice()[1] == dynamic_expected[1]); + assert(dynamic.as_slice()[2] == dynamic_expected[2]); + assert(dynamic.as_slice()[3] == dynamic_expected[3]); + assert(dynamic.as_slice().len() == 4); let res = brillig_as_slice(x); - // print(res); - // print("\n"); assert(res == 2); } From c29add0b0483cb5fc9f9b14b7ff0c28d0e76f8ed Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 12 Mar 2024 18:40:31 -0400 Subject: [PATCH 20/28] remove debugging return --- compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 65dd8821ae1..7b58b88e162 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1245,7 +1245,7 @@ impl Context { source: BlockId, destination: BlockId, array_len: usize, - ) -> Result, RuntimeError> { + ) -> Result<(), RuntimeError> { let init_values = try_vecmap(0..array_len, |i| { let index_var = self.acir_context.add_constant(i); @@ -1254,7 +1254,7 @@ impl Context { })?; let array: im::Vector = init_values.into(); self.initialize_array(destination, array_len, Some(AcirValue::Array(array.clone())))?; - Ok(array) + Ok(()) } fn get_flattened_index( From fecda34fde7e06a2d66e6393ea247b57904b236d Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 12 Mar 2024 18:44:45 -0400 Subject: [PATCH 21/28] cargo clippy / fmt --- .../src/brillig/brillig_gen/brillig_block.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 98906d6450c..505fb471e8b 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -471,10 +471,7 @@ impl<'block> BrilligBlock<'block> { dfg, ); - self.convert_ssa_as_slice( - source_variable, - destination_variable, - ); + self.convert_ssa_as_slice(source_variable, destination_variable); } Value::Intrinsic( Intrinsic::SlicePushBack @@ -1328,7 +1325,7 @@ impl<'block> BrilligBlock<'block> { Value::Param { .. } | Value::Instruction { .. } => { // All block parameters and instruction results should have already been // converted to registers so we fetch from the cache. - + self.variables.get_allocation(self.function_context, value_id, dfg) } Value::NumericConstant { constant, typ } => { From b6feea0f3e593de51afeee73fbd28f26d5e50e7a Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 13 Mar 2024 13:37:38 -0400 Subject: [PATCH 22/28] fix destination length init in brillig as_slice implementation, split out brillig and regular as_slice tests, cargo fmt/clippy --- .../src/brillig/brillig_gen/brillig_block.rs | 16 ++++++++++----- .../array_to_slice/src/main.nr | 20 +++---------------- .../brillig_array_to_slice/Nargo.toml | 7 +++++++ .../brillig_array_to_slice/Prover.toml | 1 + .../brillig_array_to_slice/src/main.nr | 18 +++++++++++++++++ 5 files changed, 40 insertions(+), 22 deletions(-) create mode 100644 test_programs/execution_success/brillig_array_to_slice/Nargo.toml create mode 100644 test_programs/execution_success/brillig_array_to_slice/Prover.toml create mode 100644 test_programs/execution_success/brillig_array_to_slice/src/main.nr diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 505fb471e8b..67daf1b7b8f 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -456,9 +456,7 @@ impl<'block> BrilligBlock<'block> { Value::Intrinsic(Intrinsic::AsSlice) => { let source_variable = self.convert_ssa_value(arguments[0], dfg); let result_ids = dfg.instruction_results(instruction_id); - - // this needs to be initialized for convert_ssa_as_slice - let _len_variable = self.variables.define_variable( + let destination_len_variable = self.variables.define_single_addr_variable( self.function_context, self.brillig_context, result_ids[0], @@ -470,8 +468,11 @@ impl<'block> BrilligBlock<'block> { result_ids[1], dfg, ); - - self.convert_ssa_as_slice(source_variable, destination_variable); + self.convert_ssa_as_slice( + source_variable, + destination_len_variable, + destination_variable, + ); } Value::Intrinsic( Intrinsic::SlicePushBack @@ -1515,6 +1516,7 @@ impl<'block> BrilligBlock<'block> { fn convert_ssa_as_slice( &mut self, source_variable: BrilligVariable, + destination_len_variable: SingleAddrVariable, destination_variable: BrilligVariable, ) { let destination_pointer = match destination_variable { @@ -1543,6 +1545,10 @@ impl<'block> BrilligBlock<'block> { _ => unreachable!("ICE: as_slice on non-array"), }; + // we need to explicitly set the destination_len_variable + self.brillig_context + .mov_instruction(destination_len_variable.address, source_size_as_register); + let one = self.brillig_context.make_usize_constant(1_usize.into()); let condition = self.brillig_context.allocate_register(); diff --git a/test_programs/execution_success/array_to_slice/src/main.nr b/test_programs/execution_success/array_to_slice/src/main.nr index 8fb7aa8d818..4f5594c6d11 100644 --- a/test_programs/execution_success/array_to_slice/src/main.nr +++ b/test_programs/execution_success/array_to_slice/src/main.nr @@ -7,41 +7,27 @@ fn as_slice_push(xs: [T; N]) -> [T] { slice } -unconstrained fn brillig_as_slice(x: Field) -> Field { - let mut dynamic: [Field; 1] = [1]; - dynamic[x] = 2; - assert(dynamic[0] == 2); - - let brillig_slice = dynamic.as_slice(); - assert(brillig_slice.len() == 1); - - brillig_slice[0] -} - fn main(x: Field, y: pub Field) { let xs: [Field; 0] = []; let ys: [Field; 1] = [1]; let zs: [Field; 2] = [1, 2]; let ws: [Field; 3] = [1; 3]; let qs: [Field; 4] = [3, 2, 1, 0]; - + let mut dynamic: [Field; 4] = [3, 2, 1, 0]; let dynamic_expected: [Field; 4] = [1000, 2, 1, 0]; dynamic[x] = 1000; - + assert(x != y); assert(xs.as_slice() == as_slice_push(xs)); assert(ys.as_slice() == as_slice_push(ys)); assert(zs.as_slice() == as_slice_push(zs)); assert(ws.as_slice() == as_slice_push(ws)); assert(qs.as_slice() == as_slice_push(qs)); - + assert(dynamic.as_slice()[0] == dynamic_expected[0]); assert(dynamic.as_slice()[1] == dynamic_expected[1]); assert(dynamic.as_slice()[2] == dynamic_expected[2]); assert(dynamic.as_slice()[3] == dynamic_expected[3]); assert(dynamic.as_slice().len() == 4); - - let res = brillig_as_slice(x); - assert(res == 2); } diff --git a/test_programs/execution_success/brillig_array_to_slice/Nargo.toml b/test_programs/execution_success/brillig_array_to_slice/Nargo.toml new file mode 100644 index 00000000000..58157c38c26 --- /dev/null +++ b/test_programs/execution_success/brillig_array_to_slice/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "brillig_array_to_slice" +type = "bin" +authors = [""] +compiler_version = ">=0.25.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_success/brillig_array_to_slice/Prover.toml b/test_programs/execution_success/brillig_array_to_slice/Prover.toml new file mode 100644 index 00000000000..11497a473bc --- /dev/null +++ b/test_programs/execution_success/brillig_array_to_slice/Prover.toml @@ -0,0 +1 @@ +x = "0" diff --git a/test_programs/execution_success/brillig_array_to_slice/src/main.nr b/test_programs/execution_success/brillig_array_to_slice/src/main.nr new file mode 100644 index 00000000000..8f7fcf24bae --- /dev/null +++ b/test_programs/execution_success/brillig_array_to_slice/src/main.nr @@ -0,0 +1,18 @@ +unconstrained fn brillig_as_slice(x: Field) -> (u64, Field, Field) { + let mut dynamic: [Field; 1] = [1]; + dynamic[x] = 2; + assert(dynamic[0] == 2); + + let brillig_slice = dynamic.as_slice(); + assert(brillig_slice.len() == 1); + + (brillig_slice.len(), dynamic[0], brillig_slice[0]) +} + +fn main(x: Field) { + let (slice_len, dynamic_0, slice_0) = brillig_as_slice(x); + assert(slice_len == 1); + assert(dynamic_0 == 2); + assert(slice_0 == 2); +} + From 269bdda12f2eb2d2585fd431bb8db617e840a604 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 13 Mar 2024 14:13:19 -0400 Subject: [PATCH 23/28] combine convert_ssa_as_slice with convert_ssa_array_set to pull in reference counting changes on master, cargo clippy / fmt --- .../src/brillig/brillig_gen/brillig_block.rs | 128 ++++-------------- 1 file changed, 28 insertions(+), 100 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 0e368e0d42f..02e8a494d4a 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -470,10 +470,10 @@ impl<'block> BrilligBlock<'block> { result_ids[1], dfg, ); - self.convert_ssa_as_slice( + self.convert_ssa_array_set( source_variable, - destination_len_variable, destination_variable, + Err(destination_len_variable), ); } Value::Intrinsic( @@ -639,8 +639,7 @@ impl<'block> BrilligBlock<'block> { self.convert_ssa_array_set( source_variable, destination_variable, - index_register.address, - value_variable, + Ok((index_register.address, value_variable)), ); } Instruction::RangeCheck { value, max_bit_size, assert_message } => { @@ -819,23 +818,27 @@ impl<'block> BrilligBlock<'block> { /// Array set operation in SSA returns a new array or slice that is a copy of the parameter array or slice /// With a specific value changed. + /// + /// Skip setting a value and store the resulting length in the given SingleAddrVariable if + /// provided fn convert_ssa_array_set( &mut self, source_variable: BrilligVariable, destination_variable: BrilligVariable, - index_register: MemoryAddress, - value_variable: BrilligVariable, + index_and_value_or_len: Result<(MemoryAddress, BrilligVariable), SingleAddrVariable>, ) { + let method_str = if index_and_value_or_len.is_ok() { "array set" } else { "as_slice" }; + let destination_pointer = match destination_variable { BrilligVariable::BrilligArray(BrilligArray { pointer, .. }) => pointer, BrilligVariable::BrilligVector(BrilligVector { pointer, .. }) => pointer, - _ => unreachable!("ICE: array set returns non-array"), + _ => unreachable!("ICE: {method_str} returns non-array"), }; let reference_count = match source_variable { BrilligVariable::BrilligArray(BrilligArray { rc, .. }) | BrilligVariable::BrilligVector(BrilligVector { rc, .. }) => rc, - _ => unreachable!("ICE: array set on non-array"), + _ => unreachable!("ICE: {method_str} on non-array"), }; let (source_pointer, source_size_as_register) = match source_variable { @@ -849,7 +852,7 @@ impl<'block> BrilligBlock<'block> { self.brillig_context.mov_instruction(source_size_register, size); (pointer, source_size_register) } - _ => unreachable!("ICE: array set on non-array"), + _ => unreachable!("ICE: {method_str} on non-array"), }; // Here we want to compare the reference count against 1. @@ -892,15 +895,24 @@ impl<'block> BrilligBlock<'block> { self.brillig_context.mov_instruction(target_size, source_size_as_register); self.brillig_context.usize_const(target_rc, 1_usize.into()); } - _ => unreachable!("ICE: array set on non-array"), + _ => unreachable!("ICE: {method_str} on non-array"), } - // Then set the value in the newly created array - self.store_variable_in_array( - destination_pointer, - SingleAddrVariable::new_usize(index_register), - value_variable, - ); + match index_and_value_or_len { + Ok((index_register, value_variable)) => { + // Then set the value in the newly created array + self.store_variable_in_array( + destination_pointer, + SingleAddrVariable::new_usize(index_register), + value_variable, + ); + } + Err(destination_len_variable) => { + // we need to explicitly set the destination_len_variable + self.brillig_context + .mov_instruction(destination_len_variable.address, source_size_as_register); + } + } self.brillig_context.deallocate_register(source_size_as_register); self.brillig_context.deallocate_register(condition); @@ -1510,90 +1522,6 @@ impl<'block> BrilligBlock<'block> { } } } - - /// Gets the "user-facing" slice from an array - /// An array of structs with two fields would be stored as an 2 * array.len() array/vector. - fn convert_ssa_as_slice( - &mut self, - source_variable: BrilligVariable, - destination_len_variable: SingleAddrVariable, - destination_variable: BrilligVariable, - ) { - let destination_pointer = match destination_variable { - BrilligVariable::BrilligArray(BrilligArray { pointer, .. }) => pointer, - BrilligVariable::BrilligVector(BrilligVector { pointer, .. }) => pointer, - _ => unreachable!("ICE: as_slice returns non-array"), - }; - - let reference_count = match source_variable { - BrilligVariable::BrilligArray(BrilligArray { rc, .. }) - | BrilligVariable::BrilligVector(BrilligVector { rc, .. }) => rc, - _ => unreachable!("ICE: as_slice on non-array"), - }; - - let (source_pointer, source_size_as_register) = match source_variable { - BrilligVariable::BrilligArray(BrilligArray { size, pointer, rc: _ }) => { - let source_size_register = self.brillig_context.allocate_register(); - self.brillig_context.usize_const(source_size_register, size.into()); - (pointer, source_size_register) - } - BrilligVariable::BrilligVector(BrilligVector { size, pointer, rc: _ }) => { - let source_size_register = self.brillig_context.allocate_register(); - self.brillig_context.mov_instruction(source_size_register, size); - (pointer, source_size_register) - } - _ => unreachable!("ICE: as_slice on non-array"), - }; - - // we need to explicitly set the destination_len_variable - self.brillig_context - .mov_instruction(destination_len_variable.address, source_size_as_register); - - let one = self.brillig_context.make_usize_constant(1_usize.into()); - let condition = self.brillig_context.allocate_register(); - - self.brillig_context.binary_instruction( - reference_count, - one, - condition, - BrilligBinaryOp::Field { op: BinaryFieldOp::Equals }, - ); - - self.brillig_context.branch_instruction(condition, |ctx, cond| { - if cond { - // Reference count is 1, we can mutate the array directly - ctx.mov_instruction(destination_pointer, source_pointer); - } else { - // First issue a array copy to the destination - ctx.allocate_array_instruction(destination_pointer, source_size_as_register); - - ctx.copy_array_instruction( - source_pointer, - destination_pointer, - source_size_as_register, - ); - } - }); - - match destination_variable { - BrilligVariable::BrilligArray(BrilligArray { rc: target_rc, .. }) => { - self.brillig_context.usize_const(target_rc, 1_usize.into()); - } - BrilligVariable::BrilligVector(BrilligVector { - size: target_size, - rc: target_rc, - .. - }) => { - self.brillig_context.mov_instruction(target_size, source_size_as_register); - self.brillig_context.usize_const(target_rc, 1_usize.into()); - } - _ => unreachable!("ICE: as_slice on non-array"), - } - - self.brillig_context.deallocate_register(source_size_as_register); - self.brillig_context.deallocate_register(one); - self.brillig_context.deallocate_register(condition); - } } /// Returns the type of the operation considering the types of the operands From f66772b0ed3f90744f54b05c907e89c461d04aa4 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 13 Mar 2024 15:15:54 -0400 Subject: [PATCH 24/28] move as_slice logic out of convert_ssa_array_set --- .../src/brillig/brillig_gen/brillig_block.rs | 51 +++++++++---------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 02e8a494d4a..a1d05972089 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -470,11 +470,17 @@ impl<'block> BrilligBlock<'block> { result_ids[1], dfg, ); - self.convert_ssa_array_set( + let source_size_as_register = self.convert_ssa_array_set( source_variable, destination_variable, - Err(destination_len_variable), + None, + "as_slice".to_string(), ); + + // we need to explicitly set the destination_len_variable + self.brillig_context + .mov_instruction(destination_len_variable.address, source_size_as_register); + self.brillig_context.deallocate_register(source_size_as_register); } Value::Intrinsic( Intrinsic::SlicePushBack @@ -635,12 +641,13 @@ impl<'block> BrilligBlock<'block> { dfg, ); self.validate_array_index(source_variable, index_register); - - self.convert_ssa_array_set( + let source_size_as_register = self.convert_ssa_array_set( source_variable, destination_variable, - Ok((index_register.address, value_variable)), + Some((index_register.address, value_variable)), + "array set".to_string(), ); + self.brillig_context.deallocate_register(source_size_as_register); } Instruction::RangeCheck { value, max_bit_size, assert_message } => { let value = self.convert_ssa_single_addr_value(*value, dfg); @@ -819,16 +826,15 @@ impl<'block> BrilligBlock<'block> { /// Array set operation in SSA returns a new array or slice that is a copy of the parameter array or slice /// With a specific value changed. /// - /// Skip setting a value and store the resulting length in the given SingleAddrVariable if - /// provided + /// Returns `source_size_as_register`, which is expected to be deallocated with: + /// `self.brillig_context.deallocate_register(source_size_as_register)` fn convert_ssa_array_set( &mut self, source_variable: BrilligVariable, destination_variable: BrilligVariable, - index_and_value_or_len: Result<(MemoryAddress, BrilligVariable), SingleAddrVariable>, - ) { - let method_str = if index_and_value_or_len.is_ok() { "array set" } else { "as_slice" }; - + opt_index_and_value: Option<(MemoryAddress, BrilligVariable)>, + method_str: String, + ) -> MemoryAddress { let destination_pointer = match destination_variable { BrilligVariable::BrilligArray(BrilligArray { pointer, .. }) => pointer, BrilligVariable::BrilligVector(BrilligVector { pointer, .. }) => pointer, @@ -898,24 +904,17 @@ impl<'block> BrilligBlock<'block> { _ => unreachable!("ICE: {method_str} on non-array"), } - match index_and_value_or_len { - Ok((index_register, value_variable)) => { - // Then set the value in the newly created array - self.store_variable_in_array( - destination_pointer, - SingleAddrVariable::new_usize(index_register), - value_variable, - ); - } - Err(destination_len_variable) => { - // we need to explicitly set the destination_len_variable - self.brillig_context - .mov_instruction(destination_len_variable.address, source_size_as_register); - } + if let Some((index_register, value_variable)) = opt_index_and_value { + // Then set the value in the newly created array + self.store_variable_in_array( + destination_pointer, + SingleAddrVariable::new_usize(index_register), + value_variable, + ); } - self.brillig_context.deallocate_register(source_size_as_register); self.brillig_context.deallocate_register(condition); + source_size_as_register } pub(crate) fn store_variable_in_array_with_ctx( From 243fcdc55422ad969f2b5bdc3c2cee3295b79d8a Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 13 Mar 2024 16:31:49 -0400 Subject: [PATCH 25/28] fix slice literals from merging master --- test_programs/execution_success/array_to_slice/src/main.nr | 2 +- test_programs/execution_success/bigint/src/main.nr | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test_programs/execution_success/array_to_slice/src/main.nr b/test_programs/execution_success/array_to_slice/src/main.nr index 4f5594c6d11..b97f68fc280 100644 --- a/test_programs/execution_success/array_to_slice/src/main.nr +++ b/test_programs/execution_success/array_to_slice/src/main.nr @@ -1,6 +1,6 @@ // Converts an array into a slice. fn as_slice_push(xs: [T; N]) -> [T] { - let mut slice = []; + let mut slice = &[]; for elem in xs { slice = slice.push_back(elem); } diff --git a/test_programs/execution_success/bigint/src/main.nr b/test_programs/execution_success/bigint/src/main.nr index 64d7d8ff9f1..db269d63ac0 100644 --- a/test_programs/execution_success/bigint/src/main.nr +++ b/test_programs/execution_success/bigint/src/main.nr @@ -19,8 +19,8 @@ fn main(mut x: [u8; 5], y: [u8; 5]) { // docs:start:big_int_example fn big_int_example(x: u8, y: u8) { - let a = Secpk1Fq::from_le_bytes([x, y, 0, 45, 2]); - let b = Secpk1Fq::from_le_bytes([y, x, 9]); + let a = Secpk1Fq::from_le_bytes(&[x, y, 0, 45, 2]); + let b = Secpk1Fq::from_le_bytes(&[y, x, 9]); let c = (a + b) * b / a; let d = c.to_le_bytes(); println(d[0]); From 74b52509291fc3053ad6a55645b725bdeb41c472 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Thu, 14 Mar 2024 16:32:09 -0400 Subject: [PATCH 26/28] add try_to_usize to complete roundtrip of usize -> Field and back for array lengths, support ArrayLen on acir var's when constant and the appropriate type, fix element type in generated ssa ir for as_slice, fix printer not distinguishing between slices and arrays, use slice literal instead of as_slice for merkle insert, update as_slice test with cases from debugging --- acvm-repo/acir_field/src/generic_ark.rs | 5 ++ .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 27 ++++++++-- .../src/ssa/ir/instruction/call.rs | 7 ++- .../noirc_evaluator/src/ssa/ir/printer.rs | 9 +++- noir_stdlib/src/merkle.nr | 2 +- .../array_to_slice/src/main.nr | 53 +++++++++++-------- 6 files changed, 72 insertions(+), 31 deletions(-) diff --git a/acvm-repo/acir_field/src/generic_ark.rs b/acvm-repo/acir_field/src/generic_ark.rs index 3178011a075..d219cb2b23d 100644 --- a/acvm-repo/acir_field/src/generic_ark.rs +++ b/acvm-repo/acir_field/src/generic_ark.rs @@ -254,6 +254,11 @@ impl FieldElement { (self.num_bits() <= 64).then(|| self.to_u128() as u64) } + pub fn try_to_usize(&self) -> Option { + let usize_bits: u32 = std::mem::size_of::().try_into().ok()?; + (self.num_bits() <= usize_bits).then(|| self.to_u128() as usize) + } + /// Computes the inverse or returns zero if the inverse does not exist /// Before using this FieldElement, please ensure that this behavior is necessary pub fn inverse(&self) -> FieldElement { diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index e9861bbbd82..5e5bf5942bb 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1663,8 +1663,25 @@ impl Context { self.acir_context.bit_decompose(endian, field, bit_size, result_type) } Intrinsic::ArrayLen => { - let len = match self.convert_value(arguments[0], dfg) { - AcirValue::Var(_, _) => unreachable!("Non-array passed to array.len() method"), + let converted = self.convert_value(arguments[0], dfg); + let len = match converted { + AcirValue::Var(acir_var, acir_type) => { + if Type::Numeric(acir_type.to_numeric_type()) != Type::length_type() { + unreachable!( + "ICE: array has length of unexpected type: {:?}", + acir_type + ); + } + + if !self.acir_context.is_constant(&acir_var) { + unreachable!("ICE: array has non-constant length: {:?}", acir_var); + } + + self.acir_context + .constant(acir_var) + .try_to_usize() + .expect("ICE: array with length > usize::MAX") + } AcirValue::Array(values) => values.len(), AcirValue::DynamicArray(array) => array.len, }; @@ -1698,8 +1715,9 @@ impl Context { }; let value_types = self.convert_value(slice_contents, dfg).flat_numeric_types(); + let result_len = value_types.len(); assert!( - array_len == value_types.len(), + array_len == result_len, "AsSlice: unexpected length difference: {:?} != {:?}", array_len, value_types.len() @@ -1707,10 +1725,11 @@ impl Context { let result = AcirValue::DynamicArray(AcirDynamicArray { block_id: result_block_id, - len: value_types.len(), + len: result_len, value_types, element_type_sizes, }); + Ok(vec![AcirValue::Var(slice_length, AcirType::field()), result]) } Intrinsic::SlicePushBack => { diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index 8b800e0db54..20ce2b46d8b 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -87,8 +87,13 @@ pub(super) fn simplify_call( Intrinsic::AsSlice => { let slice = dfg.get_array_constant(arguments[0]); if let Some((slice, element_type)) = slice { + let new_element_type = match element_type { + Type::Array(array_typ, _) => Type::Slice(array_typ), + Type::Slice(slice_typ) => Type::Slice(slice_typ), + _ => unreachable!("ICE: non-array passed to as_slice"), + }; let slice_length = dfg.make_constant(slice.len().into(), Type::length_type()); - let new_slice = dfg.make_array(slice, element_type); + let new_slice = dfg.make_array(slice, new_element_type); SimplifyResult::SimplifiedToMultiple(vec![slice_length, new_slice]) } else { SimplifyResult::None diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index 6ef618fba6f..d97df8dc85d 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -10,6 +10,7 @@ use super::{ basic_block::BasicBlockId, function::Function, instruction::{ConstrainError, Instruction, InstructionId, TerminatorInstruction}, + types::Type, value::ValueId, }; @@ -68,9 +69,13 @@ fn value(function: &Function, id: ValueId) -> String { } Value::Function(id) => id.to_string(), Value::Intrinsic(intrinsic) => intrinsic.to_string(), - Value::Array { array, .. } => { + Value::Array { array, typ } => { let elements = vecmap(array, |element| value(function, *element)); - format!("[{}]", elements.join(", ")) + match typ { + Type::Array(_, _) => format!("[{}]", elements.join(", ")), + Type::Slice(_) => format!("&[{}]", elements.join(", ")), + _ => unreachable!("ICE: Value::Array with non-array type: {:?}", typ), + } } Value::Param { .. } | Value::Instruction { .. } | Value::ForeignFunction(_) => { id.to_string() diff --git a/noir_stdlib/src/merkle.nr b/noir_stdlib/src/merkle.nr index ba65b1adae5..f58e80f9d99 100644 --- a/noir_stdlib/src/merkle.nr +++ b/noir_stdlib/src/merkle.nr @@ -13,7 +13,7 @@ pub fn compute_merkle_root(leaf: Field, index: Field, hash_path: [Field; N]) } else { (current, hash_path[i]) }; - current = crate::hash::pedersen_hash([hash_left, hash_right].as_slice()); + current = crate::hash::pedersen_hash(&[hash_left, hash_right]); } current } diff --git a/test_programs/execution_success/array_to_slice/src/main.nr b/test_programs/execution_success/array_to_slice/src/main.nr index b97f68fc280..32e8890bf42 100644 --- a/test_programs/execution_success/array_to_slice/src/main.nr +++ b/test_programs/execution_success/array_to_slice/src/main.nr @@ -7,27 +7,34 @@ fn as_slice_push(xs: [T; N]) -> [T] { slice } -fn main(x: Field, y: pub Field) { - let xs: [Field; 0] = []; - let ys: [Field; 1] = [1]; - let zs: [Field; 2] = [1, 2]; - let ws: [Field; 3] = [1; 3]; - let qs: [Field; 4] = [3, 2, 1, 0]; - - let mut dynamic: [Field; 4] = [3, 2, 1, 0]; - let dynamic_expected: [Field; 4] = [1000, 2, 1, 0]; - dynamic[x] = 1000; - - assert(x != y); - assert(xs.as_slice() == as_slice_push(xs)); - assert(ys.as_slice() == as_slice_push(ys)); - assert(zs.as_slice() == as_slice_push(zs)); - assert(ws.as_slice() == as_slice_push(ws)); - assert(qs.as_slice() == as_slice_push(qs)); - - assert(dynamic.as_slice()[0] == dynamic_expected[0]); - assert(dynamic.as_slice()[1] == dynamic_expected[1]); - assert(dynamic.as_slice()[2] == dynamic_expected[2]); - assert(dynamic.as_slice()[3] == dynamic_expected[3]); - assert(dynamic.as_slice().len() == 4); +fn main(x: Field, y: Field) { + let z = 3; + let w = 4; + + let mut dynamic_array = [0, 0]; + dynamic_array[x] = z; + dynamic_array[y] = w; + + let literal_slice = &[z, w]; + let convert_dynamic = dynamic_array.as_slice(); + let convert_literal = [z, w].as_slice(); + + assert(literal_slice.len() == 2); + assert(convert_dynamic.len() == 2); + assert(convert_literal.len() == 2); + + assert(literal_slice[x] == z); + assert(convert_dynamic[x] == z); + assert(convert_literal[x] == z); + + assert(literal_slice[y] == w); + assert(convert_dynamic[y] == w); + assert(convert_literal[y] == w); + + let hash_literal_slice = dep::std::hash::pedersen_hash(literal_slice); + let hash_convert_dynamic = dep::std::hash::pedersen_hash(convert_dynamic); + let hash_convert_literal = dep::std::hash::pedersen_hash(convert_literal); + + assert(hash_literal_slice == hash_convert_dynamic); + assert(hash_literal_slice == hash_convert_literal); } From 5e9861d9a2725aaa58b84988662ea82aefe1ec5e Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Thu, 14 Mar 2024 16:45:14 -0400 Subject: [PATCH 27/28] replace arrays with slice literals in Hash impl's, add distinct slice impl --- noir_stdlib/src/hash.nr | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/noir_stdlib/src/hash.nr b/noir_stdlib/src/hash.nr index 483c59fba82..2b7b2868d75 100644 --- a/noir_stdlib/src/hash.nr +++ b/noir_stdlib/src/hash.nr @@ -123,49 +123,49 @@ where impl Hash for Field { fn hash(self, state: &mut H) where H: Hasher{ - H::write(state, [self]); + H::write(state, &[self]); } } impl Hash for u8 { fn hash(self, state: &mut H) where H: Hasher{ - H::write(state, [self as Field]); + H::write(state, &[self as Field]); } } impl Hash for u32 { fn hash(self, state: &mut H) where H: Hasher{ - H::write(state, [self as Field]); + H::write(state, &[self as Field]); } } impl Hash for u64 { fn hash(self, state: &mut H) where H: Hasher{ - H::write(state, [self as Field]); + H::write(state, &[self as Field]); } } impl Hash for i8 { fn hash(self, state: &mut H) where H: Hasher{ - H::write(state, [self as Field]); + H::write(state, &[self as Field]); } } impl Hash for i32 { fn hash(self, state: &mut H) where H: Hasher{ - H::write(state, [self as Field]); + H::write(state, &[self as Field]); } } impl Hash for i64 { fn hash(self, state: &mut H) where H: Hasher{ - H::write(state, [self as Field]); + H::write(state, &[self as Field]); } } impl Hash for bool { fn hash(self, state: &mut H) where H: Hasher{ - H::write(state, [self as Field]); + H::write(state, &[self as Field]); } } @@ -175,7 +175,7 @@ impl Hash for () { impl Hash for U128 { fn hash(self, state: &mut H) where H: Hasher{ - H::write(state, [self.lo as Field, self.hi as Field]); + H::write(state, &[self.lo as Field, self.hi as Field]); } } @@ -187,6 +187,14 @@ impl Hash for [T; N] where T: Hash { } } +impl Hash for [T] where T: Hash { + fn hash(self, state: &mut H) where H: Hasher{ + for elem in self { + elem.hash(state); + } + } +} + impl Hash for (A, B) where A: Hash, B: Hash { fn hash(self, state: &mut H) where H: Hasher{ self.0.hash(state); From e18f47bc43fa05860779ddfad2f75936672109a9 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Fri, 15 Mar 2024 13:02:23 -0400 Subject: [PATCH 28/28] remove unused PartialEq --- compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs | 2 +- compiler/noirc_frontend/src/hir/def_collector/errors.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 42e086bcbf6..0c53bff4a54 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -136,7 +136,7 @@ pub struct DefCollector { pub(crate) type ImplMap = HashMap<(UnresolvedType, LocalModuleId), Vec<(UnresolvedGenerics, Span, UnresolvedFunctions)>>; -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub enum CompilationError { ParseError(ParserError), DefinitionError(DefCollectorErrorKind), diff --git a/compiler/noirc_frontend/src/hir/def_collector/errors.rs b/compiler/noirc_frontend/src/hir/def_collector/errors.rs index f2f33df2594..29daf5d6369 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/errors.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/errors.rs @@ -23,7 +23,7 @@ pub enum DuplicateType { TraitAssociatedFunction, } -#[derive(Error, Debug, Clone, PartialEq)] +#[derive(Error, Debug, Clone)] pub enum DefCollectorErrorKind { #[error("duplicate {typ} found in namespace")] Duplicate { typ: DuplicateType, first_def: Ident, second_def: Ident }, @@ -70,7 +70,7 @@ pub enum DefCollectorErrorKind { } /// An error struct that macro processors can return. -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct MacroError { pub primary_message: String, pub secondary_message: Option,