Skip to content

Commit

Permalink
Merge branch 'master' into tf/slice-equality-failing-test
Browse files Browse the repository at this point in the history
* master:
  fix: Add more thorough check for whether a type is valid when passing it from constrained code to unconstrained code (#5009)
  • Loading branch information
TomAFrench committed Jun 20, 2024
2 parents 5350cd4 + 318314d commit 91c2541
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 7 deletions.
8 changes: 2 additions & 6 deletions compiler/noirc_frontend/src/elaborator/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/hir/type_check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}
}
Expand Down
42 changes: 42 additions & 0 deletions compiler/noirc_frontend/src/hir_def/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions test_programs/compile_failure/regression_5008/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "regression_5008"
type = "bin"
authors = [""]
compiler_version = ">=0.28.0"

[dependencies]
17 changes: 17 additions & 0 deletions test_programs/compile_failure/regression_5008/src/main.nr
Original file line number Diff line number Diff line change
@@ -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();
}

0 comments on commit 91c2541

Please sign in to comment.