From 688489512b75bd8670e452282c9e5f060f7e9a69 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 20 Jul 2023 16:04:35 -0500 Subject: [PATCH 1/9] Start experiment to merge array and slice types --- crates/noirc_frontend/src/hir/def_map/mod.rs | 8 +- .../src/hir/resolution/resolver.rs | 2 + crates/noirc_frontend/src/hir_def/types.rs | 88 ++++++++++++++----- .../src/monomorphization/mod.rs | 27 +++--- crates/noirc_frontend/src/node_interner.rs | 2 + 5 files changed, 86 insertions(+), 41 deletions(-) diff --git a/crates/noirc_frontend/src/hir/def_map/mod.rs b/crates/noirc_frontend/src/hir/def_map/mod.rs index 8bb88abd1e9..cc16746daed 100644 --- a/crates/noirc_frontend/src/hir/def_map/mod.rs +++ b/crates/noirc_frontend/src/hir/def_map/mod.rs @@ -93,12 +93,12 @@ impl CrateDefMap { .to_str() .expect("expected std path to be convertible to str"); assert_eq!(path_as_str, "std/lib"); - // There are 2 printlns in the stdlib. If we are using the experimental SSA, we want to keep - // only the unconstrained one. Otherwise we want to keep only the constrained one. - ast.functions.retain(|func| { + // There are 2 printlns in the stdlib. If we are using the experimental SSA, we want to keep + // only the unconstrained one. Otherwise we want to keep only the constrained one. + ast.functions.retain(|func| { func.def.name.0.contents.as_str() != "println" || func.def.is_unconstrained == context.def_interner.experimental_ssa - }); + }); if !context.def_interner.experimental_ssa { ast.module_decls.retain(|ident| { diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index f314cd63e44..fed52928c2a 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -760,6 +760,8 @@ impl<'a> Resolver<'a> { | Type::PolymorphicInteger(_, _) | Type::Constant(_) | Type::NamedGeneric(_, _) + | Type::MaybeConstant(_, _) + | Type::NotConstant | Type::Forall(_, _) => (), Type::Array(length, _) => { diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index 9441307bf28..a9d3a2ffed8 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -21,9 +21,6 @@ pub enum Type { /// is either a type variable of some kind or a Type::Constant. Array(Box, Box), - /// Slice(E) is a slice with elements of type E. - Slice(Box), - /// A primitive integer type with the given sign, bit count, and whether it is known at compile-time. /// E.g. `u32` would be `Integer(CompTime::No(None), Unsigned, 32)` Integer(CompTime, Signedness, u32), @@ -88,6 +85,19 @@ pub enum Type { /// bind to an integer without special checks to bind it to a non-type. Constant(u64), + /// A type variable kind that is potentially bound to a constant. + /// Array literals are initialized with their lengths as MaybeConstant. + /// If they are not used as slices later on, the MaybeConstant size will + /// default to a Constant size. Otherwise they will be bound to either + /// NotConstant or a Constant, forcing the array literal to be used only + /// as a slice or array respectively. + MaybeConstant(TypeVariable, 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 @@ -227,6 +237,14 @@ impl std::fmt::Display for StructType { #[derive(Debug, Eq, PartialOrd, Ord)] pub struct Shared(Rc>); +impl std::ops::Deref for Shared { + type Target = T; + + fn deref(&self) -> &Self::Target { + &self.borrow() + } +} + impl std::hash::Hash for Shared { fn hash(&self, state: &mut H) { self.0.borrow().hash(state); @@ -576,14 +594,14 @@ impl Type { | Type::PolymorphicInteger(_, _) | Type::Constant(_) | Type::NamedGeneric(_, _) + | Type::NotConstant + | Type::MaybeConstant(_, _) | Type::Forall(_, _) => 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)) } @@ -621,8 +639,13 @@ impl std::fmt::Display for Type { Type::FieldElement(comp_time) => { write!(f, "{comp_time}Field") } - Type::Array(len, typ) => write!(f, "[{typ}; {len}]"), - Type::Slice(typ) => write!(f, "[{typ}]"), + Type::Array(len, typ) => { + if let Some(length) = len.evaluate_to_u64() { + write!(f, "[{typ}; {length}]") + } else { + write!(f, "[{typ}]") + } + } Type::Integer(comp_time, sign, num_bits) => match sign { Signedness::Signed => write!(f, "{comp_time}i{num_bits}"), Signedness::Unsigned => write!(f, "{comp_time}u{num_bits}"), @@ -671,6 +694,11 @@ impl std::fmt::Display for Type { Type::MutableReference(element) => { write!(f, "&mut {element}") } + Type::MaybeConstant(binding, x) => match &*binding.borrow() { + TypeBinding::Bound(binding) => binding.fmt(f), + TypeBinding::Unbound(_) => write!(f, "{x}"), + }, + Type::NotConstant => todo!(), } } } @@ -933,13 +961,19 @@ impl Type { other.try_bind_to(binding) } + (MaybeConstant(binding, _), other) | (other, MaybeConstant(binding, _)) => { + if let TypeBinding::Bound(link) = &*binding.borrow() { + return link.try_unify(other, span); + } + + other.try_bind_to(binding) + } + (Array(len_a, elem_a), Array(len_b, elem_b)) => { len_a.try_unify(len_b, span)?; elem_a.try_unify(elem_b, span) } - (Slice(elem_a), Slice(elem_b)) => elem_a.try_unify(elem_b, span), - (Tuple(elements_a), Tuple(elements_b)) => { if elements_a.len() != elements_b.len() { Err(SpanKind::None) @@ -1073,10 +1107,6 @@ impl Type { elem_a.is_subtype_of(elem_b, span) } - (Slice(elem_a), Slice(elem_b)) => elem_a.is_subtype_of(elem_b, span), - - (Array(_, elem_a), Slice(elem_b)) => elem_a.is_subtype_of(elem_b, span), - (Tuple(elements_a), Tuple(elements_b)) => { if elements_a.len() != elements_b.len() { Err(SpanKind::None) @@ -1176,6 +1206,10 @@ impl Type { TypeBinding::Bound(binding) => binding.evaluate_to_u64(), TypeBinding::Unbound(_) => None, }, + Type::MaybeConstant(binding, size) => match &*binding.borrow() { + TypeBinding::Bound(binding) => binding.evaluate_to_u64(), + TypeBinding::Unbound(_) => Some(*size), + }, Type::Array(len, _elem) => len.evaluate_to_u64(), Type::Constant(x) => Some(*x), _ => None, @@ -1226,8 +1260,9 @@ impl Type { Type::NamedGeneric(..) => unreachable!(), Type::Forall(..) => unreachable!(), Type::Function(_, _) => unreachable!(), - Type::Slice(_) => unreachable!("slices cannot be used in the abi"), Type::MutableReference(_) => unreachable!("&mut cannot be used in the abi"), + Type::MaybeConstant(_, _) => unreachable!(), + Type::NotConstant => unreachable!(), } } @@ -1309,16 +1344,13 @@ impl Type { let element = Box::new(element.substitute(type_bindings)); Type::Array(size, element) } - Type::Slice(element) => { - let element = Box::new(element.substitute(type_bindings)); - Type::Slice(element) - } Type::String(size) => { let size = Box::new(size.substitute(type_bindings)); Type::String(size) } Type::PolymorphicInteger(_, binding) | Type::NamedGeneric(binding, _) + | Type::MaybeConstant(binding, _) | Type::TypeVariable(binding) => substitute_binding(binding), // Do not substitute fields, it can lead to infinite recursion @@ -1354,6 +1386,7 @@ impl Type { | Type::Bool(_) | Type::Constant(_) | Type::Error + | Type::NotConstant | Type::Unit => self.clone(), } } @@ -1362,12 +1395,12 @@ impl Type { fn occurs(&self, target_id: TypeVariableId) -> bool { match self { Type::Array(len, elem) => len.occurs(target_id) || elem.occurs(target_id), - Type::Slice(element) => element.occurs(target_id), Type::String(len) => len.occurs(target_id), Type::Struct(_, generic_args) => generic_args.iter().any(|arg| arg.occurs(target_id)), Type::Tuple(fields) => fields.iter().any(|field| field.occurs(target_id)), Type::PolymorphicInteger(_, binding) | Type::NamedGeneric(binding, _) + | Type::MaybeConstant(binding, _) | Type::TypeVariable(binding) => match &*binding.borrow() { TypeBinding::Bound(binding) => binding.occurs(target_id), TypeBinding::Unbound(id) => *id == target_id, @@ -1385,6 +1418,7 @@ impl Type { | Type::Bool(_) | Type::Constant(_) | Type::Error + | Type::NotConstant | Type::Unit => false, } } @@ -1401,7 +1435,6 @@ 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())), Struct(def, args) => { let args = vecmap(args, |arg| arg.follow_bindings()); @@ -1409,7 +1442,10 @@ impl Type { } Tuple(args) => Tuple(vecmap(args, |arg| arg.follow_bindings())), - TypeVariable(var) | PolymorphicInteger(_, var) | NamedGeneric(var, _) => { + TypeVariable(var) + | PolymorphicInteger(_, var) + | MaybeConstant(var, _) + | NamedGeneric(var, _) => { if let TypeBinding::Bound(typ) = &*var.borrow() { return typ.follow_bindings(); } @@ -1426,9 +1462,13 @@ impl Type { // Expect that this function should only be called on instantiated types Forall(..) => unreachable!(), - FieldElement(_) | Integer(_, _, _) | Bool(_) | Constant(_) | Unit | Error => { - self.clone() - } + FieldElement(_) + | Integer(_, _, _) + | Bool(_) + | Constant(_) + | Unit + | Error + | NotConstant => self.clone(), } } } diff --git a/crates/noirc_frontend/src/monomorphization/mod.rs b/crates/noirc_frontend/src/monomorphization/mod.rs index a3188eaa14a..1ae4ff19e39 100644 --- a/crates/noirc_frontend/src/monomorphization/mod.rs +++ b/crates/noirc_frontend/src/monomorphization/mod.rs @@ -678,17 +678,16 @@ impl<'interner> Monomorphizer<'interner> { HirType::Unit => ast::Type::Unit, HirType::Array(length, element) => { - let length = length.evaluate_to_u64().unwrap_or(0); - let element = self.convert_type(element.as_ref()); - if self.interner.experimental_ssa { - return ast::Type::Array(length, Box::new(element)); - } - self.aos_to_soa_type(length, element) - } + let element = Box::new(self.convert_type(element.as_ref())); - HirType::Slice(element) => { - let element = self.convert_type(element.as_ref()); - ast::Type::Slice(Box::new(element)) + if let Some(length) = length.evaluate_to_u64() { + if self.interner.experimental_ssa { + return ast::Type::Array(length, element); + } + self.aos_to_soa_type(length, *element) + } else { + ast::Type::Slice(element) + } } HirType::PolymorphicInteger(_, binding) @@ -732,7 +731,10 @@ impl<'interner> Monomorphizer<'interner> { ast::Type::MutableReference(Box::new(element)) } - HirType::Forall(_, _) | HirType::Constant(_) | HirType::Error => { + HirType::Forall(_, _) + | HirType::Constant(_) + | HirType::NotConstant + | HirType::Error => { unreachable!("Unexpected type {} found", typ) } } @@ -778,7 +780,7 @@ impl<'interner> Monomorphizer<'interner> { if let Definition::Oracle(name) = &ident.definition { if name.as_str() == "println" { // Oracle calls are required to be wrapped in an unconstrained function - // Thus, the only argument to the `println` oracle is expected to always be an ident + // Thus, the only argument to the `println` oracle is expected to always be an ident self.append_abi_arg(&hir_arguments[0], &mut arguments); } } @@ -1147,7 +1149,6 @@ fn unwrap_struct_type(typ: &HirType) -> Vec<(String, HirType)> { fn unwrap_array_element_type(typ: &HirType) -> HirType { match typ { HirType::Array(_, elem) => *elem.clone(), - HirType::Slice(elem) => *elem.clone(), HirType::TypeVariable(binding) => match &*binding.borrow() { TypeBinding::Bound(binding) => unwrap_array_element_type(binding), TypeBinding::Unbound(_) => unreachable!(), diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index 69b313fd538..517113f365a 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -644,6 +644,8 @@ fn get_type_method_key(typ: &Type) -> Option { | Type::Forall(_, _) | Type::Constant(_) | Type::Error + | Type::MaybeConstant(_, _) + | Type::NotConstant | Type::Struct(_, _) => None, } } From 79440f20faf0e1d22b97e93a91b5772ccb11e178 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 21 Jul 2023 12:07:20 -0500 Subject: [PATCH 2/9] Finish merger of slices and arrays --- .../src/hir/resolution/resolver.rs | 15 ++-- .../noirc_frontend/src/hir/type_check/expr.rs | 14 +++- .../noirc_frontend/src/hir/type_check/stmt.rs | 1 - crates/noirc_frontend/src/hir_def/types.rs | 29 +++++--- .../src/monomorphization/mod.rs | 1 + crates/noirc_frontend/src/node_interner.rs | 2 - noir_stdlib/src/hash/poseidon.nr | 2 +- noir_stdlib/src/slice.nr | 69 ------------------- 8 files changed, 38 insertions(+), 95 deletions(-) diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index fed52928c2a..2388ff718c6 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -329,11 +329,12 @@ impl<'a> Resolver<'a> { UnresolvedType::FieldElement(comp_time) => Type::FieldElement(comp_time), UnresolvedType::Array(size, elem) => { let elem = Box::new(self.resolve_type_inner(*elem, new_variables)); - if self.interner.experimental_ssa && size.is_none() { - return Type::Slice(elem); - } - let resolved_size = self.resolve_array_size(size, new_variables); - Type::Array(Box::new(resolved_size), elem) + let size = if self.interner.experimental_ssa && size.is_none() { + Type::NotConstant + } else { + self.resolve_array_size(size, new_variables) + }; + Type::Array(Box::new(size), elem) } UnresolvedType::Expression(expr) => self.convert_expression_type(expr), UnresolvedType::Integer(comp_time, sign, bits) => Type::Integer(comp_time, sign, bits), @@ -770,10 +771,6 @@ impl<'a> Resolver<'a> { } } - Type::Slice(typ) => { - Self::find_numeric_generics_in_type(typ, found); - } - Type::Tuple(fields) => { for field in fields { Self::find_numeric_generics_in_type(field, found); diff --git a/crates/noirc_frontend/src/hir/type_check/expr.rs b/crates/noirc_frontend/src/hir/type_check/expr.rs index 1625c8a320f..37517e84187 100644 --- a/crates/noirc_frontend/src/hir/type_check/expr.rs +++ b/crates/noirc_frontend/src/hir/type_check/expr.rs @@ -47,8 +47,11 @@ impl<'interner> TypeChecker<'interner> { .cloned() .unwrap_or_else(|| self.interner.next_type_variable()); + let typevar_id = self.interner.next_type_variable_id(); + let typevar = Shared::new(TypeBinding::Unbound(typevar_id)); + let arr_type = Type::Array( - Box::new(Type::Constant(arr.len() as u64)), + Box::new(Type::MaybeConstant(typevar, arr.len() as u64)), Box::new(first_elem_type.clone()), ); @@ -78,6 +81,14 @@ impl<'interner> TypeChecker<'interner> { } HirLiteral::Array(HirArrayLiteral::Repeated { repeated_element, length }) => { let elem_type = self.check_expression(&repeated_element); + let length = match length { + Type::Constant(length) => { + let typevar_id = self.interner.next_type_variable_id(); + let typevar = Shared::new(TypeBinding::Unbound(typevar_id)); + Type::MaybeConstant(typevar, length) + }, + other => other, + }; Type::Array(Box::new(length), Box::new(elem_type)) } HirLiteral::Bool(_) => Type::Bool(CompTime::new(self.interner)), @@ -325,7 +336,6 @@ 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(&index_expr.collection); diff --git a/crates/noirc_frontend/src/hir/type_check/stmt.rs b/crates/noirc_frontend/src/hir/type_check/stmt.rs index 4354d4cc77b..d8971f28631 100644 --- a/crates/noirc_frontend/src/hir/type_check/stmt.rs +++ b/crates/noirc_frontend/src/hir/type_check/stmt.rs @@ -194,7 +194,6 @@ impl<'interner> TypeChecker<'interner> { let typ = match result { 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/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index a9d3a2ffed8..8e62f1831d2 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -237,14 +237,6 @@ impl std::fmt::Display for StructType { #[derive(Debug, Eq, PartialOrd, Ord)] pub struct Shared(Rc>); -impl std::ops::Deref for Shared { - type Target = T; - - fn deref(&self) -> &Self::Target { - &self.borrow() - } -} - impl std::hash::Hash for Shared { fn hash(&self, state: &mut H) { self.0.borrow().hash(state); @@ -640,10 +632,10 @@ impl std::fmt::Display for Type { write!(f, "{comp_time}Field") } Type::Array(len, typ) => { - if let Some(length) = len.evaluate_to_u64() { - write!(f, "[{typ}; {length}]") - } else { + if matches!(len.follow_bindings(), Type::NotConstant) { write!(f, "[{typ}]") + } else { + write!(f, "[{typ}; {len}]") } } Type::Integer(comp_time, sign, num_bits) => match sign { @@ -1102,6 +1094,21 @@ impl Type { other.try_bind_to(binding) } + (MaybeConstant(binding, _), other) => { + if let TypeBinding::Bound(link) = &*binding.borrow() { + return link.is_subtype_of(other, span); + } + + other.try_bind_to(binding) + } + (other, MaybeConstant(binding, _)) => { + if let TypeBinding::Bound(link) = &*binding.borrow() { + return other.is_subtype_of(link, span); + } + + other.try_bind_to(binding) + } + (Array(len_a, elem_a), Array(len_b, elem_b)) => { len_a.is_subtype_of(len_b, span)?; elem_a.is_subtype_of(elem_b, span) diff --git a/crates/noirc_frontend/src/monomorphization/mod.rs b/crates/noirc_frontend/src/monomorphization/mod.rs index 1ae4ff19e39..d445c285d61 100644 --- a/crates/noirc_frontend/src/monomorphization/mod.rs +++ b/crates/noirc_frontend/src/monomorphization/mod.rs @@ -733,6 +733,7 @@ impl<'interner> Monomorphizer<'interner> { HirType::Forall(_, _) | HirType::Constant(_) + | HirType::MaybeConstant(..) | HirType::NotConstant | HirType::Error => { unreachable!("Unexpected type {} found", typ) diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index 517113f365a..868d55eff56 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -614,7 +614,6 @@ enum TypeMethodKey { /// accept only fields or integers, it is just that their names may not clash. FieldOrInt, Array, - Slice, Bool, String, Unit, @@ -628,7 +627,6 @@ 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::PolymorphicInteger(_, _) => Some(FieldOrInt), Type::Bool(_) => Some(Bool), diff --git a/noir_stdlib/src/hash/poseidon.nr b/noir_stdlib/src/hash/poseidon.nr index 416f740bbdf..cb1e34927b4 100644 --- a/noir_stdlib/src/hash/poseidon.nr +++ b/noir_stdlib/src/hash/poseidon.nr @@ -101,7 +101,7 @@ fn check_security(rate: Field, width: Field, security: Field) -> bool { } // A*x where A is an n x n matrix in row-major order and x an n-vector -fn apply_matrix(a: [Field], x: [Field; N]) -> [Field; N] { +fn apply_matrix(a: [Field; M], x: [Field; N]) -> [Field; N] { let mut y = x; for i in 0..x.len() { diff --git a/noir_stdlib/src/slice.nr b/noir_stdlib/src/slice.nr index 186d535a264..8e344a40f5e 100644 --- a/noir_stdlib/src/slice.nr +++ b/noir_stdlib/src/slice.nr @@ -32,74 +32,5 @@ impl [T] { /// the removed element #[builtin(slice_remove)] fn remove(_self: Self, _index: Field) -> (Self, T) { } - - #[builtin(array_len)] - fn len(_self: Self) -> comptime Field {} - - #[builtin(arraysort)] - fn sort(_self: Self) -> Self {} - - // Sort with a custom sorting function. - fn sort_via(mut a: Self, ordering: fn(T, T) -> bool) -> Self { - for i in 1 .. a.len() { - for j in 0..i { - if ordering(a[i], a[j]) { - let old_a_j = a[j]; - a[j] = a[i]; - a[i] = old_a_j; - } - } - } - a - } - - // Apply a function to each element of a slice, returning a new slice - // containing the mapped elements. - fn map(self, f: fn(T) -> U) -> [U] { - let mut ret: [U] = []; - for elem in self { - ret = ret.push_back(f(elem)); - } - ret - } - - // Apply a function to each element of the slice and an accumulator value, - // returning the final accumulated value. This function is also sometimes - // called `foldl`, `fold_left`, `reduce`, or `inject`. - fn fold(self, mut accumulator: U, f: fn(U, T) -> U) -> U { - for elem in self { - accumulator = f(accumulator, elem); - } - accumulator - } - - // Apply a function to each element of the slice and an accumulator value, - // returning the final accumulated value. Unlike fold, reduce uses the first - // element of the given slice as its starting accumulator value. - fn reduce(self, f: fn(T, T) -> T) -> T { - let mut accumulator = self[0]; - for i in 1 .. self.len() { - accumulator = f(accumulator, self[i]); - } - accumulator - } - - // Returns true if all elements in the array satisfy the predicate - fn all(self, predicate: fn(T) -> bool) -> bool { - let mut ret = true; - for elem in self { - ret &= predicate(elem); - } - ret - } - - // Returns true if any element in the array satisfies the predicate - fn any(self, predicate: fn(T) -> bool) -> bool { - let mut ret = false; - for elem in self { - ret |= predicate(elem); - } - ret - } } From 22f2ed3179eebf545c98aa3a7043fcc5acc2c1f3 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 21 Jul 2023 12:52:43 -0500 Subject: [PATCH 3/9] Implement missing try_bind function --- .../tests/test_data/array_len/src/main.nr | 6 +- crates/noirc_frontend/src/hir_def/types.rs | 62 +++++++++++++++++-- 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/crates/nargo_cli/tests/test_data/array_len/src/main.nr b/crates/nargo_cli/tests/test_data/array_len/src/main.nr index 9099cfa2144..2c3cc0aee60 100644 --- a/crates/nargo_cli/tests/test_data/array_len/src/main.nr +++ b/crates/nargo_cli/tests/test_data/array_len/src/main.nr @@ -1,14 +1,14 @@ use dep::std; -fn len_plus_1(array: [T]) -> Field { +fn len_plus_1(array: [T; N]) -> Field { array.len() + 1 } -fn add_lens(a: [T], b: [Field]) -> Field { +fn add_lens(a: [T; N], b: [Field; M]) -> Field { a.len() + b.len() } -fn nested_call(b: [Field]) -> Field { +fn nested_call(b: [Field; N]) -> Field { len_plus_1(b) } diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index 8e62f1831d2..0a98ce2dc14 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -769,6 +769,56 @@ impl Type { } } + /// Try to bind a MaybeConstant variable to self, succeeding if self is a Constant, + /// MaybeConstant, or type variable. + pub fn try_bind_to_maybe_constant( + &self, + var: &TypeVariable, + target_length: u64, + ) -> Result<(), SpanKind> { + let target_id = match &*var.borrow() { + TypeBinding::Bound(_) => unreachable!(), + TypeBinding::Unbound(id) => *id, + }; + + match self { + Type::Constant(length) if *length == target_length => { + *var.borrow_mut() = TypeBinding::Bound(self.clone()); + Ok(()) + } + Type::MaybeConstant(binding, length) if *length == target_length => { + let borrow = binding.borrow(); + match &*borrow { + TypeBinding::Bound(typ) => typ.try_bind_to_maybe_constant(var, target_length), + // Avoid infinitely recursive bindings + TypeBinding::Unbound(id) if *id == target_id => Ok(()), + TypeBinding::Unbound(_) => { + drop(borrow); + *var.borrow_mut() = TypeBinding::Bound(self.clone()); + Ok(()) + } + } + } + Type::TypeVariable(binding) => { + let borrow = binding.borrow(); + match &*borrow { + TypeBinding::Bound(typ) => typ.try_bind_to_maybe_constant(var, target_length), + // Avoid infinitely recursive bindings + TypeBinding::Unbound(id) if *id == target_id => Ok(()), + TypeBinding::Unbound(_) => { + drop(borrow); + // MaybeConstant is more specific than TypeVariable so we bind the type + // variable to PolymorphicInt instead. + let clone = Type::MaybeConstant(var.clone(), target_length); + *binding.borrow_mut() = TypeBinding::Bound(clone); + Ok(()) + } + } + } + _ => Err(SpanKind::None), + } + } + /// Try to bind a PolymorphicInt variable to self, succeeding if self is an integer, field, /// other PolymorphicInt type, or type variable. If use_subtype is true, the CompTime fields /// of each will be checked via sub-typing rather than unification. @@ -953,12 +1003,12 @@ impl Type { other.try_bind_to(binding) } - (MaybeConstant(binding, _), other) | (other, MaybeConstant(binding, _)) => { + (MaybeConstant(binding, length), other) | (other, MaybeConstant(binding, length)) => { if let TypeBinding::Bound(link) = &*binding.borrow() { return link.try_unify(other, span); } - other.try_bind_to(binding) + other.try_bind_to_maybe_constant(binding, *length) } (Array(len_a, elem_a), Array(len_b, elem_b)) => { @@ -1094,19 +1144,19 @@ impl Type { other.try_bind_to(binding) } - (MaybeConstant(binding, _), other) => { + (MaybeConstant(binding, length), other) => { if let TypeBinding::Bound(link) = &*binding.borrow() { return link.is_subtype_of(other, span); } - other.try_bind_to(binding) + other.try_bind_to_maybe_constant(binding, *length) } - (other, MaybeConstant(binding, _)) => { + (other, MaybeConstant(binding, length)) => { if let TypeBinding::Bound(link) = &*binding.borrow() { return other.is_subtype_of(link, span); } - other.try_bind_to(binding) + other.try_bind_to_maybe_constant(binding, *length) } (Array(len_a, elem_a), Array(len_b, elem_b)) => { From c550dbb819ff8ccf9761d30449007152b0021f67 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 27 Jul 2023 11:26:49 -0500 Subject: [PATCH 4/9] Add missed case for NotConstant --- .../noirc_frontend/src/hir/type_check/expr.rs | 2 +- crates/noirc_frontend/src/hir_def/types.rs | 4 +++ .../src/monomorphization/mod.rs | 25 ++++++------------- noir_stdlib/src/array.nr | 9 +++++++ 4 files changed, 22 insertions(+), 18 deletions(-) diff --git a/crates/noirc_frontend/src/hir/type_check/expr.rs b/crates/noirc_frontend/src/hir/type_check/expr.rs index 2ad183e36c6..bfdca0f406b 100644 --- a/crates/noirc_frontend/src/hir/type_check/expr.rs +++ b/crates/noirc_frontend/src/hir/type_check/expr.rs @@ -86,7 +86,7 @@ impl<'interner> TypeChecker<'interner> { let typevar_id = self.interner.next_type_variable_id(); let typevar = Shared::new(TypeBinding::Unbound(typevar_id)); Type::MaybeConstant(typevar, length) - }, + } other => other, }; Type::Array(Box::new(length), Box::new(elem_type)) diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index 785d7f234be..e109308b20b 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -800,6 +800,10 @@ impl Type { *var.borrow_mut() = TypeBinding::Bound(self.clone()); Ok(()) } + Type::NotConstant => { + *var.borrow_mut() = TypeBinding::Bound(Type::NotConstant); + Ok(()) + } Type::MaybeConstant(binding, length) if *length == target_length => { let borrow = binding.borrow(); match &*borrow { diff --git a/crates/noirc_frontend/src/monomorphization/mod.rs b/crates/noirc_frontend/src/monomorphization/mod.rs index 3b370a31fe2..e72359ef7e2 100644 --- a/crates/noirc_frontend/src/monomorphization/mod.rs +++ b/crates/noirc_frontend/src/monomorphization/mod.rs @@ -22,7 +22,7 @@ use crate::{ }, node_interner::{self, DefinitionKind, NodeInterner, StmtId}, token::Attribute, - CompTime, ContractFunctionType, FunctionKind, Type, TypeBinding, TypeBindings, + CompTime, ContractFunctionType, FunctionKind, TypeBinding, TypeBindings, }; use self::ast::{Definition, FuncId, Function, LocalId, Program}; @@ -589,7 +589,7 @@ impl<'interner> Monomorphizer<'interner> { HirType::Unit => ast::Type::Unit, HirType::Array(length, element) => { - let element = Box::new(self.convert_type(element.as_ref())); + let element = Box::new(Self::convert_type(element.as_ref())); if let Some(length) = length.evaluate_to_u64() { ast::Type::Array(length, element) @@ -671,8 +671,12 @@ impl<'interner> Monomorphizer<'interner> { } } - self.try_evaluate_call(&func, &call.arguments, &return_type) - .unwrap_or(ast::Expression::Call(ast::Call { func, arguments, return_type, location })) + self.try_evaluate_call(&func, &return_type).unwrap_or(ast::Expression::Call(ast::Call { + func, + arguments, + return_type, + location, + })) } /// Adds a function argument that contains type metadata that is required to tell @@ -711,25 +715,12 @@ impl<'interner> Monomorphizer<'interner> { fn try_evaluate_call( &mut self, func: &ast::Expression, - arguments: &[node_interner::ExprId], result_type: &ast::Type, ) -> Option { if let ast::Expression::Ident(ident) = func { if let Definition::Builtin(opcode) = &ident.definition { // TODO(#1736): Move this builtin to the SSA pass return match opcode.as_str() { - "array_len" => { - let typ = self.interner.id_type(arguments[0]); - if let Type::Array(_, _) = typ { - let len = typ.evaluate_to_u64().unwrap(); - Some(ast::Expression::Literal(ast::Literal::Integer( - (len as u128).into(), - ast::Type::Field, - ))) - } else { - None - } - } "modulus_num_bits" => Some(ast::Expression::Literal(ast::Literal::Integer( (FieldElement::max_num_bits() as u128).into(), ast::Type::Field, diff --git a/noir_stdlib/src/array.nr b/noir_stdlib/src/array.nr index 9e44aa03fcc..db349317f91 100644 --- a/noir_stdlib/src/array.nr +++ b/noir_stdlib/src/array.nr @@ -22,6 +22,15 @@ impl [T; N] { a } + // Converts an array into a slice. + fn as_slice(self) -> [T] { + let mut slice = []; + for elem in self { + slice = slice.push_back(elem); + } + slice + } + // Apply a function to each element of an array, returning a new array // containing the mapped elements. fn map(self, f: fn(T) -> U) -> [U; N] { From d06b69c27ae505dfa5f5e42a129c7d1bd472f7f7 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 27 Jul 2023 11:45:55 -0500 Subject: [PATCH 5/9] Fix some tests --- .../tests/test_data_ssa_refactor/array_len/src/main.nr | 6 +++--- noir_stdlib/src/ecdsa_secp256k1.nr | 2 +- noir_stdlib/src/ecdsa_secp256r1.nr | 2 +- noir_stdlib/src/merkle.nr | 4 ++-- noir_stdlib/src/schnorr.nr | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr index 9099cfa2144..2c3cc0aee60 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr @@ -1,14 +1,14 @@ use dep::std; -fn len_plus_1(array: [T]) -> Field { +fn len_plus_1(array: [T; N]) -> Field { array.len() + 1 } -fn add_lens(a: [T], b: [Field]) -> Field { +fn add_lens(a: [T; N], b: [Field; M]) -> Field { a.len() + b.len() } -fn nested_call(b: [Field]) -> Field { +fn nested_call(b: [Field; N]) -> Field { len_plus_1(b) } diff --git a/noir_stdlib/src/ecdsa_secp256k1.nr b/noir_stdlib/src/ecdsa_secp256k1.nr index efeceef5df2..c46380e1988 100644 --- a/noir_stdlib/src/ecdsa_secp256k1.nr +++ b/noir_stdlib/src/ecdsa_secp256k1.nr @@ -1,2 +1,2 @@ #[foreign(ecdsa_secp256k1)] -fn verify_signature(_public_key_x : [u8; 32], _public_key_y : [u8; 32], _signature: [u8; 64], _message_hash: [u8]) -> bool {} +fn verify_signature(_public_key_x : [u8; 32], _public_key_y : [u8; 32], _signature: [u8; 64], _message_hash: [u8; N]) -> bool {} diff --git a/noir_stdlib/src/ecdsa_secp256r1.nr b/noir_stdlib/src/ecdsa_secp256r1.nr index 44df07d3590..77744384f52 100644 --- a/noir_stdlib/src/ecdsa_secp256r1.nr +++ b/noir_stdlib/src/ecdsa_secp256r1.nr @@ -1,2 +1,2 @@ #[foreign(ecdsa_secp256r1)] -fn verify_signature(_public_key_x : [u8; 32], _public_key_y : [u8; 32], _signature: [u8; 64], _message_hash: [u8]) -> bool {} +fn verify_signature(_public_key_x : [u8; 32], _public_key_y : [u8; 32], _signature: [u8; 64], _message_hash: [u8; N]) -> bool {} diff --git a/noir_stdlib/src/merkle.nr b/noir_stdlib/src/merkle.nr index 1f1a45ffe17..07588a52a5a 100644 --- a/noir_stdlib/src/merkle.nr +++ b/noir_stdlib/src/merkle.nr @@ -3,7 +3,7 @@ // XXX: In the future we can add an arity parameter // Returns the merkle root of the tree from the provided leaf, its hashpath, using a pedersen hash function. -fn compute_merkle_root(leaf: Field, index: Field, hash_path: [Field]) -> Field { +fn compute_merkle_root(leaf: Field, index: Field, hash_path: [Field; N]) -> Field { let n = hash_path.len(); let index_bits = index.to_le_bits(n as u32); let mut current = leaf; @@ -18,4 +18,4 @@ fn compute_merkle_root(leaf: Field, index: Field, hash_path: [Field]) -> Field { current = crate::hash::pedersen([hash_left, hash_right])[0]; }; current -} \ No newline at end of file +} diff --git a/noir_stdlib/src/schnorr.nr b/noir_stdlib/src/schnorr.nr index 5000efd3be4..1e69bcec821 100644 --- a/noir_stdlib/src/schnorr.nr +++ b/noir_stdlib/src/schnorr.nr @@ -1,2 +1,2 @@ #[foreign(schnorr_verify)] -fn verify_signature(_public_key_x: Field, _public_key_y: Field, _signature: [u8; 64], _message: [u8]) -> bool {} +fn verify_signature(_public_key_x: Field, _public_key_y: Field, _signature: [u8; 64], _message: [u8; N]) -> bool {} From eb3356170c4b2ec327e3040ebdd6230b906a36c9 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 27 Jul 2023 13:13:42 -0500 Subject: [PATCH 6/9] Fix poseidon test --- crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs | 11 +++-------- .../src/ssa_refactor/ir/instruction.rs | 12 +++--------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs index 5c9fde280a8..4cb95994546 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs @@ -382,14 +382,9 @@ impl DataFlowGraph { /// Returns the Type::Array associated with this ValueId if it refers to an array parameter. /// Otherwise, this returns None. - pub(crate) fn get_array_parameter_type( - &self, - value: ValueId, - ) -> Option<(Rc, usize)> { - match &self.values[self.resolve(value)] { - Value::Param { typ: Type::Array(element_type, size), .. } => { - Some((element_type.clone(), *size)) - } + pub(crate) fn try_get_array_length(&self, value: ValueId) -> Option { + match self.type_of_value(value) { + Type::Array(_, length) => Some(length), _ => None, } } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs index 416c53ba6b4..cec7408ebcd 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs @@ -420,15 +420,9 @@ fn simplify_call(func: ValueId, arguments: &[ValueId], dfg: &mut DataFlowGraph) Intrinsic::ArrayLen => { let slice = dfg.get_array_constant(arguments[0]); if let Some((slice, _)) = slice { - let slice_len = - dfg.make_constant(FieldElement::from(slice.len() as u128), Type::field()); - SimplifiedTo(slice_len) - } else if let Some((_, slice_len)) = dfg.get_array_parameter_type(arguments[0]) { - let slice_len = dfg.make_constant( - FieldElement::from(slice_len as u128), - Type::Numeric(NumericType::NativeField), - ); - SimplifiedTo(slice_len) + SimplifiedTo(dfg.make_constant((slice.len() as u128).into(), Type::field())) + } else if let Some(length) = dfg.try_get_array_length(arguments[0]) { + SimplifiedTo(dfg.make_constant((length as u128).into(), Type::field())) } else { None } From 3e366d8406d5fe27487d3f3df7ab52eedc3744d3 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 27 Jul 2023 14:58:36 -0500 Subject: [PATCH 7/9] Fix evaluation of slice length --- .../test_data_ssa_refactor/slices/src/main.nr | 4 +- .../src/ssa_refactor/ir/dfg.rs | 20 +++----- .../src/ssa_refactor/ir/function_inserter.rs | 6 +-- .../src/ssa_refactor/ir/instruction.rs | 6 ++- .../src/ssa_refactor/ir/value.rs | 10 ++-- .../src/ssa_refactor/opt/flatten_cfg.rs | 32 ++++++------- .../src/ssa_refactor/opt/inlining.rs | 4 +- .../src/ssa_refactor/ssa_builder/mod.rs | 11 ++--- .../src/ssa_refactor/ssa_gen/mod.rs | 20 +++----- .../src/monomorphization/ast.rs | 2 +- .../src/monomorphization/mod.rs | 48 +++++++------------ 11 files changed, 65 insertions(+), 98 deletions(-) diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/slices/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/slices/src/main.nr index a0460aafb40..f97078a2143 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/slices/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/slices/src/main.nr @@ -4,7 +4,7 @@ fn main(x : Field, y : pub Field) { /// TODO(#1889): Using slices in if statements where the condition is a witness /// is not yet supported - let mut slice: [Field] = [0; 2]; + let mut slice = [0; 2]; assert(slice[0] == 0); assert(slice[0] != 1); slice[0] = x; @@ -15,7 +15,7 @@ fn main(x : Field, y : pub Field) { assert(slice_plus_10[2] != 8); assert(slice_plus_10.len() == 3); - let mut new_slice: [Field] = []; + let mut new_slice = []; for i in 0..5 { new_slice = new_slice.push_back(i); } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs index 4cb95994546..b24af65723a 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs @@ -1,4 +1,4 @@ -use std::{borrow::Cow, collections::HashMap, rc::Rc}; +use std::{borrow::Cow, collections::HashMap}; use crate::ssa_refactor::ir::instruction::SimplifyResult; @@ -9,7 +9,7 @@ use super::{ Instruction, InstructionId, InstructionResultType, Intrinsic, TerminatorInstruction, }, map::DenseMap, - types::{CompositeType, Type}, + types::Type, value::{Value, ValueId}, }; @@ -226,12 +226,9 @@ impl DataFlowGraph { } /// Create a new constant array value from the given elements - pub(crate) fn make_array( - &mut self, - array: im::Vector, - element_type: Rc, - ) -> ValueId { - self.make_value(Value::Array { array, element_type }) + pub(crate) fn make_array(&mut self, array: im::Vector, typ: Type) -> ValueId { + assert!(matches!(typ, Type::Array(..) | Type::Slice(_))); + self.make_value(Value::Array { array, typ }) } /// Gets or creates a ValueId for the given FunctionId. @@ -369,13 +366,10 @@ impl DataFlowGraph { /// Returns the Value::Array associated with this ValueId if it refers to an array constant. /// Otherwise, this returns None. - pub(crate) fn get_array_constant( - &self, - value: ValueId, - ) -> Option<(im::Vector, Rc)> { + pub(crate) fn get_array_constant(&self, value: ValueId) -> Option<(im::Vector, Type)> { match &self.values[self.resolve(value)] { // Vectors are shared, so cloning them is cheap - Value::Array { array, element_type } => Some((array.clone(), element_type.clone())), + Value::Array { array, typ } => Some((array.clone(), typ.clone())), _ => None, } } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/function_inserter.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/function_inserter.rs index 22a1399ae79..38dcfbbb168 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/function_inserter.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/function_inserter.rs @@ -33,11 +33,11 @@ impl<'f> FunctionInserter<'f> { match self.values.get(&value) { Some(value) => *value, None => match &self.function.dfg[value] { - super::value::Value::Array { array, element_type } => { + super::value::Value::Array { array, typ } => { let array = array.clone(); - let element_type = element_type.clone(); + let typ = typ.clone(); let new_array = array.iter().map(|id| self.resolve(*id)).collect(); - let new_id = self.function.dfg.make_array(new_array, element_type); + let new_id = self.function.dfg.make_array(new_array, typ); self.values.insert(value, new_id); new_id } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs index cec7408ebcd..b7a3ea02ae9 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs @@ -528,9 +528,11 @@ fn constant_to_radix( while limbs.len() < limb_count_with_padding as usize { limbs.push(FieldElement::zero()); } - let result_constants = + let result_constants: im::Vector = limbs.into_iter().map(|limb| dfg.make_constant(limb, Type::unsigned(bit_size))).collect(); - dfg.make_array(result_constants, Rc::new(vec![Type::unsigned(bit_size)])) + + let typ = Type::Array(Rc::new(vec![Type::unsigned(bit_size)]), result_constants.len()); + dfg.make_array(result_constants, typ) } /// The possible return values for Instruction::return_types diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/value.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/value.rs index 03475f5f514..cea526058b4 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/value.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/value.rs @@ -1,5 +1,3 @@ -use std::rc::Rc; - use acvm::FieldElement; use crate::ssa_refactor::ir::basic_block::BasicBlockId; @@ -8,7 +6,7 @@ use super::{ function::FunctionId, instruction::{InstructionId, Intrinsic}, map::Id, - types::{CompositeType, Type}, + types::Type, }; pub(crate) type ValueId = Id; @@ -38,7 +36,7 @@ pub(crate) enum Value { NumericConstant { constant: FieldElement, typ: Type }, /// Represents a constant array value - Array { array: im::Vector, element_type: Rc }, + Array { array: im::Vector, typ: Type }, /// This Value refers to a function in the IR. /// Functions always have the type Type::Function. @@ -64,9 +62,7 @@ impl Value { Value::Instruction { typ, .. } => typ.clone(), Value::Param { typ, .. } => typ.clone(), Value::NumericConstant { typ, .. } => typ.clone(), - Value::Array { element_type, array } => { - Type::Array(element_type.clone(), array.len() / element_type.len()) - } + Value::Array { typ, .. } => typ.clone(), Value::Function { .. } => Type::Function, Value::Intrinsic { .. } => Type::Function, Value::ForeignFunction { .. } => Type::Function, diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs index ac62071d6ee..4ff857f942f 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs @@ -131,10 +131,7 @@ //! v11 = mul v4, Field 12 //! v12 = add v10, v11 //! store v12 at v5 (new store) -use std::{ - collections::{BTreeMap, HashMap, HashSet}, - rc::Rc, -}; +use std::collections::{BTreeMap, HashMap, HashSet}; use acvm::FieldElement; use iter_extended::vecmap; @@ -148,7 +145,7 @@ use crate::ssa_refactor::{ function::Function, function_inserter::FunctionInserter, instruction::{BinaryOp, Instruction, InstructionId, TerminatorInstruction}, - types::{CompositeType, Type}, + types::Type, value::ValueId, }, ssa_gen::Ssa, @@ -393,14 +390,9 @@ impl<'f> Context<'f> { Type::Numeric(_) => { self.merge_numeric_values(then_condition, else_condition, then_value, else_value) } - Type::Array(element_types, len) => self.merge_array_values( - element_types, - len, - then_condition, - else_condition, - then_value, - else_value, - ), + typ @ Type::Array(_, _) => { + self.merge_array_values(typ, then_condition, else_condition, then_value, else_value) + } // TODO(#1889) Type::Slice(_) => panic!("Cannot return slices from an if expression"), Type::Reference => panic!("Cannot return references from an if expression"), @@ -413,8 +405,7 @@ impl<'f> Context<'f> { /// by creating a new array containing the result of self.merge_values for each element. fn merge_array_values( &mut self, - element_types: Rc, - len: usize, + typ: Type, then_condition: ValueId, else_condition: ValueId, then_value: ValueId, @@ -422,6 +413,11 @@ impl<'f> Context<'f> { ) -> ValueId { let mut merged = im::Vector::new(); + let (element_types, len) = match &typ { + Type::Array(elements, len) => (elements, *len), + _ => panic!("Expected array type"), + }; + for i in 0..len { for (element_index, element_type) in element_types.iter().enumerate() { let index = ((i * element_types.len() + element_index) as u128).into(); @@ -446,7 +442,7 @@ impl<'f> Context<'f> { } } - self.inserter.function.dfg.make_array(merged, element_types) + self.inserter.function.dfg.make_array(merged, typ) } /// Merge two numeric values a and b from separate basic blocks to a single value. This @@ -1333,8 +1329,10 @@ mod test { let b3 = builder.insert_block(); let element_type = Rc::new(vec![Type::field()]); + let array_type = Type::Array(element_type.clone(), 1); + let zero = builder.field_constant(0_u128); - let zero_array = builder.array_constant(im::Vector::unit(zero), element_type.clone()); + let zero_array = builder.array_constant(im::Vector::unit(zero), array_type); let i_zero = builder.numeric_constant(0_u128, Type::unsigned(32)); let pedersen = builder.import_intrinsic_id(Intrinsic::BlackBox(acvm::acir::BlackBoxFunc::Pedersen)); diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs index 430b52ce9f6..7aa2f9d176a 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs @@ -217,9 +217,9 @@ impl<'function> PerFunctionContext<'function> { Value::ForeignFunction(function) => { self.context.builder.import_foreign_function(function) } - Value::Array { array, element_type } => { + Value::Array { array, typ } => { let elements = array.iter().map(|value| self.translate_value(*value)).collect(); - self.context.builder.array_constant(elements, element_type.clone()) + self.context.builder.array_constant(elements, typ.clone()) } }; diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs index d3d9e56b3af..02350d9ed17 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs @@ -1,4 +1,4 @@ -use std::{borrow::Cow, rc::Rc}; +use std::borrow::Cow; use acvm::FieldElement; use noirc_errors::Location; @@ -17,7 +17,6 @@ use super::{ dfg::InsertInstructionResult, function::RuntimeType, instruction::{InstructionId, Intrinsic}, - types::CompositeType, }, ssa_gen::Ssa, }; @@ -115,12 +114,8 @@ impl FunctionBuilder { } /// Insert an array constant into the current function with the given element values. - pub(crate) fn array_constant( - &mut self, - elements: im::Vector, - element_types: Rc, - ) -> ValueId { - self.current_function.dfg.make_array(elements, element_types) + pub(crate) fn array_constant(&mut self, elements: im::Vector, typ: Type) -> ValueId { + self.current_function.dfg.make_array(elements, typ) } /// Returns the type of the given value. diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs index 13e67f26cc5..2b6db4e7586 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs @@ -2,8 +2,6 @@ mod context; mod program; mod value; -use std::rc::Rc; - pub(crate) use program::Ssa; use context::SharedContext; @@ -16,12 +14,7 @@ use self::{ value::{Tree, Values}, }; -use super::ir::{ - function::RuntimeType, - instruction::BinaryOp, - types::{CompositeType, Type}, - value::ValueId, -}; +use super::ir::{function::RuntimeType, instruction::BinaryOp, types::Type, value::ValueId}; /// Generates SSA for the given monomorphized program. /// @@ -115,8 +108,8 @@ impl<'a> FunctionContext<'a> { match literal { ast::Literal::Array(array) => { let elements = vecmap(&array.contents, |element| self.codegen_expression(element)); - let element_types = Self::convert_type(&array.element_type).flatten(); - self.codegen_array(elements, element_types) + let typ = Self::convert_non_tuple_type(&array.typ); + self.codegen_array(elements, typ) } ast::Literal::Integer(value, typ) => { let typ = Self::convert_non_tuple_type(typ); @@ -129,7 +122,8 @@ impl<'a> FunctionContext<'a> { let elements = vecmap(string.as_bytes(), |byte| { self.builder.numeric_constant(*byte as u128, Type::field()).into() }); - self.codegen_array(elements, vec![Type::char()]) + let typ = Self::convert_non_tuple_type(&ast::Type::String(elements.len() as u64)); + self.codegen_array(elements, typ) } } } @@ -143,7 +137,7 @@ impl<'a> FunctionContext<'a> { /// stored the same as the array [1, 2, 3, 4]. /// /// The value returned from this function is always that of the allocate instruction. - fn codegen_array(&mut self, elements: Vec, element_types: CompositeType) -> Values { + fn codegen_array(&mut self, elements: Vec, typ: Type) -> Values { let mut array = im::Vector::new(); for element in elements { @@ -153,7 +147,7 @@ impl<'a> FunctionContext<'a> { }); } - self.builder.array_constant(array, Rc::new(element_types)).into() + self.builder.array_constant(array, typ).into() } fn codegen_block(&mut self, block: &[Expression]) -> Values { diff --git a/crates/noirc_frontend/src/monomorphization/ast.rs b/crates/noirc_frontend/src/monomorphization/ast.rs index 7cac2ed8e4f..488d05c6509 100644 --- a/crates/noirc_frontend/src/monomorphization/ast.rs +++ b/crates/noirc_frontend/src/monomorphization/ast.rs @@ -119,7 +119,7 @@ pub struct Cast { #[derive(Debug, Clone)] pub struct ArrayLiteral { pub contents: Vec, - pub element_type: Type, + pub typ: Type, } #[derive(Debug, Clone)] diff --git a/crates/noirc_frontend/src/monomorphization/mod.rs b/crates/noirc_frontend/src/monomorphization/mod.rs index 8140065a2ac..bb0228091da 100644 --- a/crates/noirc_frontend/src/monomorphization/mod.rs +++ b/crates/noirc_frontend/src/monomorphization/mod.rs @@ -269,7 +269,7 @@ impl<'interner> Monomorphizer<'interner> { HirExpression::Literal(HirLiteral::Array(array)) => match array { HirArrayLiteral::Standard(array) => self.standard_array(expr, array), HirArrayLiteral::Repeated { repeated_element, length } => { - self.repeated_array(repeated_element, length) + self.repeated_array(expr, repeated_element, length) } }, HirExpression::Literal(HirLiteral::Unit) => ast::Expression::Block(vec![]), @@ -354,25 +354,26 @@ impl<'interner> Monomorphizer<'interner> { array: node_interner::ExprId, array_elements: Vec, ) -> ast::Expression { - let element_type = - Self::convert_type(&unwrap_array_element_type(&self.interner.id_type(array))); + let typ = Self::convert_type(&self.interner.id_type(array)); let contents = vecmap(array_elements, |id| self.expr(id)); - ast::Expression::Literal(ast::Literal::Array(ast::ArrayLiteral { contents, element_type })) + ast::Expression::Literal(ast::Literal::Array(ast::ArrayLiteral { contents, typ })) } fn repeated_array( &mut self, + array: node_interner::ExprId, repeated_element: node_interner::ExprId, length: HirType, ) -> ast::Expression { - let element_type = Self::convert_type(&self.interner.id_type(repeated_element)); + let typ = Self::convert_type(&self.interner.id_type(array)); + let contents = self.expr(repeated_element); let length = length .evaluate_to_u64() .expect("Length of array is unknown when evaluating numeric generic"); let contents = vec![contents; length as usize]; - ast::Expression::Literal(ast::Literal::Array(ast::ArrayLiteral { contents, element_type })) + ast::Expression::Literal(ast::Literal::Array(ast::ArrayLiteral { contents, typ })) } fn index(&mut self, id: node_interner::ExprId, index: HirIndexExpression) -> ast::Expression { @@ -764,17 +765,17 @@ impl<'interner> Monomorphizer<'interner> { } fn modulus_array_literal(&self, bytes: Vec, arr_elem_bits: u32) -> ast::Expression { + use ast::*; + let int_type = Type::Integer(crate::Signedness::Unsigned, arr_elem_bits); + let bytes_as_expr = vecmap(bytes, |byte| { - ast::Expression::Literal(ast::Literal::Integer( - (byte as u128).into(), - ast::Type::Integer(crate::Signedness::Unsigned, arr_elem_bits), - )) + Expression::Literal(Literal::Integer((byte as u128).into(), int_type.clone())) }); - let arr_literal = ast::ArrayLiteral { - contents: bytes_as_expr, - element_type: ast::Type::Integer(crate::Signedness::Unsigned, arr_elem_bits), - }; - ast::Expression::Literal(ast::Literal::Array(arr_literal)) + + let typ = Type::Array(bytes_as_expr.len() as u64, Box::new(int_type)); + + let arr_literal = ArrayLiteral { typ, contents: bytes_as_expr }; + Expression::Literal(Literal::Array(arr_literal)) } fn queue_function( @@ -915,7 +916,7 @@ impl<'interner> Monomorphizer<'interner> { let element = self.zeroed_value_of_type(element_type.as_ref()); ast::Expression::Literal(ast::Literal::Array(ast::ArrayLiteral { contents: vec![element; *length as usize], - element_type: element_type.as_ref().clone(), + typ: ast::Type::Array(*length, element_type.clone()), })) } ast::Type::String(length) => { @@ -930,7 +931,7 @@ impl<'interner> Monomorphizer<'interner> { ast::Type::Slice(element_type) => { ast::Expression::Literal(ast::Literal::Array(ast::ArrayLiteral { contents: vec![], - element_type: *element_type.clone(), + typ: ast::Type::Slice(element_type.clone()), })) } ast::Type::MutableReference(element) => { @@ -1001,19 +1002,6 @@ fn unwrap_struct_type(typ: &HirType) -> Vec<(String, HirType)> { } } -fn unwrap_array_element_type(typ: &HirType) -> HirType { - match typ { - HirType::Array(_, elem) => *elem.clone(), - HirType::TypeVariable(binding, TypeVariableKind::Normal) => match &*binding.borrow() { - TypeBinding::Bound(binding) => unwrap_array_element_type(binding), - TypeBinding::Unbound(_) => unreachable!(), - }, - other => { - unreachable!("unwrap_array_element_type: expected an array or slice, found {:?}", other) - } - } -} - fn perform_instantiation_bindings(bindings: &TypeBindings) { for (var, binding) in bindings.values() { *var.borrow_mut() = TypeBinding::Bound(binding.clone()); From fe00354fef7c21e8a8c39b8469dd605fc018e797 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 27 Jul 2023 15:17:56 -0500 Subject: [PATCH 8/9] Fix tests --- crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs | 3 ++- .../src/ssa_refactor/opt/constant_folding.rs | 7 +++++-- crates/noirc_evaluator/src/ssa_refactor/opt/mem2reg.rs | 5 +++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs index b0ade9419fe..3bf18a2d86a 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -1057,7 +1057,8 @@ mod tests { let one = builder.field_constant(FieldElement::one()); let element_type = Rc::new(vec![Type::field()]); - let array = builder.array_constant(im::Vector::unit(one), element_type); + let array_type = Type::Array(element_type, 1); + let array = builder.array_constant(im::Vector::unit(one), array_type); builder.terminate_with_return(vec![array]); diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/constant_folding.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/constant_folding.rs index 3c40e2a15c5..acf048595d7 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/constant_folding.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/constant_folding.rs @@ -92,6 +92,8 @@ impl Context { #[cfg(test)] mod test { + use std::rc::Rc; + use crate::ssa_refactor::{ ir::{ function::RuntimeType, @@ -176,8 +178,9 @@ mod test { let v0 = builder.add_parameter(Type::field()); let one = builder.field_constant(1u128); let v1 = builder.insert_binary(v0, BinaryOp::Add, one); - let arr = - builder.current_function.dfg.make_array(vec![v1].into(), vec![Type::field()].into()); + + let array_type = Type::Array(Rc::new(vec![Type::field()]), 1); + let arr = builder.current_function.dfg.make_array(vec![v1].into(), array_type); builder.terminate_with_return(vec![arr]); let ssa = builder.finish().fold_constants(); diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/mem2reg.rs index 145ba25f5a5..15108abc490 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/mem2reg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/mem2reg.rs @@ -212,10 +212,11 @@ mod tests { let two = builder.field_constant(FieldElement::one()); let element_type = Rc::new(vec![Type::field()]); - let array = builder.array_constant(vector![one, two], element_type.clone()); + let array_type = Type::Array(element_type, 2); + let array = builder.array_constant(vector![one, two], array_type.clone()); builder.insert_store(v0, array); - let v1 = builder.insert_load(v0, Type::Array(element_type, 2)); + let v1 = builder.insert_load(v0, array_type); let v2 = builder.insert_array_get(v1, one, Type::field()); builder.terminate_with_return(vec![v2]); From 045166a8bd58fa0f8ceae1c90613904ca39c8839 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 27 Jul 2023 15:32:57 -0500 Subject: [PATCH 9/9] Fix 2070 --- .../tests/test_data_ssa_refactor/array_len/Prover.toml | 1 + .../tests/test_data_ssa_refactor/array_len/src/main.nr | 7 ++++++- .../src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs | 5 +++++ crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs | 8 ++++++++ 4 files changed, 20 insertions(+), 1 deletion(-) diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/Prover.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/Prover.toml index 3c3295e6848..a5ffe607b73 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/Prover.toml +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/Prover.toml @@ -1,2 +1,3 @@ len3 = [1, 2, 3] len4 = [1, 2, 3, 4] +x = 123 diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr index 2c3cc0aee60..65c2295cefb 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr @@ -12,7 +12,7 @@ fn nested_call(b: [Field; N]) -> Field { len_plus_1(b) } -fn main(len3: [u8; 3], len4: [Field; 4]) { +fn main(x: Field, len3: [u8; 3], len4: [Field; 4]) { assert(len_plus_1(len3) == 4); assert(len_plus_1(len4) == 5); assert(add_lens(len3, len4) == 7); @@ -20,4 +20,9 @@ fn main(len3: [u8; 3], len4: [Field; 4]) { // std::array::len returns a comptime value assert(len4[len3.len()] == 4); + + // Regression for #1023, ensure .len still works after calling to_le_bytes on a witness. + // This was needed because normally .len is evaluated before acir-gen where to_le_bytes + // on a witness is only evaluated during/after acir-gen. + assert(x.to_le_bytes(8).len() != 0); } diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs index 2fdfb0bd10f..d6a7bb86401 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs @@ -55,6 +55,11 @@ impl AcirType { } } + /// Returns a field type + pub(crate) fn field() -> Self { + AcirType::NumericType(NumericType::NativeField) + } + /// Returns a boolean type fn boolean() -> Self { AcirType::NumericType(NumericType::Unsigned { bit_size: 1 }) diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs index 3bf18a2d86a..43ba027082b 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -933,6 +933,14 @@ impl Context { Ok(Self::convert_vars_to_values(out_vars, dfg, result_ids)) } + Intrinsic::ArrayLen => { + let len = match self.convert_value(arguments[0], dfg) { + AcirValue::Var(_, _) => unreachable!("Non-array passed to array.len() method"), + AcirValue::Array(values) => (values.len() as u128).into(), + AcirValue::DynamicArray(array) => (array.len as u128).into(), + }; + Ok(vec![AcirValue::Var(self.acir_context.add_constant(len), AcirType::field())]) + } _ => todo!("expected a black box function"), } }