From 6932503a78f1ce0720427455ebda46bbcebc1383 Mon Sep 17 00:00:00 2001 From: kek kek kek Date: Thu, 27 Jul 2023 09:07:42 -0700 Subject: [PATCH] =?UTF-8?q?chore:=20replace=20`Type::TypeVariable`,=20`Typ?= =?UTF-8?q?e::PolymorphicInteger`,=20and=20=E2=80=A6=20(#2065)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore: replace `Type::TypeVariable`, `Type::PolymorphicInteger`, and all other type variables with one kind * update comments --- .../src/hir/resolution/resolver.rs | 3 +- .../noirc_frontend/src/hir/type_check/expr.rs | 27 ++-- crates/noirc_frontend/src/hir_def/types.rs | 130 ++++++++++-------- .../src/monomorphization/mod.rs | 11 +- crates/noirc_frontend/src/node_interner.rs | 6 +- 5 files changed, 90 insertions(+), 87 deletions(-) diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index 480cbf8411a..27fa91a086b 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -777,8 +777,7 @@ impl<'a> Resolver<'a> { | Type::String(_) | Type::Unit | Type::Error - | Type::TypeVariable(_) - | Type::PolymorphicInteger(_, _) + | Type::TypeVariable(_, _) | Type::Constant(_) | Type::NamedGeneric(_, _) | Type::Forall(_, _) => (), diff --git a/crates/noirc_frontend/src/hir/type_check/expr.rs b/crates/noirc_frontend/src/hir/type_check/expr.rs index b360e16df1b..a2ff1c23a63 100644 --- a/crates/noirc_frontend/src/hir/type_check/expr.rs +++ b/crates/noirc_frontend/src/hir/type_check/expr.rs @@ -11,7 +11,7 @@ use crate::{ types::Type, }, node_interner::{ExprId, FuncId}, - CompTime, Shared, TypeBinding, UnaryOp, + CompTime, Shared, TypeBinding, TypeVariableKind, UnaryOp, }; use super::{errors::TypeCheckError, TypeChecker}; @@ -185,7 +185,10 @@ impl<'interner> TypeChecker<'interner> { }; let fresh_id = self.interner.next_type_variable_id(); let type_variable = Shared::new(TypeBinding::Unbound(fresh_id)); - let expected_type = Type::PolymorphicInteger(expected_comptime, type_variable); + let expected_type = Type::TypeVariable( + type_variable, + TypeVariableKind::IntegerOrField(expected_comptime), + ); self.unify(&start_range_type, &expected_type, range_span, || { TypeCheckError::TypeCannotBeUsed { @@ -343,8 +346,8 @@ impl<'interner> TypeChecker<'interner> { let is_comp_time = match from.follow_bindings() { Type::Integer(is_comp_time, ..) => is_comp_time, Type::FieldElement(is_comp_time) => is_comp_time, - Type::PolymorphicInteger(is_comp_time, _) => is_comp_time, - Type::TypeVariable(_) => { + Type::TypeVariable(_, TypeVariableKind::IntegerOrField(is_comp_time)) => is_comp_time, + Type::TypeVariable(_, _) => { self.errors.push(TypeCheckError::TypeAnnotationsNeeded { span }); return Type::Error; } @@ -616,12 +619,9 @@ impl<'interner> TypeChecker<'interner> { // Avoid reporting errors multiple times (Error, _) | (_, Error) => Ok(Bool(CompTime::Yes(None))), - // Matches on PolymorphicInteger and TypeVariable must be first to follow any type + // Matches on TypeVariable must be first to follow any type // bindings. - (var @ PolymorphicInteger(_, int), other) - | (other, var @ PolymorphicInteger(_, int)) - | (var @ TypeVariable(int), other) - | (other, var @ TypeVariable(int)) => { + (var @ TypeVariable(int, _), other) | (other, var @ TypeVariable(int, _)) => { if let TypeBinding::Bound(binding) = &*int.borrow() { return self.comparator_operand_type_rules(other, binding, op, span); } @@ -798,7 +798,7 @@ impl<'interner> TypeChecker<'interner> { // Could do a single unification for the entire function type, but matching beforehand // lets us issue a more precise error on the individual argument that fails to type check. match function { - Type::TypeVariable(binding) => { + Type::TypeVariable(binding, TypeVariableKind::Normal) => { if let TypeBinding::Bound(typ) = &*binding.borrow() { return self.bind_function_type(typ.clone(), args, span); } @@ -860,12 +860,9 @@ impl<'interner> TypeChecker<'interner> { // An error type on either side will always return an error (Error, _) | (_, Error) => Ok(Error), - // Matches on PolymorphicInteger and TypeVariable must be first so that we follow any type + // Matches on TypeVariable must be first so that we follow any type // bindings. - (var @ PolymorphicInteger(_, int), other) - | (other, var @ PolymorphicInteger(_, int)) - | (var @ TypeVariable(int), other) - | (other, var @ TypeVariable(int)) => { + (var @ TypeVariable(int, _), other) | (other, var @ TypeVariable(int, _)) => { if let TypeBinding::Bound(binding) = &*int.borrow() { return self.infix_operand_type_rules(binding, op, other, span); } diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index 9321a57b92b..143e59f9434 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -58,14 +58,7 @@ pub enum Type { /// is a process that replaces each NamedGeneric in a generic function with a TypeVariable. /// Doing this at each call site of a generic function is how they can be called with /// different argument types each time. - TypeVariable(TypeVariable), - - /// A generic integer or field type. This is a more specific kind of TypeVariable - /// that can only be bound to Type::Field, Type::Integer, or other PolymorphicIntegers. - /// This is the type of undecorated integer literals like `46`. Typing them in this way - /// allows them to be polymorphic over the actual integer/field type used without requiring - /// type annotations on each integer literal. - PolymorphicInteger(CompTime, TypeVariable), + TypeVariable(TypeVariable, TypeVariableKind), /// NamedGenerics are the 'T' or 'U' in a user-defined generic function /// like `fn foo(...) {}`. Unlike TypeVariables, they cannot be bound over. @@ -278,6 +271,18 @@ pub enum BinaryTypeOperator { Modulo, } +#[derive(Debug, PartialEq, Eq, Clone, Hash)] +pub enum TypeVariableKind { + /// Can bind to any type + Normal, + /// A generic integer or field type. This is a more specific kind of TypeVariable + /// that can only be bound to Type::Field, Type::Integer, or other polymorphic integers. + /// This is the type of undecorated integer literals like `46`. Typing them in this way + /// allows them to be polymorphic over the actual integer/field type used without requiring + /// type annotations on each integer literal. + IntegerOrField(CompTime), +} + /// A TypeVariable is a mutable reference that is either /// bound to some type, or unbound with a given TypeVariableId. pub type TypeVariable = Shared; @@ -536,12 +541,15 @@ impl Type { } pub fn type_variable(id: TypeVariableId) -> Type { - Type::TypeVariable(Shared::new(TypeBinding::Unbound(id))) + Type::TypeVariable(Shared::new(TypeBinding::Unbound(id)), TypeVariableKind::Normal) } pub fn polymorphic_integer(interner: &mut NodeInterner) -> Type { let id = interner.next_type_variable_id(); - Type::PolymorphicInteger(CompTime::new(interner), Shared::new(TypeBinding::Unbound(id))) + Type::TypeVariable( + Shared::new(TypeBinding::Unbound(id)), + TypeVariableKind::IntegerOrField(CompTime::new(interner)), + ) } /// A bit of an awkward name for this function - this function returns @@ -550,12 +558,10 @@ impl Type { /// they shouldn't be bound over until monomorphization. pub fn is_bindable(&self) -> bool { match self { - Type::PolymorphicInteger(_, binding) | Type::TypeVariable(binding) => { - match &*binding.borrow() { - TypeBinding::Bound(binding) => binding.is_bindable(), - TypeBinding::Unbound(_) => true, - } - } + Type::TypeVariable(binding, _) => match &*binding.borrow() { + TypeBinding::Bound(binding) => binding.is_bindable(), + TypeBinding::Unbound(_) => true, + }, _ => false, } } @@ -586,8 +592,7 @@ impl Type { | Type::String(_) | Type::Unit | Type::Error - | Type::TypeVariable(_) - | Type::PolymorphicInteger(_, _) + | Type::TypeVariable(_, _) | Type::Constant(_) | Type::NamedGeneric(_, _) | Type::Forall(_, _) => false, @@ -623,7 +628,9 @@ impl Type { Type::FieldElement(comptime) | Type::Integer(comptime, _, _) | Type::Bool(comptime) - | Type::PolymorphicInteger(comptime, _) => Cow::Borrowed(comptime), + | Type::TypeVariable(_, TypeVariableKind::IntegerOrField(comptime)) => { + Cow::Borrowed(comptime) + } _ => Cow::Owned(CompTime::No(None)), } } @@ -641,9 +648,9 @@ impl std::fmt::Display for Type { Signedness::Signed => write!(f, "{comp_time}i{num_bits}"), Signedness::Unsigned => write!(f, "{comp_time}u{num_bits}"), }, - Type::PolymorphicInteger(_, binding) => { + Type::TypeVariable(binding, TypeVariableKind::IntegerOrField(_)) => { if let TypeBinding::Unbound(_) = &*binding.borrow() { - // Show a Field by default if this PolymorphicInteger is unbound, since that is + // 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, "Field") @@ -667,7 +674,7 @@ impl std::fmt::Display for Type { Type::String(len) => write!(f, "str<{len}>"), Type::Unit => write!(f, "()"), Type::Error => write!(f, "error"), - Type::TypeVariable(id) => write!(f, "{}", id.borrow()), + Type::TypeVariable(id, TypeVariableKind::Normal) => write!(f, "{}", id.borrow()), Type::NamedGeneric(binding, name) => match &*binding.borrow() { TypeBinding::Bound(binding) => binding.fmt(f), TypeBinding::Unbound(_) if name.is_empty() => write!(f, "_"), @@ -738,7 +745,7 @@ impl Type { Type::FieldElement(comptime) | Type::Integer(comptime, _, _) => { comptime.set_span(new_span); } - Type::PolymorphicInteger(span, binding) => { + Type::TypeVariable(binding, TypeVariableKind::IntegerOrField(span)) => { if let TypeBinding::Bound(binding) = &mut *binding.borrow_mut() { return binding.set_comp_time_span(new_span); } @@ -753,7 +760,7 @@ impl Type { Type::FieldElement(comptime) | Type::Integer(comptime, _, _) => { *comptime = new_comptime; } - Type::PolymorphicInteger(comptime, binding) => { + Type::TypeVariable(binding, TypeVariableKind::IntegerOrField(comptime)) => { if let TypeBinding::Bound(binding) = &mut *binding.borrow_mut() { return binding.set_comp_time(new_comptime); } @@ -797,7 +804,7 @@ impl Type { Type::FieldElement(int_comp_time, ..) | Type::Integer(int_comp_time, ..) => { bind(int_comp_time) } - Type::PolymorphicInteger(int_comp_time, self_var) => { + Type::TypeVariable(self_var, TypeVariableKind::IntegerOrField(int_comp_time)) => { let borrow = self_var.borrow(); match &*borrow { TypeBinding::Bound(typ) => { @@ -811,7 +818,7 @@ impl Type { } } } - Type::TypeVariable(binding) => { + Type::TypeVariable(binding, TypeVariableKind::Normal) => { let borrow = binding.borrow(); match &*borrow { TypeBinding::Bound(typ) => { @@ -823,8 +830,10 @@ impl Type { drop(borrow); // PolymorphicInt is more specific than TypeVariable so we bind the type // variable to PolymorphicInt instead. - let mut clone = - Type::PolymorphicInteger(var_comp_time.clone(), var.clone()); + let mut clone = Type::TypeVariable( + var.clone(), + TypeVariableKind::IntegerOrField(var_comp_time.clone()), + ); clone.set_comp_time_span(span); *binding.borrow_mut() = TypeBinding::Bound(clone); Ok(()) @@ -862,9 +871,7 @@ impl Type { fn get_inner_type_variable(&self) -> Option> { match self { - Type::PolymorphicInteger(_, var) - | Type::TypeVariable(var) - | Type::NamedGeneric(var, _) => Some(var.clone()), + Type::TypeVariable(var, _) | Type::NamedGeneric(var, _) => Some(var.clone()), _ => None, } } @@ -873,7 +880,7 @@ impl Type { match self { Type::FieldElement(comptime) => comptime.is_comp_time(), Type::Integer(comptime, ..) => comptime.is_comp_time(), - Type::PolymorphicInteger(comptime, binding) => { + Type::TypeVariable(binding, TypeVariableKind::IntegerOrField(comptime)) => { if let TypeBinding::Bound(binding) = &*binding.borrow() { return binding.is_comp_time(); } @@ -926,8 +933,8 @@ impl Type { match (self, other) { (Error, _) | (_, Error) => Ok(()), - (PolymorphicInteger(comptime, binding), other) - | (other, PolymorphicInteger(comptime, binding)) => { + (TypeVariable(binding, TypeVariableKind::IntegerOrField(comptime)), other) + | (other, TypeVariable(binding, TypeVariableKind::IntegerOrField(comptime))) => { // If it is already bound, unify against what it is bound to if let TypeBinding::Bound(link) = &*binding.borrow() { return link.try_unify(other, span); @@ -937,7 +944,7 @@ impl Type { other.try_bind_to_polymorphic_int(binding, comptime, false, span) } - (TypeVariable(binding), other) | (other, TypeVariable(binding)) => { + (TypeVariable(binding, _), other) | (other, TypeVariable(binding, _)) => { if let TypeBinding::Bound(link) = &*binding.borrow() { return link.try_unify(other, span); } @@ -1046,7 +1053,7 @@ impl Type { match (self, other) { (Error, _) | (_, Error) => Ok(()), - (PolymorphicInteger(comptime, binding), other) => { + (TypeVariable(binding, TypeVariableKind::IntegerOrField(comptime)), other) => { if let TypeBinding::Bound(link) = &*binding.borrow() { return link.is_subtype_of(other, span); } @@ -1055,7 +1062,7 @@ impl Type { other.try_bind_to_polymorphic_int(binding, comptime, true, span) } // These needs to be a separate case to keep the argument order of is_subtype_of - (other, PolymorphicInteger(comptime, binding)) => { + (other, TypeVariable(binding, TypeVariableKind::IntegerOrField(comptime))) => { if let TypeBinding::Bound(link) = &*binding.borrow() { return other.is_subtype_of(link, span); } @@ -1065,14 +1072,14 @@ impl Type { other.try_bind_to_polymorphic_int(binding, comptime, false, span) } - (TypeVariable(binding), other) => { + (TypeVariable(binding, _), other) => { if let TypeBinding::Bound(link) = &*binding.borrow() { return link.is_subtype_of(other, span); } other.try_bind_to(binding) } - (other, TypeVariable(binding)) => { + (other, TypeVariable(binding, _)) => { if let TypeBinding::Bound(link) = &*binding.borrow() { return other.is_subtype_of(link, span); } @@ -1182,12 +1189,12 @@ impl Type { /// to a Type::Constant, return the constant as a u64. pub fn evaluate_to_u64(&self) -> Option { match self { - Type::PolymorphicInteger(_, binding) - | Type::NamedGeneric(binding, _) - | Type::TypeVariable(binding) => match &*binding.borrow() { - TypeBinding::Bound(binding) => binding.evaluate_to_u64(), - TypeBinding::Unbound(_) => None, - }, + Type::NamedGeneric(binding, _) | Type::TypeVariable(binding, _) => { + match &*binding.borrow() { + TypeBinding::Bound(binding) => binding.evaluate_to_u64(), + TypeBinding::Unbound(_) => None, + } + } Type::Array(len, _elem) => len.evaluate_to_u64(), Type::Constant(x) => Some(*x), _ => None, @@ -1213,10 +1220,12 @@ impl Type { AbiType::Integer { sign, width: *bit_width } } - Type::PolymorphicInteger(_, binding) => match &*binding.borrow() { - TypeBinding::Bound(typ) => typ.as_abi_type(), - TypeBinding::Unbound(_) => Type::default_int_type(None).as_abi_type(), - }, + Type::TypeVariable(binding, TypeVariableKind::IntegerOrField(_)) => { + match &*binding.borrow() { + TypeBinding::Bound(typ) => typ.as_abi_type(), + TypeBinding::Unbound(_) => Type::default_int_type(None).as_abi_type(), + } + } Type::Bool(_) => AbiType::Boolean, Type::String(size) => { let size = size @@ -1234,7 +1243,7 @@ impl Type { AbiType::Struct { fields } } Type::Tuple(_) => todo!("as_abi_type not yet implemented for tuple types"), - Type::TypeVariable(_) => unreachable!(), + Type::TypeVariable(_, _) => unreachable!(), Type::NamedGeneric(..) => unreachable!(), Type::Forall(..) => unreachable!(), Type::Function(_, _) => unreachable!(), @@ -1329,10 +1338,9 @@ impl Type { let size = Box::new(size.substitute(type_bindings)); Type::String(size) } - Type::PolymorphicInteger(_, binding) - | Type::NamedGeneric(binding, _) - | Type::TypeVariable(binding) => substitute_binding(binding), - + Type::NamedGeneric(binding, _) | Type::TypeVariable(binding, _) => { + substitute_binding(binding) + } // Do not substitute fields, it can lead to infinite recursion // and we should not match fields when type checking anyway. Type::Struct(fields, args) => { @@ -1378,12 +1386,12 @@ impl Type { Type::String(len) => len.occurs(target_id), Type::Struct(_, generic_args) => generic_args.iter().any(|arg| arg.occurs(target_id)), Type::Tuple(fields) => fields.iter().any(|field| field.occurs(target_id)), - Type::PolymorphicInteger(_, binding) - | Type::NamedGeneric(binding, _) - | Type::TypeVariable(binding) => match &*binding.borrow() { - TypeBinding::Bound(binding) => binding.occurs(target_id), - TypeBinding::Unbound(id) => *id == target_id, - }, + Type::NamedGeneric(binding, _) | Type::TypeVariable(binding, _) => { + match &*binding.borrow() { + TypeBinding::Bound(binding) => binding.occurs(target_id), + TypeBinding::Unbound(id) => *id == target_id, + } + } Type::Forall(typevars, typ) => { !typevars.iter().any(|(id, _)| *id == target_id) && typ.occurs(target_id) } @@ -1421,7 +1429,7 @@ impl Type { } Tuple(args) => Tuple(vecmap(args, |arg| arg.follow_bindings())), - TypeVariable(var) | PolymorphicInteger(_, var) | NamedGeneric(var, _) => { + TypeVariable(var, _) | NamedGeneric(var, _) => { if let TypeBinding::Bound(typ) = &*var.borrow() { return typ.follow_bindings(); } diff --git a/crates/noirc_frontend/src/monomorphization/mod.rs b/crates/noirc_frontend/src/monomorphization/mod.rs index 41831de644e..d9ee9666e3c 100644 --- a/crates/noirc_frontend/src/monomorphization/mod.rs +++ b/crates/noirc_frontend/src/monomorphization/mod.rs @@ -23,6 +23,7 @@ use crate::{ node_interner::{self, DefinitionKind, NodeInterner, StmtId}, token::Attribute, CompTime, ContractFunctionType, FunctionKind, Type, TypeBinding, TypeBindings, + TypeVariableKind, }; use self::ast::{Definition, FuncId, Function, LocalId, Program}; @@ -599,9 +600,7 @@ impl<'interner> Monomorphizer<'interner> { ast::Type::Slice(Box::new(element)) } - HirType::PolymorphicInteger(_, binding) - | HirType::TypeVariable(binding) - | HirType::NamedGeneric(binding, _) => { + HirType::TypeVariable(binding, _) | HirType::NamedGeneric(binding, _) => { if let TypeBinding::Bound(binding) = &*binding.borrow() { return Self::convert_type(binding); } @@ -974,7 +973,7 @@ impl<'interner> Monomorphizer<'interner> { fn unwrap_tuple_type(typ: &HirType) -> Vec { match typ { HirType::Tuple(fields) => fields.clone(), - HirType::TypeVariable(binding) => match &*binding.borrow() { + HirType::TypeVariable(binding, TypeVariableKind::Normal) => match &*binding.borrow() { TypeBinding::Bound(binding) => unwrap_tuple_type(binding), TypeBinding::Unbound(_) => unreachable!(), }, @@ -985,7 +984,7 @@ fn unwrap_tuple_type(typ: &HirType) -> Vec { fn unwrap_struct_type(typ: &HirType) -> Vec<(String, HirType)> { match typ { HirType::Struct(def, args) => def.borrow().get_fields(args), - HirType::TypeVariable(binding) => match &*binding.borrow() { + HirType::TypeVariable(binding, TypeVariableKind::Normal) => match &*binding.borrow() { TypeBinding::Bound(binding) => unwrap_struct_type(binding), TypeBinding::Unbound(_) => unreachable!(), }, @@ -997,7 +996,7 @@ fn unwrap_array_element_type(typ: &HirType) -> HirType { match typ { HirType::Array(_, elem) => *elem.clone(), HirType::Slice(elem) => *elem.clone(), - HirType::TypeVariable(binding) => match &*binding.borrow() { + HirType::TypeVariable(binding, TypeVariableKind::Normal) => match &*binding.borrow() { TypeBinding::Bound(binding) => unwrap_array_element_type(binding), TypeBinding::Unbound(_) => unreachable!(), }, diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index c03c18bb7a5..fa2ce49ed11 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -17,7 +17,7 @@ use crate::hir_def::{ function::{FuncMeta, HirFunction}, stmt::HirStatement, }; -use crate::{Shared, TypeBinding, TypeBindings, TypeVariable, TypeVariableId}; +use crate::{Shared, TypeBinding, TypeBindings, TypeVariable, TypeVariableId, TypeVariableKind}; /// The node interner is the central storage location of all nodes in Noir's Hir (the /// various node types can be found in hir_def). The interner is also used to collect @@ -624,7 +624,7 @@ fn get_type_method_key(typ: &Type) -> Option { Type::Array(_, _) => Some(Array), Type::Slice(_) => Some(Slice), Type::Integer(_, _, _) => Some(FieldOrInt), - Type::PolymorphicInteger(_, _) => Some(FieldOrInt), + Type::TypeVariable(_, TypeVariableKind::IntegerOrField(_)) => Some(FieldOrInt), Type::Bool(_) => Some(Bool), Type::String(_) => Some(String), Type::Unit => Some(Unit), @@ -633,7 +633,7 @@ fn get_type_method_key(typ: &Type) -> Option { Type::MutableReference(element) => get_type_method_key(element), // We do not support adding methods to these types - Type::TypeVariable(_) + Type::TypeVariable(_, _) | Type::NamedGeneric(_, _) | Type::Forall(_, _) | Type::Constant(_)