From 3e8e90c4392a3b284a6daca1a0ede010c2148d96 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 26 Mar 2024 12:58:38 -0500 Subject: [PATCH 01/15] Fix issue --- .../noirc_frontend/src/hir/type_check/expr.rs | 86 +++++++------------ compiler/noirc_frontend/src/hir_def/types.rs | 1 + .../regression_4635/Nargo.toml | 7 ++ .../regression_4635/src/main.nr | 59 +++++++++++++ 4 files changed, 99 insertions(+), 54 deletions(-) create mode 100644 test_programs/compile_success_empty/regression_4635/Nargo.toml create mode 100644 test_programs/compile_success_empty/regression_4635/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 10476b6caef..569d2a8185b 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -152,14 +152,16 @@ impl<'interner> TypeChecker<'interner> { Ok((typ, use_impl)) => { if use_impl { let id = infix_expr.trait_method_id; - // Assume operators have no trait generics - self.verify_trait_constraint( - &lhs_type, - id.trait_id, - &[], - *expr_id, - span, - ); + + // Delay checking the trait constraint until the end of the function. + // Checking it now could bind an unbound type variable to any type + // that implements the trait. + let constraint = crate::hir_def::traits::TraitConstraint { + typ: lhs_type.clone(), + trait_id: id.trait_id, + trait_generics: Vec::new(), + }; + self.trait_constraints.push((constraint, *expr_id)); self.typecheck_operator_method(*expr_id, id, &lhs_type, span); } typ @@ -836,6 +838,10 @@ impl<'interner> TypeChecker<'interner> { match (lhs_type, rhs_type) { // Avoid reporting errors multiple times (Error, _) | (_, Error) => Ok((Bool, false)), + (Alias(alias, args), other) | (other, Alias(alias, args)) => { + let alias = alias.borrow().get_type(args); + self.comparator_operand_type_rules(&alias, other, op, span) + } // Matches on TypeVariable must be first to follow any type // bindings. @@ -844,12 +850,13 @@ impl<'interner> TypeChecker<'interner> { return self.comparator_operand_type_rules(other, binding, op, span); } - self.bind_type_variables_for_infix(lhs_type, op, rhs_type, span); - Ok((Bool, false)) - } - (Alias(alias, args), other) | (other, Alias(alias, args)) => { - let alias = alias.borrow().get_type(args); - self.comparator_operand_type_rules(&alias, other, op, span) + self.unify(lhs_type, rhs_type, || TypeCheckError::TypeMismatchWithSource { + expected: lhs_type.clone(), + actual: rhs_type.clone(), + source: Source::Binary, + span, + }); + Ok((Bool, true)) } (Integer(sign_x, bit_width_x), Integer(sign_y, bit_width_y)) => { if sign_x != sign_y { @@ -1079,38 +1086,6 @@ impl<'interner> TypeChecker<'interner> { } } - fn bind_type_variables_for_infix( - &mut self, - lhs_type: &Type, - op: &HirBinaryOp, - rhs_type: &Type, - span: Span, - ) { - self.unify(lhs_type, rhs_type, || TypeCheckError::TypeMismatchWithSource { - expected: lhs_type.clone(), - actual: rhs_type.clone(), - source: Source::Binary, - span, - }); - - // In addition to unifying both types, we also have to bind either - // the lhs or rhs to an integer type variable. This ensures if both lhs - // and rhs are type variables, that they will have the correct integer - // type variable kind instead of TypeVariableKind::Normal. - let target = if op.kind.is_valid_for_field_type() { - Type::polymorphic_integer_or_field(self.interner) - } else { - Type::polymorphic_integer(self.interner) - }; - - self.unify(lhs_type, &target, || TypeCheckError::TypeMismatchWithSource { - expected: lhs_type.clone(), - actual: rhs_type.clone(), - source: Source::Binary, - span, - }); - } - // Given a binary operator and another type. This method will produce the output type // and a boolean indicating whether to use the trait impl corresponding to the operator // or not. A value of false indicates the caller to use a primitive operation for this @@ -1130,6 +1105,10 @@ impl<'interner> TypeChecker<'interner> { match (lhs_type, rhs_type) { // An error type on either side will always return an error (Error, _) | (_, Error) => Ok((Error, false)), + (Alias(alias, args), other) | (other, Alias(alias, args)) => { + let alias = alias.borrow().get_type(args); + self.infix_operand_type_rules(&alias, op, other, span) + } // Matches on TypeVariable must be first so that we follow any type // bindings. @@ -1138,14 +1117,13 @@ impl<'interner> TypeChecker<'interner> { return self.infix_operand_type_rules(binding, op, other, span); } - self.bind_type_variables_for_infix(lhs_type, op, rhs_type, span); - - // Both types are unified so the choice of which to return is arbitrary - Ok((other.clone(), false)) - } - (Alias(alias, args), other) | (other, Alias(alias, args)) => { - let alias = alias.borrow().get_type(args); - self.infix_operand_type_rules(&alias, op, other, span) + self.unify(lhs_type, rhs_type, || TypeCheckError::TypeMismatchWithSource { + expected: lhs_type.clone(), + actual: rhs_type.clone(), + source: Source::Binary, + span, + }); + Ok((other.clone(), true)) } (Integer(sign_x, bit_width_x), Integer(sign_y, bit_width_y)) => { if sign_x != sign_y { diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 3c5627f739b..03487f1f1ba 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -589,6 +589,7 @@ impl Type { TypeBinding::Bound(binding) => binding.is_bindable(), TypeBinding::Unbound(_) => true, }, + Type::Alias(alias, args) => alias.borrow().get_type(args).is_bindable(), _ => false, } } diff --git a/test_programs/compile_success_empty/regression_4635/Nargo.toml b/test_programs/compile_success_empty/regression_4635/Nargo.toml new file mode 100644 index 00000000000..563e262410f --- /dev/null +++ b/test_programs/compile_success_empty/regression_4635/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "regression_4635" +type = "bin" +authors = [""] +compiler_version = ">=0.26.0" + +[dependencies] diff --git a/test_programs/compile_success_empty/regression_4635/src/main.nr b/test_programs/compile_success_empty/regression_4635/src/main.nr new file mode 100644 index 00000000000..23918e30785 --- /dev/null +++ b/test_programs/compile_success_empty/regression_4635/src/main.nr @@ -0,0 +1,59 @@ +trait FromField { + fn from_field(field: Field) -> Self; +} + +impl FromField for Field { + fn from_field(value: Field) -> Self { + value + } +} + +trait Deserialize { + fn deserialize(fields: [Field; N]) -> Self; +} + +global AZTEC_ADDRESS_LENGTH = 1; + +struct AztecAddress { + inner : Field +} + +impl FromField for AztecAddress { + fn from_field(value: Field) -> Self { + Self { inner: value } + } +} + +impl Deserialize for AztecAddress { + fn deserialize(fields: [Field; AZTEC_ADDRESS_LENGTH]) -> Self { + AztecAddress::from_field(fields[0]) + } +} + +impl Eq for AztecAddress { + fn eq(self, other: Self) -> bool { + self.inner == other.inner + } +} + +// Custom code + +struct MyStruct { + a: T +} + +impl Deserialize<1> for MyStruct { + fn deserialize(fields: [Field; 1]) -> Self where T: FromField { + Self{ a: FromField::from_field(fields[0]) } + } +} + +fn main() { + let fields = [5; 1]; + let foo = MyStruct::deserialize(fields); // Note I don't specify T here (the type of `foo.a`) + + let bar = AztecAddress { inner: 5 }; + + // Here `T` is apparently inferred to be `AztecAddress`, presumably because of the comparison. + assert(foo.a == bar); +} From f55006894bf7b9a4431d511a44e916d1f18ff41c Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 27 Mar 2024 09:19:27 -0500 Subject: [PATCH 02/15] Reorder stdlib impls --- noir_stdlib/src/cmp.nr | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/noir_stdlib/src/cmp.nr b/noir_stdlib/src/cmp.nr index 0eed50eb42b..dde29d7ee87 100644 --- a/noir_stdlib/src/cmp.nr +++ b/noir_stdlib/src/cmp.nr @@ -6,10 +6,10 @@ trait Eq { impl Eq for Field { fn eq(self, other: Field) -> bool { self == other } } -impl Eq for u1 { fn eq(self, other: u1) -> bool { self == other } } -impl Eq for u8 { fn eq(self, other: u8) -> bool { self == other } } -impl Eq for u32 { fn eq(self, other: u32) -> bool { self == other } } impl Eq for u64 { fn eq(self, other: u64) -> bool { self == other } } +impl Eq for u32 { fn eq(self, other: u32) -> bool { self == other } } +impl Eq for u8 { fn eq(self, other: u8) -> bool { self == other } } +impl Eq for u1 { fn eq(self, other: u1) -> bool { self == other } } impl Eq for i8 { fn eq(self, other: i8) -> bool { self == other } } impl Eq for i32 { fn eq(self, other: i32) -> bool { self == other } } @@ -107,8 +107,8 @@ trait Ord { // Note: Field deliberately does not implement Ord -impl Ord for u8 { - fn cmp(self, other: u8) -> Ordering { +impl Ord for u64 { + fn cmp(self, other: u64) -> Ordering { if self < other { Ordering::less() } else if self > other { @@ -131,8 +131,8 @@ impl Ord for u32 { } } -impl Ord for u64 { - fn cmp(self, other: u64) -> Ordering { +impl Ord for u8 { + fn cmp(self, other: u8) -> Ordering { if self < other { Ordering::less() } else if self > other { From 42ff186a07f1c4a08c106a1c35310575821b6bb0 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 27 Mar 2024 09:30:02 -0500 Subject: [PATCH 03/15] Fix signed bit shifts --- noir_stdlib/src/ops.nr | 14 ++++++-------- .../bit_shifts_runtime/src/main.nr | 2 +- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/noir_stdlib/src/ops.nr b/noir_stdlib/src/ops.nr index e561265629e..10f9048b485 100644 --- a/noir_stdlib/src/ops.nr +++ b/noir_stdlib/src/ops.nr @@ -134,10 +134,9 @@ impl Shl for u8 { fn shl(self, other: u8) -> u8 { self << other } } impl Shl for u32 { fn shl(self, other: u32) -> u32 { self << other } } impl Shl for u64 { fn shl(self, other: u64) -> u64 { self << other } } -// Bit shifting is not currently supported for signed integer types -// impl Shl for i8 { fn shl(self, other: i8) -> i8 { self << other } } -// impl Shl for i32 { fn shl(self, other: i32) -> i32 { self << other } } -// impl Shl for i64 { fn shl(self, other: i64) -> i64 { self << other } } +impl Shl for i8 { fn shl(self, other: i8) -> i8 { self << other } } +impl Shl for i32 { fn shl(self, other: i32) -> i32 { self << other } } +impl Shl for i64 { fn shl(self, other: i64) -> i64 { self << other } } // docs:start:shr-trait trait Shr { @@ -149,7 +148,6 @@ impl Shr for u8 { fn shr(self, other: u8) -> u8 { self >> other } } impl Shr for u32 { fn shr(self, other: u32) -> u32 { self >> other } } impl Shr for u64 { fn shr(self, other: u64) -> u64 { self >> other } } -// Bit shifting is not currently supported for signed integer types -// impl Shr for i8 { fn shr(self, other: i8) -> i8 { self >> other } } -// impl Shr for i32 { fn shr(self, other: i32) -> i32 { self >> other } } -// impl Shr for i64 { fn shr(self, other: i64) -> i64 { self >> other } } +impl Shr for i8 { fn shr(self, other: i8) -> i8 { self >> other } } +impl Shr for i32 { fn shr(self, other: i32) -> i32 { self >> other } } +impl Shr for i64 { fn shr(self, other: i64) -> i64 { self >> other } } diff --git a/test_programs/execution_success/bit_shifts_runtime/src/main.nr b/test_programs/execution_success/bit_shifts_runtime/src/main.nr index 28b3ef656c1..ff424c9fbf6 100644 --- a/test_programs/execution_success/bit_shifts_runtime/src/main.nr +++ b/test_programs/execution_success/bit_shifts_runtime/src/main.nr @@ -7,7 +7,7 @@ fn main(x: u64, y: u64) { assert(x >> y == 32); // Bit-shift with signed integers - let mut a :i8 = y as i8; + let mut a: i8 = y as i8; let mut b: i8 = x as i8; assert(b << 1 == -128); assert(b >> 2 == 16); From 21b6b72cd5e805d50f4dff5a030d853ef4d674a8 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 27 Mar 2024 10:03:30 -0500 Subject: [PATCH 04/15] Fix remaining tests --- .../noirc_frontend/src/hir/type_check/mod.rs | 50 +++++++++---------- noir_stdlib/src/ops.nr | 40 ++++++++------- .../hashmap_load_factor/Prover.toml | 23 +++++++++ 3 files changed, 68 insertions(+), 45 deletions(-) create mode 100644 test_programs/compile_failure/hashmap_load_factor/Prover.toml diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 137608f8037..bb42ebf68fb 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -86,31 +86,13 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec Vec Vec Vec Field { self + other } } -impl Add for u8 { fn add(self, other: u8) -> u8 { self + other } } -impl Add for u32 { fn add(self, other: u32) -> u32 { self + other } } impl Add for u64 { fn add(self, other: u64) -> u64 { self + other } } +impl Add for u32 { fn add(self, other: u32) -> u32 { self + other } } +impl Add for u8 { fn add(self, other: u8) -> u8 { self + other } } impl Add for i8 { fn add(self, other: i8) -> i8 { self + other } } impl Add for i32 { fn add(self, other: i32) -> i32 { self + other } } @@ -22,9 +22,9 @@ trait Sub { impl Sub for Field { fn sub(self, other: Field) -> Field { self - other } } -impl Sub for u8 { fn sub(self, other: u8) -> u8 { self - other } } -impl Sub for u32 { fn sub(self, other: u32) -> u32 { self - other } } impl Sub for u64 { fn sub(self, other: u64) -> u64 { self - other } } +impl Sub for u32 { fn sub(self, other: u32) -> u32 { self - other } } +impl Sub for u8 { fn sub(self, other: u8) -> u8 { self - other } } impl Sub for i8 { fn sub(self, other: i8) -> i8 { self - other } } impl Sub for i32 { fn sub(self, other: i32) -> i32 { self - other } } @@ -38,9 +38,9 @@ trait Mul { impl Mul for Field { fn mul(self, other: Field) -> Field { self * other } } -impl Mul for u8 { fn mul(self, other: u8) -> u8 { self * other } } -impl Mul for u32 { fn mul(self, other: u32) -> u32 { self * other } } impl Mul for u64 { fn mul(self, other: u64) -> u64 { self * other } } +impl Mul for u32 { fn mul(self, other: u32) -> u32 { self * other } } +impl Mul for u8 { fn mul(self, other: u8) -> u8 { self * other } } impl Mul for i8 { fn mul(self, other: i8) -> i8 { self * other } } impl Mul for i32 { fn mul(self, other: i32) -> i32 { self * other } } @@ -54,9 +54,9 @@ trait Div { impl Div for Field { fn div(self, other: Field) -> Field { self / other } } -impl Div for u8 { fn div(self, other: u8) -> u8 { self / other } } -impl Div for u32 { fn div(self, other: u32) -> u32 { self / other } } impl Div for u64 { fn div(self, other: u64) -> u64 { self / other } } +impl Div for u32 { fn div(self, other: u32) -> u32 { self / other } } +impl Div for u8 { fn div(self, other: u8) -> u8 { self / other } } impl Div for i8 { fn div(self, other: i8) -> i8 { self / other } } impl Div for i32 { fn div(self, other: i32) -> i32 { self / other } } @@ -68,9 +68,9 @@ trait Rem{ } // docs:end:rem-trait -impl Rem for u8 { fn rem(self, other: u8) -> u8 { self % other } } -impl Rem for u32 { fn rem(self, other: u32) -> u32 { self % other } } impl Rem for u64 { fn rem(self, other: u64) -> u64 { self % other } } +impl Rem for u32 { fn rem(self, other: u32) -> u32 { self % other } } +impl Rem for u8 { fn rem(self, other: u8) -> u8 { self % other } } impl Rem for i8 { fn rem(self, other: i8) -> i8 { self % other } } impl Rem for i32 { fn rem(self, other: i32) -> i32 { self % other } } @@ -84,9 +84,9 @@ trait BitOr { impl BitOr for bool { fn bitor(self, other: bool) -> bool { self | other } } -impl BitOr for u8 { fn bitor(self, other: u8) -> u8 { self | other } } -impl BitOr for u32 { fn bitor(self, other: u32) -> u32 { self | other } } impl BitOr for u64 { fn bitor(self, other: u64) -> u64 { self | other } } +impl BitOr for u32 { fn bitor(self, other: u32) -> u32 { self | other } } +impl BitOr for u8 { fn bitor(self, other: u8) -> u8 { self | other } } impl BitOr for i8 { fn bitor(self, other: i8) -> i8 { self | other } } impl BitOr for i32 { fn bitor(self, other: i32) -> i32 { self | other } } @@ -100,9 +100,9 @@ trait BitAnd { impl BitAnd for bool { fn bitand(self, other: bool) -> bool { self & other } } -impl BitAnd for u8 { fn bitand(self, other: u8) -> u8 { self & other } } -impl BitAnd for u32 { fn bitand(self, other: u32) -> u32 { self & other } } impl BitAnd for u64 { fn bitand(self, other: u64) -> u64 { self & other } } +impl BitAnd for u32 { fn bitand(self, other: u32) -> u32 { self & other } } +impl BitAnd for u8 { fn bitand(self, other: u8) -> u8 { self & other } } impl BitAnd for i8 { fn bitand(self, other: i8) -> i8 { self & other } } impl BitAnd for i32 { fn bitand(self, other: i32) -> i32 { self & other } } @@ -116,9 +116,9 @@ trait BitXor { impl BitXor for bool { fn bitxor(self, other: bool) -> bool { self ^ other } } -impl BitXor for u8 { fn bitxor(self, other: u8) -> u8 { self ^ other } } -impl BitXor for u32 { fn bitxor(self, other: u32) -> u32 { self ^ other } } impl BitXor for u64 { fn bitxor(self, other: u64) -> u64 { self ^ other } } +impl BitXor for u32 { fn bitxor(self, other: u32) -> u32 { self ^ other } } +impl BitXor for u8 { fn bitxor(self, other: u8) -> u8 { self ^ other } } impl BitXor for i8 { fn bitxor(self, other: i8) -> i8 { self ^ other } } impl BitXor for i32 { fn bitxor(self, other: i32) -> i32 { self ^ other } } @@ -130,9 +130,10 @@ trait Shl { } // docs:end:shl-trait -impl Shl for u8 { fn shl(self, other: u8) -> u8 { self << other } } impl Shl for u32 { fn shl(self, other: u32) -> u32 { self << other } } impl Shl for u64 { fn shl(self, other: u64) -> u64 { self << other } } +impl Shl for u8 { fn shl(self, other: u8) -> u8 { self << other } } +impl Shl for u1 { fn shl(self, other: u1) -> u1 { self << other } } impl Shl for i8 { fn shl(self, other: i8) -> i8 { self << other } } impl Shl for i32 { fn shl(self, other: i32) -> i32 { self << other } } @@ -144,9 +145,10 @@ trait Shr { } // docs:end:shr-trait -impl Shr for u8 { fn shr(self, other: u8) -> u8 { self >> other } } -impl Shr for u32 { fn shr(self, other: u32) -> u32 { self >> other } } impl Shr for u64 { fn shr(self, other: u64) -> u64 { self >> other } } +impl Shr for u32 { fn shr(self, other: u32) -> u32 { self >> other } } +impl Shr for u8 { fn shr(self, other: u8) -> u8 { self >> other } } +impl Shr for u1 { fn shr(self, other: u1) -> u1 { self >> other } } impl Shr for i8 { fn shr(self, other: i8) -> i8 { self >> other } } impl Shr for i32 { fn shr(self, other: i32) -> i32 { self >> other } } diff --git a/test_programs/compile_failure/hashmap_load_factor/Prover.toml b/test_programs/compile_failure/hashmap_load_factor/Prover.toml new file mode 100644 index 00000000000..6d72cab47fa --- /dev/null +++ b/test_programs/compile_failure/hashmap_load_factor/Prover.toml @@ -0,0 +1,23 @@ +[[input]] +key = 1 +value = 0 + +[[input]] +key = 2 +value = 0 + +[[input]] +key = 3 +value = 0 + +[[input]] +key = 4 +value = 0 + +[[input]] +key = 5 +value = 0 + +[[input]] +key = 6 +value = 0 From f56db1d6c2ef1f398b4471ffb7e2c82249451038 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 27 Mar 2024 10:19:26 -0500 Subject: [PATCH 05/15] Move hashmap_load_factor to execution_failure --- .../hashmap_load_factor/Nargo.toml | 0 .../hashmap_load_factor/Prover.toml | 0 .../hashmap_load_factor/src/main.nr | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename test_programs/{compile_failure => execution_failure}/hashmap_load_factor/Nargo.toml (100%) rename test_programs/{compile_failure => execution_failure}/hashmap_load_factor/Prover.toml (100%) rename test_programs/{compile_failure => execution_failure}/hashmap_load_factor/src/main.nr (100%) diff --git a/test_programs/compile_failure/hashmap_load_factor/Nargo.toml b/test_programs/execution_failure/hashmap_load_factor/Nargo.toml similarity index 100% rename from test_programs/compile_failure/hashmap_load_factor/Nargo.toml rename to test_programs/execution_failure/hashmap_load_factor/Nargo.toml diff --git a/test_programs/compile_failure/hashmap_load_factor/Prover.toml b/test_programs/execution_failure/hashmap_load_factor/Prover.toml similarity index 100% rename from test_programs/compile_failure/hashmap_load_factor/Prover.toml rename to test_programs/execution_failure/hashmap_load_factor/Prover.toml diff --git a/test_programs/compile_failure/hashmap_load_factor/src/main.nr b/test_programs/execution_failure/hashmap_load_factor/src/main.nr similarity index 100% rename from test_programs/compile_failure/hashmap_load_factor/src/main.nr rename to test_programs/execution_failure/hashmap_load_factor/src/main.nr From 4835609c73a31d1922c5319721df1e8634f5ba4f Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 27 Mar 2024 11:17:39 -0500 Subject: [PATCH 06/15] Use primitive ops more often; spam type annotations in frontend tests --- .../noirc_frontend/src/hir/type_check/expr.rs | 8 ++++-- compiler/noirc_frontend/src/hir_def/types.rs | 5 ++++ compiler/noirc_frontend/src/tests.rs | 27 ++++++++++--------- 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 569d2a8185b..f62c4512df2 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -856,7 +856,9 @@ impl<'interner> TypeChecker<'interner> { source: Source::Binary, span, }); - Ok((Bool, true)) + + let use_primitive_op = lhs_type.is_numeric(); + Ok((Bool, !use_primitive_op)) } (Integer(sign_x, bit_width_x), Integer(sign_y, bit_width_y)) => { if sign_x != sign_y { @@ -1123,7 +1125,9 @@ impl<'interner> TypeChecker<'interner> { source: Source::Binary, span, }); - Ok((other.clone(), true)) + + let use_primitive_op = lhs_type.is_numeric(); + Ok((other.clone(), !use_primitive_op)) } (Integer(sign_x, bit_width_x), Integer(sign_y, bit_width_y)) => { if sign_x != sign_y { diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 03487f1f1ba..6b3260d390c 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -606,6 +606,11 @@ impl Type { matches!(self.follow_bindings(), Type::Integer(Signedness::Unsigned, _)) } + pub fn is_numeric(&self) -> bool { + use Type::*; + matches!(self.follow_bindings(), FieldElement | Integer(..) | Bool) + } + fn contains_numeric_typevar(&self, target_id: TypeVariableId) -> bool { // True if the given type is a NamedGeneric with the target_id let named_generic_id_matches_target = |typ: &Type| { diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 6f92cb52a88..76a59f0d25c 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -1033,22 +1033,22 @@ mod test { fn resolve_complex_closures() { let src = r#" fn main(x: Field) -> pub Field { - let closure_without_captures = |x| x + x; - let a = closure_without_captures(1); + let closure_without_captures = |x: Field| -> Field { x + x }; + let a: Field = closure_without_captures(1); - let closure_capturing_a_param = |y| y + x; - let b = closure_capturing_a_param(2); + let closure_capturing_a_param = |y: Field| -> Field { y + x }; + let b: Field = closure_capturing_a_param(2); - let closure_capturing_a_local_var = |y| y + b; - let c = closure_capturing_a_local_var(3); + let closure_capturing_a_local_var = |y: Field| -> Field { y + b }; + let c: Field = closure_capturing_a_local_var(3); - let closure_with_transitive_captures = |y| { - let d = 5; - let nested_closure = |z| { - let doubly_nested_closure = |w| w + x + b; + let closure_with_transitive_captures = |y: Field| -> Field { + let d: Field = 5; + let nested_closure = |z: Field| -> Field { + let doubly_nested_closure = |w: Field| -> Field { w + x + b }; a + z + y + d + x + doubly_nested_closure(4) + x + y }; - let res = nested_closure(5); + let res: Field = nested_closure(5); res }; @@ -1219,8 +1219,8 @@ fn lambda$f1(mut env$l1: (Field)) -> Field { #[test] fn operators_in_global_used_in_type() { let src = r#" - global ONE = 1; - global COUNT = ONE + 2; + global ONE: Field = 1; + global COUNT: Field = ONE + 2; fn main() { let _array: [Field; COUNT] = [1, 2, 3]; } @@ -1233,6 +1233,7 @@ fn lambda$f1(mut env$l1: (Field)) -> Field { let src = r#" fn main() { for i in 0 .. 10 { + let i: u64 = i; if i == 2 { continue; } From 2d130d63dcead9fcb33622a0f2fb9bad841beabf Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 27 Mar 2024 11:59:29 -0500 Subject: [PATCH 07/15] Refine use_impl some more --- .../noirc_frontend/src/hir/type_check/expr.rs | 60 +++++++++++++------ compiler/noirc_frontend/src/hir_def/types.rs | 10 +++- 2 files changed, 51 insertions(+), 19 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index f62c4512df2..e705cf41fa5 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -850,15 +850,8 @@ impl<'interner> TypeChecker<'interner> { return self.comparator_operand_type_rules(other, binding, op, span); } - self.unify(lhs_type, rhs_type, || TypeCheckError::TypeMismatchWithSource { - expected: lhs_type.clone(), - actual: rhs_type.clone(), - source: Source::Binary, - span, - }); - - let use_primitive_op = lhs_type.is_numeric(); - Ok((Bool, !use_primitive_op)) + let use_impl = self.bind_type_variables_for_infix(lhs_type, op, rhs_type, span); + Ok((Bool, use_impl)) } (Integer(sign_x, bit_width_x), Integer(sign_y, bit_width_y)) => { if sign_x != sign_y { @@ -1088,6 +1081,44 @@ impl<'interner> TypeChecker<'interner> { } } + /// Handles the TypeVariable case for checking binary operators. + /// Returns true if we should use the impl for the operator instead of the primitive + /// version of it. + fn bind_type_variables_for_infix( + &mut self, + lhs_type: &Type, + op: &HirBinaryOp, + rhs_type: &Type, + span: Span, + ) -> bool { + self.unify(lhs_type, rhs_type, || TypeCheckError::TypeMismatchWithSource { + expected: lhs_type.clone(), + actual: rhs_type.clone(), + source: Source::Binary, + span, + }); + + let use_impl = !lhs_type.is_numeric(); + + // if the type variable is an integer or field we have to narrow it to only an integer + if !op.kind.is_valid_for_field_type() && lhs_type.is_numeric() { + // In addition to unifying both types, we also have to bind either + // the lhs or rhs to an integer type variable. This ensures if both lhs + // and rhs are type variables, that they will have the correct integer + // type variable kind instead of TypeVariableKind::Normal. + let target = Type::polymorphic_integer(self.interner); + + self.unify(lhs_type, &target, || TypeCheckError::TypeMismatchWithSource { + expected: lhs_type.clone(), + actual: rhs_type.clone(), + source: Source::Binary, + span, + }); + } + + use_impl + } + // Given a binary operator and another type. This method will produce the output type // and a boolean indicating whether to use the trait impl corresponding to the operator // or not. A value of false indicates the caller to use a primitive operation for this @@ -1119,15 +1150,8 @@ impl<'interner> TypeChecker<'interner> { return self.infix_operand_type_rules(binding, op, other, span); } - self.unify(lhs_type, rhs_type, || TypeCheckError::TypeMismatchWithSource { - expected: lhs_type.clone(), - actual: rhs_type.clone(), - source: Source::Binary, - span, - }); - - let use_primitive_op = lhs_type.is_numeric(); - Ok((other.clone(), !use_primitive_op)) + let use_impl = self.bind_type_variables_for_infix(lhs_type, op, rhs_type, span); + Ok((other.clone(), use_impl)) } (Integer(sign_x, bit_width_x), Integer(sign_y, bit_width_y)) => { if sign_x != sign_y { diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index f533d942384..970ff0f8e3b 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -608,7 +608,15 @@ impl Type { pub fn is_numeric(&self) -> bool { use Type::*; - matches!(self.follow_bindings(), FieldElement | Integer(..) | Bool) + use TypeVariableKind as K; + matches!( + self.follow_bindings(), + FieldElement | Integer(..) | Bool | TypeVariable(_, K::Integer | K::IntegerOrField) + ) + } + + pub(crate) fn is_integer_or_field_typevar(&self) -> bool { + matches!(self.follow_bindings(), Type::TypeVariable(_, TypeVariableKind::IntegerOrField)) } fn contains_numeric_typevar(&self, target_id: TypeVariableId) -> bool { From 17ca35f89d18a7d34d7c672037e0250b65fd222f Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 27 Mar 2024 12:03:50 -0500 Subject: [PATCH 08/15] Undo most type annotations in tests --- compiler/noirc_frontend/src/hir_def/types.rs | 4 ---- compiler/noirc_frontend/src/tests.rs | 15 +++++++-------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 970ff0f8e3b..a2aee5e4716 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -615,10 +615,6 @@ impl Type { ) } - pub(crate) fn is_integer_or_field_typevar(&self) -> bool { - matches!(self.follow_bindings(), Type::TypeVariable(_, TypeVariableKind::IntegerOrField)) - } - fn contains_numeric_typevar(&self, target_id: TypeVariableId) -> bool { // True if the given type is a NamedGeneric with the target_id let named_generic_id_matches_target = |typ: &Type| { diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 76a59f0d25c..c4f0a8d67ba 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -1034,21 +1034,21 @@ mod test { let src = r#" fn main(x: Field) -> pub Field { let closure_without_captures = |x: Field| -> Field { x + x }; - let a: Field = closure_without_captures(1); + let a = closure_without_captures(1); let closure_capturing_a_param = |y: Field| -> Field { y + x }; - let b: Field = closure_capturing_a_param(2); + let b = closure_capturing_a_param(2); let closure_capturing_a_local_var = |y: Field| -> Field { y + b }; - let c: Field = closure_capturing_a_local_var(3); + let c = closure_capturing_a_local_var(3); let closure_with_transitive_captures = |y: Field| -> Field { - let d: Field = 5; + let d = 5; let nested_closure = |z: Field| -> Field { let doubly_nested_closure = |w: Field| -> Field { w + x + b }; a + z + y + d + x + doubly_nested_closure(4) + x + y }; - let res: Field = nested_closure(5); + let res = nested_closure(5); res }; @@ -1219,8 +1219,8 @@ fn lambda$f1(mut env$l1: (Field)) -> Field { #[test] fn operators_in_global_used_in_type() { let src = r#" - global ONE: Field = 1; - global COUNT: Field = ONE + 2; + global ONE = 1; + global COUNT = ONE + 2; fn main() { let _array: [Field; COUNT] = [1, 2, 3]; } @@ -1233,7 +1233,6 @@ fn lambda$f1(mut env$l1: (Field)) -> Field { let src = r#" fn main() { for i in 0 .. 10 { - let i: u64 = i; if i == 2 { continue; } From 6554cd2797fe60f83a4f6457723c35ec5b378d70 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 27 Mar 2024 13:02:54 -0500 Subject: [PATCH 09/15] Fix impl search binding type variables --- .../noirc_frontend/src/hir/type_check/expr.rs | 16 ++++---- .../noirc_frontend/src/hir/type_check/mod.rs | 39 ++++++++++++++++++- compiler/noirc_frontend/src/node_interner.rs | 19 +++++---- 3 files changed, 57 insertions(+), 17 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index e705cf41fa5..5c0203e8085 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -126,7 +126,7 @@ impl<'interner> TypeChecker<'interner> { } } HirLiteral::Bool(_) => Type::Bool, - HirLiteral::Integer(_, _) => Type::polymorphic_integer_or_field(self.interner), + HirLiteral::Integer(_, _) => self.polymorphic_integer_or_field(), HirLiteral::Str(string) => { let len = Type::Constant(string.len() as u64); Type::String(Box::new(len)) @@ -560,15 +560,13 @@ impl<'interner> TypeChecker<'interner> { let index_type = self.check_expression(&index_expr.index); let span = self.interner.expr_span(&index_expr.index); - index_type.unify( - &Type::polymorphic_integer_or_field(self.interner), - &mut self.errors, - || TypeCheckError::TypeMismatch { + index_type.unify(&self.polymorphic_integer_or_field(), &mut self.errors, || { + TypeCheckError::TypeMismatch { expected_typ: "an integer".to_owned(), expr_typ: index_type.to_string(), expr_span: span, - }, - ); + } + }); // When writing `a[i]`, if `a : &mut ...` then automatically dereference `a` as many // times as needed to get the underlying array. @@ -1106,7 +1104,7 @@ impl<'interner> TypeChecker<'interner> { // the lhs or rhs to an integer type variable. This ensures if both lhs // and rhs are type variables, that they will have the correct integer // type variable kind instead of TypeVariableKind::Normal. - let target = Type::polymorphic_integer(self.interner); + let target = self.polymorphic_integer(); self.unify(lhs_type, &target, || TypeCheckError::TypeMismatchWithSource { expected: lhs_type.clone(), @@ -1217,7 +1215,7 @@ impl<'interner> TypeChecker<'interner> { self.errors .push(TypeCheckError::InvalidUnaryOp { kind: rhs_type.to_string(), span }); } - let expected = Type::polymorphic_integer_or_field(self.interner); + let expected = self.polymorphic_integer_or_field(); rhs_type.unify(&expected, &mut self.errors, || TypeCheckError::InvalidUnaryOp { kind: rhs_type.to_string(), span, diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index bb42ebf68fb..51e23f0cd6a 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -37,6 +37,11 @@ pub struct TypeChecker<'interner> { /// 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)>, + + /// All type variables created in the current function. + /// This map is used to default any integer type variables at the end of + /// a function (before checking trait constraints) if a type wasn't already chosen. + type_variables: Vec, } /// Type checks a function and assigns the @@ -127,6 +132,15 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec TypeChecker<'interner> { fn new(interner: &'interner mut NodeInterner) -> Self { - Self { interner, errors: Vec::new(), trait_constraints: Vec::new(), current_function: None } + Self { + interner, + errors: Vec::new(), + trait_constraints: Vec::new(), + type_variables: Vec::new(), + current_function: None, + } } fn check_function_body(&mut self, body: &ExprId) -> Type { @@ -348,6 +368,7 @@ impl<'interner> TypeChecker<'interner> { interner, errors: Vec::new(), trait_constraints: Vec::new(), + type_variables: Vec::new(), current_function: None, }; let statement = this.interner.get_global(id).let_statement; @@ -381,6 +402,22 @@ impl<'interner> TypeChecker<'interner> { make_error, ); } + + /// Return a fresh integer or field type variable and log it + /// in self.type_variables to default it later. + fn polymorphic_integer_or_field(&mut self) -> Type { + let typ = Type::polymorphic_integer_or_field(self.interner); + self.type_variables.push(typ.clone()); + typ + } + + /// Return a fresh integer type variable and log it + /// in self.type_variables to default it later. + fn polymorphic_integer(&mut self) -> Type { + let typ = Type::polymorphic_integer(self.interner); + self.type_variables.push(typ.clone()); + typ + } } // XXX: These tests are all manual currently. diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 09e0cb04d26..7905433ed67 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -1159,12 +1159,14 @@ impl NodeInterner { let impls = self.trait_implementation_map.get(&trait_id).ok_or_else(|| vec![make_constraint()])?; + let mut matching_impls = Vec::new(); + for (existing_object_type2, impl_kind) in impls { // Bug: We're instantiating only the object type's generics here, not all of the trait's generics like we need to let (existing_object_type, instantiation_bindings) = existing_object_type2.instantiate(self); - let mut fresh_bindings = TypeBindings::new(); + let mut fresh_bindings = type_bindings.clone(); let mut check_trait_generics = |impl_generics: &[Type]| { trait_generics.iter().zip(impl_generics).all(|(trait_generic, impl_generic2)| { @@ -1189,16 +1191,13 @@ impl NodeInterner { } if object_type.try_unify(&existing_object_type, &mut fresh_bindings).is_ok() { - // The unification was successful so we can append fresh_bindings to our bindings list - type_bindings.extend(fresh_bindings); - if let TraitImplKind::Normal(impl_id) = impl_kind { let trait_impl = self.get_trait_implementation(*impl_id); let trait_impl = trait_impl.borrow(); if let Err(mut errors) = self.validate_where_clause( &trait_impl.where_clause, - type_bindings, + &mut fresh_bindings, &instantiation_bindings, recursion_limit, ) { @@ -1207,11 +1206,17 @@ impl NodeInterner { } } - return Ok(impl_kind.clone()); + matching_impls.push((impl_kind.clone(), fresh_bindings)); } } - Err(vec![make_constraint()]) + if matching_impls.len() == 1 { + let (impl_, fresh_bindings) = matching_impls.pop().unwrap(); + *type_bindings = fresh_bindings; + Ok(impl_) + } else { + Err(vec![make_constraint()]) + } } /// Verifies that each constraint in the given where clause is valid. From d7babb1cda9e2b84e5d6270ab927b90358584a3a Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 27 Mar 2024 15:56:12 -0500 Subject: [PATCH 10/15] Improve trait resolution --- compiler/noirc_frontend/src/monomorphization/mod.rs | 7 ++++++- compiler/noirc_frontend/src/node_interner.rs | 10 +++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 618eba8f190..37bad901e01 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -1014,7 +1014,12 @@ impl<'interner> Monomorphizer<'interner> { Err(constraints) => { let failed_constraints = vecmap(constraints, |constraint| { let id = constraint.trait_id; - let name = self.interner.get_trait(id).name.to_string(); + let mut name = self.interner.get_trait(id).name.to_string(); + if !constraint.trait_generics.is_empty() { + let types = + vecmap(&constraint.trait_generics, |t| format!("{t:?}")); + name += &format!("<{}>", types.join(", ")); + } format!(" {}: {name}", constraint.typ) }) .join("\n"); diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 7905433ed67..756374f95c9 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -1232,12 +1232,11 @@ impl NodeInterner { // Instantiation bindings are generally safe to force substitute into the same type. // This is needed here to undo any bindings done to trait methods by monomorphization. // Otherwise, an impl for (A, B) could get narrowed to only an impl for e.g. (u8, u16). - let constraint_type = constraint.typ.force_substitute(instantiation_bindings); - let constraint_type = constraint_type.substitute(type_bindings); + let constraint_type = + constraint.typ.force_substitute(instantiation_bindings).substitute(type_bindings); let trait_generics = vecmap(&constraint.trait_generics, |generic| { - let generic = generic.force_substitute(instantiation_bindings); - generic.substitute(type_bindings) + generic.force_substitute(instantiation_bindings).substitute(type_bindings) }); self.lookup_trait_implementation_helper( @@ -1246,10 +1245,11 @@ impl NodeInterner { &trait_generics, // Use a fresh set of type bindings here since the constraint_type originates from // our impl list, which we don't want to bind to. - &mut TypeBindings::new(), + type_bindings, recursion_limit - 1, )?; } + Ok(()) } From e696da787c632203a370941f9bdfd0639bca413e Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 28 Mar 2024 15:43:59 -0500 Subject: [PATCH 11/15] Improve error message --- .../noirc_frontend/src/hir/type_check/expr.rs | 36 ++++++++++--------- compiler/noirc_frontend/src/node_interner.rs | 20 ++++++++++- 2 files changed, 39 insertions(+), 17 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 5c0203e8085..ec5645a4084 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -418,23 +418,27 @@ impl<'interner> TypeChecker<'interner> { self.interner.select_impl_for_expression(function_ident_id, impl_kind); } Err(erroring_constraints) => { - // Don't show any errors where try_get_trait returns None. - // This can happen if a trait is used that was never declared. - let constraints = erroring_constraints - .into_iter() - .map(|constraint| { - let r#trait = self.interner.try_get_trait(constraint.trait_id)?; - let mut name = r#trait.name.to_string(); - if !constraint.trait_generics.is_empty() { - let generics = vecmap(&constraint.trait_generics, ToString::to_string); - name += &format!("<{}>", generics.join(", ")); - } - Some((constraint.typ, name)) - }) - .collect::>>(); + if erroring_constraints.is_empty() { + self.errors.push(TypeCheckError::TypeAnnotationsNeeded { span }); + } else { + // Don't show any errors where try_get_trait returns None. + // This can happen if a trait is used that was never declared. + let constraints = erroring_constraints + .into_iter() + .map(|constraint| { + let r#trait = self.interner.try_get_trait(constraint.trait_id)?; + let mut name = r#trait.name.to_string(); + if !constraint.trait_generics.is_empty() { + let generics = vecmap(&constraint.trait_generics, ToString::to_string); + name += &format!("<{}>", generics.join(", ")); + } + Some((constraint.typ, name)) + }) + .collect::>>(); - if let Some(constraints) = constraints { - self.errors.push(TypeCheckError::NoMatchingImplFound { constraints, span }); + if let Some(constraints) = constraints { + self.errors.push(TypeCheckError::NoMatchingImplFound { constraints, span }); + } } } } diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 756374f95c9..dcfceccdb57 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -1079,6 +1079,7 @@ impl NodeInterner { /// constraint, but when where clauses are involved, the failing constraint may be several /// constraints deep. In this case, all of the constraints are returned, starting with the /// failing one. + /// If this list of failing constraints is empty, this means type annotations are required. pub fn lookup_trait_implementation( &self, object_type: &Type, @@ -1121,6 +1122,10 @@ impl NodeInterner { } /// Similar to `lookup_trait_implementation` but does not apply any type bindings on success. + /// On error returns either: + /// - 1+ failing trait constraints, including the original. + /// Each constraint after the first represents a `where` clause that was followed. + /// - 0 trait constraints indicating type annotations are needed to choose an impl. pub fn try_lookup_trait_implementation( &self, object_type: &Type, @@ -1138,6 +1143,11 @@ impl NodeInterner { Ok((impl_kind, bindings)) } + /// Returns the trait implementation if found. + /// On error returns either: + /// - 1+ failing trait constraints, including the original. + /// Each constraint after the first represents a `where` clause that was followed. + /// - 0 trait constraints indicating type annotations are needed to choose an impl. fn lookup_trait_implementation_helper( &self, object_type: &Type, @@ -1156,6 +1166,11 @@ impl NodeInterner { let object_type = object_type.substitute(type_bindings); + // If the object type isn't known, just return an error saying type annotations are needed. + if object_type.is_bindable() { + return Err(Vec::new()); + } + let impls = self.trait_implementation_map.get(&trait_id).ok_or_else(|| vec![make_constraint()])?; @@ -1214,8 +1229,11 @@ impl NodeInterner { let (impl_, fresh_bindings) = matching_impls.pop().unwrap(); *type_bindings = fresh_bindings; Ok(impl_) - } else { + } else if matching_impls.is_empty() { Err(vec![make_constraint()]) + } else { + // multiple matching impls, type annotations needed + Err(vec![]) } } From 308a786dbdd6e986adda9d457e806399e58bd738 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 29 Mar 2024 10:18:36 -0500 Subject: [PATCH 12/15] Add missed calls --- compiler/noirc_frontend/src/hir/type_check/stmt.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/stmt.rs b/compiler/noirc_frontend/src/hir/type_check/stmt.rs index 69363d5f00a..37f42ebaaad 100644 --- a/compiler/noirc_frontend/src/hir/type_check/stmt.rs +++ b/compiler/noirc_frontend/src/hir/type_check/stmt.rs @@ -71,7 +71,7 @@ impl<'interner> TypeChecker<'interner> { expr_span: range_span, }); - let expected_type = Type::polymorphic_integer(self.interner); + let expected_type = self.polymorphic_integer(); self.unify(&start_range_type, &expected_type, || TypeCheckError::TypeCannotBeUsed { typ: start_range_type.clone(), @@ -232,7 +232,7 @@ impl<'interner> TypeChecker<'interner> { let expr_span = self.interner.expr_span(index); index_type.unify( - &Type::polymorphic_integer_or_field(self.interner), + &self.polymorphic_integer_or_field(), &mut self.errors, || TypeCheckError::TypeMismatch { expected_typ: "an integer".to_owned(), From 0ee0176b869d376ccec21ba670cbc98d29c64a2f Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 29 Mar 2024 10:18:46 -0500 Subject: [PATCH 13/15] fmt --- compiler/noirc_frontend/src/hir/type_check/stmt.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/stmt.rs b/compiler/noirc_frontend/src/hir/type_check/stmt.rs index 37f42ebaaad..300bde87f6c 100644 --- a/compiler/noirc_frontend/src/hir/type_check/stmt.rs +++ b/compiler/noirc_frontend/src/hir/type_check/stmt.rs @@ -231,15 +231,13 @@ impl<'interner> TypeChecker<'interner> { let index_type = self.check_expression(index); let expr_span = self.interner.expr_span(index); - index_type.unify( - &self.polymorphic_integer_or_field(), - &mut self.errors, - || TypeCheckError::TypeMismatch { + index_type.unify(&self.polymorphic_integer_or_field(), &mut self.errors, || { + TypeCheckError::TypeMismatch { expected_typ: "an integer".to_owned(), expr_typ: index_type.to_string(), expr_span, - }, - ); + } + }); let (mut lvalue_type, mut lvalue, mut mutable) = self.check_lvalue(array, assign_span); From bd6b50f6046182db3f17a8af5f4b9c855ab0c3a8 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 29 Mar 2024 10:21:58 -0500 Subject: [PATCH 14/15] Fix merge --- compiler/noirc_frontend/src/hir/type_check/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 51e23f0cd6a..5bda4b9345e 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -137,7 +137,8 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec Date: Fri, 29 Mar 2024 10:33:48 -0500 Subject: [PATCH 15/15] Add type annotation to global --- noir_stdlib/src/hash/poseidon2.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir_stdlib/src/hash/poseidon2.nr b/noir_stdlib/src/hash/poseidon2.nr index 5b97d809896..12bf373e671 100644 --- a/noir_stdlib/src/hash/poseidon2.nr +++ b/noir_stdlib/src/hash/poseidon2.nr @@ -1,7 +1,7 @@ use crate::hash::Hasher; use crate::default::Default; -global RATE = 3; +global RATE: u32 = 3; struct Poseidon2 { cache: [Field;3],