From d1cd0ecc01484b9739338ef54b8337675326ea72 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 9 Feb 2024 13:40:48 -0600 Subject: [PATCH 1/7] Display an error for cyclic aliases --- .../src/hir/resolution/resolver.rs | 23 +++++++++++-------- .../src/hir/resolution/type_aliases.rs | 6 ++--- compiler/noirc_frontend/src/hir_def/types.rs | 23 +++++++++++-------- compiler/noirc_frontend/src/node_interner.rs | 17 +++++++++----- 4 files changed, 41 insertions(+), 28 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index cf1018a4927..e7a132541bc 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -29,7 +29,7 @@ use crate::hir::def_map::{LocalModuleId, ModuleDefId, TryFromModuleDefId, MAIN_F use crate::hir_def::stmt::{HirAssignStatement, HirForStatement, HirLValue, HirPattern}; use crate::node_interner::{ DefinitionId, DefinitionKind, DependencyId, ExprId, FuncId, GlobalId, NodeInterner, StmtId, - StructId, TraitId, TraitImplId, TraitMethodId, + StructId, TraitId, TraitImplId, TraitMethodId, TypeAliasId, }; use crate::{ hir::{def_map::CrateDefMap, resolution::path_resolver::PathResolver}, @@ -39,9 +39,9 @@ use crate::{ use crate::{ ArrayLiteral, ContractFunctionType, Distinctness, ForRange, FunctionDefinition, FunctionReturnType, FunctionVisibility, Generics, LValue, NoirStruct, NoirTypeAlias, Param, - Path, PathKind, Pattern, Shared, StructType, Type, TypeAliasType, TypeVariable, - TypeVariableKind, UnaryOp, UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType, - UnresolvedTypeData, UnresolvedTypeExpression, Visibility, ERROR_IDENT, + Path, PathKind, Pattern, Shared, StructType, Type, TypeAlias, TypeVariable, TypeVariableKind, + UnaryOp, UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData, + UnresolvedTypeExpression, Visibility, ERROR_IDENT, }; use fm::FileId; use iter_extended::vecmap; @@ -582,7 +582,9 @@ impl<'a> Resolver<'a> { type_alias_string }); - let result = self.interner.get_type_alias(id).get_type(&args); + if let Some(item) = self.current_item { + self.interner.add_type_alias_dependency(item, id); + } // Collecting Type Alias references [Location]s to be used by LSP in order // to resolve the definition of the type alias @@ -593,9 +595,7 @@ impl<'a> Resolver<'a> { // equal to another type alias. Fixing this fully requires an analysis to create a DFG // of definition ordering, but for now we have an explicit check here so that we at // least issue an error that the type was not found instead of silently passing. - if result != Type::Error { - return result; - } + return self.interner.get_type_alias(id).get_type(&args); } match self.lookup_struct_or_error(path) { @@ -752,12 +752,15 @@ impl<'a> Resolver<'a> { resolved_type } - pub fn resolve_type_aliases( + pub fn resolve_type_alias( mut self, unresolved: NoirTypeAlias, + alias_id: TypeAliasId, ) -> (Type, Generics, Vec) { let generics = self.add_generics(&unresolved.generics); self.resolve_local_globals(); + + self.current_item = Some(DependencyId::Alias(alias_id)); let typ = self.resolve_type(unresolved.typ); (typ, generics, self.errors) @@ -1791,7 +1794,7 @@ impl<'a> Resolver<'a> { } } - fn lookup_type_alias(&mut self, path: Path) -> Option<&TypeAliasType> { + fn lookup_type_alias(&mut self, path: Path) -> Option<&TypeAlias> { self.lookup(path).ok().map(|id| self.interner.get_type_alias(id)) } diff --git a/compiler/noirc_frontend/src/hir/resolution/type_aliases.rs b/compiler/noirc_frontend/src/hir/resolution/type_aliases.rs index f66f6c8dfa7..2e5ce611a7f 100644 --- a/compiler/noirc_frontend/src/hir/resolution/type_aliases.rs +++ b/compiler/noirc_frontend/src/hir/resolution/type_aliases.rs @@ -17,7 +17,7 @@ pub(crate) fn resolve_type_aliases( crate_id: CrateId, ) -> Vec<(CompilationError, FileId)> { let mut errors: Vec<(CompilationError, FileId)> = vec![]; - for (type_id, unresolved_typ) in type_aliases { + for (alias_id, unresolved_typ) in type_aliases { let path_resolver = StandardPathResolver::new(ModuleId { local_id: unresolved_typ.module_id, krate: crate_id, @@ -25,9 +25,9 @@ pub(crate) fn resolve_type_aliases( let file = unresolved_typ.file_id; let (typ, generics, resolver_errors) = Resolver::new(&mut context.def_interner, &path_resolver, &context.def_maps, file) - .resolve_type_aliases(unresolved_typ.type_alias_def); + .resolve_type_alias(unresolved_typ.type_alias_def, alias_id); errors.extend(resolver_errors.iter().cloned().map(|e| (e.into(), file))); - context.def_interner.set_type_alias(type_id, typ, generics); + context.def_interner.set_type_alias(alias_id, typ, generics); } errors } diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 00e24de279b..de19dcd94bd 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -44,13 +44,18 @@ pub enum Type { /// The unit type `()`. Unit, + /// A tuple type with the given list of fields in the order they appear in source code. + Tuple(Vec), + /// A user-defined struct type. The `Shared` field here refers to /// the shared definition for each instance of this struct type. The `Vec` /// represents the generic arguments (if any) to this struct type. Struct(Shared, Vec), - /// A tuple type with the given list of fields in the order they appear in source code. - Tuple(Vec), + /// A user-defined alias to another type. Similar to a Struct, this carries a shared + /// reference to the definition of the alias along with any generics that may have + /// been applied to the alias. + Alias(Shared, Vec), /// TypeVariables are stand-in variables for some type which is not yet known. /// They are not to be confused with NamedGenerics. While the later mostly works @@ -309,7 +314,7 @@ impl std::fmt::Display for StructType { /// Wrap around an unsolved type #[derive(Debug, Clone, Eq)] -pub struct TypeAliasType { +pub struct TypeAlias { pub name: Ident, pub id: TypeAliasId, pub typ: Type, @@ -317,19 +322,19 @@ pub struct TypeAliasType { pub location: Location, } -impl std::hash::Hash for TypeAliasType { +impl std::hash::Hash for TypeAlias { fn hash(&self, state: &mut H) { self.id.hash(state); } } -impl PartialEq for TypeAliasType { +impl PartialEq for TypeAlias { fn eq(&self, other: &Self) -> bool { self.id == other.id } } -impl std::fmt::Display for TypeAliasType { +impl std::fmt::Display for TypeAlias { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{}", self.name)?; @@ -342,15 +347,15 @@ impl std::fmt::Display for TypeAliasType { } } -impl TypeAliasType { +impl TypeAlias { pub fn new( id: TypeAliasId, name: Ident, location: Location, typ: Type, generics: Generics, - ) -> TypeAliasType { - TypeAliasType { id, typ, name, location, generics } + ) -> TypeAlias { + TypeAlias { id, typ, name, location, generics } } pub fn set_type_and_generics(&mut self, new_typ: Type, new_generics: Generics) { diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 0051c1b4f5f..67b80053856 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -29,7 +29,7 @@ use crate::hir_def::{ use crate::token::{Attributes, SecondaryAttribute}; use crate::{ BinaryOpKind, ContractFunctionType, FunctionDefinition, FunctionVisibility, Generics, Shared, - TypeAliasType, TypeBindings, TypeVariable, TypeVariableId, TypeVariableKind, + TypeAlias, TypeBindings, TypeVariable, TypeVariableId, TypeVariableKind, }; /// An arbitrary number to limit the recursion depth when searching for trait impls. @@ -90,11 +90,12 @@ pub struct NodeInterner { structs: HashMap>, struct_attributes: HashMap, - // Type Aliases map. + + // Maps TypeAliasId -> Shared // // Map type aliases to the actual type. // When resolving types, check against this map to see if a type alias is defined. - pub(crate) type_aliases: Vec, + pub(crate) type_aliases: Vec>, // Trait map. // @@ -604,7 +605,7 @@ impl NodeInterner { pub fn push_type_alias(&mut self, typ: &UnresolvedTypeAlias) -> TypeAliasId { let type_id = TypeAliasId(self.type_aliases.len()); - self.type_aliases.push(TypeAliasType::new( + self.type_aliases.push(TypeAlias::new( type_id, typ.type_alias_def.name.clone(), Location::new(typ.type_alias_def.span, typ.file_id), @@ -957,8 +958,8 @@ impl NodeInterner { self.traits.get(&id) } - pub fn get_type_alias(&self, id: TypeAliasId) -> &TypeAliasType { - &self.type_aliases[id.0] + pub fn get_type_alias(&self, id: TypeAliasId) -> Shared { + self.type_aliases[id.0].clone() } pub fn get_global(&self, global_id: GlobalId) -> &GlobalInfo { @@ -1539,6 +1540,10 @@ impl NodeInterner { self.add_dependency(dependent, DependencyId::Function(dependency)); } + pub fn add_type_alias_dependency(&mut self, dependent: DependencyId, dependency: TypeAliasId) { + self.add_dependency(dependent, DependencyId::Alias(dependency)); + } + fn add_dependency(&mut self, dependent: DependencyId, dependency: DependencyId) { let dependent_index = self.get_or_insert_dependency(dependent); let dependency_index = self.get_or_insert_dependency(dependency); From 182b478ecec79091c8270b5d0d9d8558c4d9219a Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 12 Feb 2024 15:41:30 -0600 Subject: [PATCH 2/7] Add Type::Alias --- .../src/hir/resolution/resolver.rs | 25 ++++-- .../noirc_frontend/src/hir/type_check/expr.rs | 8 ++ compiler/noirc_frontend/src/hir_def/types.rs | 79 ++++++++++++++++--- .../src/monomorphization/mod.rs | 2 + compiler/noirc_frontend/src/node_interner.rs | 10 ++- .../noirc_frontend/src/resolve_locations.rs | 4 +- tooling/noirc_abi/src/lib.rs | 3 +- 7 files changed, 106 insertions(+), 25 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index e7a132541bc..186cbecbc11 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -573,10 +573,11 @@ impl<'a> Resolver<'a> { let span = path.span(); let mut args = vecmap(args, |arg| self.resolve_type_inner(arg, new_variables)); - if let Some(type_alias_type) = self.lookup_type_alias(path.clone()) { - let expected_generic_count = type_alias_type.generics.len(); - let type_alias_string = type_alias_type.to_string(); - let id = type_alias_type.id; + if let Some(type_alias) = self.lookup_type_alias(path.clone()) { + let type_alias = type_alias.borrow(); + let expected_generic_count = type_alias.generics.len(); + let type_alias_string = type_alias.to_string(); + let id = type_alias.id; self.verify_generics_count(expected_generic_count, &mut args, span, || { type_alias_string @@ -595,7 +596,8 @@ impl<'a> Resolver<'a> { // equal to another type alias. Fixing this fully requires an analysis to create a DFG // of definition ordering, but for now we have an explicit check here so that we at // least issue an error that the type was not found instead of silently passing. - return self.interner.get_type_alias(id).get_type(&args); + let alias = self.interner.get_type_alias(id); + return Type::Alias(alias, args); } match self.lookup_struct_or_error(path) { @@ -1123,6 +1125,17 @@ impl<'a> Resolver<'a> { } } } + Type::Alias(alias, generics) => { + for (i, generic) in generics.iter().enumerate() { + if let Type::NamedGeneric(type_variable, name) = generic { + if alias.borrow().generic_is_numeric(i) { + found.insert(name.to_string(), type_variable.clone()); + } + } else { + Self::find_numeric_generics_in_type(generic, found); + } + } + } Type::MutableReference(element) => Self::find_numeric_generics_in_type(element, found), Type::String(length) => { if let Type::NamedGeneric(type_variable, name) = length.as_ref() { @@ -1794,7 +1807,7 @@ impl<'a> Resolver<'a> { } } - fn lookup_type_alias(&mut self, path: Path) -> Option<&TypeAlias> { + fn lookup_type_alias(&mut self, path: Path) -> Option> { self.lookup(path).ok().map(|id| self.interner.get_type_alias(id)) } diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 998abeedcec..95329908533 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -839,6 +839,10 @@ impl<'interner> TypeChecker<'interner> { }) } } + (Alias(alias, args), other) | (other, Alias(alias, args)) => { + let alias = alias.borrow().get_type(args); + self.comparator_operand_type_rules(&alias, other, op, span) + } (Integer(sign_x, bit_width_x), Integer(sign_y, bit_width_y)) => { if sign_x != sign_y { return Err(TypeCheckError::IntegerSignedness { @@ -1130,6 +1134,10 @@ impl<'interner> TypeChecker<'interner> { }) } } + (Alias(alias, args), other) | (other, Alias(alias, args)) => { + let alias = alias.borrow().get_type(args); + self.infix_operand_type_rules(&alias, op, other, span) + } (Integer(sign_x, bit_width_x), Integer(sign_y, bit_width_y)) => { if sign_x != sign_y { return Err(TypeCheckError::IntegerSignedness { diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index de19dcd94bd..3b93be1d141 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -121,11 +121,16 @@ impl Type { let typ = typ.as_ref(); (length as u32) * typ.field_count() } - Type::Struct(ref def, args) => { + Type::Struct(def, args) => { let struct_type = def.borrow(); let fields = struct_type.get_fields(args); fields.iter().fold(0, |acc, (_, field_type)| acc + field_type.field_count()) } + Type::Alias(def, _) => { + // It is safe to access `typ` without instantiating generics here since generics + // cannot change the number of fields in `typ`. + def.borrow().typ.field_count() + } Type::Tuple(fields) => { fields.iter().fold(0, |acc, field_typ| acc + field_typ.field_count()) } @@ -336,14 +341,7 @@ impl PartialEq for TypeAlias { impl std::fmt::Display for TypeAlias { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.name)?; - - if !self.generics.is_empty() { - let generics = vecmap(&self.generics, |binding| binding.borrow().to_string()); - write!(f, "{}", generics.join(", "))?; - } - - Ok(()) + write!(f, "{}", self.name) } } @@ -376,6 +374,14 @@ impl TypeAlias { self.typ.substitute(&substitutions) } + + /// True if the given index is the same index as a generic type of this alias + /// which is expected to be a numeric generic. + /// This is needed because we infer type kinds in Noir and don't have extensive kind checking. + pub fn generic_is_numeric(&self, index_of_generic: usize) -> bool { + let target_id = self.generics[index_of_generic].0; + self.typ.contains_numeric_typevar(target_id) + } } /// A shared, mutable reference to some T. @@ -642,6 +648,13 @@ impl Type { } }) } + Type::Alias(alias, generics) => generics.iter().enumerate().any(|(i, generic)| { + if named_generic_id_matches_target(generic) { + alias.borrow().generic_is_numeric(i) + } else { + generic.contains_numeric_typevar(target_id) + } + }), Type::MutableReference(element) => element.contains_numeric_typevar(target_id), Type::String(length) => named_generic_id_matches_target(length), Type::FmtString(length, elements) => { @@ -678,6 +691,11 @@ impl Type { | Type::TraitAsType(..) | Type::NotConstant => false, + // This function is called during name resolution before we've verified aliases + // are not cyclic. As a result, it wouldn't be safe to check this alias' definition + // to see if the aliased type is valid. + Type::Alias(..) => false, + Type::Array(length, element) => { length.is_valid_for_program_input() && element.is_valid_for_program_input() } @@ -777,6 +795,14 @@ impl std::fmt::Display for Type { write!(f, "{}<{}>", s.borrow(), args.join(", ")) } } + Type::Alias(alias, args) => { + let args = vecmap(args, |arg| arg.to_string()); + if args.is_empty() { + write!(f, "{}", alias.borrow()) + } else { + write!(f, "{}<{}>", alias.borrow(), args.join(", ")) + } + } Type::TraitAsType(_id, name, generics) => { write!(f, "impl {}", name)?; if !generics.is_empty() { @@ -1071,6 +1097,11 @@ impl Type { other.try_bind_to_maybe_constant(var, *length, bindings) }), + (Alias(alias, args), other) | (other, Alias(alias, args)) => { + let alias = alias.borrow().get_type(args); + alias.try_unify(other, bindings) + } + (Array(len_a, elem_a), Array(len_b, elem_b)) => { len_a.try_unify(len_b, bindings)?; elem_a.try_unify(elem_b, bindings) @@ -1413,7 +1444,7 @@ impl Type { Type::NamedGeneric(binding, _) | Type::TypeVariable(binding, _) => { substitute_binding(binding) } - // Do not substitute_helper fields, it ca, substitute_bound_typevarsn lead to infinite recursion + // Do not substitute_helper fields, it can lead to infinite recursion // and we should not match fields when type checking anyway. Type::Struct(fields, args) => { let args = vecmap(args, |arg| { @@ -1421,6 +1452,12 @@ impl Type { }); Type::Struct(fields.clone(), args) } + Type::Alias(alias, args) => { + let args = vecmap(args, |arg| { + arg.substitute_helper(type_bindings, substitute_bound_typevars) + }); + Type::Alias(alias.clone(), args) + } Type::Tuple(fields) => { let fields = vecmap(fields, |field| { field.substitute_helper(type_bindings, substitute_bound_typevars) @@ -1469,7 +1506,9 @@ impl Type { let field_occurs = fields.occurs(target_id); len_occurs || field_occurs } - Type::Struct(_, generic_args) => generic_args.iter().any(|arg| arg.occurs(target_id)), + Type::Struct(_, generic_args) | Type::Alias(_, generic_args) => { + generic_args.iter().any(|arg| arg.occurs(target_id)) + } Type::Tuple(fields) => fields.iter().any(|field| field.occurs(target_id)), Type::NamedGeneric(binding, _) | Type::TypeVariable(binding, _) => { match &*binding.borrow() { @@ -1520,6 +1559,11 @@ impl Type { let args = vecmap(args, |arg| arg.follow_bindings()); Struct(def.clone(), args) } + Alias(def, args) => { + // We don't need to vecmap(args, follow_bindings) since we're recursively + // calling follow_bindings here already. + def.borrow().get_type(&args).follow_bindings() + } Tuple(args) => Tuple(vecmap(args, |arg| arg.follow_bindings())), TypeVariable(var, _) | NamedGeneric(var, _) => { if let TypeBinding::Bound(typ) = &*var.borrow() { @@ -1651,6 +1695,7 @@ impl From<&Type> for PrintableType { let fields = vecmap(fields, |(name, typ)| (name, typ.into())); PrintableType::Struct { fields, name: struct_type.name.to_string() } } + Type::Alias(alias, args) => alias.borrow().get_type(args).into(), Type::TraitAsType(_, _, _) => unreachable!(), Type::Tuple(types) => PrintableType::Tuple { types: vecmap(types, |typ| typ.into()) }, Type::TypeVariable(_, _) => unreachable!(), @@ -1694,9 +1739,17 @@ impl std::fmt::Debug for Type { Type::Struct(s, args) => { let args = vecmap(args, |arg| format!("{:?}", arg)); if args.is_empty() { - write!(f, "{:?}", s.borrow()) + write!(f, "{}", s.borrow()) + } else { + write!(f, "{}<{}>", s.borrow(), args.join(", ")) + } + } + Type::Alias(alias, args) => { + let args = vecmap(args, |arg| format!("{:?}", arg)); + if args.is_empty() { + write!(f, "{}", alias.borrow()) } else { - write!(f, "{:?}<{}>", s.borrow(), args.join(", ")) + write!(f, "{}<{}>", alias.borrow(), args.join(", ")) } } Type::TraitAsType(_id, name, generics) => { diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 21c095eb877..655e6e6a44b 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -820,6 +820,8 @@ impl<'interner> Monomorphizer<'interner> { ast::Type::Tuple(fields) } + HirType::Alias(def, args) => self.convert_type(&def.borrow().get_type(args)), + HirType::Tuple(fields) => { let fields = vecmap(fields, |x| self.convert_type(x)); ast::Type::Tuple(fields) diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 67b80053856..9071cf73547 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -605,13 +605,13 @@ impl NodeInterner { pub fn push_type_alias(&mut self, typ: &UnresolvedTypeAlias) -> TypeAliasId { let type_id = TypeAliasId(self.type_aliases.len()); - self.type_aliases.push(TypeAlias::new( + self.type_aliases.push(Shared::new(TypeAlias::new( type_id, typ.type_alias_def.name.clone(), Location::new(typ.type_alias_def.span, typ.file_id), Type::Error, vecmap(&typ.type_alias_def.generics, |_| TypeVariable::unbound(TypeVariableId(0))), - )); + ))); type_id } @@ -633,7 +633,7 @@ impl NodeInterner { pub fn set_type_alias(&mut self, type_id: TypeAliasId, typ: Type, generics: Generics) { let type_alias_type = &mut self.type_aliases[type_id.0]; - type_alias_type.set_type_and_generics(typ, generics); + type_alias_type.borrow_mut().set_type_and_generics(typ, generics); } /// Returns the interned statement corresponding to `stmt_id` @@ -1590,6 +1590,7 @@ impl NodeInterner { } DependencyId::Alias(alias_id) => { let alias = self.get_type_alias(alias_id); + let alias = alias.borrow(); push_error(alias.name.to_string(), &scc, i, alias.location); break; } @@ -1611,7 +1612,7 @@ impl NodeInterner { DependencyId::Struct(id) => Cow::Owned(self.get_struct(id).borrow().name.to_string()), DependencyId::Function(id) => Cow::Borrowed(self.function_name(&id)), DependencyId::Alias(id) => { - Cow::Borrowed(self.get_type_alias(id).name.0.contents.as_ref()) + Cow::Owned(self.get_type_alias(id).borrow().name.to_string()) } DependencyId::Global(id) => { Cow::Borrowed(self.get_global(id).ident.0.contents.as_ref()) @@ -1713,6 +1714,7 @@ fn get_type_method_key(typ: &Type) -> Option { Type::Function(_, _, _) => Some(Function), Type::NamedGeneric(_, _) => Some(Generic), Type::MutableReference(element) => get_type_method_key(element), + Type::Alias(alias, _) => get_type_method_key(&alias.borrow().typ), // We do not support adding methods to these types Type::TypeVariable(_, _) diff --git a/compiler/noirc_frontend/src/resolve_locations.rs b/compiler/noirc_frontend/src/resolve_locations.rs index cfb88966b9d..b5f1b1d0c64 100644 --- a/compiler/noirc_frontend/src/resolve_locations.rs +++ b/compiler/noirc_frontend/src/resolve_locations.rs @@ -212,6 +212,8 @@ impl NodeInterner { self.type_alias_ref .iter() .find(|(_, named_type_location)| named_type_location.span.contains(&location.span)) - .map(|(type_alias_id, _found_location)| self.get_type_alias(*type_alias_id).location) + .map(|(type_alias_id, _found_location)| { + self.get_type_alias(*type_alias_id).borrow().location + }) } } diff --git a/tooling/noirc_abi/src/lib.rs b/tooling/noirc_abi/src/lib.rs index 1fc257c1676..4d67d74b476 100644 --- a/tooling/noirc_abi/src/lib.rs +++ b/tooling/noirc_abi/src/lib.rs @@ -158,7 +158,7 @@ impl AbiType { Self::String { length: size } } - Type::Struct(def, ref args) => { + Type::Struct(def, args) => { let struct_type = def.borrow(); let fields = struct_type.get_fields(args); let fields = vecmap(fields, |(name, typ)| (name, Self::from_type(context, &typ))); @@ -167,6 +167,7 @@ impl AbiType { context.fully_qualified_struct_path(context.root_crate_id(), struct_type.id); Self::Struct { fields, path } } + Type::Alias(def, args) => Self::from_type(context, &def.borrow().get_type(args)), Type::Tuple(fields) => { let fields = vecmap(fields, |typ| Self::from_type(context, typ)); Self::Tuple { fields } From 5df49e269a2a54a6e5276a194cf68e60edce0589 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 12 Feb 2024 15:46:19 -0600 Subject: [PATCH 3/7] Break type alias cycles --- compiler/noirc_frontend/src/node_interner.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 9071cf73547..60e28c9c1ba 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -1590,6 +1590,11 @@ impl NodeInterner { } DependencyId::Alias(alias_id) => { let alias = self.get_type_alias(alias_id); + // If type aliases form a cycle, we have to manually break the cycle + // here to prevent infinite recursion in the type checker. + alias.borrow_mut().typ = Type::Error; + + // push_error will borrow the alias so we have to drop the mutable borrow let alias = alias.borrow(); push_error(alias.name.to_string(), &scc, i, alias.location); break; From eb511f7a26684c615e7e6d29ab13d77dbced2a2b Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 12 Feb 2024 15:50:59 -0600 Subject: [PATCH 4/7] Add tests --- compiler/noirc_frontend/src/tests.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 1deff446d7e..8a56b337398 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -1184,4 +1184,26 @@ fn lambda$f1(mut env$l1: (Field)) -> Field { "#; assert_eq!(get_program_errors(src).len(), 1); } + + #[test] + fn deny_cyclic_type_aliases() { + let src = r#" + type A = B; + type B = A; + fn main() {} + "#; + assert_eq!(get_program_errors(src).len(), 1); + } + + #[test] + fn ensure_nested_type_aliases_type_check() { + let src = r#" + type A = B; + type B = u8; + fn main() { + let _a: A = 0 as u16; + } + "#; + assert_eq!(get_program_errors(src).len(), 1); + } } From 8f37f45f04c0d49c409b6dd5f96c85b47eff7938 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 12 Feb 2024 15:56:16 -0600 Subject: [PATCH 5/7] Add note to docs --- docs/docs/noir/concepts/data_types/index.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/docs/noir/concepts/data_types/index.md b/docs/docs/noir/concepts/data_types/index.md index 3c9cd4c2437..97b3b2cb094 100644 --- a/docs/docs/noir/concepts/data_types/index.md +++ b/docs/docs/noir/concepts/data_types/index.md @@ -91,6 +91,20 @@ fn main() { } ``` +Type aliases can even refer to other aliases. An error will be issued if they form a cycle: + +```rust +// Ok! +type A = B; +type B = Field; + +type Bad1 = Bad2; + +// error: Dependency cycle found +type Bad2 = Bad1; +// ^^^^^^^^^^^ 'Bad2' recursively depends on itself: Bad2 -> Bad1 -> Bad2 +``` + ### BigInt You can achieve BigInt functionality using the [Noir BigInt](https://github.com/shuklaayush/noir-bigint) library. From 5707fab06a81cd224c881ba2b4cc45a9799ecac6 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 12 Feb 2024 15:58:41 -0600 Subject: [PATCH 6/7] Appease Clippy --- compiler/noirc_frontend/src/hir_def/types.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 3b93be1d141..ea6b10fefaa 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1562,7 +1562,7 @@ impl Type { Alias(def, args) => { // We don't need to vecmap(args, follow_bindings) since we're recursively // calling follow_bindings here already. - def.borrow().get_type(&args).follow_bindings() + def.borrow().get_type(args).follow_bindings() } Tuple(args) => Tuple(vecmap(args, |arg| arg.follow_bindings())), TypeVariable(var, _) | NamedGeneric(var, _) => { From c8814fbec5c23605f6952cb570538097a0d1869d Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 13 Feb 2024 10:42:22 -0600 Subject: [PATCH 7/7] Add follow_bindings when resolving type variables --- compiler/noirc_frontend/src/hir/type_check/stmt.rs | 6 +++--- compiler/noirc_frontend/src/hir_def/types.rs | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/stmt.rs b/compiler/noirc_frontend/src/hir/type_check/stmt.rs index d6a19bb74be..03d61b93e0c 100644 --- a/compiler/noirc_frontend/src/hir/type_check/stmt.rs +++ b/compiler/noirc_frontend/src/hir/type_check/stmt.rs @@ -93,7 +93,7 @@ impl<'interner> TypeChecker<'interner> { match pattern { HirPattern::Identifier(ident) => self.interner.push_definition_type(ident.id, typ), HirPattern::Mutable(pattern, _) => self.bind_pattern(pattern, typ), - HirPattern::Tuple(fields, location) => match typ { + HirPattern::Tuple(fields, location) => match typ.follow_bindings() { Type::Tuple(field_types) if field_types.len() == fields.len() => { for (field, field_type) in fields.iter().zip(field_types) { self.bind_pattern(field, field_type); @@ -120,12 +120,12 @@ impl<'interner> TypeChecker<'interner> { source: Source::Assignment, }); - if let Type::Struct(struct_type, generics) = struct_type { + if let Type::Struct(struct_type, generics) = struct_type.follow_bindings() { let struct_type = struct_type.borrow(); for (field_name, field_pattern) in fields { if let Some((type_field, _)) = - struct_type.get_field(&field_name.0.contents, generics) + struct_type.get_field(&field_name.0.contents, &generics) { self.bind_pattern(field_pattern, type_field); } diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index d9a43c56136..dae6ec207fb 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -895,7 +895,7 @@ impl Type { TypeBinding::Unbound(id) => *id, }; - let this = self.substitute(bindings); + let this = self.substitute(bindings).follow_bindings(); match &this { Type::Constant(length) if *length == target_length => { @@ -962,7 +962,7 @@ impl Type { TypeBinding::Unbound(id) => *id, }; - let this = self.substitute(bindings); + let this = self.substitute(bindings).follow_bindings(); match &this { Type::FieldElement | Type::Integer(..) => { bindings.insert(target_id, (var.clone(), this)); @@ -1078,6 +1078,11 @@ impl Type { match (self, other) { (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, Kind::IntegerOrField), other) | (other, TypeVariable(var, Kind::IntegerOrField)) => { other.try_unify_to_type_variable(var, bindings, |bindings| { @@ -1097,11 +1102,6 @@ impl Type { other.try_bind_to_maybe_constant(var, *length, bindings) }), - (Alias(alias, args), other) | (other, Alias(alias, args)) => { - let alias = alias.borrow().get_type(args); - alias.try_unify(other, bindings) - } - (Array(len_a, elem_a), Array(len_b, elem_b)) => { len_a.try_unify(len_b, bindings)?; elem_a.try_unify(elem_b, bindings)