fix: Add more thorough check for whether a type is valid when passing…

… it from constrained code to unconstrained code (#5009)

# Description

## Problem\*

Resolves #5008 

## Summary\*

Thank you to @nventuro for the minimal example and @TomAFrench for
looking into this. As a result it was a quick fix.

## Additional Context

All type system fixes must currently be duplicated in the elaborator so
I've included this there as well.

## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: TomAFrench <[email protected]>
Co-authored-by: Tom French <[email protected]>

compiler/noirc_frontend/src/elaborator/lints.rs

@@ -137,7 +137,7 @@ pub(super) fn unconstrained_function_args(
     function_args
         .iter()
         .filter_map(|(typ, _, span)| {
-            if type_contains_mutable_reference(typ) {
+            if !typ.is_valid_for_unconstrained_boundary() {
                 Some(TypeCheckError::ConstrainedReferenceToUnconstrained { span: *span })
             } else {
                 None
@@ -153,17 +153,13 @@ pub(super) fn unconstrained_function_return(
 ) -> Option<TypeCheckError> {
     if return_type.contains_slice() {
         Some(TypeCheckError::UnconstrainedSliceReturnToConstrained { span })
-    } else if type_contains_mutable_reference(return_type) {
+    } else if !return_type.is_valid_for_unconstrained_boundary() {
         Some(TypeCheckError::UnconstrainedReferenceToConstrained { span })
     } else {
         None
     }
 }
 
-fn type_contains_mutable_reference(typ: &Type) -> bool {
-    matches!(&typ.follow_bindings(), Type::MutableReference(_))
-}
-
 /// Only entrypoint functions require a `pub` visibility modifier applied to their return types.
 ///
 /// Application of `pub` to other functions is not meaningful and is a mistake.

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

@@ -340,7 +340,7 @@ impl<'interner> TypeChecker<'interner> {
         // Check that we are not passing a mutable reference from a constrained runtime to an unconstrained runtime
         if is_current_func_constrained && is_unconstrained_call {
             for (typ, _, _) in args.iter() {
-                if matches!(&typ.follow_bindings(), Type::MutableReference(_)) {
+                if !typ.is_valid_for_unconstrained_boundary() {
                     self.errors.push(TypeCheckError::ConstrainedReferenceToUnconstrained { span });
                 }
             }

compiler/noirc_frontend/src/hir_def/types.rs

@@ -788,6 +788,48 @@ impl Type {
         }
     }
 
+    /// Returns true if a value of this type can safely pass between constrained and
+    /// unconstrained functions (and vice-versa).
+    pub(crate) fn is_valid_for_unconstrained_boundary(&self) -> bool {
+        match self {
+            Type::FieldElement
+            | Type::Integer(_, _)
+            | Type::Bool
+            | Type::Unit
+            | Type::Constant(_)
+            | Type::Slice(_)
+            | Type::TypeVariable(_, _)
+            | Type::NamedGeneric(_, _)
+            | Type::Function(_, _, _)
+            | Type::FmtString(_, _)
+            | Type::Error => true,
+
+            Type::MutableReference(_)
+            | Type::Forall(_, _)
+            | Type::Quoted(_)
+            | Type::TraitAsType(..) => false,
+
+            Type::Alias(alias, generics) => {
+                let alias = alias.borrow();
+                alias.get_type(generics).is_valid_for_unconstrained_boundary()
+            }
+
+            Type::Array(length, element) => {
+                length.is_valid_for_unconstrained_boundary()
+                    && element.is_valid_for_unconstrained_boundary()
+            }
+            Type::String(length) => length.is_valid_for_unconstrained_boundary(),
+            Type::Tuple(elements) => {
+                elements.iter().all(|elem| elem.is_valid_for_unconstrained_boundary())
+            }
+            Type::Struct(definition, generics) => definition
+                .borrow()
+                .get_fields(generics)
+                .into_iter()
+                .all(|(_, field)| field.is_valid_for_unconstrained_boundary()),
+        }
+    }
+
     /// Returns the number of `Forall`-quantified type variables on this type.
     /// Returns 0 if this is not a Type::Forall
     pub fn generic_count(&self) -> usize {

test_programs/compile_failure/regression_5008/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "regression_5008"
+type = "bin"
+authors = [""]
+compiler_version = ">=0.28.0"
+
+[dependencies]

test_programs/compile_failure/regression_5008/src/main.nr

@@ -0,0 +1,17 @@
+struct Bar {
+    value: Field,
+}
+
+struct Foo{
+    bar: &mut Bar,
+}
+
+impl Foo {
+    unconstrained fn crash_fn(self) {}
+}
+
+fn main() {
+    let foo = Foo { bar: &mut Bar { value: 0 } };
+
+    foo.crash_fn();
+}