chore(debug): Add ownership analysis pass

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

@@ -901,11 +901,16 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
                 // and become usize::MAX, and then return to 1, then it will indicate
                 // an array as mutable when it probably shouldn't be.
                 if self.brillig_context.enable_debug_assertions() {
-                    let min_addr = ReservedRegisters::usize_one();
+                    let zero =
+                        SingleAddrVariable::new(self.brillig_context.allocate_register(), 32);
+
+                    self.brillig_context.const_instruction(zero, FieldElement::zero());
+
                     let condition =
                         SingleAddrVariable::new(self.brillig_context.allocate_register(), 1);
+
                     self.brillig_context.memory_op_instruction(
-                        min_addr,
+                        zero.address,
                         rc_register,
                         condition.address,
                         BrilligBinaryOp::LessThan,

compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs

@@ -24,7 +24,7 @@ use crate::ssa::ir::value::ValueId;
 
 use super::GlobalsGraph;
 use super::value::{Tree, Value, Values};
-use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet};
+use fxhash::FxHashMap as HashMap;
 
 /// The FunctionContext is the main context object for translating a
 /// function into SSA form during the SSA-gen pass.
@@ -173,7 +173,7 @@ impl<'a> FunctionContext<'a> {
             let value = self.builder.add_parameter(typ);
             if mutable {
                 // This will wrap any `mut var: T` in a reference and increase the rc of an array if needed
-                self.new_mutable_variable(value, true)
+                self.new_mutable_variable(value)
             } else {
                 value.into()
             }
@@ -184,17 +184,9 @@ impl<'a> FunctionContext<'a> {
 
     /// Allocate a single slot of memory and store into it the given initial value of the variable.
     /// Always returns a Value::Mutable wrapping the allocate instruction.
-    pub(super) fn new_mutable_variable(
-        &mut self,
-        value_to_store: ValueId,
-        increment_array_rc: bool,
-    ) -> Value {
+    pub(super) fn new_mutable_variable(&mut self, value_to_store: ValueId) -> Value {
         let element_type = self.builder.current_function.dfg.type_of_value(value_to_store);
 
-        if increment_array_rc {
-            self.builder.increment_array_reference_count(value_to_store);
-        }
-
         let alloc = self.builder.insert_allocate(element_type);
         self.builder.insert_store(alloc, value_to_store);
         let typ = self.builder.type_of_value(value_to_store);
@@ -219,7 +211,7 @@ impl<'a> FunctionContext<'a> {
             ast::Type::Unit => Tree::empty(),
             // A mutable reference wraps each element into a reference.
             // This can be multiple values if the element type is a tuple.
-            ast::Type::MutableReference(element) => {
+            ast::Type::Reference(element) => {
                 Self::map_type_helper(element, &mut |typ| f(Type::Reference(Arc::new(typ))))
             }
             ast::Type::FmtString(len, fields) => {
@@ -272,7 +264,7 @@ impl<'a> FunctionContext<'a> {
             ast::Type::Tuple(_) => panic!("convert_non_tuple_type called on a tuple: {typ}"),
             ast::Type::Function(_, _, _, _) => Type::Function,
             ast::Type::Slice(_) => panic!("convert_non_tuple_type called on a slice: {typ}"),
-            ast::Type::MutableReference(element) => {
+            ast::Type::Reference(element) => {
                 // Recursive call to panic if element is a tuple
                 let element = Self::convert_non_tuple_type(element);
                 Type::Reference(Arc::new(element))
@@ -957,65 +949,6 @@ impl<'a> FunctionContext<'a> {
         }
     }
 
-    /// Increments the reference count of mutable reference array parameters.
-    /// Any mutable-value (`mut a: [T; N]` versus `a: &mut [T; N]`) are already incremented
-    /// by `FunctionBuilder::add_parameter_to_scope`.
-    /// Returns each array id that was incremented.
-    ///
-    /// This is done on parameters rather than call arguments so that we can optimize out
-    /// paired inc/dec instructions within brillig functions more easily.
-    ///
-    /// Returns the list of parameters incremented, together with the value ID of the arrays they refer to.
-    pub(crate) fn increment_parameter_rcs(&mut self) -> Vec<(ValueId, ValueId)> {
-        let entry = self.builder.current_function.entry_block();
-        let parameters = self.builder.current_function.dfg.block_parameters(entry).to_vec();
-
-        let mut incremented = Vec::default();
-        let mut seen_array_types = HashSet::default();
-
-        for parameter in parameters {
-            // Avoid reference counts for immutable arrays that aren't behind references.
-            let typ = self.builder.current_function.dfg.type_of_value(parameter);
-
-            if let Type::Reference(element) = typ {
-                if element.contains_an_array() {
-                    // If we haven't already seen this array type, the value may be possibly
-                    // aliased, so issue an inc_rc for it.
-                    if seen_array_types.insert(element.get_contained_array().clone()) {
-                        continue;
-                    }
-                    if let Some(id) = self.builder.increment_array_reference_count(parameter) {
-                        incremented.push((parameter, id));
-                    }
-                }
-            }
-        }
-
-        incremented
-    }
-
-    /// Ends a local scope of a function.
-    /// This will issue DecrementRc instructions for any arrays in the given starting scope
-    /// block's parameters. Arrays that are also used in terminator instructions for the scope are
-    /// ignored.
-    pub(crate) fn end_scope(
-        &mut self,
-        mut incremented_params: Vec<(ValueId, ValueId)>,
-        terminator_args: &[ValueId],
-    ) {
-        // TODO: This check likely leads to unsoundness.
-        // It is here to avoid decrementing the RC of a parameter we're returning but we
-        // only check the exact ValueId which can be easily circumvented by storing to and
-        // loading from a temporary reference.
-        incremented_params.retain(|(parameter, _)| !terminator_args.contains(parameter));
-
-        for (parameter, original) in incremented_params {
-            if self.builder.current_function.dfg.value_is_reference(parameter) {
-                self.builder.decrement_array_reference_count(original);
-            }
-        }
-    }
-
     pub(crate) fn enter_loop(&mut self, loop_: Loop) {
         self.loops.push(loop_);
     }

compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs

@@ -136,10 +136,10 @@ impl FunctionContext<'_> {
     /// Codegen a function's body and set its return value to that of its last parameter.
     /// For functions returning nothing, this will be an empty list.
     fn codegen_function_body(&mut self, body: &Expression) -> Result<(), RuntimeError> {
-        let incremented_params = self.increment_parameter_rcs();
+        // let incremented_params = self.increment_parameter_rcs();
         let return_value = self.codegen_expression(body)?;
         let results = return_value.into_value_list(self);
-        self.end_scope(incremented_params, &results);
+        // self.end_scope(incremented_params, &results);
 
         self.builder.terminate_with_return(results);
         Ok(())
@@ -172,6 +172,8 @@ impl FunctionContext<'_> {
             Expression::Semi(semi) => self.codegen_semi(semi),
             Expression::Break => Ok(self.codegen_break()),
             Expression::Continue => Ok(self.codegen_continue()),
+            Expression::Clone(expr) => self.codegen_clone(expr),
+            Expression::Drop(expr) => self.codegen_drop(expr),
         }
     }
 
@@ -277,17 +279,13 @@ impl FunctionContext<'_> {
     fn codegen_array_elements(
         &mut self,
         elements: &[Expression],
-    ) -> Result<Vec<(Values, bool)>, RuntimeError> {
-        try_vecmap(elements, |element| {
-            let value = self.codegen_expression(element)?;
-            Ok((value, element.is_array_or_slice_literal()))
-        })
+    ) -> Result<Vec<Values>, RuntimeError> {
+        try_vecmap(elements, |element| self.codegen_expression(element))
     }
 
     fn codegen_string(&mut self, string: &str) -> Values {
         let elements = vecmap(string.as_bytes(), |byte| {
-            let char = self.builder.numeric_constant(*byte as u128, NumericType::char());
-            (char.into(), false)
+            self.builder.numeric_constant(*byte as u128, NumericType::char()).into()
         });
         let typ = Self::convert_non_tuple_type(&ast::Type::String(elements.len() as u32));
         self.codegen_array(elements, typ)
@@ -300,7 +298,7 @@ impl FunctionContext<'_> {
     /// constant to be moved into this larger array constant.
     fn codegen_array_checked(
         &mut self,
-        elements: Vec<(Values, bool)>,
+        elements: Vec<Values>,
         typ: Type,
     ) -> Result<Values, RuntimeError> {
         if typ.is_nested_slice() {
@@ -322,22 +320,12 @@ impl FunctionContext<'_> {
     /// constant to be moved into this larger array constant.
     ///
     /// The value returned from this function is always that of the allocate instruction.
-    fn codegen_array(&mut self, elements: Vec<(Values, bool)>, typ: Type) -> Values {
+    fn codegen_array(&mut self, elements: Vec<Values>, typ: Type) -> Values {
         let mut array = im::Vector::new();
 
-        for (element, is_array_constant) in elements {
+        for element in elements {
             element.for_each(|element| {
                 let element = element.eval(self);
-
-                // If we're referencing a sub-array in a larger nested array we need to
-                // increase the reference count of the sub array. This maintains a
-                // pessimistic reference count (since some are likely moved rather than shared)
-                // which is important for Brillig's copy on write optimization. This has no
-                // effect in ACIR code.
-                if !is_array_constant {
-                    self.builder.increment_array_reference_count(element);
-                }
-
                 array.push_back(element);
             });
         }
@@ -486,7 +474,7 @@ impl FunctionContext<'_> {
             // counts when nested arrays/slices are constructed or indexed. This
             // has no effect in ACIR code.
             let result = self.builder.insert_array_get(array, offset, typ);
-            self.builder.increment_array_reference_count(result);
+            // self.builder.increment_array_reference_count(result);
             result.into()
         }))
     }
@@ -1029,22 +1017,12 @@ impl FunctionContext<'_> {
     fn codegen_let(&mut self, let_expr: &ast::Let) -> Result<Values, RuntimeError> {
         let mut values = self.codegen_expression(&let_expr.expression)?;
 
-        // Don't mutate the reference count if we're assigning an array literal to a Let:
-        // `let mut foo = [1, 2, 3];`
-        // we consider the array to be moved, so we should have an initial rc of just 1.
-        let should_inc_rc = !let_expr.expression.is_array_or_slice_literal();
-
         values = values.map(|value| {
             let value = value.eval(self);
 
             Tree::Leaf(if let_expr.mutable {
-                self.new_mutable_variable(value, should_inc_rc)
+                self.new_mutable_variable(value)
             } else {
-                // `new_mutable_variable` increments rcs internally so we have to
-                // handle it separately for the immutable case
-                if should_inc_rc {
-                    self.builder.increment_array_reference_count(value);
-                }
                 value::Value::Normal(value)
             })
         });
@@ -1103,15 +1081,6 @@ impl FunctionContext<'_> {
     fn codegen_assign(&mut self, assign: &ast::Assign) -> Result<Values, RuntimeError> {
         let lhs = self.extract_current_value(&assign.lvalue)?;
         let rhs = self.codegen_expression(&assign.expression)?;
-        let should_inc_rc = !assign.expression.is_array_or_slice_literal();
-
-        rhs.clone().for_each(|value| {
-            let value = value.eval(self);
-
-            if should_inc_rc {
-                self.builder.increment_array_reference_count(value);
-            }
-        });
 
         self.assign_new_value(lhs, rhs);
         Ok(Self::unit_value())
@@ -1140,4 +1109,26 @@ impl FunctionContext<'_> {
         }
         Self::unit_value()
     }
+
+    /// Evaluate the given expression, increment the reference count of each array within,
+    /// and return the evaluated expression.
+    fn codegen_clone(&mut self, expr: &Expression) -> Result<Values, RuntimeError> {
+        let values = self.codegen_expression(expr)?;
+        Ok(values.map(|value| {
+            let value = value.eval(self);
+            self.builder.increment_array_reference_count(value);
+            Values::from(value)
+        }))
+    }
+
+    /// Evaluate the given expression, decrement the reference count of each array within,
+    /// and return unit.
+    fn codegen_drop(&mut self, expr: &Expression) -> Result<Values, RuntimeError> {
+        let values = self.codegen_expression(expr)?;
+        values.for_each(|value| {
+            let value = value.eval(self);
+            self.builder.decrement_array_reference_count(value);
+        });
+        Ok(Self::unit_value())
+    }
 }

compiler/noirc_frontend/src/lib.rs

@@ -22,6 +22,7 @@ pub mod lexer;
 pub mod locations;
 pub mod monomorphization;
 pub mod node_interner;
+pub(crate) mod ownership;
 pub mod parser;
 pub mod resolve_locations;
 pub mod shared;

compiler/noirc_frontend/src/monomorphization/ast.rs

@@ -50,6 +50,8 @@ pub enum Expression {
     Constrain(Box<Expression>, Location, Option<Box<(Expression, HirType)>>),
     Assign(Assign),
     Semi(Box<Expression>),
+    Clone(Box<Expression>),
+    Drop(Box<Expression>),
     Break,
     Continue,
 }
@@ -338,7 +340,7 @@ pub enum Type {
     Unit,
     Tuple(Vec<Type>),
     Slice(Box<Type>),
-    MutableReference(Box<Type>),
+    Reference(Box<Type>),
     Function(
         /*args:*/ Vec<Type>,
         /*ret:*/ Box<Type>,
@@ -354,6 +356,14 @@ impl Type {
             _ => vec![self.clone()],
         }
     }
+
+    /// Returns the element type of this array or slice
+    pub fn array_element_type(&self) -> Option<&Type> {
+        match self {
+            Type::Array(_, elem) | Type::Slice(elem) => Some(elem),
+            _ => None,
+        }
+    }
 }
 
 #[derive(Debug, Clone, Hash, Default)]
@@ -490,7 +500,7 @@ impl std::fmt::Display for Type {
                 write!(f, "fn({}) -> {}{}", args.join(", "), ret, closure_env_text)
             }
             Type::Slice(element) => write!(f, "[{element}]"),
-            Type::MutableReference(element) => write!(f, "&mut {element}"),
+            Type::Reference(element) => write!(f, "&mut {element}"),
         }
     }
 }

compiler/noirc_frontend/src/monomorphization/mod.rs

@@ -201,7 +201,7 @@ pub fn monomorphize_debug(
         debug_functions,
         debug_types,
     );
-    Ok(program)
+    Ok(program.handle_ownership(monomorphizer.next_local_id))
 }
 
 impl<'interner> Monomorphizer<'interner> {
@@ -1312,7 +1312,7 @@ impl<'interner> Monomorphizer<'interner> {
             // Lower both mutable & immutable references to the same reference type
             HirType::Reference(element, _mutable) => {
                 let element = Self::convert_type_helper(element, location, seen_types)?;
-                ast::Type::MutableReference(Box::new(element))
+                ast::Type::Reference(Box::new(element))
             }
 
             HirType::Forall(_, _) | HirType::Constant(..) | HirType::InfixExpr(..) => {
@@ -2131,7 +2131,7 @@ impl<'interner> Monomorphizer<'interner> {
                     typ: ast::Type::Slice(element_type.clone()),
                 }))
             }
-            ast::Type::MutableReference(element) => {
+            ast::Type::Reference(element) => {
                 use UnaryOp::Reference;
                 let rhs = Box::new(self.zeroed_value_of_type(element, location));
                 let result_type = typ.clone();