fix(frontend)!: Restrict capturing mutable variable in lambdas

compiler/noirc_frontend/src/elaborator/expressions.rs

@@ -1220,7 +1220,20 @@ impl<'context> Elaborator<'context> {
         });
 
         let captured_vars = vecmap(&lambda_context.captures, |capture| {
-            self.interner.definition_type(capture.ident.id)
+            let typ = self.interner.definition_type(capture.ident.id);
+            if let Some(definition) = self.interner.try_definition(capture.ident.id) {
+                if definition.mutable && !typ.is_mutable_ref() {
+                    let location = definition.location;
+                    self.push_err(
+                        TypeCheckError::MutableCaptureWithoutRef {
+                            name: definition.name.clone(),
+                            span: location.span,
+                        },
+                        location.file,
+                    );
+                }
+            }
+            typ
         });
 
         let env_type =

compiler/noirc_frontend/src/hir/type_check/errors.rs

@@ -94,6 +94,8 @@ pub enum TypeCheckError {
     VariableMustBeMutable { name: String, span: Span },
     #[error("Cannot mutate immutable variable `{name}`")]
     CannotMutateImmutableVariable { name: String, span: Span },
+    #[error("Variable {name} captured in lambda must be a mutable reference")]
+    MutableCaptureWithoutRef { name: String, span: Span },
     #[error("No method named '{method_name}' found for type '{object_type}'")]
     UnresolvedMethodCall { method_name: String, object_type: Type, span: Span },
     #[error("Cannot invoke function field '{method_name}' on type '{object_type}' as a method")]
@@ -370,6 +372,11 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic {
             | TypeCheckError::InvalidShiftSize { span } => {
                 Diagnostic::simple_error(error.to_string(), String::new(), *span)
             }
+            TypeCheckError::MutableCaptureWithoutRef { name, span } => Diagnostic::simple_error(
+                format!("Mutable variable {name} captured in lambda must be a mutable reference"),
+                "Use '&mut' instead of 'mut' to capture a mutable variable.".to_string(),
+                *span,
+            ),
             TypeCheckError::PublicReturnType { typ, span } => Diagnostic::simple_error(
                 "Functions cannot declare a public return type".to_string(),
                 format!("return type is {typ}"),

compiler/noirc_frontend/src/hir_def/types.rs

@@ -1245,6 +1245,10 @@
         }
     }
 
+    pub(crate) fn is_mutable_ref(&self) -> bool {
+        matches!(self.follow_bindings_shallow().as_ref(), Type::MutableReference(_))
+    }
+
     /// True if this type can be used as a parameter to `main` or a contract function.
     /// This is only false for unsized types like slices or slices that do not make sense
     /// as a program input such as named generics or mutable references.
@@ -2347,7 +2351,7 @@
         }
 
         let recur_on_binding = |id, replacement: &Type| {
             // Prevent recuring forever if there's a `T := T` binding
             if replacement.type_variable_id() == Some(id) {
                 replacement.clone()
             } else {
@@ -2433,7 +2437,7 @@
                 Type::Tuple(fields)
             }
             Type::Forall(typevars, typ) => {
                 // Trying to substitute_helper a variable de, substitute_bound_typevarsfined within a nested Forall
                 // is usually impossible and indicative of an error in the type checker somewhere.
                 for var in typevars {
                     assert!(!type_bindings.contains_key(&var.id()));
@@ -2599,7 +2603,7 @@
         }
     }
 
     /// Follow bindings if this is a type variable or generic to the first non-typevariable
     /// type. Unlike `follow_bindings`, this won't recursively follow any bindings on any
     /// fields or arguments of this type.
     pub fn follow_bindings_shallow(&self) -> Cow<Type> {
@@ -2624,7 +2628,7 @@
 
     /// Replace any `Type::NamedGeneric` in this type with a `Type::TypeVariable`
     /// using to the same inner `TypeVariable`. This is used during monomorphization
     /// to bind to named generics since they are unbindable during type checking.
     pub fn replace_named_generics_with_type_variables(&mut self) {
         match self {
             Type::FieldElement
@@ -3095,7 +3099,7 @@
                 len.hash(state);
                 env.hash(state);
             }
             Type::Tuple(elems) => elems.hash(state),
             Type::DataType(def, args) => {
                 def.hash(state);
                 args.hash(state);

compiler/noirc_frontend/src/tests.rs

@@ -3197,7 +3197,7 @@
 }
 
 #[test]
 fn dont_infer_globals_to_u32_from_type_use() {
     let src = r#"
         global ARRAY_LEN = 3;
         global STR_LEN: _ = 2;
@@ -3227,7 +3227,7 @@
 }
 
 #[test]
 fn dont_infer_partial_global_types() {
     let src = r#"
         pub global ARRAY: [Field; _] = [0; 3];
         pub global NESTED_ARRAY: [[Field; _]; 3] = [[]; 3];
@@ -3470,7 +3470,7 @@
     // wouldn't panic due to infinite recursion, but the errors asserted here
     // come from the compilation checks, which does static analysis to catch the
     // problem before it even has a chance to cause a panic.
     let srcs = vec![
         r#"
         fn main() {
             main()
@@ -3532,7 +3532,7 @@
         "#,
     ];
 
     for src in srcs {
         let errors = get_program_errors(src);
         assert!(
             !errors.is_empty(),
@@ -4475,3 +4475,59 @@
 
     assert!(matches!(error.reason(), Some(ParserErrorReason::ExperimentalFeature(_))));
 }
+
+#[test]
+fn mutate_with_reference_in_lambda() {
+    let src = r#"
+    fn main() {
+        let x = &mut 3;
+        let f = || {
+            *x += 2;
+        };
+        f();
+        assert(*x == 5);
+    }
+    "#;
+
+    let errors = get_program_errors(src);
+    assert_eq!(errors.len(), 0);
+}
+
+#[test]
+fn mutate_with_reference_marked_mutable_in_lambda() {
+    let src = r#"
+    fn main() {
+        let mut x = &mut 3;
+        let f = || {
+            *x += 2;
+        };
+        f();
+        assert(*x == 5);
+    }
+    "#;
+
+    let errors = get_program_errors(src);
+    assert_eq!(errors.len(), 0);
+}
+
+#[test]
+fn deny_capturing_mut_variable_without_reference_in_lambda() {
+    let src = r#"
+    fn main() {
+        let mut x = 3;
+        let f = || {
+            x += 2;
+        };
+        f();
+        assert(x == 5);
+    }
+    "#;
+
+    let errors = get_program_errors(src);
+    assert_eq!(errors.len(), 1);
+
+    assert!(matches!(
+        errors[0].0,
+        CompilationError::TypeError(TypeCheckError::MutableCaptureWithoutRef { .. })
+    ));
+}