feat: Make arrays and slices polymorphic over each other (#2070)

* Start experiment to merge array and slice types

* Finish merger of slices and arrays

* Implement missing try_bind function

* Add missed case for NotConstant

* Fix some tests

* Fix poseidon test

* Fix evaluation of slice length

* Fix tests

* fix: Slice initialization (#2080)

* fix: Initialize Value::Array of type Slice

* test: improved brillig test after bug fix

* Add array -> slice coercion

* Update comment

* Clippy suggestion

* Use coercions in more places to match rust

---------

Co-authored-by: Álvaro Rodríguez <[email protected]>

crates/nargo_cli/tests/test_data/array_len/src/main.nr

@@ -1,14 +1,14 @@
 use dep::std;
 
-fn len_plus_1<T>(array: [T]) -> Field {
+fn len_plus_1<T, N>(array: [T; N]) -> Field {
     array.len() + 1
 }
 
-fn add_lens<T>(a: [T], b: [Field]) -> Field {
+fn add_lens<T, N, M>(a: [T; N], b: [Field; M]) -> Field {
     a.len() + b.len()
 }
 
-fn nested_call(b: [Field]) -> Field {
+fn nested_call<N>(b: [Field; N]) -> Field {
     len_plus_1(b)
 }
 

crates/nargo_cli/tests/test_data/brillig_slices/src/main.nr

@@ -2,71 +2,75 @@ use dep::std::slice;
 use dep::std;
 
 unconstrained fn main(x: Field, y: Field) {
-    // Mark it as mut so the compiler doesn't simplify the following operations
-    // But don't reuse the mut slice variable until this is fixed https://github.com/noir-lang/noir/issues/1931
-    let slice: [Field] = [y, x];
+    let mut slice: [Field] = [y, x];
     assert(slice.len() == 2);
 
-    let mut pushed_back_slice = slice.push_back(7);
-    assert(pushed_back_slice.len() == 3);
-    assert(pushed_back_slice[0] == y);
-    assert(pushed_back_slice[1] == x);
-    assert(pushed_back_slice[2] == 7);
+    slice = slice.push_back(7);
+    assert(slice.len() == 3);
+    assert(slice[0] == y);
+    assert(slice[1] == x);
+    assert(slice[2] == 7);
 
     // Array set on slice target
-    pushed_back_slice[0] = x;
-    pushed_back_slice[1] = y;
-    pushed_back_slice[2] = 1;
-
-    assert(pushed_back_slice[0] == x);
-    assert(pushed_back_slice[1] == y);
-    assert(pushed_back_slice[2] == 1);
-
-    assert(slice.len() == 2);
-
-    let pushed_front_slice = pushed_back_slice.push_front(2);
-    assert(pushed_front_slice.len() == 4);
-    assert(pushed_front_slice[0] == 2);
-    assert(pushed_front_slice[1] == x);
-    assert(pushed_front_slice[2] == y);
-    assert(pushed_front_slice[3] == 1);
-
-    let (item, popped_front_slice) = pushed_front_slice.pop_front();
+    slice[0] = x;
+    slice[1] = y;
+    slice[2] = 1;
+
+    assert(slice[0] == x);
+    assert(slice[1] == y);
+    assert(slice[2] == 1);
+
+    slice = push_front_to_slice(slice, 2);
+    assert(slice.len() == 4);
+    assert(slice[0] == 2);
+    assert(slice[1] == x);
+    assert(slice[2] == y);
+    assert(slice[3] == 1);
+
+    let (item, popped_front_slice) = slice.pop_front();
+    slice = popped_front_slice;
     assert(item == 2);
 
-    assert(popped_front_slice.len() == 3);
-    assert(popped_front_slice[0] == x);
-    assert(popped_front_slice[1] == y);
-    assert(popped_front_slice[2] == 1);
+    assert(slice.len() == 3);
+    assert(slice[0] == x);
+    assert(slice[1] == y);
+    assert(slice[2] == 1);
 
-    let (popped_back_slice, another_item) = popped_front_slice.pop_back();
+    let (popped_back_slice, another_item) = slice.pop_back();
+    slice = popped_back_slice;
     assert(another_item == 1);
 
-    assert(popped_back_slice.len() == 2);
-    assert(popped_back_slice[0] == x);
-    assert(popped_back_slice[1] == y);
+    assert(slice.len() == 2);
+    assert(slice[0] == x);
+    assert(slice[1] == y);
 
-    let inserted_slice = popped_back_slice.insert(1, 2);
-    assert(inserted_slice.len() == 3);
-    assert(inserted_slice[0] == x);
-    assert(inserted_slice[1] == 2);
-    assert(inserted_slice[2] == y);
+    slice = slice.insert(1, 2);
+    assert(slice.len() == 3);
+    assert(slice[0] == x);
+    assert(slice[1] == 2);
+    assert(slice[2] == y);
 
-    let (removed_slice, should_be_2) = inserted_slice.remove(1);
+    let (removed_slice, should_be_2) = slice.remove(1);
+    slice = removed_slice;
     assert(should_be_2 == 2);
 
-    assert(removed_slice.len() == 2);
-    assert(removed_slice[0] == x);
-    assert(removed_slice[1] == y);
+    assert(slice.len() == 2);
+    assert(slice[0] == x);
+    assert(slice[1] == y);
 
-    let (slice_with_only_x, should_be_y) = removed_slice.remove(1);
+    let (slice_with_only_x, should_be_y) = slice.remove(1);
+    slice = slice_with_only_x;
     assert(should_be_y == y);
 
-    assert(slice_with_only_x.len() == 1);
-    assert(removed_slice[0] == x);
+    assert(slice.len() == 1);
+    assert(slice[0] == x);
 
-    let (empty_slice, should_be_x) = slice_with_only_x.remove(0);
+    let (empty_slice, should_be_x) = slice.remove(0);
     assert(should_be_x == x);
     assert(empty_slice.len() == 0);
 }
 
+// Tests slice passing to/from functions
+unconstrained fn push_front_to_slice<T>(slice: [T], item: T) -> [T] {
+    slice.push_front(item)
+}

crates/nargo_cli/tests/test_data/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);
     }

crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

@@ -800,22 +800,37 @@ impl<'block> BrilligBlock<'block> {
                     value_id,
                     dfg,
                 );
-                let heap_array = self.function_context.extract_heap_array(new_variable);
 
-                self.brillig_context
-                    .allocate_fixed_length_array(heap_array.pointer, heap_array.size);
+                // Initialize the variable
+                let pointer = match new_variable {
+                    RegisterOrMemory::HeapArray(heap_array) => {
+                        self.brillig_context
+                            .allocate_fixed_length_array(heap_array.pointer, array.len());
+
+                        heap_array.pointer
+                    }
+                    RegisterOrMemory::HeapVector(heap_vector) => {
+                        self.brillig_context
+                            .const_instruction(heap_vector.size, array.len().into());
+                        self.brillig_context
+                            .allocate_array_instruction(heap_vector.pointer, heap_vector.size);
+
+                        heap_vector.pointer
+                    }
+                    _ => unreachable!(
+                        "ICE: Cannot initialize array value created as {new_variable:?}"
+                    ),
+                };
+
+                // Write the items
 
                 // Allocate a register for the iterator
                 let iterator_register = self.brillig_context.make_constant(0_usize.into());
 
                 for element_id in array.iter() {
                     let element_variable = self.convert_ssa_value(*element_id, dfg);
                     // Store the item in memory
-                    self.store_variable_in_array(
-                        heap_array.pointer,
-                        iterator_register,
-                        element_variable,
-                    );
+                    self.store_variable_in_array(pointer, iterator_register, element_variable);
                     // Increment the iterator
                     self.brillig_context.usize_op_in_place(iterator_register, BinaryIntOp::Add, 1);
                 }

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]);
 

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<ValueId>,
-        element_type: Rc<CompositeType>,
-    ) -> ValueId {
-        self.make_value(Value::Array { array, element_type })
+    pub(crate) fn make_array(&mut self, array: im::Vector<ValueId>, 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,27 +366,19 @@ 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<ValueId>, Rc<CompositeType>)> {
+    pub(crate) fn get_array_constant(&self, value: ValueId) -> Option<(im::Vector<ValueId>, 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,
         }
     }
 
-    /// 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<CompositeType>, usize)> {
-        match &self.values[self.resolve(value)] {
-            Value::Param { typ: Type::Array(element_type, size), .. } => {
-                Some((element_type.clone(), *size))
-            }
+    /// If this value is an array, return the length of the array as indicated by its type.
+    /// Otherwise, return None.
+    pub(crate) fn try_get_array_length(&self, value: ValueId) -> Option<usize> {
+        match self.type_of_value(value) {
+            Type::Array(_, length) => Some(length),
             _ => None,
         }
     }

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
                 }

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
             }
@@ -534,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<ValueId> =
         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

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<Value>;
@@ -38,7 +36,7 @@ pub(crate) enum Value {
     NumericConstant { constant: FieldElement, typ: Type },
 
     /// Represents a constant array value
-    Array { array: im::Vector<ValueId>, element_type: Rc<CompositeType> },
+    Array { array: im::Vector<ValueId>, 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,

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();