diff --git a/compiler/noirc_frontend/src/hir/type_check/errors.rs b/compiler/noirc_frontend/src/hir/type_check/errors.rs index 6beb6929ce1..fe9cc9ea2d6 100644 --- a/compiler/noirc_frontend/src/hir/type_check/errors.rs +++ b/compiler/noirc_frontend/src/hir/type_check/errors.rs @@ -65,8 +65,6 @@ pub enum TypeCheckError { VariableMustBeMutable { name: String, span: Span }, #[error("No method named '{method_name}' found for type '{object_type}'")] UnresolvedMethodCall { method_name: String, object_type: Type, span: Span }, - #[error("Comparisons are invalid on Field types. Try casting the operands to a sized integer type first")] - InvalidComparisonOnField { span: Span }, #[error("Integers must have the same signedness LHS is {sign_x:?}, RHS is {sign_y:?}")] IntegerSignedness { sign_x: Signedness, sign_y: Signedness, span: Span }, #[error("Integers must have the same bit width LHS is {bit_width_x}, RHS is {bit_width_y}")] @@ -76,7 +74,7 @@ pub enum TypeCheckError { #[error("{kind} cannot be used in a unary operation")] InvalidUnaryOp { kind: String, span: Span }, #[error("Bitwise operations are invalid on Field types. Try casting the operands to a sized integer type first.")] - InvalidBitwiseOperationOnField { span: Span }, + FieldBitwiseOp { span: Span }, #[error("Integer cannot be used with type {typ}")] IntegerTypeMismatch { typ: Type, span: Span }, #[error("Cannot use an integer and a Field in a binary operation, try converting the Field into an integer first")] @@ -224,12 +222,11 @@ impl From for Diagnostic { | TypeCheckError::TupleIndexOutOfBounds { span, .. } | TypeCheckError::VariableMustBeMutable { span, .. } | TypeCheckError::UnresolvedMethodCall { span, .. } - | TypeCheckError::InvalidComparisonOnField { span } | TypeCheckError::IntegerSignedness { span, .. } | TypeCheckError::IntegerBitWidth { span, .. } | TypeCheckError::InvalidInfixOp { span, .. } | TypeCheckError::InvalidUnaryOp { span, .. } - | TypeCheckError::InvalidBitwiseOperationOnField { span, .. } + | TypeCheckError::FieldBitwiseOp { span, .. } | TypeCheckError::IntegerTypeMismatch { span, .. } | TypeCheckError::FieldComparison { span, .. } | TypeCheckError::AmbiguousBitWidth { span, .. } diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 10476b6caef..fb66bdeae6e 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,8 @@ 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) + 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 { @@ -1079,13 +1081,16 @@ 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(), @@ -1093,22 +1098,26 @@ impl<'interner> TypeChecker<'interner> { 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) - }; + let use_impl = !lhs_type.is_numeric(); + + // If this operator isn't valid for fields we have to possibly narrow + // TypeVariableKind::IntegerOrField to TypeVariableKind::Integer. + // Doing so also ensures a type error if Field is used. + // The is_numeric check is to allow impls for custom types to bypass this. + if !op.kind.is_valid_for_field_type() && lhs_type.is_numeric() { + let target = Type::polymorphic_integer(self.interner); + + use BinaryOpKind::*; + use TypeCheckError::*; + self.unify(lhs_type, &target, || match op.kind { + Less | LessEqual | Greater | GreaterEqual => FieldComparison { span }, + And | Or | Xor | ShiftRight | ShiftLeft => FieldBitwiseOp { span }, + Modulo => FieldModulo { span }, + other => unreachable!("Operator {other:?} should be valid for Field"), + }); + } - 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 @@ -1130,6 +1139,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 +1151,8 @@ 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) + 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 { @@ -1170,7 +1177,7 @@ impl<'interner> TypeChecker<'interner> { if op.kind == BinaryOpKind::Modulo { return Err(TypeCheckError::FieldModulo { span }); } else { - return Err(TypeCheckError::InvalidBitwiseOperationOnField { span }); + return Err(TypeCheckError::FieldBitwiseOp { span }); } } Ok((FieldElement, false)) 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 binding.is_bindable(), TypeBinding::Unbound(_) => true, }, + Type::Alias(alias, args) => alias.borrow().get_type(args).is_bindable(), _ => false, } } @@ -605,6 +606,15 @@ impl Type { matches!(self.follow_bindings(), Type::Integer(Signedness::Unsigned, _)) } + pub fn is_numeric(&self) -> bool { + use Type::*; + use TypeVariableKind as K; + matches!( + self.follow_bindings(), + FieldElement | Integer(..) | Bool | TypeVariable(_, K::Integer | K::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 6f92cb52a88..c4f0a8d67ba 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -1033,19 +1033,19 @@ mod test { fn resolve_complex_closures() { let src = r#" fn main(x: Field) -> pub Field { - let closure_without_captures = |x| x + x; + let closure_without_captures = |x: Field| -> Field { x + x }; let a = closure_without_captures(1); - let closure_capturing_a_param = |y| y + x; + let closure_capturing_a_param = |y: Field| -> Field { y + x }; let b = closure_capturing_a_param(2); - let closure_capturing_a_local_var = |y| y + b; + let closure_capturing_a_local_var = |y: Field| -> Field { y + b }; let c = closure_capturing_a_local_var(3); - let closure_with_transitive_captures = |y| { + let closure_with_transitive_captures = |y: Field| -> Field { let d = 5; - let nested_closure = |z| { - let doubly_nested_closure = |w| w + x + b; + 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); 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 { diff --git a/noir_stdlib/src/ops.nr b/noir_stdlib/src/ops.nr index e561265629e..d855e794fb4 100644 --- a/noir_stdlib/src/ops.nr +++ b/noir_stdlib/src/ops.nr @@ -6,9 +6,9 @@ trait Add { impl Add for Field { fn add(self, other: Field) -> 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,14 +130,14 @@ 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 } } -// 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 { @@ -145,11 +145,11 @@ 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 } } -// 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/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); +} 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/execution_failure/hashmap_load_factor/Prover.toml b/test_programs/execution_failure/hashmap_load_factor/Prover.toml new file mode 100644 index 00000000000..6d72cab47fa --- /dev/null +++ b/test_programs/execution_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 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 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);