From be6268e54968a6bb74884ba3d3f9a0c7b7b961ba Mon Sep 17 00:00:00 2001 From: guipublic Date: Mon, 18 Nov 2024 09:38:36 +0000 Subject: [PATCH 1/7] iterative unification --- .../src/hir_def/iterative_unification.rs | 329 ++++++++++++++++++ compiler/noirc_frontend/src/hir_def/mod.rs | 1 + compiler/noirc_frontend/src/hir_def/types.rs | 13 +- 3 files changed, 337 insertions(+), 6 deletions(-) create mode 100644 compiler/noirc_frontend/src/hir_def/iterative_unification.rs diff --git a/compiler/noirc_frontend/src/hir_def/iterative_unification.rs b/compiler/noirc_frontend/src/hir_def/iterative_unification.rs new file mode 100644 index 00000000000..2c126b452dd --- /dev/null +++ b/compiler/noirc_frontend/src/hir_def/iterative_unification.rs @@ -0,0 +1,329 @@ +use std::borrow::Cow; + +use noirc_errors::Span; + +use crate::{Kind, Type, TypeBinding, TypeBindings, TypeVariable, UnificationError}; + +pub(crate) struct Unifier { + // Temporary storage in order to get references to the types to be processed during unification + types: Vec, +} + +impl Unifier { + pub(crate) fn new() -> Unifier { + Unifier { types: Vec::new() } + } + + fn add(&mut self, typ: &Type) -> usize { + let len = self.types.len(); + self.types.push(typ.clone()); + len + } + + // Adds types to the temporary storage and returns their index + fn to_unite(&mut self, lhs: &Type, rhs: &Type) -> (usize, usize) { + let lhs_id = self.add(lhs); + let rhs_id = self.add(rhs); + (lhs_id, rhs_id) + } + + /// Iterative version of type unification + /// Unifying types may requires to unify other types which are + /// put in a queue and processed sequentially. + pub(crate) fn unify( + &mut self, + lhs: &Type, + rhs: &Type, + bindings: &mut TypeBindings, + ) -> Result<(), UnificationError> { + let mut to_process = vec![self.to_unite(lhs, rhs)]; + while let Some((a, b)) = to_process.pop() { + let (a, b) = (self.types[a].clone(), self.types[b].clone()); + let to_unit = self.try_unify_single(&a, &b, bindings)?; + to_process.extend(to_unit); + } + Ok(()) + } + + fn try_unify_to_type_variable_iter( + &mut self, + lhs: usize, + type_variable: &TypeVariable, + bindings: &mut TypeBindings, + + // Bind the type variable to a type. This is factored out since depending on the + // Kind, there are different methods to check whether the variable can + // bind to the given type or not. + bind_variable: impl FnOnce(&mut TypeBindings) -> Result<(), UnificationError>, + ) -> Result, UnificationError> { + match &*type_variable.borrow() { + // If it is already bound, unify against what it is bound to + TypeBinding::Bound(link) => { + let link_id = self.add(link); + return Ok(vec![(link_id, lhs)]); + } + TypeBinding::Unbound(id, _) => { + // We may have already "bound" this type variable in this call to + // try_unify, so check those bindings as well. + match bindings.clone().get(id) { + Some((_, kind, binding)) => { + if !self.kind_unifies_iter(&kind, &binding.kind()) { + return Err(UnificationError); + } + let bind_id = self.add(binding); + return Ok(vec![(bind_id, lhs)]); + } + None => (), + } + // Otherwise, bind it + bind_variable(bindings)?; + } + } + Ok(Vec::new()) + } + + fn try_unify_single( + &mut self, + lhs: &Type, + rhs: &Type, + bindings: &mut TypeBindings, + ) -> Result, UnificationError> { + use Type::*; + + let lhs: Cow = match lhs { + Type::InfixExpr(..) => Cow::Owned(lhs.canonicalize()), + other => Cow::Borrowed(other), + }; + + let rhs: Cow = match rhs { + Type::InfixExpr(..) => Cow::Owned(rhs.canonicalize()), + other => Cow::Borrowed(other), + }; + + match (lhs.as_ref(), rhs.as_ref()) { + (Error, _) | (_, Error) => Ok(Vec::new()), + + (Alias(alias, args), other) | (other, Alias(alias, args)) => { + let alias = alias.borrow().get_type(args); + Ok(vec![self.to_unite(other, &alias)]) + } + + (TypeVariable(var), other) | (other, TypeVariable(var)) => { + let other_id = self.add(other); + + match &*var.borrow() { + TypeBinding::Bound(typ) => { + if typ.is_numeric_value() { + self.try_unify_to_type_variable_iter( + other_id, + var, + bindings, + |bindings| { + let only_integer = matches!(typ, Type::Integer(..)); + other.try_bind_to_polymorphic_int(var, bindings, only_integer) + }, + ) + } else { + self.try_unify_to_type_variable_iter( + other_id, + var, + bindings, + |bindings| other.try_bind_to(var, bindings, typ.kind()), + ) + } + } + TypeBinding::Unbound(_id, Kind::IntegerOrField) => self + .try_unify_to_type_variable_iter(other_id, var, bindings, |bindings| { + let only_integer = false; + other.try_bind_to_polymorphic_int(var, bindings, only_integer) + }), + TypeBinding::Unbound(_id, Kind::Integer) => self + .try_unify_to_type_variable_iter(other_id, var, bindings, |bindings| { + let only_integer = true; + other.try_bind_to_polymorphic_int(var, bindings, only_integer) + }), + TypeBinding::Unbound(_id, type_var_kind) => self + .try_unify_to_type_variable_iter(other_id, var, bindings, |bindings| { + other.try_bind_to(var, bindings, type_var_kind.clone()) + }), + } + } + + (Array(len_a, elem_a), Array(len_b, elem_b)) => { + Ok(vec![self.to_unite(len_a, len_b), self.to_unite(elem_a, elem_b)]) + } + + (Slice(elem_a), Slice(elem_b)) => Ok(vec![self.to_unite(elem_a, elem_b)]), + + (String(len_a), String(len_b)) => Ok(vec![self.to_unite(len_a, len_b)]), + + (FmtString(len_a, elements_a), FmtString(len_b, elements_b)) => { + Ok(vec![self.to_unite(len_a, len_b), self.to_unite(elements_a, elements_b)]) + } + + (Tuple(elements_a), Tuple(elements_b)) => { + if elements_a.len() != elements_b.len() { + Err(UnificationError) + } else { + let mut to_unit = Vec::new(); + for (a, b) in elements_a.iter().zip(elements_b) { + to_unit.push(self.to_unite(a, b)); + } + Ok(to_unit) + } + } + + // No recursive try_unify call for struct fields. Don't want + // to mutate shared type variables within struct definitions. + // This isn't possible currently but will be once noir gets generic types + (Struct(id_a, args_a), Struct(id_b, args_b)) => { + if id_a == id_b && args_a.len() == args_b.len() { + let mut to_unit = Vec::new(); + for (a, b) in args_a.iter().zip(args_b) { + to_unit.push(self.to_unite(a, b)); + } + Ok(to_unit) + } else { + Err(UnificationError) + } + } + + (CheckedCast { to, .. }, other) | (other, CheckedCast { to, .. }) => { + Ok(vec![self.to_unite(to, other)]) + } + + (NamedGeneric(binding, _), other) | (other, NamedGeneric(binding, _)) + if !binding.borrow().is_unbound() => + { + if let TypeBinding::Bound(link) = &*binding.borrow() { + Ok(vec![self.to_unite(link, other)]) + } else { + unreachable!("If guard ensures binding is bound") + } + } + + (NamedGeneric(binding_a, name_a), NamedGeneric(binding_b, name_b)) => { + // Bound NamedGenerics are caught by the check above + assert!(binding_a.borrow().is_unbound()); + assert!(binding_b.borrow().is_unbound()); + + if name_a == name_b { + self.kind_unify(&binding_a.kind(), &binding_b.kind())?; + Ok(vec![]) + } else { + Err(UnificationError) + } + } + + ( + Function(params_a, ret_a, env_a, unconstrained_a), + Function(params_b, ret_b, env_b, unconstrained_b), + ) => { + if unconstrained_a == unconstrained_b && params_a.len() == params_b.len() { + let mut to_unit = Vec::new(); + for (a, b) in params_a.iter().zip(params_b.iter()) { + to_unit.push(self.to_unite(a, b)); + } + to_unit.push(self.to_unite(env_a, env_b)); + to_unit.push(self.to_unite(ret_b, ret_a)); + Ok(to_unit) + } else { + Err(UnificationError) + } + } + + (MutableReference(elem_a), MutableReference(elem_b)) => { + Ok(vec![self.to_unite(elem_a, elem_b)]) + } + + (InfixExpr(lhs_a, op_a, rhs_a), InfixExpr(lhs_b, op_b, rhs_b)) => { + if op_a == op_b { + // Try to simplify if both expressions have constant terms + if op_a.approx_inverse().is_some() { + let kind_a = lhs_a.infix_kind(rhs_a); + let kind_b = lhs_a.infix_kind(rhs_b); + let dummy_span = Span::default(); + let rhs_a_value = rhs_a.evaluate_to_field_element(&kind_a, dummy_span); + let rhs_b_value = rhs_b.evaluate_to_field_element(&kind_b, dummy_span); + if rhs_a_value.is_ok() && rhs_b_value.is_ok() { + return Ok(vec![self.to_unite(lhs_a, lhs_b)]); + } + } + + Ok(vec![self.to_unite(lhs_a, lhs_b), self.to_unite(rhs_a, rhs_b)]) + } else { + Err(UnificationError) + } + } + + (Constant(value, kind), other) | (other, Constant(value, kind)) => { + let dummy_span = Span::default(); + if let Ok(other_value) = other.evaluate_to_field_element(kind, dummy_span) { + if *value == other_value && self.kind_unifies_iter(&kind, &other.kind()) { + Ok(Vec::new()) + } else { + Err(UnificationError) + } + } else if let InfixExpr(lhs, op, rhs) = other { + if let Some(inverse) = op.approx_inverse() { + // Handle cases like `4 = a + b` by trying to solve to `a = 4 - b` + let new_type = InfixExpr( + Box::new(Constant(*value, kind.clone())), + inverse, + rhs.clone(), + ); + Ok(vec![self.to_unite(&new_type, lhs)]) + } else { + Err(UnificationError) + } + } else { + Err(UnificationError) + } + } + + (other_a, other_b) => { + if other_a == other_b { + Ok(Vec::new()) + } else { + Err(UnificationError) + } + } + } + } + + pub(crate) fn kind_unifies_iter(&mut self, lhs: &Kind, other: &Kind) -> bool { + match (lhs, other) { + // Kind::Any unifies with everything + (Kind::Any, _) | (_, Kind::Any) => true, + + // Kind::Normal unifies with Kind::Integer and Kind::IntegerOrField + (Kind::Normal, Kind::Integer | Kind::IntegerOrField) + | (Kind::Integer | Kind::IntegerOrField, Kind::Normal) => true, + + // Kind::Integer unifies with Kind::IntegerOrField + (Kind::Integer | Kind::IntegerOrField, Kind::Integer | Kind::IntegerOrField) => true, + + // Kind::Numeric unifies along its Type argument + (Kind::Numeric(lhs), Kind::Numeric(rhs)) => { + let mut bindings = TypeBindings::new(); + let unifies = self.unify(lhs, rhs, &mut bindings).is_ok(); + // let unifies = lhs.try_unify_iter(rhs, &mut bindings).is_ok(); + if unifies { + Type::apply_type_bindings(bindings); + } + unifies + } + + // everything unifies with itself + (lhs, rhs) => lhs == rhs, + } + } + + pub(crate) fn kind_unify(&mut self, lhs: &Kind, other: &Kind) -> Result<(), UnificationError> { + if self.kind_unifies_iter(lhs, other) { + Ok(()) + } else { + Err(UnificationError) + } + } +} diff --git a/compiler/noirc_frontend/src/hir_def/mod.rs b/compiler/noirc_frontend/src/hir_def/mod.rs index 206fc3ddda5..b0d04ef81fe 100644 --- a/compiler/noirc_frontend/src/hir_def/mod.rs +++ b/compiler/noirc_frontend/src/hir_def/mod.rs @@ -17,6 +17,7 @@ //! (monomorphization::ast::Expression). pub mod expr; pub mod function; +mod iterative_unification; pub mod stmt; pub mod traits; pub mod types; diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index a0fea3aa774..f7326f0d56c 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -13,6 +13,7 @@ use acvm::{AcirField, FieldElement}; use crate::{ ast::{IntegerBitSize, ItemVisibility}, hir::type_check::{generics::TraitGenerics, TypeCheckError}, + hir_def::iterative_unification::Unifier, node_interner::{ExprId, NodeInterner, TraitId, TypeAliasId}, }; use iter_extended::vecmap; @@ -1425,7 +1426,7 @@ impl Type { } /// Unifies self and other kinds or fails with a Kind error - fn infix_kind(&self, other: &Self) -> Kind { + pub(crate) fn infix_kind(&self, other: &Self) -> Kind { let self_kind = self.kind(); let other_kind = other.kind(); if self_kind.unifies(&other_kind) { @@ -1511,7 +1512,7 @@ impl Type { /// Try to bind a PolymorphicInt variable to self, succeeding if self is an integer, field, /// other PolymorphicInt type, or type variable. If successful, the binding is placed in the /// given TypeBindings map rather than linked immediately. - fn try_bind_to_polymorphic_int( + pub(crate) fn try_bind_to_polymorphic_int( &self, var: &TypeVariable, bindings: &mut TypeBindings, @@ -1589,7 +1590,7 @@ impl Type { /// /// If successful, the binding is placed in the /// given TypeBindings map rather than linked immediately. - fn try_bind_to( + pub(crate) fn try_bind_to( &self, var: &TypeVariable, bindings: &mut TypeBindings, @@ -1639,8 +1640,8 @@ impl Type { /// will correctly handle generic types. pub fn unify(&self, expected: &Type) -> Result<(), UnificationError> { let mut bindings = TypeBindings::new(); - - self.try_unify(expected, &mut bindings).map(|()| { + let mut unifier = Unifier::new(); + unifier.unify(self, expected, &mut bindings).map(|()| { // Commit any type bindings on success Self::apply_type_bindings(bindings); }) @@ -2727,7 +2728,7 @@ impl BinaryTypeOperator { } /// Return the operator that will "undo" this operation if applied to the rhs - fn approx_inverse(self) -> Option { + pub(crate) fn approx_inverse(self) -> Option { match self { BinaryTypeOperator::Addition => Some(BinaryTypeOperator::Subtraction), BinaryTypeOperator::Subtraction => Some(BinaryTypeOperator::Addition), From 5cabf55053e76d6c7c3f0d7f8687e079967a8a28 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 19 Nov 2024 09:22:38 +0000 Subject: [PATCH 2/7] iterative version for check_type and convert_type --- .../noirc_evaluator/src/ssa/opt/databus.rs | 46 ++++ .../noirc_frontend/src/elaborator/traits.rs | 5 +- .../noirc_frontend/src/elaborator/types.rs | 3 +- .../src/hir_def/iterative_unification.rs | 64 +++-- compiler/noirc_frontend/src/hir_def/mod.rs | 2 +- compiler/noirc_frontend/src/hir_def/types.rs | 237 +----------------- .../src/hir_def/types/arithmetic.rs | 46 +--- .../src/monomorphization/mod.rs | 150 +++++++---- compiler/noirc_frontend/src/node_interner.rs | 15 +- 9 files changed, 201 insertions(+), 367 deletions(-) create mode 100644 compiler/noirc_evaluator/src/ssa/opt/databus.rs diff --git a/compiler/noirc_evaluator/src/ssa/opt/databus.rs b/compiler/noirc_evaluator/src/ssa/opt/databus.rs new file mode 100644 index 00000000000..4cfcd0b9156 --- /dev/null +++ b/compiler/noirc_evaluator/src/ssa/opt/databus.rs @@ -0,0 +1,46 @@ +use crate::ssa::ir::instruction::Instruction; + + + +impl Ssa { + /// Map arrays with the last instruction that uses it + /// For this we simply process all the instructions in execution order + /// and update the map whenever there is a match + pub(crate) fn map_to_call_data_array(&self) -> HashMap { + let mut array_use = HashMap::default(); + for func in self.functions.values() { + let mut reverse_post_order = PostOrder::with_function(func).into_vec(); + reverse_post_order.reverse(); + for block in reverse_post_order { + last_use(block, &func.dfg, &mut array_use); + } + } + array_use + } +//la question c comment on remplace ? +//on pourrait simplement garder une liste instruction_id -> new instruction +// et on les remplace dans la liste des instructions instructions_mut() + pub(crate) fn array_me(block_id: BasicBlockId, dfg: &mut DataFlowGraph) { + let block = &dfg[block_id]; + for instruction_id in block.instructions() { + match &dfg[*instruction_id] { + Instruction::ArrayGet { array, index } => { + // Get operations to call-data parameters are replaced by a get to the call-data-bus array + if let Some(call_data) = dfg.data_bus.call_data { + let array_id = dfg.resolve(*array); + if dfg.data_bus.call_data_map.contains_key(&array_id) { + // on doit remplacer ce get par un autre, mais on doit d'abord fiare un calcul + // + dfg.make_constant(FieldElement::from( + self.data_bus.call_data_map[&array_id] as i128, + ), type_of_value(index)); + let new_index = self.acir_context.add_var(index, bus_index)?; //TODO add instruction + Instruction::ArrayGet { array: call_data, index: new_index };; //on veut remplacer par ca + } + } + } + + } + } +} +} diff --git a/compiler/noirc_frontend/src/elaborator/traits.rs b/compiler/noirc_frontend/src/elaborator/traits.rs index ae278616e03..10538f5239f 100644 --- a/compiler/noirc_frontend/src/elaborator/traits.rs +++ b/compiler/noirc_frontend/src/elaborator/traits.rs @@ -12,6 +12,7 @@ use crate::{ hir::{def_collector::dc_crate::UnresolvedTrait, type_check::TypeCheckError}, hir_def::{ function::Parameters, + iterative_unification::Unifier, traits::{ResolvedTraitBound, TraitFunction}, }, node_interner::{DependencyId, FuncId, NodeInterner, ReferenceId, TraitId}, @@ -345,7 +346,7 @@ fn check_function_type_matches_expected_type( if params_a.len() == params_b.len() { for (i, (a, b)) in params_a.iter().zip(params_b.iter()).enumerate() { - if a.try_unify(b, &mut bindings).is_err() { + if Unifier::try_unify(a, b, &mut bindings).is_err() { errors.push(TypeCheckError::TraitMethodParameterTypeMismatch { method_name: method_name.to_string(), expected_typ: a.to_string(), @@ -356,7 +357,7 @@ fn check_function_type_matches_expected_type( } } - if ret_b.try_unify(ret_a, &mut bindings).is_err() { + if Unifier::try_unify(ret_b, ret_a, &mut bindings).is_err() { errors.push(TypeCheckError::TypeMismatch { expected_typ: ret_a.to_string(), expr_typ: ret_b.to_string(), diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index b296c4f1805..4c75ba49d4b 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -26,6 +26,7 @@ use crate::{ HirMethodReference, HirPrefixExpression, TraitMethod, }, function::{FuncMeta, Parameters}, + iterative_unification::Unifier, stmt::HirStatement, traits::{NamedType, ResolvedTraitBound, Trait, TraitConstraint}, }, @@ -682,7 +683,7 @@ impl<'context> Elaborator<'context> { make_error: impl FnOnce() -> TypeCheckError, ) { let mut bindings = TypeBindings::new(); - if actual.try_unify(expected, &mut bindings).is_err() { + if Unifier::try_unify(actual, expected, &mut bindings).is_err() { self.errors.push((make_error().into(), file)); } } diff --git a/compiler/noirc_frontend/src/hir_def/iterative_unification.rs b/compiler/noirc_frontend/src/hir_def/iterative_unification.rs index 2c126b452dd..0ed9984f771 100644 --- a/compiler/noirc_frontend/src/hir_def/iterative_unification.rs +++ b/compiler/noirc_frontend/src/hir_def/iterative_unification.rs @@ -21,12 +21,23 @@ impl Unifier { } // Adds types to the temporary storage and returns their index - fn to_unite(&mut self, lhs: &Type, rhs: &Type) -> (usize, usize) { + fn for_unite(&mut self, lhs: &Type, rhs: &Type) -> (usize, usize) { let lhs_id = self.add(lhs); let rhs_id = self.add(rhs); (lhs_id, rhs_id) } + /// `try_unify` is a bit of a misnomer since although errors are not committed, + /// any unified bindings are on success. + pub(crate) fn try_unify( + lhs: &Type, + rhs: &Type, + bindings: &mut TypeBindings, + ) -> Result<(), UnificationError> { + let mut unifier = Unifier::new(); + unifier.unify(lhs, rhs, bindings) + } + /// Iterative version of type unification /// Unifying types may requires to unify other types which are /// put in a queue and processed sequentially. @@ -36,7 +47,7 @@ impl Unifier { rhs: &Type, bindings: &mut TypeBindings, ) -> Result<(), UnificationError> { - let mut to_process = vec![self.to_unite(lhs, rhs)]; + let mut to_process = vec![self.for_unite(lhs, rhs)]; while let Some((a, b)) = to_process.pop() { let (a, b) = (self.types[a].clone(), self.types[b].clone()); let to_unit = self.try_unify_single(&a, &b, bindings)?; @@ -45,6 +56,8 @@ impl Unifier { Ok(()) } + /// Try to unify a type variable to `self`. + /// This is a helper function factored out from try_unify. fn try_unify_to_type_variable_iter( &mut self, lhs: usize, @@ -65,15 +78,12 @@ impl Unifier { TypeBinding::Unbound(id, _) => { // We may have already "bound" this type variable in this call to // try_unify, so check those bindings as well. - match bindings.clone().get(id) { - Some((_, kind, binding)) => { - if !self.kind_unifies_iter(&kind, &binding.kind()) { - return Err(UnificationError); - } - let bind_id = self.add(binding); - return Ok(vec![(bind_id, lhs)]); + if let Some((_, kind, binding)) = bindings.clone().get(id) { + if !self.kind_unifies_iter(kind, &binding.kind()) { + return Err(UnificationError); } - None => (), + let bind_id = self.add(binding); + return Ok(vec![(bind_id, lhs)]); } // Otherwise, bind it bind_variable(bindings)?; @@ -105,7 +115,7 @@ impl Unifier { (Alias(alias, args), other) | (other, Alias(alias, args)) => { let alias = alias.borrow().get_type(args); - Ok(vec![self.to_unite(other, &alias)]) + Ok(vec![self.for_unite(other, &alias)]) } (TypeVariable(var), other) | (other, TypeVariable(var)) => { @@ -150,15 +160,15 @@ impl Unifier { } (Array(len_a, elem_a), Array(len_b, elem_b)) => { - Ok(vec![self.to_unite(len_a, len_b), self.to_unite(elem_a, elem_b)]) + Ok(vec![self.for_unite(len_a, len_b), self.for_unite(elem_a, elem_b)]) } - (Slice(elem_a), Slice(elem_b)) => Ok(vec![self.to_unite(elem_a, elem_b)]), + (Slice(elem_a), Slice(elem_b)) => Ok(vec![self.for_unite(elem_a, elem_b)]), - (String(len_a), String(len_b)) => Ok(vec![self.to_unite(len_a, len_b)]), + (String(len_a), String(len_b)) => Ok(vec![self.for_unite(len_a, len_b)]), (FmtString(len_a, elements_a), FmtString(len_b, elements_b)) => { - Ok(vec![self.to_unite(len_a, len_b), self.to_unite(elements_a, elements_b)]) + Ok(vec![self.for_unite(len_a, len_b), self.for_unite(elements_a, elements_b)]) } (Tuple(elements_a), Tuple(elements_b)) => { @@ -167,7 +177,7 @@ impl Unifier { } else { let mut to_unit = Vec::new(); for (a, b) in elements_a.iter().zip(elements_b) { - to_unit.push(self.to_unite(a, b)); + to_unit.push(self.for_unite(a, b)); } Ok(to_unit) } @@ -180,7 +190,7 @@ impl Unifier { if id_a == id_b && args_a.len() == args_b.len() { let mut to_unit = Vec::new(); for (a, b) in args_a.iter().zip(args_b) { - to_unit.push(self.to_unite(a, b)); + to_unit.push(self.for_unite(a, b)); } Ok(to_unit) } else { @@ -189,14 +199,14 @@ impl Unifier { } (CheckedCast { to, .. }, other) | (other, CheckedCast { to, .. }) => { - Ok(vec![self.to_unite(to, other)]) + Ok(vec![self.for_unite(to, other)]) } (NamedGeneric(binding, _), other) | (other, NamedGeneric(binding, _)) if !binding.borrow().is_unbound() => { if let TypeBinding::Bound(link) = &*binding.borrow() { - Ok(vec![self.to_unite(link, other)]) + Ok(vec![self.for_unite(link, other)]) } else { unreachable!("If guard ensures binding is bound") } @@ -222,10 +232,10 @@ impl Unifier { if unconstrained_a == unconstrained_b && params_a.len() == params_b.len() { let mut to_unit = Vec::new(); for (a, b) in params_a.iter().zip(params_b.iter()) { - to_unit.push(self.to_unite(a, b)); + to_unit.push(self.for_unite(a, b)); } - to_unit.push(self.to_unite(env_a, env_b)); - to_unit.push(self.to_unite(ret_b, ret_a)); + to_unit.push(self.for_unite(env_a, env_b)); + to_unit.push(self.for_unite(ret_b, ret_a)); Ok(to_unit) } else { Err(UnificationError) @@ -233,7 +243,7 @@ impl Unifier { } (MutableReference(elem_a), MutableReference(elem_b)) => { - Ok(vec![self.to_unite(elem_a, elem_b)]) + Ok(vec![self.for_unite(elem_a, elem_b)]) } (InfixExpr(lhs_a, op_a, rhs_a), InfixExpr(lhs_b, op_b, rhs_b)) => { @@ -246,11 +256,11 @@ impl Unifier { let rhs_a_value = rhs_a.evaluate_to_field_element(&kind_a, dummy_span); let rhs_b_value = rhs_b.evaluate_to_field_element(&kind_b, dummy_span); if rhs_a_value.is_ok() && rhs_b_value.is_ok() { - return Ok(vec![self.to_unite(lhs_a, lhs_b)]); + return Ok(vec![self.for_unite(lhs_a, lhs_b)]); } } - Ok(vec![self.to_unite(lhs_a, lhs_b), self.to_unite(rhs_a, rhs_b)]) + Ok(vec![self.for_unite(lhs_a, lhs_b), self.for_unite(rhs_a, rhs_b)]) } else { Err(UnificationError) } @@ -259,7 +269,7 @@ impl Unifier { (Constant(value, kind), other) | (other, Constant(value, kind)) => { let dummy_span = Span::default(); if let Ok(other_value) = other.evaluate_to_field_element(kind, dummy_span) { - if *value == other_value && self.kind_unifies_iter(&kind, &other.kind()) { + if *value == other_value && self.kind_unifies_iter(kind, &other.kind()) { Ok(Vec::new()) } else { Err(UnificationError) @@ -272,7 +282,7 @@ impl Unifier { inverse, rhs.clone(), ); - Ok(vec![self.to_unite(&new_type, lhs)]) + Ok(vec![self.for_unite(&new_type, lhs)]) } else { Err(UnificationError) } diff --git a/compiler/noirc_frontend/src/hir_def/mod.rs b/compiler/noirc_frontend/src/hir_def/mod.rs index b0d04ef81fe..cf9f4cf77b7 100644 --- a/compiler/noirc_frontend/src/hir_def/mod.rs +++ b/compiler/noirc_frontend/src/hir_def/mod.rs @@ -17,7 +17,7 @@ //! (monomorphization::ast::Expression). pub mod expr; pub mod function; -mod iterative_unification; +pub mod iterative_unification; pub mod stmt; pub mod traits; pub mod types; diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index f7326f0d56c..a660ff1fada 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -234,7 +234,7 @@ impl Kind { // Kind::Numeric unifies along its Type argument (Kind::Numeric(lhs), Kind::Numeric(rhs)) => { let mut bindings = TypeBindings::new(); - let unifies = lhs.try_unify(rhs, &mut bindings).is_ok(); + let unifies = Unifier::try_unify(lhs, rhs, &mut bindings).is_ok(); if unifies { Type::apply_type_bindings(bindings); } @@ -1647,237 +1647,6 @@ impl Type { }) } - /// `try_unify` is a bit of a misnomer since although errors are not committed, - /// any unified bindings are on success. - pub fn try_unify( - &self, - other: &Type, - bindings: &mut TypeBindings, - ) -> Result<(), UnificationError> { - use Type::*; - - let lhs = match self { - Type::InfixExpr(..) => Cow::Owned(self.canonicalize()), - other => Cow::Borrowed(other), - }; - - let rhs = match other { - Type::InfixExpr(..) => Cow::Owned(other.canonicalize()), - other => Cow::Borrowed(other), - }; - - match (lhs.as_ref(), rhs.as_ref()) { - (Error, _) | (_, Error) => Ok(()), - - (Alias(alias, args), other) | (other, Alias(alias, args)) => { - let alias = alias.borrow().get_type(args); - alias.try_unify(other, bindings) - } - - (TypeVariable(var), other) | (other, TypeVariable(var)) => match &*var.borrow() { - TypeBinding::Bound(typ) => { - if typ.is_numeric_value() { - other.try_unify_to_type_variable(var, bindings, |bindings| { - let only_integer = matches!(typ, Type::Integer(..)); - other.try_bind_to_polymorphic_int(var, bindings, only_integer) - }) - } else { - other.try_unify_to_type_variable(var, bindings, |bindings| { - other.try_bind_to(var, bindings, typ.kind()) - }) - } - } - TypeBinding::Unbound(_id, Kind::IntegerOrField) => other - .try_unify_to_type_variable(var, bindings, |bindings| { - let only_integer = false; - other.try_bind_to_polymorphic_int(var, bindings, only_integer) - }), - TypeBinding::Unbound(_id, 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) - }) - } - TypeBinding::Unbound(_id, type_var_kind) => { - other.try_unify_to_type_variable(var, bindings, |bindings| { - other.try_bind_to(var, bindings, type_var_kind.clone()) - }) - } - }, - - (Array(len_a, elem_a), Array(len_b, elem_b)) => { - len_a.try_unify(len_b, bindings)?; - elem_a.try_unify(elem_b, bindings) - } - - (Slice(elem_a), Slice(elem_b)) => elem_a.try_unify(elem_b, bindings), - - (String(len_a), String(len_b)) => len_a.try_unify(len_b, bindings), - - (FmtString(len_a, elements_a), FmtString(len_b, elements_b)) => { - len_a.try_unify(len_b, bindings)?; - elements_a.try_unify(elements_b, bindings) - } - - (Tuple(elements_a), Tuple(elements_b)) => { - if elements_a.len() != elements_b.len() { - Err(UnificationError) - } else { - for (a, b) in elements_a.iter().zip(elements_b) { - a.try_unify(b, bindings)?; - } - Ok(()) - } - } - - // No recursive try_unify call for struct fields. Don't want - // to mutate shared type variables within struct definitions. - // This isn't possible currently but will be once noir gets generic types - (Struct(id_a, args_a), Struct(id_b, args_b)) => { - if id_a == id_b && args_a.len() == args_b.len() { - for (a, b) in args_a.iter().zip(args_b) { - a.try_unify(b, bindings)?; - } - Ok(()) - } else { - Err(UnificationError) - } - } - - (CheckedCast { to, .. }, other) | (other, CheckedCast { to, .. }) => { - to.try_unify(other, bindings) - } - - (NamedGeneric(binding, _), other) | (other, NamedGeneric(binding, _)) - if !binding.borrow().is_unbound() => - { - if let TypeBinding::Bound(link) = &*binding.borrow() { - link.try_unify(other, bindings) - } else { - unreachable!("If guard ensures binding is bound") - } - } - - (NamedGeneric(binding_a, name_a), NamedGeneric(binding_b, name_b)) => { - // Bound NamedGenerics are caught by the check above - assert!(binding_a.borrow().is_unbound()); - assert!(binding_b.borrow().is_unbound()); - - if name_a == name_b { - binding_a.kind().unify(&binding_b.kind()) - } else { - Err(UnificationError) - } - } - - ( - Function(params_a, ret_a, env_a, unconstrained_a), - Function(params_b, ret_b, env_b, unconstrained_b), - ) => { - if unconstrained_a == unconstrained_b && params_a.len() == params_b.len() { - for (a, b) in params_a.iter().zip(params_b.iter()) { - a.try_unify(b, bindings)?; - } - - env_a.try_unify(env_b, bindings)?; - ret_b.try_unify(ret_a, bindings) - } else { - Err(UnificationError) - } - } - - (MutableReference(elem_a), MutableReference(elem_b)) => { - elem_a.try_unify(elem_b, bindings) - } - - (InfixExpr(lhs_a, op_a, rhs_a), InfixExpr(lhs_b, op_b, rhs_b)) => { - if op_a == op_b { - // We need to preserve the original bindings since if syntactic equality - // fails we fall back to other equality strategies. - let mut new_bindings = bindings.clone(); - let lhs_result = lhs_a.try_unify(lhs_b, &mut new_bindings); - let rhs_result = rhs_a.try_unify(rhs_b, &mut new_bindings); - - if lhs_result.is_ok() && rhs_result.is_ok() { - *bindings = new_bindings; - Ok(()) - } else { - lhs.try_unify_by_moving_constant_terms(&rhs, bindings) - } - } else { - Err(UnificationError) - } - } - - (Constant(value, kind), other) | (other, Constant(value, kind)) => { - let dummy_span = Span::default(); - if let Ok(other_value) = other.evaluate_to_field_element(kind, dummy_span) { - if *value == other_value && kind.unifies(&other.kind()) { - Ok(()) - } else { - Err(UnificationError) - } - } else if let InfixExpr(lhs, op, rhs) = other { - if let Some(inverse) = op.approx_inverse() { - // Handle cases like `4 = a + b` by trying to solve to `a = 4 - b` - let new_type = InfixExpr( - Box::new(Constant(*value, kind.clone())), - inverse, - rhs.clone(), - ); - new_type.try_unify(lhs, bindings)?; - Ok(()) - } else { - Err(UnificationError) - } - } else { - Err(UnificationError) - } - } - - (other_a, other_b) => { - if other_a == other_b { - Ok(()) - } else { - Err(UnificationError) - } - } - } - } - - /// Try to unify a type variable to `self`. - /// This is a helper function factored out from try_unify. - fn try_unify_to_type_variable( - &self, - type_variable: &TypeVariable, - bindings: &mut TypeBindings, - - // Bind the type variable to a type. This is factored out since depending on the - // Kind, there are different methods to check whether the variable can - // bind to the given type or not. - bind_variable: impl FnOnce(&mut TypeBindings) -> Result<(), UnificationError>, - ) -> Result<(), UnificationError> { - match &*type_variable.borrow() { - // If it is already bound, unify against what it is bound to - TypeBinding::Bound(link) => link.try_unify(self, bindings), - TypeBinding::Unbound(id, _) => { - // We may have already "bound" this type variable in this call to - // try_unify, so check those bindings as well. - match bindings.get(id) { - Some((_, kind, binding)) => { - if !kind.unifies(&binding.kind()) { - return Err(UnificationError); - } - binding.clone().try_unify(self, bindings) - } - - // Otherwise, bind it - None => bind_variable(bindings), - } - } - } - } - /// Similar to `unify` but if the check fails this will attempt to coerce the /// argument to the target type. When this happens, the given expression is wrapped in /// a new expression to convert its type. E.g. `array` -> `array.as_slice()` @@ -1894,7 +1663,7 @@ impl Type { ) { let mut bindings = TypeBindings::new(); - if let Ok(()) = self.try_unify(expected, &mut bindings) { + if let Ok(()) = Unifier::try_unify(self, expected, &mut bindings) { Type::apply_type_bindings(bindings); return; } @@ -1959,7 +1728,7 @@ impl Type { // Still have to ensure the element types match. // Don't need to issue an error here if not, it will be done in unify_with_coercions let mut bindings = TypeBindings::new(); - if element1.try_unify(element2, &mut bindings).is_ok() { + if Unifier::try_unify(element1, element2, &mut bindings).is_ok() { convert_array_expression_to_slice(expression, this, target, as_slice, interner); Self::apply_type_bindings(bindings); return true; diff --git a/compiler/noirc_frontend/src/hir_def/types/arithmetic.rs b/compiler/noirc_frontend/src/hir_def/types/arithmetic.rs index 263a102bee1..a1cdccce66c 100644 --- a/compiler/noirc_frontend/src/hir_def/types/arithmetic.rs +++ b/compiler/noirc_frontend/src/hir_def/types/arithmetic.rs @@ -3,7 +3,7 @@ use std::collections::BTreeMap; use acvm::{AcirField, FieldElement}; use noirc_errors::Span; -use crate::{BinaryTypeOperator, Type, TypeBindings, UnificationError}; +use crate::{BinaryTypeOperator, Type}; impl Type { /// Try to canonicalize the representation of this type. @@ -308,48 +308,4 @@ impl Type { _ => None, } } - - /// Try to unify equations like `(..) + 3 = (..) + 1` - /// by transforming them to `(..) + 2 = (..)` - pub(super) fn try_unify_by_moving_constant_terms( - &self, - other: &Type, - bindings: &mut TypeBindings, - ) -> Result<(), UnificationError> { - if let Type::InfixExpr(lhs_a, op_a, rhs_a) = self { - if let Some(inverse) = op_a.approx_inverse() { - let kind = lhs_a.infix_kind(rhs_a); - let dummy_span = Span::default(); - if let Ok(rhs_a_value) = rhs_a.evaluate_to_field_element(&kind, dummy_span) { - let rhs_a = Box::new(Type::Constant(rhs_a_value, kind)); - let new_other = Type::InfixExpr(Box::new(other.clone()), inverse, rhs_a); - - let mut tmp_bindings = bindings.clone(); - if lhs_a.try_unify(&new_other, &mut tmp_bindings).is_ok() { - *bindings = tmp_bindings; - return Ok(()); - } - } - } - } - - if let Type::InfixExpr(lhs_b, op_b, rhs_b) = other { - if let Some(inverse) = op_b.approx_inverse() { - let kind = lhs_b.infix_kind(rhs_b); - let dummy_span = Span::default(); - if let Ok(rhs_b_value) = rhs_b.evaluate_to_field_element(&kind, dummy_span) { - let rhs_b = Box::new(Type::Constant(rhs_b_value, kind)); - let new_self = Type::InfixExpr(Box::new(self.clone()), inverse, rhs_b); - - let mut tmp_bindings = bindings.clone(); - if new_self.try_unify(lhs_b, &mut tmp_bindings).is_ok() { - *bindings = tmp_bindings; - return Ok(()); - } - } - } - } - - Err(UnificationError) - } } diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index ce2c58e71c1..b15c140cad1 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -11,6 +11,7 @@ use crate::ast::{FunctionKind, IntegerBitSize, Signedness, UnaryOp, Visibility}; use crate::hir::comptime::InterpreterError; use crate::hir::type_check::{NoMatchingImplFoundError, TypeCheckError}; +use crate::hir_def::iterative_unification::Unifier; use crate::node_interner::{ExprId, ImplSearchErrorKind}; use crate::{ debug::DebugInstrumenter, @@ -957,6 +958,59 @@ impl<'interner> Monomorphizer<'interner> { Ok(ident) } + /// Iterative version of convert_type, for a subset of types + /// It defaults to the recursive version for the other types + fn convert_type_iter( + typ: &HirType, + location: Location, + ) -> Result { + let mut queue = Vec::new(); + queue.push(typ.clone()); + let mut result = Err(MonomorphizationError::InternalError { + message: "ICE - Could not convert type variable", + location, + }); + while let Some(typ) = queue.pop() { + match typ { + HirType::TypeVariable(ref binding) => { + match &*binding.borrow() { + TypeBinding::Bound(ref binding) => { + queue.push(binding.clone()); + } + TypeBinding::Unbound(_, ref type_var_kind) => { + // 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 = match type_var_kind.default_type() { + Some(typ) => typ, + None => { + return Err(MonomorphizationError::NoDefaultType { location }) + } + }; + queue.push(default.clone()); + binding.bind(default.clone()); + } + } + } + HirType::NamedGeneric(binding, _) => { + if let TypeBinding::Bound(ref binding) = &*binding.borrow() { + queue.push(binding.clone()); + } else { + // Default any remaining unbound type variables. + // This should only happen if the variable in question is unused + // and within a larger generic type. + binding.bind(HirType::default_int_or_field_type()); + result = Ok(ast::Type::Field); + } + } + _ => { + result = Monomorphizer::convert_type(&typ, location); + } + } + } + result + } + /// Convert a non-tuple/struct type to a monomorphized type fn convert_type(typ: &HirType, location: Location) -> Result { Ok(match typ { @@ -1019,43 +1073,14 @@ impl<'interner> Monomorphizer<'interner> { HirType::TraitAsType(..) => { unreachable!("All TraitAsType should be replaced before calling convert_type"); } - HirType::NamedGeneric(binding, _) => { - if let TypeBinding::Bound(ref binding) = &*binding.borrow() { - return Self::convert_type(binding, location); - } - - // Default any remaining unbound type variables. - // This should only happen if the variable in question is unused - // and within a larger generic type. - binding.bind(HirType::default_int_or_field_type()); - ast::Type::Field - } + HirType::NamedGeneric(_, _) => Self::convert_type_iter(typ, location)?, HirType::CheckedCast { from, to } => { Self::check_checked_cast(from, to, location)?; Self::convert_type(to, location)? } - HirType::TypeVariable(ref binding) => { - let type_var_kind = match &*binding.borrow() { - TypeBinding::Bound(ref binding) => { - return Self::convert_type(binding, location); - } - TypeBinding::Unbound(_, ref type_var_kind) => type_var_kind.clone(), - }; - - // 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 = match type_var_kind.default_type() { - Some(typ) => typ, - None => return Err(MonomorphizationError::NoDefaultType { location }), - }; - - let monomorphized_default = Self::convert_type(&default, location)?; - binding.bind(default); - monomorphized_default - } + HirType::TypeVariable(_) => Self::convert_type_iter(typ, location)?, HirType::Struct(def, args) => { // Not all generic arguments may be used in a struct's fields so we have to check @@ -1063,7 +1088,6 @@ impl<'interner> Monomorphizer<'interner> { for arg in args { Self::check_type(arg, location)?; } - let fields = def.borrow().get_fields(args); let fields = try_vecmap(fields, |(_, field)| Self::convert_type(&field, location))?; ast::Type::Tuple(fields) @@ -1075,7 +1099,6 @@ impl<'interner> Monomorphizer<'interner> { for arg in args { Self::check_type(arg, location)?; } - Self::convert_type(&def.borrow().get_type(args), location)? } @@ -1123,6 +1146,45 @@ impl<'interner> Monomorphizer<'interner> { }) } + /// Iterative version of check_type dedicated for TypeVariable + fn check_type_iter(typ: &HirType, location: Location) -> Result<(), MonomorphizationError> { + let mut queue = Vec::new(); + + queue.push(typ.clone()); + let mut result = Err(MonomorphizationError::InternalError { + message: "Unexpected Type::Error found during monomorphization", + location, + }); + while let Some(typ) = queue.pop() { + match typ { + HirType::TypeVariable(ref binding) => { + match &*binding.borrow() { + TypeBinding::Bound(binding) => { + queue.push(binding.clone()); + } + TypeBinding::Unbound(_, ref type_var_kind) => { + // 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 = match type_var_kind.default_type() { + Some(typ) => typ, + None => { + return Err(MonomorphizationError::NoDefaultType { location }) + } + }; + queue.push(default); + } + } + } + _ => { + // Fall back to the recursive version. + result = Self::check_type(&typ, location); + } + } + } + result + } + // Similar to `convert_type` but returns an error if any type variable can't be defaulted. fn check_type(typ: &HirType, location: Location) -> Result<(), MonomorphizationError> { match typ { @@ -1158,24 +1220,7 @@ impl<'interner> Monomorphizer<'interner> { Ok(()) } - HirType::TypeVariable(ref binding) => { - let type_var_kind = match &*binding.borrow() { - TypeBinding::Bound(binding) => { - return Self::check_type(binding, location); - } - TypeBinding::Unbound(_, ref type_var_kind) => type_var_kind.clone(), - }; - - // 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 = match type_var_kind.default_type() { - Some(typ) => typ, - None => return Err(MonomorphizationError::NoDefaultType { location }), - }; - - Self::check_type(&default, location) - } + HirType::TypeVariable(_) => Self::check_type_iter(typ, location), HirType::Struct(_def, args) => { for arg in args { @@ -2075,8 +2120,7 @@ pub fn perform_impl_bindings( // with the same internal id, binding. trait_method_type.replace_named_generics_with_type_variables(); impl_method_type.replace_named_generics_with_type_variables(); - - trait_method_type.try_unify(&impl_method_type, &mut bindings).map_err(|_| { + Unifier::try_unify(&trait_method_type, &impl_method_type, &mut bindings).map_err(|_| { InterpreterError::ImplMethodTypeMismatch { expected: trait_method_type.follow_bindings(), actual: impl_method_type.follow_bindings(), diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 736d37fe83f..089a18863c8 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -23,6 +23,7 @@ use crate::hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait, Unr use crate::hir::def_map::DefMaps; use crate::hir::def_map::{LocalModuleId, ModuleDefId, ModuleId}; use crate::hir::type_check::generics::TraitGenerics; +use crate::hir_def::iterative_unification::Unifier; use crate::hir_def::traits::NamedType; use crate::hir_def::traits::ResolvedTraitBound; use crate::QuotedType; @@ -1569,12 +1570,18 @@ impl NodeInterner { |impl_generics: &[Type], impl_associated_types: &[NamedType]| { trait_generics.iter().zip(impl_generics).all(|(trait_generic, impl_generic)| { let impl_generic = impl_generic.force_substitute(&instantiation_bindings); - trait_generic.try_unify(&impl_generic, &mut fresh_bindings).is_ok() + Unifier::try_unify(trait_generic, &impl_generic, &mut fresh_bindings) + .is_ok() }) && trait_associated_types.iter().zip(impl_associated_types).all( |(trait_generic, impl_generic)| { let impl_generic2 = impl_generic.typ.force_substitute(&instantiation_bindings); - trait_generic.typ.try_unify(&impl_generic2, &mut fresh_bindings).is_ok() + Unifier::try_unify( + &trait_generic.typ, + &impl_generic2, + &mut fresh_bindings, + ) + .is_ok() }, ) }; @@ -1593,8 +1600,8 @@ impl NodeInterner { if !check_trait_generics(&trait_generics.ordered, &trait_generics.named) { continue; } - - if object_type.try_unify(&existing_object_type, &mut fresh_bindings).is_ok() { + if Unifier::try_unify(&object_type, &existing_object_type, &mut fresh_bindings).is_ok() + { if let TraitImplKind::Normal(impl_id) = impl_kind { let trait_impl = self.get_trait_implementation(*impl_id); let trait_impl = trait_impl.borrow(); From ab55466120197387b259efceea5bb730d9ec98c4 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 19 Nov 2024 10:01:40 +0000 Subject: [PATCH 3/7] remove old file that should not have been commit. --- .../noirc_evaluator/src/ssa/opt/databus.rs | 46 ------------------- 1 file changed, 46 deletions(-) delete mode 100644 compiler/noirc_evaluator/src/ssa/opt/databus.rs diff --git a/compiler/noirc_evaluator/src/ssa/opt/databus.rs b/compiler/noirc_evaluator/src/ssa/opt/databus.rs deleted file mode 100644 index 4cfcd0b9156..00000000000 --- a/compiler/noirc_evaluator/src/ssa/opt/databus.rs +++ /dev/null @@ -1,46 +0,0 @@ -use crate::ssa::ir::instruction::Instruction; - - - -impl Ssa { - /// Map arrays with the last instruction that uses it - /// For this we simply process all the instructions in execution order - /// and update the map whenever there is a match - pub(crate) fn map_to_call_data_array(&self) -> HashMap { - let mut array_use = HashMap::default(); - for func in self.functions.values() { - let mut reverse_post_order = PostOrder::with_function(func).into_vec(); - reverse_post_order.reverse(); - for block in reverse_post_order { - last_use(block, &func.dfg, &mut array_use); - } - } - array_use - } -//la question c comment on remplace ? -//on pourrait simplement garder une liste instruction_id -> new instruction -// et on les remplace dans la liste des instructions instructions_mut() - pub(crate) fn array_me(block_id: BasicBlockId, dfg: &mut DataFlowGraph) { - let block = &dfg[block_id]; - for instruction_id in block.instructions() { - match &dfg[*instruction_id] { - Instruction::ArrayGet { array, index } => { - // Get operations to call-data parameters are replaced by a get to the call-data-bus array - if let Some(call_data) = dfg.data_bus.call_data { - let array_id = dfg.resolve(*array); - if dfg.data_bus.call_data_map.contains_key(&array_id) { - // on doit remplacer ce get par un autre, mais on doit d'abord fiare un calcul - // - dfg.make_constant(FieldElement::from( - self.data_bus.call_data_map[&array_id] as i128, - ), type_of_value(index)); - let new_index = self.acir_context.add_var(index, bus_index)?; //TODO add instruction - Instruction::ArrayGet { array: call_data, index: new_index };; //on veut remplacer par ca - } - } - } - - } - } -} -} From 8303eb01ab1c9157df247a5f53679177dc63ba64 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 19 Nov 2024 10:37:09 +0000 Subject: [PATCH 4/7] code review --- compiler/noirc_frontend/src/monomorphization/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index b15c140cad1..81797ec4865 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -960,6 +960,8 @@ impl<'interner> Monomorphizer<'interner> { /// Iterative version of convert_type, for a subset of types /// It defaults to the recursive version for the other types + /// Because a program can potentially have extremely long chains of type variables that are bind to other types variables, + /// recursively handling these types can lead to stack overflows, thus we do it iteratively. fn convert_type_iter( typ: &HirType, location: Location, From 2b9e976d8f643661c97032af35e59e8da86e6dd4 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 19 Nov 2024 15:00:05 +0000 Subject: [PATCH 5/7] adding back the previous handling for constant terms --- .../src/hir_def/iterative_unification.rs | 25 +++++----- .../src/hir_def/types/arithmetic.rs | 47 +++++++++++++++++++ 2 files changed, 59 insertions(+), 13 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/iterative_unification.rs b/compiler/noirc_frontend/src/hir_def/iterative_unification.rs index 0ed9984f771..8adc867ebe8 100644 --- a/compiler/noirc_frontend/src/hir_def/iterative_unification.rs +++ b/compiler/noirc_frontend/src/hir_def/iterative_unification.rs @@ -248,24 +248,23 @@ impl Unifier { (InfixExpr(lhs_a, op_a, rhs_a), InfixExpr(lhs_b, op_b, rhs_b)) => { if op_a == op_b { - // Try to simplify if both expressions have constant terms - if op_a.approx_inverse().is_some() { - let kind_a = lhs_a.infix_kind(rhs_a); - let kind_b = lhs_a.infix_kind(rhs_b); - let dummy_span = Span::default(); - let rhs_a_value = rhs_a.evaluate_to_field_element(&kind_a, dummy_span); - let rhs_b_value = rhs_b.evaluate_to_field_element(&kind_b, dummy_span); - if rhs_a_value.is_ok() && rhs_b_value.is_ok() { - return Ok(vec![self.for_unite(lhs_a, lhs_b)]); - } + // We need to preserve the original bindings since if syntactic equality + // fails we fall back to other equality strategies. + let mut new_bindings = bindings.clone(); + let lhs_result = Unifier::try_unify(lhs_a, lhs_b, &mut new_bindings); + let rhs_result = Unifier::try_unify(rhs_a, rhs_b, &mut new_bindings); + + if lhs_result.is_ok() && rhs_result.is_ok() { + *bindings = new_bindings; + Ok(Vec::new()) + } else { + lhs.try_unify_by_moving_constant_terms(&rhs, bindings)?; + Ok(Vec::new()) } - - Ok(vec![self.for_unite(lhs_a, lhs_b), self.for_unite(rhs_a, rhs_b)]) } else { Err(UnificationError) } } - (Constant(value, kind), other) | (other, Constant(value, kind)) => { let dummy_span = Span::default(); if let Ok(other_value) = other.evaluate_to_field_element(kind, dummy_span) { diff --git a/compiler/noirc_frontend/src/hir_def/types/arithmetic.rs b/compiler/noirc_frontend/src/hir_def/types/arithmetic.rs index f5a39125d84..84c4213406e 100644 --- a/compiler/noirc_frontend/src/hir_def/types/arithmetic.rs +++ b/compiler/noirc_frontend/src/hir_def/types/arithmetic.rs @@ -3,6 +3,7 @@ use std::collections::BTreeMap; use acvm::{AcirField, FieldElement}; use noirc_errors::Span; +use super::{TypeBindings, UnificationError, Unifier}; use crate::{BinaryTypeOperator, Type}; impl Type { @@ -323,6 +324,52 @@ impl Type { _ => None, } } + + /// Try to unify equations like `(..) + 3 = (..) + 1` + /// by transforming them to `(..) + 2 = (..)` + pub fn try_unify_by_moving_constant_terms( + &self, + other: &Type, + bindings: &mut TypeBindings, + ) -> Result<(), UnificationError> { + if let Type::InfixExpr(lhs_a, op_a, rhs_a) = self { + if let Some(inverse) = op_a.approx_inverse() { + let kind = lhs_a.infix_kind(rhs_a); + let dummy_span = Span::default(); + if let Ok(rhs_a_value) = rhs_a.evaluate_to_field_element(&kind, dummy_span) { + let rhs_a = Box::new(Type::Constant(rhs_a_value, kind)); + let new_other = Type::InfixExpr(Box::new(other.clone()), inverse, rhs_a); + + let mut tmp_bindings = bindings.clone(); + if Unifier::try_unify(lhs_a, &new_other, &mut tmp_bindings).is_ok() { + //if lhs_a.try_unify(&new_other, &mut tmp_bindings).is_ok() { + *bindings = tmp_bindings; + return Ok(()); + } + } + } + } + + if let Type::InfixExpr(lhs_b, op_b, rhs_b) = other { + if let Some(inverse) = op_b.approx_inverse() { + let kind = lhs_b.infix_kind(rhs_b); + let dummy_span = Span::default(); + if let Ok(rhs_b_value) = rhs_b.evaluate_to_field_element(&kind, dummy_span) { + let rhs_b = Box::new(Type::Constant(rhs_b_value, kind)); + let new_self = Type::InfixExpr(Box::new(self.clone()), inverse, rhs_b); + + let mut tmp_bindings = bindings.clone(); + if Unifier::try_unify(&new_self, lhs_b, &mut tmp_bindings).is_ok() { + // if new_self.try_unify(lhs_b, &mut tmp_bindings).is_ok() { + *bindings = tmp_bindings; + return Ok(()); + } + } + } + } + + Err(UnificationError) + } } #[cfg(test)] From 177608f80fd1603581cde154085fa9a677bf7f7a Mon Sep 17 00:00:00 2001 From: guipublic Date: Wed, 20 Nov 2024 10:00:19 +0000 Subject: [PATCH 6/7] code review --- compiler/noirc_frontend/src/hir_def/iterative_unification.rs | 3 --- compiler/noirc_frontend/src/hir_def/types/arithmetic.rs | 2 -- 2 files changed, 5 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/iterative_unification.rs b/compiler/noirc_frontend/src/hir_def/iterative_unification.rs index 8adc867ebe8..aa31a389b34 100644 --- a/compiler/noirc_frontend/src/hir_def/iterative_unification.rs +++ b/compiler/noirc_frontend/src/hir_def/iterative_unification.rs @@ -183,9 +183,6 @@ impl Unifier { } } - // No recursive try_unify call for struct fields. Don't want - // to mutate shared type variables within struct definitions. - // This isn't possible currently but will be once noir gets generic types (Struct(id_a, args_a), Struct(id_b, args_b)) => { if id_a == id_b && args_a.len() == args_b.len() { let mut to_unit = Vec::new(); diff --git a/compiler/noirc_frontend/src/hir_def/types/arithmetic.rs b/compiler/noirc_frontend/src/hir_def/types/arithmetic.rs index 84c4213406e..a5c9cabf3f9 100644 --- a/compiler/noirc_frontend/src/hir_def/types/arithmetic.rs +++ b/compiler/noirc_frontend/src/hir_def/types/arithmetic.rs @@ -342,7 +342,6 @@ impl Type { let mut tmp_bindings = bindings.clone(); if Unifier::try_unify(lhs_a, &new_other, &mut tmp_bindings).is_ok() { - //if lhs_a.try_unify(&new_other, &mut tmp_bindings).is_ok() { *bindings = tmp_bindings; return Ok(()); } @@ -360,7 +359,6 @@ impl Type { let mut tmp_bindings = bindings.clone(); if Unifier::try_unify(&new_self, lhs_b, &mut tmp_bindings).is_ok() { - // if new_self.try_unify(lhs_b, &mut tmp_bindings).is_ok() { *bindings = tmp_bindings; return Ok(()); } From 2dcc13fcbe82496017aa9a6fa4ea3ca448997c6b Mon Sep 17 00:00:00 2001 From: guipublic Date: Wed, 20 Nov 2024 10:36:32 +0000 Subject: [PATCH 7/7] fix merge issue --- compiler/noirc_frontend/src/monomorphization/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 3926dd20df5..ce496e028a0 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -1070,14 +1070,14 @@ impl<'interner> Monomorphizer<'interner> { HirType::TraitAsType(..) => { unreachable!("All TraitAsType should be replaced before calling convert_type"); } - HirType::NamedGeneric(_, _) => Self::convert_type_iter(typ, location)?, + HirType::NamedGeneric(_, _) => Self::convert_type_iter(&typ, location)?, HirType::CheckedCast { from, to } => { Self::check_checked_cast(from, to, location)?; Self::convert_type(to, location)? } - HirType::TypeVariable(_) => Self::convert_type_iter(typ, location)?, + HirType::TypeVariable(_) => Self::convert_type_iter(&typ, location)?, HirType::Struct(def, args) => { // Not all generic arguments may be used in a struct's fields so we have to check @@ -1218,7 +1218,7 @@ impl<'interner> Monomorphizer<'interner> { Ok(()) } - HirType::TypeVariable(_) => Self::check_type_iter(typ, location), + HirType::TypeVariable(_) => Self::check_type_iter(&typ, location), HirType::Struct(_def, args) => { for arg in args {