From 0436280813f5b66bbae643b5f8b31a717a4a8245 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 10 Nov 2023 12:13:44 +0300 Subject: [PATCH 1/3] Verify impls arising from function calls exist --- .../noirc_frontend/src/hir/type_check/expr.rs | 14 +++- .../noirc_frontend/src/hir/type_check/mod.rs | 38 ++++++--- .../src/monomorphization/mod.rs | 9 +- .../no_impl_from_function/Nargo.toml | 6 ++ .../no_impl_from_function/src/main.nr | 82 +++++++++++++++++++ 5 files changed, 136 insertions(+), 13 deletions(-) create mode 100644 tooling/nargo_cli/tests/compile_failure/no_impl_from_function/Nargo.toml create mode 100644 tooling/nargo_cli/tests/compile_failure/no_impl_from_function/src/main.nr diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 5e8ea9f08d3..561ab37d31c 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -53,6 +53,18 @@ 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. + let definition = self.interner.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 } @@ -294,7 +306,7 @@ impl<'interner> TypeChecker<'interner> { typ } - fn verify_trait_constraint( + pub fn verify_trait_constraint( &mut self, object_type: &Type, trait_id: TraitId, diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 01c68adbb6d..956992a54d0 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -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, }; @@ -28,6 +28,12 @@ pub struct TypeChecker<'interner> { interner: &'interner mut NodeInterner, errors: Vec, current_function: Option, + + /// 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 @@ -66,11 +72,9 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec Vec (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, Vec) { + fn check_function_body(&mut self, body: &ExprId) -> (Type, Vec) { 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 { 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); diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 634569bbc7a..57e4e6cdeb0 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -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}") } } } diff --git a/tooling/nargo_cli/tests/compile_failure/no_impl_from_function/Nargo.toml b/tooling/nargo_cli/tests/compile_failure/no_impl_from_function/Nargo.toml new file mode 100644 index 00000000000..0d243d0029d --- /dev/null +++ b/tooling/nargo_cli/tests/compile_failure/no_impl_from_function/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "no_impl_from_function" +type = "bin" +authors = [""] + +[dependencies] diff --git a/tooling/nargo_cli/tests/compile_failure/no_impl_from_function/src/main.nr b/tooling/nargo_cli/tests/compile_failure/no_impl_from_function/src/main.nr new file mode 100644 index 00000000000..59ce2d8d0cc --- /dev/null +++ b/tooling/nargo_cli/tests/compile_failure/no_impl_from_function/src/main.nr @@ -0,0 +1,82 @@ + +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(x: T) -> bool where T: Eq { + x.eq(x) +} + +trait Eq { + fn eq(self, other: Self) -> bool; +} + +impl 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 + } +} + +// fn main() { +// let mut ops: [Field] = [1, 10, 2]; +// let mut numbers: [Field] = [1, 2, 4]; +// let mut final = 0; +// +// for i in 0..2 { +// if ops[i] == 10 { +// // This line seems to cause the error. +// numbers[i+1] += numbers[i]; +// final = numbers[i+1]; +// } +// } +// } +// fn main() { +// let mut a: [Field] = [1, 2, 3]; +// let mut b: [Field] = [4, 5, 6]; +// +// for i in 0..2 { +// if a[i] == 2 { +// // Bug occurs in following line +// b[i + 1] = b[i] * b[i + 1]; +// } +// } +// } +// +// #[test] +// fn test_main() { +// main(); +// } +/// fn main() { +/// foo(&mut 2, [1]); +/// } +// unconstrained fn foo(x: &mut Field, y: [Field]){} + +// Speed test: +// use dep::std::collections::vec::Vec; +// +// fn main() { +// assert(to_bits(1234) != 5678); +// } +// +// fn to_bits(self: Self) -> [u1; MAX_BITS] { +// let mut res = [0 as u1; MAX_BITS]; +// +// for i in 0..NUM_LIMBS { +// let limb_bits = (self.limbs[i] as Field).to_le_bits(BITS_PER_LIMB as u32); +// for j in 0..BITS_PER_LIMB { +// let idx = i * (BITS_PER_LIMB as Field) + (j as Field); +// res[idx] = limb_bits[j as Field]; +// } +// } +// +// res +// } From f3f9497ba969883548625a10743f0d6512a55f2d Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 10 Nov 2023 12:39:54 +0300 Subject: [PATCH 2/3] Shorten test --- .../no_impl_from_function/src/main.nr | 56 ------------------- 1 file changed, 56 deletions(-) diff --git a/tooling/nargo_cli/tests/compile_failure/no_impl_from_function/src/main.nr b/tooling/nargo_cli/tests/compile_failure/no_impl_from_function/src/main.nr index 59ce2d8d0cc..b0c485c2bf5 100644 --- a/tooling/nargo_cli/tests/compile_failure/no_impl_from_function/src/main.nr +++ b/tooling/nargo_cli/tests/compile_failure/no_impl_from_function/src/main.nr @@ -1,4 +1,3 @@ - fn main() { let array: [Field; 3] = [1, 2, 3]; assert(foo(array)); @@ -25,58 +24,3 @@ impl Eq for [T; N] where T: Eq { ret } } - -// fn main() { -// let mut ops: [Field] = [1, 10, 2]; -// let mut numbers: [Field] = [1, 2, 4]; -// let mut final = 0; -// -// for i in 0..2 { -// if ops[i] == 10 { -// // This line seems to cause the error. -// numbers[i+1] += numbers[i]; -// final = numbers[i+1]; -// } -// } -// } -// fn main() { -// let mut a: [Field] = [1, 2, 3]; -// let mut b: [Field] = [4, 5, 6]; -// -// for i in 0..2 { -// if a[i] == 2 { -// // Bug occurs in following line -// b[i + 1] = b[i] * b[i + 1]; -// } -// } -// } -// -// #[test] -// fn test_main() { -// main(); -// } -/// fn main() { -/// foo(&mut 2, [1]); -/// } -// unconstrained fn foo(x: &mut Field, y: [Field]){} - -// Speed test: -// use dep::std::collections::vec::Vec; -// -// fn main() { -// assert(to_bits(1234) != 5678); -// } -// -// fn to_bits(self: Self) -> [u1; MAX_BITS] { -// let mut res = [0 as u1; MAX_BITS]; -// -// for i in 0..NUM_LIMBS { -// let limb_bits = (self.limbs[i] as Field).to_le_bits(BITS_PER_LIMB as u32); -// for j in 0..BITS_PER_LIMB { -// let idx = i * (BITS_PER_LIMB as Field) + (j as Field); -// res[idx] = limb_bits[j as Field]; -// } -// } -// -// res -// } From 5915ec3214e35af80b5d417f6bd6e27b726da919 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 10 Nov 2023 12:57:25 +0300 Subject: [PATCH 3/3] Fix test panic --- compiler/noirc_frontend/src/hir/type_check/expr.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 561ab37d31c..8222f212190 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -56,12 +56,13 @@ impl<'interner> TypeChecker<'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. - let definition = self.interner.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)); + 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)); + } } }