From e28c297bffa1b9aab6cb00e6d053108816f90966 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 9 May 2024 16:09:18 -0500 Subject: [PATCH 1/5] Fix bug --- .../noirc_frontend/src/elaborator/types.rs | 4 +- .../noirc_frontend/src/hir/type_check/expr.rs | 4 +- compiler/noirc_frontend/src/hir_def/types.rs | 39 +++++++++++++++++++ .../regression_5008/Nargo.toml | 7 ++++ .../regression_5008/src/main.nr | 17 ++++++++ 5 files changed, 67 insertions(+), 4 deletions(-) create mode 100644 test_programs/compile_failure/regression_5008/Nargo.toml create mode 100644 test_programs/compile_failure/regression_5008/src/main.nr diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 4c8364b6dd..d507af1afd 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -1228,7 +1228,7 @@ impl<'context> Elaborator<'context> { // 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.push_err(TypeCheckError::ConstrainedReferenceToUnconstrained { span }); } } @@ -1240,7 +1240,7 @@ impl<'context> Elaborator<'context> { if is_current_func_constrained && is_unconstrained_call { if return_type.contains_slice() { self.push_err(TypeCheckError::UnconstrainedSliceReturnToConstrained { span }); - } else if matches!(&return_type.follow_bindings(), Type::MutableReference(_)) { + } else if !return_type.is_valid_for_unconstrained_boundary() { self.push_err(TypeCheckError::UnconstrainedReferenceToConstrained { span }); } }; diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 4859810982..f1d8477923 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -196,7 +196,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: self.interner.expr_span(expr_id), }); @@ -215,7 +215,7 @@ impl<'interner> TypeChecker<'interner> { span: self.interner.expr_span(expr_id), }); return Type::Error; - } else if matches!(&return_type.follow_bindings(), Type::MutableReference(_)) { + } else if !return_type.is_valid_for_unconstrained_boundary() { self.errors.push(TypeCheckError::UnconstrainedReferenceToConstrained { span: self.interner.expr_span(expr_id), }); diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index f31aeea055..f1a3b312c0 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -779,6 +779,45 @@ 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::TypeVariable(_, _) + | Type::NamedGeneric(_, _) + | Type::Function(_, _, _) + | Type::FmtString(_, _) + | Type::Error => true, + + Type::Slice(_) + | Type::MutableReference(_) + | Type::Forall(_, _) + | Type::Code + | 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 { diff --git a/test_programs/compile_failure/regression_5008/Nargo.toml b/test_programs/compile_failure/regression_5008/Nargo.toml new file mode 100644 index 0000000000..920c00660c --- /dev/null +++ b/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] diff --git a/test_programs/compile_failure/regression_5008/src/main.nr b/test_programs/compile_failure/regression_5008/src/main.nr new file mode 100644 index 0000000000..6d9645ee6e --- /dev/null +++ b/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(); +} From 96bfe7288232e72b3ae50f4d12453a03b3fc5ef4 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 9 May 2024 16:21:04 -0500 Subject: [PATCH 2/5] Format --- compiler/noirc_frontend/src/hir_def/types.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index f1a3b312c0..466f57b9bd 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -806,10 +806,13 @@ impl Type { } Type::Array(length, element) => { - length.is_valid_for_unconstrained_boundary() && element.is_valid_for_unconstrained_boundary() + 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::Tuple(elements) => { + elements.iter().all(|elem| elem.is_valid_for_unconstrained_boundary()) + } Type::Struct(definition, generics) => definition .borrow() .get_fields(generics) From 37e6a8a5aaadcf6fa08975086cee7feada7d7325 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 3 Jun 2024 16:16:39 +0100 Subject: [PATCH 3/5] Update compiler/noirc_frontend/src/hir/type_check/expr.rs --- compiler/noirc_frontend/src/hir/type_check/expr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index dacae8df6b..b9a3c58ed7 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/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 typ.is_valid_for_unconstrained_boundary() { + if !typ.is_valid_for_unconstrained_boundary() { self.errors.push(TypeCheckError::ConstrainedReferenceToUnconstrained { span }); } } From 54723968b3d222147b6d2edc8a3a48f95284d729 Mon Sep 17 00:00:00 2001 From: Tom French Date: Thu, 20 Jun 2024 01:36:35 +0100 Subject: [PATCH 4/5] chore: clippy --- compiler/noirc_frontend/src/elaborator/lints.rs | 4 ---- compiler/noirc_frontend/src/hir_def/types.rs | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/lints.rs b/compiler/noirc_frontend/src/elaborator/lints.rs index e531c9d6fd..45eaf5dda6 100644 --- a/compiler/noirc_frontend/src/elaborator/lints.rs +++ b/compiler/noirc_frontend/src/elaborator/lints.rs @@ -158,10 +158,6 @@ pub(super) fn unconstrained_function_return( } } -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. diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 4b4bd99068..ecf429d70e 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -806,7 +806,7 @@ impl Type { Type::Slice(_) | Type::MutableReference(_) | Type::Forall(_, _) - | Type::Code + | Type::Quoted(_) | Type::TraitAsType(..) => false, Type::Alias(alias, generics) => { From 24e14b1630b7b138f77974015888f5caffb3eef6 Mon Sep 17 00:00:00 2001 From: Tom French Date: Thu, 20 Jun 2024 01:48:54 +0100 Subject: [PATCH 5/5] fix: allow passing slices to unconstrained functions --- compiler/noirc_frontend/src/elaborator/lints.rs | 4 +++- compiler/noirc_frontend/src/hir_def/types.rs | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/lints.rs b/compiler/noirc_frontend/src/elaborator/lints.rs index 45eaf5dda6..af6f4cdb42 100644 --- a/compiler/noirc_frontend/src/elaborator/lints.rs +++ b/compiler/noirc_frontend/src/elaborator/lints.rs @@ -151,7 +151,9 @@ pub(super) fn unconstrained_function_return( return_type: &Type, span: Span, ) -> Option { - if !return_type.is_valid_for_unconstrained_boundary() { + if return_type.contains_slice() { + Some(TypeCheckError::UnconstrainedSliceReturnToConstrained { span }) + } else if !return_type.is_valid_for_unconstrained_boundary() { Some(TypeCheckError::UnconstrainedReferenceToConstrained { span }) } else { None diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index ecf429d70e..772558ec31 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -797,14 +797,14 @@ impl Type { | Type::Bool | Type::Unit | Type::Constant(_) + | Type::Slice(_) | Type::TypeVariable(_, _) | Type::NamedGeneric(_, _) | Type::Function(_, _, _) | Type::FmtString(_, _) | Type::Error => true, - Type::Slice(_) - | Type::MutableReference(_) + Type::MutableReference(_) | Type::Forall(_, _) | Type::Quoted(_) | Type::TraitAsType(..) => false,