From 9510292b9278f9fba3ba688f4852ec7178199e8b Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 22 Jan 2024 13:36:38 -0500 Subject: [PATCH 01/13] WIP: first pass of adding a TypeVariableKind specifically for Integer(s) --- compiler/noirc_frontend/src/hir_def/types.rs | 47 ++++++++++++++++++- .../src/main.nr | 9 ++-- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 2c08a980d23..b91b5120fab 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -441,6 +441,10 @@ pub enum TypeVariableKind { /// type annotations on each integer literal. IntegerOrField, + /// A generic integer type. This is a more specific kind of TypeVariable + /// that can only be bound to Type::Integer, or other polymorphic integers. + Integer, + /// A potentially constant array size. This will only bind to itself, Type::NotConstant, or /// Type::Constant(n) with a matching size. This defaults to Type::Constant(n) if still unbound /// during monomorphization. @@ -745,7 +749,17 @@ impl std::fmt::Display for Type { Signedness::Signed => write!(f, "i{num_bits}"), Signedness::Unsigned => write!(f, "u{num_bits}"), }, - Type::TypeVariable(var, TypeVariableKind::Normal) => write!(f, "{}", var.borrow()), + Type::TypeVariable(var, TypeVariableKind::Normal) => write!(f, "{}", var.0.borrow()), + Type::TypeVariable(binding, TypeVariableKind::Integer) => { + if let TypeBinding::Unbound(_) = &*binding.0.borrow() { + // Show a Field by default if this TypeVariableKind::IntegerOrField is unbound, since that is + // what they bind to by default anyway. It is less confusing than displaying it + // as a generic. + write!(f, "Integer") + } else { + write!(f, "{}", binding.0.borrow()) + } + } Type::TypeVariable(binding, TypeVariableKind::IntegerOrField) => { if let TypeBinding::Unbound(_) = &*binding.borrow() { // Show a Field by default if this TypeVariableKind::IntegerOrField is unbound, since that is @@ -949,6 +963,20 @@ impl Type { } } } + Type::TypeVariable(self_var, TypeVariableKind::Integer) => { + let borrow = self_var.0.borrow(); + match &*borrow { + TypeBinding::Bound(typ) => typ.try_bind_to_polymorphic_int(var, bindings), + // Avoid infinitely recursive bindings + TypeBinding::Unbound(id) if *id == target_id => Ok(()), + TypeBinding::Unbound(id) => { + + // we want to bind var to self_var (the less specific to the more specififc) + bindings.insert(id, (var.clone(), _)); + Ok(()) + } + } + } Type::TypeVariable(binding, TypeVariableKind::Normal) => { let borrow = binding.borrow(); match &*borrow { @@ -1054,6 +1082,16 @@ impl Type { }) } + (TypeVariable(var, Kind::Integer), other) + | (other, TypeVariable(var, Kind::Integer)) => { + () + } + // { + // other.try_unify_to_type_variable(var, bindings, |bindings| { + // other.try_bind_to_polymorphic_int(var, bindings) + // }) + // } + (TypeVariable(var, Kind::Normal), other) | (other, TypeVariable(var, Kind::Normal)) => { other.try_unify_to_type_variable(var, bindings, |bindings| { other.try_bind_to(var, bindings) @@ -1599,6 +1637,7 @@ impl TypeVariableKind { pub(crate) fn default_type(&self) -> Type { match self { TypeVariableKind::IntegerOrField | TypeVariableKind::Normal => Type::default_int_type(), + TypeVariableKind::Integer => Type::default_range_loop_type(), TypeVariableKind::Constant(length) => Type::Constant(*length), } } @@ -1625,6 +1664,12 @@ impl From<&Type> for PrintableType { Signedness::Unsigned => PrintableType::UnsignedInteger { width: *bit_width }, Signedness::Signed => PrintableType::SignedInteger { width: *bit_width }, }, + Type::TypeVariable(binding, TypeVariableKind::Integer) => { + match &*binding.0.borrow() { + TypeBinding::Bound(typ) => typ.into(), + TypeBinding::Unbound(_) => Type::default_range_loop_type().into(), + } + } Type::TypeVariable(binding, TypeVariableKind::IntegerOrField) => { match &*binding.borrow() { TypeBinding::Bound(typ) => typ.into(), diff --git a/test_programs/compile_success_empty/brillig_integer_binary_operations/src/main.nr b/test_programs/compile_success_empty/brillig_integer_binary_operations/src/main.nr index a873bcd5dbd..64251f3c247 100644 --- a/test_programs/compile_success_empty/brillig_integer_binary_operations/src/main.nr +++ b/test_programs/compile_success_empty/brillig_integer_binary_operations/src/main.nr @@ -1,7 +1,7 @@ // Tests arithmetic operations on integers fn main() { - let x: u32 = 6; - let y: u32 = 2; + let x = 6; + let y = 2; assert((x + y) == add(x, y)); @@ -59,8 +59,9 @@ unconstrained fn and(x: u32, y: u32) -> u32 { x & y } -unconstrained fn or(x: u32, y: u32) -> u32 { - x | y +unconstrained fn or(x: u32, y: u32) -> () { + let z = x | y; + () } unconstrained fn check_xor(x: u32, y: u32, result: u32) -> bool { From ffb8301a4cd6e3708714acb3ac9c6414aa132f76 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 23 Jan 2024 14:20:36 -0500 Subject: [PATCH 02/13] WIP working through cargo errors --- compiler/noirc_frontend/src/hir_def/types.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index b91b5120fab..0da2f916e2d 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1,5 +1,5 @@ use std::{ - borrow::Cow, + borrow::{Borrow, Cow}, cell::RefCell, collections::{BTreeSet, HashMap}, rc::Rc, @@ -972,7 +972,7 @@ impl Type { TypeBinding::Unbound(id) => { // we want to bind var to self_var (the less specific to the more specififc) - bindings.insert(id, (var.clone(), _)); + bindings.insert(*id, (var.clone(), ())); Ok(()) } } From c8ebfb9a6dd8305837a58b7e4a61c72dd9e4b0f5 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 31 Jan 2024 14:41:19 -0500 Subject: [PATCH 03/13] revert unrelated test programs change, update printing of TypeVariableKind::Integer to default integer type name: 'u64' --- compiler/noirc_frontend/src/hir_def/types.rs | 2 +- .../brillig_integer_binary_operations/src/main.nr | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 23d0ce00550..ea57344f6c1 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -755,7 +755,7 @@ impl std::fmt::Display for Type { // Show a Field by default if this TypeVariableKind::IntegerOrField is unbound, since that is // what they bind to by default anyway. It is less confusing than displaying it // as a generic. - write!(f, "Integer") + write!(f, "u64") } else { write!(f, "{}", binding.0.borrow()) } diff --git a/test_programs/compile_success_empty/brillig_integer_binary_operations/src/main.nr b/test_programs/compile_success_empty/brillig_integer_binary_operations/src/main.nr index 64251f3c247..a873bcd5dbd 100644 --- a/test_programs/compile_success_empty/brillig_integer_binary_operations/src/main.nr +++ b/test_programs/compile_success_empty/brillig_integer_binary_operations/src/main.nr @@ -1,7 +1,7 @@ // Tests arithmetic operations on integers fn main() { - let x = 6; - let y = 2; + let x: u32 = 6; + let y: u32 = 2; assert((x + y) == add(x, y)); @@ -59,9 +59,8 @@ unconstrained fn and(x: u32, y: u32) -> u32 { x & y } -unconstrained fn or(x: u32, y: u32) -> () { - let z = x | y; - () +unconstrained fn or(x: u32, y: u32) -> u32 { + x | y } unconstrained fn check_xor(x: u32, y: u32, result: u32) -> bool { From 940ec3a0a0254be52139405c97c98f59e6dee0e2 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 7 Feb 2024 13:27:16 -0500 Subject: [PATCH 04/13] wip implementing type check rules for Kind::Integer --- compiler/noirc_frontend/src/hir_def/types.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index ea57344f6c1..6c541d17cbe 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -751,7 +751,7 @@ impl std::fmt::Display for Type { }, Type::TypeVariable(var, TypeVariableKind::Normal) => write!(f, "{}", var.0.borrow()), Type::TypeVariable(binding, TypeVariableKind::Integer) => { - if let TypeBinding::Unbound(_) = &*binding.0.borrow() { + if let TypeBinding::Unbound(_) = &*binding.borrow() { // Show a Field by default if this TypeVariableKind::IntegerOrField is unbound, since that is // what they bind to by default anyway. It is less confusing than displaying it // as a generic. @@ -964,7 +964,7 @@ impl Type { } } Type::TypeVariable(self_var, TypeVariableKind::Integer) => { - let borrow = self_var.0.borrow(); + let borrow = self_var.borrow(); match &*borrow { TypeBinding::Bound(typ) => typ.try_bind_to_polymorphic_int(var, bindings), // Avoid infinitely recursive bindings @@ -1084,13 +1084,10 @@ impl Type { (TypeVariable(var, Kind::Integer), other) | (other, TypeVariable(var, Kind::Integer)) => { - () + other.try_unify_to_type_variable(var, bindings, |bindings| { + other.try_bind_to_polymorphic_int(var, bindings) + }) } - // { - // other.try_unify_to_type_variable(var, bindings, |bindings| { - // other.try_bind_to_polymorphic_int(var, bindings) - // }) - // } (TypeVariable(var, Kind::Normal), other) | (other, TypeVariable(var, Kind::Normal)) => { other.try_unify_to_type_variable(var, bindings, |bindings| { @@ -1665,7 +1662,7 @@ impl From<&Type> for PrintableType { Signedness::Signed => PrintableType::SignedInteger { width: *bit_width }, }, Type::TypeVariable(binding, TypeVariableKind::Integer) => { - match &*binding.0.borrow() { + match &*binding.borrow() { TypeBinding::Bound(typ) => typ.into(), TypeBinding::Unbound(_) => Type::default_range_loop_type().into(), } From 56886b00276a4217c06dab50557ca704f6bcbe1f Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 7 Feb 2024 14:08:09 -0500 Subject: [PATCH 05/13] resolve typing with TypeVariableKind::Integer (bindings), update borrow stuff that changed from updated master, extend typ.try_bind_to_polymorphic_int to work with both IntegerOrField and Integer, added formatter for TypeVariableKind::Integer, testing.. --- .../noirc_frontend/src/hir/type_check/expr.rs | 20 ++++- compiler/noirc_frontend/src/hir_def/types.rs | 76 ++++++++++++------- 2 files changed, 65 insertions(+), 31 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 4bd7779587d..d9f2a66130f 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -793,7 +793,7 @@ impl<'interner> TypeChecker<'interner> { // Matches on TypeVariable must be first to follow any type // bindings. - (TypeVariable(int, _), other) | (other, TypeVariable(int, _)) => { + (TypeVariable(int, int_kind), other) | (other, TypeVariable(int, int_kind)) => { if let TypeBinding::Bound(binding) = &*int.borrow() { return self.comparator_operand_type_rules(other, binding, op, span); } @@ -811,7 +811,13 @@ impl<'interner> TypeChecker<'interner> { } let mut bindings = TypeBindings::new(); - if other.try_bind_to_polymorphic_int(int, &mut bindings).is_ok() + if other + .try_bind_to_polymorphic_int( + int, + &mut bindings, + *int_kind == TypeVariableKind::Integer, + ) + .is_ok() || other == &Type::Error { Type::apply_type_bindings(bindings); @@ -1069,7 +1075,7 @@ impl<'interner> TypeChecker<'interner> { // Matches on TypeVariable must be first so that we follow any type // bindings. - (TypeVariable(int, _), other) | (other, TypeVariable(int, _)) => { + (TypeVariable(int, int_kind), other) | (other, TypeVariable(int, int_kind)) => { if let TypeBinding::Bound(binding) = &*int.borrow() { return self.infix_operand_type_rules(binding, op, other, span); } @@ -1102,7 +1108,13 @@ impl<'interner> TypeChecker<'interner> { } let mut bindings = TypeBindings::new(); - if other.try_bind_to_polymorphic_int(int, &mut bindings).is_ok() + if other + .try_bind_to_polymorphic_int( + int, + &mut bindings, + *int_kind == TypeVariableKind::Integer, + ) + .is_ok() || other == &Type::Error { Type::apply_type_bindings(bindings); diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 6c541d17cbe..1492e6939f6 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1,5 +1,5 @@ use std::{ - borrow::{Borrow, Cow}, + borrow::Cow, cell::RefCell, collections::{BTreeSet, HashMap}, rc::Rc, @@ -749,7 +749,7 @@ impl std::fmt::Display for Type { Signedness::Signed => write!(f, "i{num_bits}"), Signedness::Unsigned => write!(f, "u{num_bits}"), }, - Type::TypeVariable(var, TypeVariableKind::Normal) => write!(f, "{}", var.0.borrow()), + Type::TypeVariable(var, TypeVariableKind::Normal) => write!(f, "{}", var.borrow()), Type::TypeVariable(binding, TypeVariableKind::Integer) => { if let TypeBinding::Unbound(_) = &*binding.borrow() { // Show a Field by default if this TypeVariableKind::IntegerOrField is unbound, since that is @@ -757,7 +757,7 @@ impl std::fmt::Display for Type { // as a generic. write!(f, "u64") } else { - write!(f, "{}", binding.0.borrow()) + write!(f, "{}", binding.borrow()) } } Type::TypeVariable(binding, TypeVariableKind::IntegerOrField) => { @@ -924,6 +924,7 @@ impl Type { Ok(()) } TypeVariableKind::IntegerOrField => Err(UnificationError), + TypeVariableKind::Integer => Err(UnificationError), }, } } @@ -938,6 +939,7 @@ impl Type { &self, var: &TypeVariable, bindings: &mut TypeBindings, + only_integer: bool, ) -> Result<(), UnificationError> { let target_id = match &*var.borrow() { TypeBinding::Bound(_) => unreachable!(), @@ -954,11 +956,20 @@ impl Type { Type::TypeVariable(self_var, TypeVariableKind::IntegerOrField) => { let borrow = self_var.borrow(); match &*borrow { - TypeBinding::Bound(typ) => typ.try_bind_to_polymorphic_int(var, bindings), + TypeBinding::Bound(typ) => { + typ.try_bind_to_polymorphic_int(var, bindings, only_integer) + } // Avoid infinitely recursive bindings TypeBinding::Unbound(id) if *id == target_id => Ok(()), - TypeBinding::Unbound(_) => { - bindings.insert(target_id, (var.clone(), this.clone())); + TypeBinding::Unbound(new_target_id) => { + if only_integer { + // Integer is more specific than IntegerOrField so we bind the type + // variable to Integer instead. + let clone = Type::TypeVariable(var.clone(), TypeVariableKind::Integer); + bindings.insert(*new_target_id, (self_var.clone(), clone)); + } else { + bindings.insert(target_id, (var.clone(), this.clone())); + } Ok(()) } } @@ -966,29 +977,37 @@ impl Type { Type::TypeVariable(self_var, TypeVariableKind::Integer) => { let borrow = self_var.borrow(); match &*borrow { - TypeBinding::Bound(typ) => typ.try_bind_to_polymorphic_int(var, bindings), + TypeBinding::Bound(typ) => { + typ.try_bind_to_polymorphic_int(var, bindings, only_integer) + } // Avoid infinitely recursive bindings TypeBinding::Unbound(id) if *id == target_id => Ok(()), - TypeBinding::Unbound(id) => { - - // we want to bind var to self_var (the less specific to the more specififc) - bindings.insert(*id, (var.clone(), ())); + TypeBinding::Unbound(_) => { + bindings.insert(target_id, (var.clone(), this.clone())); Ok(()) } } } - Type::TypeVariable(binding, TypeVariableKind::Normal) => { - let borrow = binding.borrow(); + Type::TypeVariable(self_var, TypeVariableKind::Normal) => { + let borrow = self_var.borrow(); match &*borrow { - TypeBinding::Bound(typ) => typ.try_bind_to_polymorphic_int(var, bindings), + TypeBinding::Bound(typ) => { + typ.try_bind_to_polymorphic_int(var, bindings, only_integer) + } // Avoid infinitely recursive bindings TypeBinding::Unbound(id) if *id == target_id => Ok(()), TypeBinding::Unbound(new_target_id) => { - // IntegerOrField is more specific than TypeVariable so we bind the type - // variable to IntegerOrField instead. - let clone = - Type::TypeVariable(var.clone(), TypeVariableKind::IntegerOrField); - bindings.insert(*new_target_id, (binding.clone(), clone)); + let clone_kind = if only_integer { + // Integer is more specific than TypeVariable so we bind the type + // variable to Integer instead. + TypeVariableKind::Integer + } else { + // IntegerOrField is more specific than TypeVariable so we bind the type + // variable to IntegerOrField instead. + TypeVariableKind::IntegerOrField + }; + let clone = Type::TypeVariable(var.clone(), clone_kind); + bindings.insert(*new_target_id, (self_var.clone(), clone)); Ok(()) } } @@ -1078,14 +1097,16 @@ impl Type { (TypeVariable(var, Kind::IntegerOrField), other) | (other, TypeVariable(var, Kind::IntegerOrField)) => { other.try_unify_to_type_variable(var, bindings, |bindings| { - other.try_bind_to_polymorphic_int(var, bindings) + let only_integer = false; + other.try_bind_to_polymorphic_int(var, bindings, only_integer) }) } (TypeVariable(var, Kind::Integer), other) | (other, TypeVariable(var, Kind::Integer)) => { other.try_unify_to_type_variable(var, bindings, |bindings| { - other.try_bind_to_polymorphic_int(var, bindings) + let only_integer = true; + other.try_bind_to_polymorphic_int(var, bindings, only_integer) }) } @@ -1661,12 +1682,10 @@ impl From<&Type> for PrintableType { Signedness::Unsigned => PrintableType::UnsignedInteger { width: *bit_width }, Signedness::Signed => PrintableType::SignedInteger { width: *bit_width }, }, - Type::TypeVariable(binding, TypeVariableKind::Integer) => { - match &*binding.borrow() { - TypeBinding::Bound(typ) => typ.into(), - TypeBinding::Unbound(_) => Type::default_range_loop_type().into(), - } - } + Type::TypeVariable(binding, TypeVariableKind::Integer) => match &*binding.borrow() { + TypeBinding::Bound(typ) => typ.into(), + TypeBinding::Unbound(_) => Type::default_range_loop_type().into(), + }, Type::TypeVariable(binding, TypeVariableKind::IntegerOrField) => { match &*binding.borrow() { TypeBinding::Bound(typ) => typ.into(), @@ -1725,6 +1744,9 @@ impl std::fmt::Debug for Type { Type::TypeVariable(binding, TypeVariableKind::IntegerOrField) => { write!(f, "IntOrField{:?}", binding) } + Type::TypeVariable(binding, TypeVariableKind::Integer) => { + write!(f, "Int{:?}", binding) + } Type::TypeVariable(binding, TypeVariableKind::Constant(n)) => { write!(f, "{}{:?}", n, binding) } From 4a2b99e040de35b2fca32d2c34c5f8852aca9fc1 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 7 Feb 2024 14:31:30 -0500 Subject: [PATCH 06/13] add test programs for regressions --- .../infer_int_loop_bounds/Nargo.toml | 7 +++++++ .../infer_int_loop_bounds/src/main.nr | 18 ++++++++++++++++++ .../int_or_field_casting_loop/Nargo.toml | 7 +++++++ .../int_or_field_casting_loop/src/main.nr | 15 +++++++++++++++ 4 files changed, 47 insertions(+) create mode 100644 test_programs/compile_success_empty/infer_int_loop_bounds/Nargo.toml create mode 100644 test_programs/compile_success_empty/infer_int_loop_bounds/src/main.nr create mode 100644 test_programs/compile_success_empty/int_or_field_casting_loop/Nargo.toml create mode 100644 test_programs/compile_success_empty/int_or_field_casting_loop/src/main.nr diff --git a/test_programs/compile_success_empty/infer_int_loop_bounds/Nargo.toml b/test_programs/compile_success_empty/infer_int_loop_bounds/Nargo.toml new file mode 100644 index 00000000000..36d613ec2ec --- /dev/null +++ b/test_programs/compile_success_empty/infer_int_loop_bounds/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "infer_int_loop_bounds" +type = "bin" +authors = [""] +compiler_version = ">=0.23.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/compile_success_empty/infer_int_loop_bounds/src/main.nr b/test_programs/compile_success_empty/infer_int_loop_bounds/src/main.nr new file mode 100644 index 00000000000..8b199d5a095 --- /dev/null +++ b/test_programs/compile_success_empty/infer_int_loop_bounds/src/main.nr @@ -0,0 +1,18 @@ +// issue #4290 +fn loop_from(_arr: [Field; N], _another: [Field; K]) -> u32 { + let mut acc = 0; + // N and K are type Field but the iterator is u32 + for i in N..K { + acc += i; + } + acc +} + +fn main() { + let _ = loop_from([0; 10], [0; 20]); +} + +#[test] +fn test_main() { + main(); +} diff --git a/test_programs/compile_success_empty/int_or_field_casting_loop/Nargo.toml b/test_programs/compile_success_empty/int_or_field_casting_loop/Nargo.toml new file mode 100644 index 00000000000..a6220d21f5e --- /dev/null +++ b/test_programs/compile_success_empty/int_or_field_casting_loop/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "int_or_field_casting_loop" +type = "bin" +authors = [""] +compiler_version = ">=0.23.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/compile_success_empty/int_or_field_casting_loop/src/main.nr b/test_programs/compile_success_empty/int_or_field_casting_loop/src/main.nr new file mode 100644 index 00000000000..e0d8ab96475 --- /dev/null +++ b/test_programs/compile_success_empty/int_or_field_casting_loop/src/main.nr @@ -0,0 +1,15 @@ +// issue #3639 +fn main() { + for i in 0..8 { + for j in 0..8 { + if (i as u64 + j as u64 < 8) { + // pass + } + } + } +} + +#[test] +fn test_main() { + main(); +} From db62dfce20f7ee53f73f525fce4e578c0d89a933 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 7 Feb 2024 15:05:03 -0500 Subject: [PATCH 07/13] ensure IntegerOrField and Integer TypeVariableKind's are equivalent where expected --- compiler/noirc_frontend/src/hir/type_check/expr.rs | 1 + .../noirc_frontend/src/monomorphization/mod.rs | 14 ++++++++------ compiler/noirc_frontend/src/node_interner.rs | 1 + tooling/noirc_abi/src/lib.rs | 11 +++++------ 4 files changed, 15 insertions(+), 12 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index d9f2a66130f..6950ba28520 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -553,6 +553,7 @@ impl<'interner> TypeChecker<'interner> { Type::Integer(..) | Type::FieldElement | Type::TypeVariable(_, TypeVariableKind::IntegerOrField) + | Type::TypeVariable(_, TypeVariableKind::Integer) | Type::Bool => (), Type::TypeVariable(_, _) => { diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 3323f1e24c4..9ba135401c8 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -793,12 +793,14 @@ impl<'interner> Monomorphizer<'interner> { // Default any remaining unbound type variables. // This should only happen if the variable in question is unused // and within a larger generic type. - let default = - if self.is_range_loop && matches!(kind, TypeVariableKind::IntegerOrField) { - Type::default_range_loop_type() - } else { - kind.default_type() - }; + let default = if self.is_range_loop + && (matches!(kind, TypeVariableKind::IntegerOrField) + || matches!(kind, TypeVariableKind::Integer)) + { + Type::default_range_loop_type() + } else { + kind.default_type() + }; let monomorphized_default = self.convert_type(&default); binding.bind(default); diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 53583bb0f17..d08bbdd52e8 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -1667,6 +1667,7 @@ fn get_type_method_key(typ: &Type) -> Option { Type::Array(_, _) => Some(Array), Type::Integer(_, _) => Some(FieldOrInt), Type::TypeVariable(_, TypeVariableKind::IntegerOrField) => Some(FieldOrInt), + Type::TypeVariable(_, TypeVariableKind::Integer) => Some(FieldOrInt), Type::Bool => Some(Bool), Type::String(_) => Some(String), Type::FmtString(_, _) => Some(FmtString), diff --git a/tooling/noirc_abi/src/lib.rs b/tooling/noirc_abi/src/lib.rs index 1fc257c1676..0666545c367 100644 --- a/tooling/noirc_abi/src/lib.rs +++ b/tooling/noirc_abi/src/lib.rs @@ -144,12 +144,11 @@ impl AbiType { Self::Integer { sign, width: *bit_width } } - Type::TypeVariable(binding, TypeVariableKind::IntegerOrField) => { - match &*binding.borrow() { - TypeBinding::Bound(typ) => Self::from_type(context, typ), - TypeBinding::Unbound(_) => Self::from_type(context, &Type::default_int_type()), - } - } + Type::TypeVariable(binding, TypeVariableKind::IntegerOrField) + | Type::TypeVariable(binding, TypeVariableKind::Integer) => match &*binding.borrow() { + TypeBinding::Bound(typ) => Self::from_type(context, typ), + TypeBinding::Unbound(_) => Self::from_type(context, &Type::default_int_type()), + }, Type::Bool => Self::Boolean, Type::String(size) => { let size = size From 082d3cccbaaa9e673cd1f59f304395b53b98a4e3 Mon Sep 17 00:00:00 2001 From: Michael J Klein Date: Mon, 12 Feb 2024 16:16:31 -0500 Subject: [PATCH 08/13] Update test_programs/compile_success_empty/infer_int_loop_bounds/src/main.nr Co-authored-by: jfecher --- .../compile_success_empty/infer_int_loop_bounds/src/main.nr | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test_programs/compile_success_empty/infer_int_loop_bounds/src/main.nr b/test_programs/compile_success_empty/infer_int_loop_bounds/src/main.nr index 8b199d5a095..d0aec831338 100644 --- a/test_programs/compile_success_empty/infer_int_loop_bounds/src/main.nr +++ b/test_programs/compile_success_empty/infer_int_loop_bounds/src/main.nr @@ -11,8 +11,3 @@ fn loop_from(_arr: [Field; N], _another: [Field; K]) -> u32 { fn main() { let _ = loop_from([0; 10], [0; 20]); } - -#[test] -fn test_main() { - main(); -} From f0918c8fabcc822384e11df0f32bcd53728a8b6b Mon Sep 17 00:00:00 2001 From: Michael J Klein Date: Mon, 12 Feb 2024 16:16:37 -0500 Subject: [PATCH 09/13] Update test_programs/compile_success_empty/int_or_field_casting_loop/src/main.nr Co-authored-by: jfecher --- .../int_or_field_casting_loop/src/main.nr | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test_programs/compile_success_empty/int_or_field_casting_loop/src/main.nr b/test_programs/compile_success_empty/int_or_field_casting_loop/src/main.nr index e0d8ab96475..e2caaac59f0 100644 --- a/test_programs/compile_success_empty/int_or_field_casting_loop/src/main.nr +++ b/test_programs/compile_success_empty/int_or_field_casting_loop/src/main.nr @@ -8,8 +8,3 @@ fn main() { } } } - -#[test] -fn test_main() { - main(); -} From a913bb4c9f0f37f5e193ccc061ad11b2f781a8d6 Mon Sep 17 00:00:00 2001 From: Michael J Klein Date: Mon, 12 Feb 2024 16:16:46 -0500 Subject: [PATCH 10/13] Update compiler/noirc_frontend/src/hir_def/types.rs Co-authored-by: jfecher --- compiler/noirc_frontend/src/hir_def/types.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index efb21f3e24a..332bf96085b 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -997,13 +997,10 @@ impl Type { // Avoid infinitely recursive bindings TypeBinding::Unbound(id) if *id == target_id => Ok(()), TypeBinding::Unbound(new_target_id) => { + // Bind to the most specific type variable kind let clone_kind = if only_integer { - // Integer is more specific than TypeVariable so we bind the type - // variable to Integer instead. TypeVariableKind::Integer } else { - // IntegerOrField is more specific than TypeVariable so we bind the type - // variable to IntegerOrField instead. TypeVariableKind::IntegerOrField }; let clone = Type::TypeVariable(var.clone(), clone_kind); From e6eb976c631e93d615ae9423ad8b4a4c465f43ae Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 13 Feb 2024 09:48:27 -0500 Subject: [PATCH 11/13] update comment w/ type checking context --- .../compile_success_empty/infer_int_loop_bounds/src/main.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_programs/compile_success_empty/infer_int_loop_bounds/src/main.nr b/test_programs/compile_success_empty/infer_int_loop_bounds/src/main.nr index d0aec831338..65ec4a2f3b6 100644 --- a/test_programs/compile_success_empty/infer_int_loop_bounds/src/main.nr +++ b/test_programs/compile_success_empty/infer_int_loop_bounds/src/main.nr @@ -1,7 +1,7 @@ // issue #4290 fn loop_from(_arr: [Field; N], _another: [Field; K]) -> u32 { let mut acc = 0; - // N and K are type Field but the iterator is u32 + // N and K were of type Field but the iterator is u32 for i in N..K { acc += i; } From 7cdacc261924debb50ea0e723ec8890368d67468 Mon Sep 17 00:00:00 2001 From: Michael J Klein Date: Tue, 13 Feb 2024 10:22:24 -0500 Subject: [PATCH 12/13] Update compiler/noirc_frontend/src/hir_def/types.rs Co-authored-by: jfecher --- compiler/noirc_frontend/src/hir_def/types.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index b648276542c..7a6d809003a 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -752,10 +752,7 @@ impl std::fmt::Display for Type { Type::TypeVariable(var, TypeVariableKind::Normal) => write!(f, "{}", var.borrow()), Type::TypeVariable(binding, TypeVariableKind::Integer) => { if let TypeBinding::Unbound(_) = &*binding.borrow() { - // Show a Field by default if this TypeVariableKind::IntegerOrField is unbound, since that is - // what they bind to by default anyway. It is less confusing than displaying it - // as a generic. - write!(f, "u64") + write!(f, "{}", TypeVariableKind::Integer.default_type()) } else { write!(f, "{}", binding.borrow()) } From ea3fd62588f4bb3c5f3bd833e4b2fc39560a0d2e Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 13 Feb 2024 10:26:13 -0500 Subject: [PATCH 13/13] remove tests that pass on master branch --- .../infer_int_loop_bounds/Nargo.toml | 7 ------- .../infer_int_loop_bounds/src/main.nr | 13 ------------- .../int_or_field_casting_loop/Nargo.toml | 7 ------- .../int_or_field_casting_loop/src/main.nr | 10 ---------- 4 files changed, 37 deletions(-) delete mode 100644 test_programs/compile_success_empty/infer_int_loop_bounds/Nargo.toml delete mode 100644 test_programs/compile_success_empty/infer_int_loop_bounds/src/main.nr delete mode 100644 test_programs/compile_success_empty/int_or_field_casting_loop/Nargo.toml delete mode 100644 test_programs/compile_success_empty/int_or_field_casting_loop/src/main.nr diff --git a/test_programs/compile_success_empty/infer_int_loop_bounds/Nargo.toml b/test_programs/compile_success_empty/infer_int_loop_bounds/Nargo.toml deleted file mode 100644 index 36d613ec2ec..00000000000 --- a/test_programs/compile_success_empty/infer_int_loop_bounds/Nargo.toml +++ /dev/null @@ -1,7 +0,0 @@ -[package] -name = "infer_int_loop_bounds" -type = "bin" -authors = [""] -compiler_version = ">=0.23.0" - -[dependencies] \ No newline at end of file diff --git a/test_programs/compile_success_empty/infer_int_loop_bounds/src/main.nr b/test_programs/compile_success_empty/infer_int_loop_bounds/src/main.nr deleted file mode 100644 index 65ec4a2f3b6..00000000000 --- a/test_programs/compile_success_empty/infer_int_loop_bounds/src/main.nr +++ /dev/null @@ -1,13 +0,0 @@ -// issue #4290 -fn loop_from(_arr: [Field; N], _another: [Field; K]) -> u32 { - let mut acc = 0; - // N and K were of type Field but the iterator is u32 - for i in N..K { - acc += i; - } - acc -} - -fn main() { - let _ = loop_from([0; 10], [0; 20]); -} diff --git a/test_programs/compile_success_empty/int_or_field_casting_loop/Nargo.toml b/test_programs/compile_success_empty/int_or_field_casting_loop/Nargo.toml deleted file mode 100644 index a6220d21f5e..00000000000 --- a/test_programs/compile_success_empty/int_or_field_casting_loop/Nargo.toml +++ /dev/null @@ -1,7 +0,0 @@ -[package] -name = "int_or_field_casting_loop" -type = "bin" -authors = [""] -compiler_version = ">=0.23.0" - -[dependencies] \ No newline at end of file diff --git a/test_programs/compile_success_empty/int_or_field_casting_loop/src/main.nr b/test_programs/compile_success_empty/int_or_field_casting_loop/src/main.nr deleted file mode 100644 index e2caaac59f0..00000000000 --- a/test_programs/compile_success_empty/int_or_field_casting_loop/src/main.nr +++ /dev/null @@ -1,10 +0,0 @@ -// issue #3639 -fn main() { - for i in 0..8 { - for j in 0..8 { - if (i as u64 + j as u64 < 8) { - // pass - } - } - } -}