feat(ssa_refactor)!: Add Slices (#1728)

* initial slices work on frontend

* cargo  clippy

* develop comment cleanup

* working push_back and len slices commands

* cargo clippy

* cleanup

* fix clippy

* empty slice being processed

* cargo clippy

* delete old comment

* remove unused import

* add enable_slices flag to avoid slices in old SSA

* add new SSA type for slices

* missing Slice conversion

* Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs

Co-authored-by: jfecher <jake@aztecprotocol.com>

* PR comments, fix slice/array subtyping

* hack to handle duplicate methods and mismatched types when compiling w/ old SSA pass that does not handle slices

* cleanup enable_slices tech debt to be part of the NodeInterner

* reference enable_slices flag issue

* cleanup name

* remove SSA Value::Slice

* remove dbg

* fix array len from slice params

* remove old debug

* unwrap_array_element_type method in monomorphization pass

* cargo clippy

* mark issue in TODO comment for tech debt that removes slice module in stdlib

* Update crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs

Co-authored-by: jfecher <jake@aztecprotocol.com>

* fix stdlib crate check

* Update noir_stdlib/src/slice.nr

Co-authored-by: jfecher <jake@aztecprotocol.com>

* Update noir_stdlib/src/slice.nr

Co-authored-by: jfecher <jake@aztecprotocol.com>

* Update noir_stdlib/src/slice.nr

Co-authored-by: jfecher <jake@aztecprotocol.com>

* Update noir_stdlib/src/slice.nr

Co-authored-by: jfecher <jake@aztecprotocol.com>

* PR comments and stdlib crate assert

* cargo fmt

* keep enable slices check in expr type check

* increase timeout time

* increase timeout to 60m

* back to 30m timeout

* excldue 6_array from old ssa test and move 6_array into ssa_refactor tests

* unwrap_or_else for first_elem_type

* only test new ssa :D

* add back old ssa tests

* remove Vec type

* remove Vec type check

* remove Vec from stdlib as we have no primitive type until we add in Vec using mutable refs

* try w/ a pub input to 6_array

* im dumb and misplaced the pub

* remove pub

* temp: remove unused tests for debugging

* Revert "temp: remove unused tests for debugging"

This reverts commit daac684.

---------

Co-authored-by: jfecher <jake@aztecprotocol.com>
Co-authored-by: kevaundray <kevtheappdev@gmail.com>

Author: 3 people

crates/fm/src/lib.rs

@@ -70,7 +70,7 @@ impl FileManager {
         // Unwrap as we ensure that all file_id's map to a corresponding file in the file map
         self.file_map.get_file(file_id).unwrap()
     }
-    fn path(&mut self, file_id: FileId) -> &Path {
+    pub fn path(&mut self, file_id: FileId) -> &Path {
         // Unwrap as we ensure that all file_ids are created by the file manager
         // So all file_ids will points to a corresponding path
         self.id_to_path.get(&file_id).unwrap().0.as_path()

crates/lsp/src/lib.rs

@@ -139,7 +139,7 @@ fn on_code_lens_request(
 
     // We ignore the warnings and errors produced by compilation for producing codelenses
     // because we can still get the test functions even if compilation fails
-    let _ = check_crate(&mut state.context, false);
+    let _ = check_crate(&mut state.context, false, false);
 
     let fm = &state.context.file_manager;
     let files = fm.as_simple_files();
@@ -224,7 +224,7 @@ fn on_did_save_text_document(
 
     let mut diagnostics = Vec::new();
 
-    let file_diagnostics = match check_crate(&mut state.context, false) {
+    let file_diagnostics = match check_crate(&mut state.context, false, false) {
         Ok(warnings) => warnings,
         Err(errors_and_warnings) => errors_and_warnings,
     };

crates/nargo_cli/src/cli/check_cmd.rs

@@ -37,7 +37,7 @@ fn check_from_path<B: Backend>(
     compile_options: &CompileOptions,
 ) -> Result<(), CliError<B>> {
     let mut context = resolve_root_manifest(program_dir)?;
-    check_crate_and_report_errors(&mut context, compile_options.deny_warnings)?;
+    check_crate_and_report_errors(&mut context, compile_options.deny_warnings, compile_options.experimental_ssa)?;
 
     // XXX: We can have a --overwrite flag to determine if you want to overwrite the Prover/Verifier.toml files
     if let Some((parameters, return_type)) = compute_function_signature(&context) {
@@ -214,7 +214,8 @@ d2 = ["", "", ""]
 pub(crate) fn check_crate_and_report_errors(
     context: &mut Context,
     deny_warnings: bool,
+    enable_slices: bool,
 ) -> Result<(), ReportedErrors> {
-    let result = check_crate(context, deny_warnings).map(|warnings| ((), warnings));
+    let result = check_crate(context, deny_warnings, enable_slices).map(|warnings| ((), warnings));
     super::compile_cmd::report_errors(result, context, deny_warnings)
 }

crates/nargo_cli/src/cli/mod.rs

@@ -132,7 +132,7 @@ mod tests {
         let mut context = Context::default();
         create_local_crate(&mut context, &root_file, CrateType::Binary);
 
-        let result = check_crate(&mut context, false);
+        let result = check_crate(&mut context, false, false);
         let success = result.is_ok();
 
         let errors = match result {

crates/nargo_cli/src/cli/test_cmd.rs

@@ -41,7 +41,7 @@ fn run_tests<B: Backend>(
     compile_options: &CompileOptions,
 ) -> Result<(), CliError<B>> {
     let mut context = resolve_root_manifest(program_dir)?;
-    check_crate_and_report_errors(&mut context, compile_options.deny_warnings)?;
+    check_crate_and_report_errors(&mut context, compile_options.deny_warnings, compile_options.experimental_ssa)?;
 
     let test_functions = context.get_all_test_functions_in_crate_matching(&LOCAL_CRATE, test_name);
     println!("Running {} test functions...", test_functions.len());

crates/nargo_cli/tests/test_data_ssa_refactor/6_array/Prover.toml

@@ -5,5 +5,4 @@ t = "10"
 
 #7128
 #15309
-#16349
-
+#16349

crates/nargo_cli/tests/test_data_ssa_refactor/6_array/src/main.nr

@@ -51,4 +51,3 @@ fn main(x: [u32; 5], y: [u32; 5], mut z: u32, t: u32) {
         }
     }
 }
-

crates/nargo_cli/tests/test_data_ssa_refactor/slices/Nargo.toml

@@ -0,0 +1,5 @@
+[package]
+authors = [""]
+compiler_version = "0.6.0"
+
+[dependencies]

crates/nargo_cli/tests/test_data_ssa_refactor/slices/Prover.toml

@@ -0,0 +1,2 @@
+x = "5"
+y = "10"

crates/nargo_cli/tests/test_data_ssa_refactor/slices/src/main.nr

@@ -0,0 +1,23 @@
+use dep::std::slice;
+
+fn main(x : Field, y : pub Field) {
+
+    let mut slice: [Field] = [0; 2];
+
+    assert(slice[0] == 0);
+    assert(slice[0] != 1);
+    slice[0] = x;
+    assert(slice[0] == x);
+
+    let slice_plus_10 = slice.push_back(y);
+    assert(slice_plus_10[2] == 10);
+    assert(slice_plus_10[2] != 8);
+    assert(slice_plus_10.len() == 3);
+
+    let mut new_slice: [Field] = [];
+    for i in 0..5 {
+        new_slice = new_slice.push_back(i);
+    }
+    assert(new_slice.len() == 5);
+}
+

crates/noirc_driver/src/lib.rs

@@ -157,6 +157,7 @@ pub fn propagate_dep(
 pub fn check_crate(
     context: &mut Context,
     deny_warnings: bool,
+    enable_slices: bool,
 ) -> Result<Warnings, ErrorsAndWarnings> {
     // Add the stdlib before we check the crate
     // TODO: This should actually be done when constructing the driver and then propagated to each dependency when added;
@@ -167,6 +168,8 @@ pub fn check_crate(
     let std_crate = create_non_local_crate(context, path_to_std_lib_file, CrateType::Library);
     propagate_dep(context, std_crate, &CrateName::new(std_crate_name).unwrap());
 
+    context.def_interner.enable_slices = enable_slices;
+
     let mut errors = vec![];
     CrateDefMap::collect_defs(LOCAL_CRATE, context, &mut errors);
 
@@ -195,7 +198,7 @@ pub fn compile_main(
     is_opcode_supported: &impl Fn(&Opcode) -> bool,
     options: &CompileOptions,
 ) -> Result<(CompiledProgram, Warnings), ErrorsAndWarnings> {
-    let warnings = check_crate(context, options.deny_warnings)?;
+    let warnings = check_crate(context, options.deny_warnings, options.experimental_ssa)?;
 
     let main = match context.get_main_function(&LOCAL_CRATE) {
         Some(m) => m,
@@ -226,7 +229,7 @@ pub fn compile_contracts(
     is_opcode_supported: &impl Fn(&Opcode) -> bool,
     options: &CompileOptions,
 ) -> Result<(Vec<CompiledContract>, Warnings), ErrorsAndWarnings> {
-    let warnings = check_crate(context, options.deny_warnings)?;
+    let warnings = check_crate(context, options.deny_warnings, options.experimental_ssa)?;
 
     let contracts = context.get_all_contracts(&LOCAL_CRATE);
     let mut compiled_contracts = vec![];

crates/noirc_evaluator/src/ssa/context.rs

@@ -1212,7 +1212,7 @@ impl SsaContext {
             Type::Function(..) => ObjectType::Function,
             Type::Tuple(_) => todo!("Conversion to ObjectType is unimplemented for tuples"),
             Type::String(_) => todo!("Conversion to ObjectType is unimplemented for strings"),
-            Type::Vec(_) => todo!("Conversion to ObjectType is unimplemented for Vecs"),
+            Type::Slice(_) => todo!("Conversion to ObjectType is unimplemented for slices"),
         }
     }
 

crates/noirc_evaluator/src/ssa/value.rs

@@ -96,7 +96,7 @@ impl Value {
             Type::Unit
             | Type::Function(..)
             | Type::Array(..)
-            | Type::Vec(..)
+            | Type::Slice(..)
             | Type::String(..)
             | Type::Integer(..)
             | Type::Bool

crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs

@@ -578,6 +578,9 @@ impl Context {
             (_, Type::Array(..)) | (Type::Array(..), _) => {
                 unreachable!("Arrays are invalid in binary operations")
             }
+            (_, Type::Slice(..)) | (Type::Slice(..), _) => {
+                unreachable!("Arrays are invalid in binary operations")
+            }
             // If either side is a Field constant then, we coerce into the type
             // of the other operand
             (Type::Numeric(NumericType::NativeField), typ)
@@ -732,6 +735,7 @@ impl Context {
 
                 Self::convert_vars_to_values(out_vars, dfg, result_ids)
             }
+            _ => todo!("expected a black box function"),
         }
     }
 
@@ -743,6 +747,10 @@ impl Context {
                 assert_eq!(elements.len(), 1);
                 (&elements[0]).into()
             }
+            Type::Slice(elements) => {
+                assert_eq!(elements.len(), 1);
+                (&elements[0]).into()
+            }
             _ => unreachable!("Expected array type"),
         }
     }

crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs

@@ -238,10 +238,16 @@ impl DataFlowGraph {
 
     /// Gets or creates a ValueId for the given Intrinsic.
     pub(crate) fn import_intrinsic(&mut self, intrinsic: Intrinsic) -> ValueId {
-        if let Some(existing) = self.intrinsics.get(&intrinsic) {
+        if let Some(existing) = self.get_intrinsic(intrinsic) {
             return *existing;
         }
-        self.values.insert(Value::Intrinsic(intrinsic))
+        let intrinsic_value_id = self.values.insert(Value::Intrinsic(intrinsic));
+        self.intrinsics.insert(intrinsic, intrinsic_value_id);
+        intrinsic_value_id
+    }
+
+    pub(crate) fn get_intrinsic(&mut self, intrinsic: Intrinsic) -> Option<&ValueId> {
+        self.intrinsics.get(&intrinsic)
     }
 
     /// Attaches results to the instruction, clearing any previous results.
@@ -360,6 +366,20 @@ 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<CompositeType>, usize)> {
+        match &self.values[self.resolve(value)] {
+            Value::Param { typ: Type::Array(element_type, size), .. } => {
+                Some((element_type.clone(), *size))
+            }
+            _ => None,
+        }
+    }
+
     /// Sets the terminator instruction for the given basic block
     pub(crate) fn set_block_terminator(
         &mut self,

crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs

@@ -32,6 +32,8 @@ pub(crate) type InstructionId = Id<Instruction>;
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
 pub(crate) enum Intrinsic {
     Sort,
+    ArrayLen,
+    SlicePushBack,
     Println,
     ToBits(Endian),
     ToRadix(Endian),
@@ -43,6 +45,8 @@ impl std::fmt::Display for Intrinsic {
         match self {
             Intrinsic::Println => write!(f, "println"),
             Intrinsic::Sort => write!(f, "arraysort"),
+            Intrinsic::ArrayLen => write!(f, "array_len"),
+            Intrinsic::SlicePushBack => write!(f, "slice_push_back"),
             Intrinsic::ToBits(Endian::Big) => write!(f, "to_be_bits"),
             Intrinsic::ToBits(Endian::Little) => write!(f, "to_le_bits"),
             Intrinsic::ToRadix(Endian::Big) => write!(f, "to_be_radix"),
@@ -59,6 +63,8 @@ impl Intrinsic {
         match name {
             "println" => Some(Intrinsic::Println),
             "arraysort" => Some(Intrinsic::Sort),
+            "array_len" => Some(Intrinsic::ArrayLen),
+            "slice_push_back" => Some(Intrinsic::SlicePushBack),
             "to_le_radix" => Some(Intrinsic::ToRadix(Endian::Little)),
             "to_be_radix" => Some(Intrinsic::ToRadix(Endian::Big)),
             "to_le_bits" => Some(Intrinsic::ToBits(Endian::Little)),
@@ -276,7 +282,6 @@ impl Instruction {
             Instruction::ArrayGet { array, index } => {
                 let array = dfg.get_array_constant(*array);
                 let index = dfg.get_numeric_constant(*index);
-
                 if let (Some((array, _)), Some(index)) = (array, index) {
                     let index =
                         index.try_to_u64().expect("Expected array index to fit in u64") as usize;
@@ -290,7 +295,6 @@ impl Instruction {
             Instruction::ArraySet { array, index, value } => {
                 let array = dfg.get_array_constant(*array);
                 let index = dfg.get_numeric_constant(*index);
-
                 if let (Some((array, element_type)), Some(index)) = (array, index) {
                     let index =
                         index.try_to_u64().expect("Expected array index to fit in u64") as usize;
@@ -375,23 +379,55 @@ fn simplify_call(func: ValueId, arguments: &[ValueId], dfg: &mut DataFlowGraph)
         Value::Intrinsic(intrinsic) => *intrinsic,
         _ => return None,
     };
+
     let constant_args: Option<Vec<_>> =
         arguments.iter().map(|value_id| dfg.get_numeric_constant(*value_id)).collect();
-    let constant_args = match constant_args {
-        Some(constant_args) => constant_args,
-        Option::None => return None,
-    };
+
     match intrinsic {
         Intrinsic::ToBits(endian) => {
-            let field = constant_args[0];
-            let limb_count = constant_args[1].to_u128() as u32;
-            SimplifiedTo(constant_to_radix(endian, field, 2, limb_count, dfg))
+            if let Some(constant_args) = constant_args {
+                let field = constant_args[0];
+                let limb_count = constant_args[1].to_u128() as u32;
+                SimplifiedTo(constant_to_radix(endian, field, 2, limb_count, dfg))
+            } else {
+                None
+            }
         }
         Intrinsic::ToRadix(endian) => {
-            let field = constant_args[0];
-            let radix = constant_args[1].to_u128() as u32;
-            let limb_count = constant_args[2].to_u128() as u32;
-            SimplifiedTo(constant_to_radix(endian, field, radix, limb_count, dfg))
+            if let Some(constant_args) = constant_args {
+                let field = constant_args[0];
+                let radix = constant_args[1].to_u128() as u32;
+                let limb_count = constant_args[2].to_u128() as u32;
+                SimplifiedTo(constant_to_radix(endian, field, radix, limb_count, dfg))
+            } else {
+                None
+            }
+        }
+        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)
+            } else {
+                None
+            }
+        }
+        Intrinsic::SlicePushBack => {
+            let slice = dfg.get_array_constant(arguments[0]);
+            if let (Some((mut slice, element_type)), elem) = (slice, arguments[1]) {
+                slice.push_back(elem);
+                let new_slice = dfg.make_array(slice, element_type);
+                SimplifiedTo(new_slice)
+            } else {
+                None
+            }
         }
         Intrinsic::BlackBox(_) | Intrinsic::Println | Intrinsic::Sort => None,
     }

crates/noirc_evaluator/src/ssa_refactor/ir/types.rs

@@ -29,6 +29,9 @@ pub(crate) enum Type {
     /// An immutable array value with the given element type and length
     Array(Rc<CompositeType>, usize),
 
+    /// An immutable slice value with a given element type
+    Slice(Rc<CompositeType>),
+
     /// A function that may be called directly
     Function,
 }
@@ -74,6 +77,10 @@ impl std::fmt::Display for Type {
                 let elements = vecmap(element.iter(), |element| element.to_string());
                 write!(f, "[{}; {length}]", elements.join(", "))
             }
+            Type::Slice(element) => {
+                let elements = vecmap(element.iter(), |element| element.to_string());
+                write!(f, "[{}]", elements.join(", "))
+            }
             Type::Function => write!(f, "function"),
         }
     }

crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs

@@ -388,6 +388,7 @@ impl<'f> Context<'f> {
                 then_value,
                 else_value,
             ),
+            Type::Slice(_) => panic!("Cannot return slices from an if expression"),
             Type::Reference => panic!("Cannot return references from an if expression"),
             Type::Function => panic!("Cannot return functions from an if expression"),
         }

crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs

@@ -206,16 +206,15 @@ impl<'a> FunctionContext<'a> {
             ast::Type::Unit => panic!("convert_non_tuple_type called on a unit type"),
             ast::Type::Tuple(_) => panic!("convert_non_tuple_type called on a tuple: {typ}"),
             ast::Type::Function(_, _) => Type::Function,
+            ast::Type::Slice(element) => {
+                let element_types = Self::convert_type(element).flatten();
+                Type::Slice(Rc::new(element_types))
+            }
             ast::Type::MutableReference(element) => {
                 // Recursive call to panic if element is a tuple
                 Self::convert_non_tuple_type(element);
                 Type::Reference
             }
-
-            // How should we represent Vecs?
-            // Are they a struct of array + length + capacity?
-            // Or are they just references?
-            ast::Type::Vec(_) => Type::Reference,
         }
     }