Skip to content

Commit

Permalink
fix: Verify impls arising from function calls exist (#3472)
Browse files Browse the repository at this point in the history
  • Loading branch information
jfecher authored and TomAFrench committed Nov 14, 2023
1 parent a002e8c commit c8a4312
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 13 deletions.
15 changes: 14 additions & 1 deletion compiler/noirc_frontend/src/hir/type_check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,19 @@ impl<'interner> TypeChecker<'interner> {
// variable to handle generic functions.
let t = self.interner.id_type_substitute_trait_as_type(ident.id);
let (typ, bindings) = t.instantiate(self.interner);

// Push any trait constraints required by this definition to the context
// to be checked later when the type of this variable is further constrained.
if let Some(definition) = self.interner.try_definition(ident.id) {
if let DefinitionKind::Function(function) = definition.kind {
let function = self.interner.function_meta(&function);
for mut constraint in function.trait_constraints.clone() {
constraint.typ = constraint.typ.substitute(&bindings);
self.trait_constraints.push((constraint, *expr_id));
}
}
}

self.interner.store_instantiation_bindings(*expr_id, bindings);
typ
}
Expand Down Expand Up @@ -294,7 +307,7 @@ impl<'interner> TypeChecker<'interner> {
typ
}

fn verify_trait_constraint(
pub fn verify_trait_constraint(
&mut self,
object_type: &Type,
trait_id: TraitId,
Expand Down
38 changes: 27 additions & 11 deletions compiler/noirc_frontend/src/hir/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ mod stmt;
pub use errors::TypeCheckError;

use crate::{
hir_def::{expr::HirExpression, stmt::HirStatement},
hir_def::{expr::HirExpression, stmt::HirStatement, traits::TraitConstraint},
node_interner::{ExprId, FuncId, NodeInterner, StmtId},
Type,
};
Expand All @@ -28,6 +28,12 @@ pub struct TypeChecker<'interner> {
interner: &'interner mut NodeInterner,
errors: Vec<TypeCheckError>,
current_function: Option<FuncId>,

/// Trait constraints are collected during type checking until they are
/// verified at the end of a function. This is because constraints arise
/// on each variable, but it is only until function calls when the types
/// needed for the trait constraint may become known.
trait_constraints: Vec<(TraitConstraint, ExprId)>,
}

/// Type checks a function and assigns the
Expand Down Expand Up @@ -66,18 +72,24 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec<Type
type_checker.bind_pattern(&param.0, param.1);
}

let (function_last_type, delayed_type_check_functions, mut body_errors) =
let (function_last_type, delayed_type_check_functions) =
type_checker.check_function_body(function_body_id);

errors.append(&mut body_errors);

// Go through any delayed type checking errors to see if they are resolved, or error otherwise.
for type_check_fn in delayed_type_check_functions {
if let Err(error) = type_check_fn() {
errors.push(error);
}
}

// Verify any remaining trait constraints arising from the function body
for (constraint, expr_id) in std::mem::take(&mut type_checker.trait_constraints) {
let span = type_checker.interner.expr_span(&expr_id);
type_checker.verify_trait_constraint(&constraint.typ, constraint.trait_id, expr_id, span);
}

errors.append(&mut type_checker.errors);

// Now remove all the `where` clause constraints we added
for constraint in &meta.trait_constraints {
interner.remove_assumed_trait_implementations_for_trait(constraint.trait_id);
Expand Down Expand Up @@ -146,26 +158,30 @@ fn function_info(interner: &NodeInterner, function_body_id: &ExprId) -> (noirc_e

impl<'interner> TypeChecker<'interner> {
fn new(interner: &'interner mut NodeInterner) -> Self {
Self { delayed_type_checks: Vec::new(), interner, errors: vec![], current_function: None }
Self {
delayed_type_checks: Vec::new(),
interner,
errors: Vec::new(),
trait_constraints: Vec::new(),
current_function: None,
}
}

pub fn push_delayed_type_check(&mut self, f: TypeCheckFn) {
self.delayed_type_checks.push(f);
}

fn check_function_body(
mut self,
body: &ExprId,
) -> (Type, Vec<TypeCheckFn>, Vec<TypeCheckError>) {
fn check_function_body(&mut self, body: &ExprId) -> (Type, Vec<TypeCheckFn>) {
let body_type = self.check_expression(body);
(body_type, self.delayed_type_checks, self.errors)
(body_type, std::mem::take(&mut self.delayed_type_checks))
}

pub fn check_global(id: &StmtId, interner: &'interner mut NodeInterner) -> Vec<TypeCheckError> {
let mut this = Self {
delayed_type_checks: Vec::new(),
interner,
errors: vec![],
errors: Vec::new(),
trait_constraints: Vec::new(),
current_function: None,
};
this.check_statement(id);
Expand Down
9 changes: 8 additions & 1 deletion compiler/noirc_frontend/src/monomorphization/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -837,7 +837,14 @@ impl<'interner> Monomorphizer<'interner> {
"There should be no remaining Assumed impls during monomorphization"
),
Err(constraints) => {
unreachable!("Failed to find trait impl during monomorphization. The failed constraint(s) are:\n {constraints:?}")
let failed_constraints = vecmap(constraints, |constraint| {
let id = constraint.trait_id;
let name = self.interner.get_trait(id).name.to_string();
format!(" {}: {name}", constraint.typ)
})
.join("\n");

unreachable!("Failed to find trait impl during monomorphization. The failed constraint(s) are:\n{failed_constraints}")
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "no_impl_from_function"
type = "bin"
authors = [""]

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
fn main() {
let array: [Field; 3] = [1, 2, 3];
assert(foo(array));

// Ensure this still works if we have to infer the type of the integer literals
let array = [1, 2, 3];
assert(foo(array));
}

fn foo<T>(x: T) -> bool where T: Eq {
x.eq(x)
}

trait Eq {
fn eq(self, other: Self) -> bool;
}

impl<T, N> Eq for [T; N] where T: Eq {
fn eq(self, other: Self) -> bool {
let mut ret = true;
for i in 0 .. self.len() {
ret &= self[i].eq(other[i]);
}
ret
}
}

0 comments on commit c8a4312

Please sign in to comment.