Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Type check ACIR mutable reference passed to brillig #4281

Merged
merged 4 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion compiler/noirc_frontend/src/hir/type_check/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ pub enum TypeCheckError {
NoMatchingImplFound { constraints: Vec<(Type, String)>, span: Span },
#[error("Constraint for `{typ}: {trait_name}` is not needed, another matching impl is already in scope")]
UnneededTraitConstraint { trait_name: String, typ: Type, span: Span },
#[error(
"Cannot pass a mutable reference from a constrained runtime to an unconstrained runtime"
)]
ConstrainedReferenceToUnconstrained { span: Span },
}

impl TypeCheckError {
Expand Down Expand Up @@ -202,7 +206,8 @@ impl From<TypeCheckError> for Diagnostic {
| TypeCheckError::AmbiguousBitWidth { span, .. }
| TypeCheckError::IntegerAndFieldBinaryOperation { span }
| TypeCheckError::OverflowingAssignment { span, .. }
| TypeCheckError::FieldModulo { span } => {
| TypeCheckError::FieldModulo { span }
| TypeCheckError::ConstrainedReferenceToUnconstrained { span } => {
Diagnostic::simple_error(error.to_string(), String::new(), span)
}
TypeCheckError::PublicReturnType { typ, span } => Diagnostic::simple_error(
Expand Down
34 changes: 34 additions & 0 deletions compiler/noirc_frontend/src/hir/type_check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,18 @@ impl<'interner> TypeChecker<'interner> {
}
}

fn is_unconstrained_call(&self, expr: &ExprId) -> bool {
if let HirExpression::Ident(expr::HirIdent { id, .. }) = self.interner.expression(expr) {
if let Some(DefinitionKind::Function(func_id)) =
self.interner.try_definition(id).map(|def| &def.kind)
{
let modifiers = self.interner.function_modifiers(func_id);
return modifiers.is_unconstrained;
}
}
false
}

/// Infers a type for a given expression, and return this type.
/// As a side-effect, this function will also remember this type in the NodeInterner
/// for the given expr_id key.
Expand Down Expand Up @@ -139,6 +151,15 @@ impl<'interner> TypeChecker<'interner> {
}
HirExpression::Index(index_expr) => self.check_index_expression(expr_id, index_expr),
HirExpression::Call(call_expr) => {
// Need to setup these flags here as `self` is borrowed mutably to type check the rest of the call expression
// These flags are later used to type check calls to unconstrained functions from constrained functions
let current_func = self
.current_function
.expect("Can only have call expression inside of a function body");
let func_mod = self.interner.function_modifiers(&current_func);
let is_current_func_constrained = !func_mod.is_unconstrained;
let is_unconstrained_call = self.is_unconstrained_call(&call_expr.func);

self.check_if_deprecated(&call_expr.func);

let function = self.check_expression(&call_expr.func);
Expand All @@ -147,6 +168,19 @@ impl<'interner> TypeChecker<'interner> {
let typ = self.check_expression(arg);
(typ, *arg, self.interner.expr_span(arg))
});

for (typ, _, _) in args.iter() {
if is_current_func_constrained
&& is_unconstrained_call
&& matches!(&typ, Type::MutableReference(_))
{
self.errors.push(TypeCheckError::ConstrainedReferenceToUnconstrained {
span: self.interner.expr_span(expr_id),
});
return Type::Error;
}
}

let span = self.interner.expr_span(expr_id);
self.bind_function_type(function, args, span)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "brillig_mut_ref_from_acir"
type = "bin"
authors = [""]
compiler_version = ">=0.23.0"

[dependencies]
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
unconstrained fn mut_ref_identity(value: &mut Field) -> Field {
*value
}

fn main(mut x: Field, y: pub Field) {
let returned_x = mut_ref_identity(&mut x);
assert(returned_x == x);
}
Loading