Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: TypeVariableKind for just Integers #4118

Merged
merged 29 commits into from
Feb 13, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
9510292
WIP: first pass of adding a TypeVariableKind specifically for Integer(s)
michaeljklein Jan 22, 2024
ffb8301
WIP working through cargo errors
michaeljklein Jan 23, 2024
5a9e434
Merge branch 'master' into michaeljklein/integer-kind
michaeljklein Jan 31, 2024
c8ebfb9
revert unrelated test programs change, update printing of TypeVariabl…
michaeljklein Jan 31, 2024
a169552
Merge branch 'master' into michaeljklein/integer-kind
michaeljklein Feb 7, 2024
940ec3a
wip implementing type check rules for Kind::Integer
michaeljklein Feb 7, 2024
5a1b8b6
Merge branch 'master' into michaeljklein/integer-kind
michaeljklein Feb 7, 2024
56886b0
resolve typing with TypeVariableKind::Integer (bindings), update borr…
michaeljklein Feb 7, 2024
83e776c
Merge branch 'master' into michaeljklein/integer-kind
michaeljklein Feb 7, 2024
4a2b99e
add test programs for regressions
michaeljklein Feb 7, 2024
442051d
Merge branch 'master' into michaeljklein/integer-kind
michaeljklein Feb 7, 2024
db62dfc
ensure IntegerOrField and Integer TypeVariableKind's are equivalent w…
michaeljklein Feb 7, 2024
cc739c3
Merge branch 'master' into michaeljklein/integer-kind
michaeljklein Feb 7, 2024
5a9781e
Merge branch 'master' into michaeljklein/integer-kind
michaeljklein Feb 7, 2024
d7c8098
Merge branch 'master' into michaeljklein/integer-kind
michaeljklein Feb 8, 2024
130d19d
Merge branch 'master' into michaeljklein/integer-kind
michaeljklein Feb 8, 2024
7102e4d
Merge branch 'master' into michaeljklein/integer-kind
michaeljklein Feb 8, 2024
78747a2
Merge branch 'master' into michaeljklein/integer-kind
TomAFrench Feb 9, 2024
c98ad67
Merge branch 'master' into michaeljklein/integer-kind
michaeljklein Feb 12, 2024
6e34306
Merge branch 'master' into michaeljklein/integer-kind
michaeljklein Feb 12, 2024
082d3cc
Update test_programs/compile_success_empty/infer_int_loop_bounds/src/…
michaeljklein Feb 12, 2024
f0918c8
Update test_programs/compile_success_empty/int_or_field_casting_loop/…
michaeljklein Feb 12, 2024
a913bb4
Update compiler/noirc_frontend/src/hir_def/types.rs
michaeljklein Feb 12, 2024
e6eb976
update comment w/ type checking context
michaeljklein Feb 13, 2024
d404ad4
Merge branch 'master' into michaeljklein/integer-kind
michaeljklein Feb 13, 2024
7cdacc2
Update compiler/noirc_frontend/src/hir_def/types.rs
michaeljklein Feb 13, 2024
ea3fd62
remove tests that pass on master branch
michaeljklein Feb 13, 2024
126965a
Merge branch 'master' into michaeljklein/integer-kind
michaeljklein Feb 13, 2024
77fc805
Merge branch 'master' into michaeljklein/integer-kind
michaeljklein Feb 13, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions compiler/noirc_frontend/src/hir/type_check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@ impl<'interner> TypeChecker<'interner> {
Type::Integer(..)
| Type::FieldElement
| Type::TypeVariable(_, TypeVariableKind::IntegerOrField)
| Type::TypeVariable(_, TypeVariableKind::Integer)
| Type::Bool => (),

Type::TypeVariable(_, _) => {
Expand Down Expand Up @@ -807,7 +808,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);
}
Expand All @@ -825,7 +826,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);
Expand Down Expand Up @@ -1083,7 +1090,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);
}
Expand Down Expand Up @@ -1116,7 +1123,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);
Expand Down
84 changes: 74 additions & 10 deletions compiler/noirc_frontend/src/hir_def/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -746,6 +750,16 @@ impl std::fmt::Display for Type {
Signedness::Unsigned => write!(f, "u{num_bits}"),
},
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")
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
} else {
write!(f, "{}", binding.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
Expand Down Expand Up @@ -910,6 +924,7 @@ impl Type {
Ok(())
}
TypeVariableKind::IntegerOrField => Err(UnificationError),
TypeVariableKind::Integer => Err(UnificationError),
},
}
}
Expand All @@ -924,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!(),
Expand All @@ -940,7 +956,30 @@ 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(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(())
}
}
}
Type::TypeVariable(self_var, TypeVariableKind::Integer) => {
let borrow = self_var.borrow();
match &*borrow {
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(_) => {
Expand All @@ -949,18 +988,26 @@ impl Type {
}
}
}
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
};
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
let clone = Type::TypeVariable(var.clone(), clone_kind);
bindings.insert(*new_target_id, (self_var.clone(), clone));
Ok(())
}
}
Expand Down Expand Up @@ -1050,7 +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| {
let only_integer = true;
other.try_bind_to_polymorphic_int(var, bindings, only_integer)
})
}

Expand Down Expand Up @@ -1599,6 +1655,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),
}
}
Expand All @@ -1625,6 +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::IntegerOrField) => {
match &*binding.borrow() {
TypeBinding::Bound(typ) => typ.into(),
Expand Down Expand Up @@ -1683,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)
}
Expand Down
14 changes: 8 additions & 6 deletions compiler/noirc_frontend/src/monomorphization/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
};
jfecher marked this conversation as resolved.
Show resolved Hide resolved

let monomorphized_default = self.convert_type(&default);
binding.bind(default);
Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1667,6 +1667,7 @@ fn get_type_method_key(typ: &Type) -> Option<TypeMethodKey> {
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),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "infer_int_loop_bounds"
type = "bin"
authors = [""]
compiler_version = ">=0.23.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// issue #4290
fn loop_from<N, K>(_arr: [Field; N], _another: [Field; K]) -> u32 {
let mut acc = 0;
// N and K are type Field but the iterator is u32
jfecher marked this conversation as resolved.
Show resolved Hide resolved
for i in N..K {
acc += i;
}
acc
}

fn main() {
let _ = loop_from([0; 10], [0; 20]);
}

#[test]
fn test_main() {
main();
}
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "int_or_field_casting_loop"
type = "bin"
authors = [""]
compiler_version = ">=0.23.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -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();
}
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
11 changes: 5 additions & 6 deletions tooling/noirc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading