chore: Remove TypeVariableId from Generics

compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs

@@ -456,7 +456,7 @@
         .collect()
 }
 
 // TODO(vitkov): Move this out of here and into type_check
 #[allow(clippy::too_many_arguments)]
 pub(crate) fn check_methods_signatures(
     resolver: &mut Resolver,
@@ -491,7 +491,7 @@
     }
 
     // We also need to bind the traits generics to the trait's generics on the impl
-    for ((_, generic), binding) in the_trait.generics.iter().zip(trait_generics) {
+    for (generic, binding) in the_trait.generics.iter().zip(trait_generics) {
         generic.bind(binding);
     }
 
@@ -599,7 +599,7 @@
     the_trait.set_methods(trait_methods);
     the_trait.self_type_typevar.unbind(the_trait.self_type_typevar_id);
 
-    for (old_id, generic) in &the_trait.generics {
-        generic.unbind(*old_id);
+    for generic in &the_trait.generics {
+        generic.unbind(generic.id());
     }
 }

compiler/noirc_frontend/src/hir/resolution/resolver.rs

@@ -39,7 +39,7 @@ use crate::{
 use crate::{
     ArrayLiteral, ContractFunctionType, Distinctness, ForRange, FunctionDefinition,
     FunctionReturnType, FunctionVisibility, Generics, LValue, NoirStruct, NoirTypeAlias, Param,
-    Path, PathKind, Pattern, Shared, StructType, Type, TypeAliasType, TypeBinding, TypeVariable,
+    Path, PathKind, Pattern, Shared, StructType, Type, TypeAliasType, TypeVariable,
     TypeVariableKind, UnaryOp, UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType,
     UnresolvedTypeData, UnresolvedTypeExpression, Visibility, ERROR_IDENT,
 };
@@ -643,7 +643,7 @@ impl<'a> Resolver<'a> {
             None => {
                 let id = self.interner.next_type_variable_id();
                 let typevar = TypeVariable::unbound(id);
-                new_variables.push((id, typevar.clone()));
+                new_variables.push(typevar.clone());
 
                 // 'Named'Generic is a bit of a misnomer here, we want a type variable that
                 // wont be bound over but this one has no name since we do not currently
@@ -768,7 +768,7 @@ impl<'a> Resolver<'a> {
                 self.generics.push((name, typevar.clone(), span));
             }
 
-            (id, typevar)
+            typevar
         })
     }
 
@@ -778,7 +778,7 @@ impl<'a> Resolver<'a> {
     pub fn add_existing_generics(&mut self, names: &UnresolvedGenerics, generics: &Generics) {
         assert_eq!(names.len(), generics.len());
 
-        for (name, (_id, typevar)) in names.iter().zip(generics) {
+        for (name, typevar) in names.iter().zip(generics) {
             self.add_existing_generic(&name.0.contents, name.0.span(), typevar.clone());
         }
     }
@@ -846,14 +846,7 @@ impl<'a> Resolver<'a> {
 
         let attributes = func.attributes().clone();
 
-        let mut generics =
-            vecmap(self.generics.clone(), |(name, typevar, _)| match &*typevar.borrow() {
-                TypeBinding::Unbound(id) => (*id, typevar.clone()),
-                TypeBinding::Bound(binding) => {
-                    unreachable!("Expected {} to be unbound, but it is bound to {}", name, binding)
-                }
-            });
-
+        let mut generics = vecmap(&self.generics, |(_, typevar, _)| typevar.clone());
         let mut parameters = vec![];
         let mut parameter_types = vec![];
 

compiler/noirc_frontend/src/hir/resolution/traits.rs

@@ -18,7 +18,7 @@ use crate::{
     },
     hir_def::traits::{TraitConstant, TraitFunction, TraitImpl, TraitType},
     node_interner::{FuncId, NodeInterner, TraitId},
-    Generics, Path, Shared, TraitItem, Type, TypeBinding, TypeVariable, TypeVariableKind,
+    Generics, Path, Shared, TraitItem, Type, TypeVariable, TypeVariableKind,
 };
 
 use super::{
@@ -42,8 +42,7 @@ pub(crate) fn resolve_traits(
 
     for (trait_id, unresolved_trait) in traits {
         let generics = vecmap(&unresolved_trait.trait_def.generics, |_| {
-            let id = context.def_interner.next_type_variable_id();
-            (id, TypeVariable::unbound(id))
+            TypeVariable::unbound(context.def_interner.next_type_variable_id())
         });
 
         // Resolve order
@@ -142,17 +141,7 @@ fn resolve_trait_methods(
             let arguments = vecmap(parameters, |param| resolver.resolve_type(param.1.clone()));
             let return_type = resolver.resolve_type(return_type.get_type().into_owned());
 
-            let generics =
-                vecmap(resolver.get_generics(), |(_, type_var, _)| match &*type_var.borrow() {
-                    TypeBinding::Unbound(id) => (*id, type_var.clone()),
-                    TypeBinding::Bound(binding) => {
-                        unreachable!("Trait generic was bound to {binding}")
-                    }
-                });
-
-            // Ensure the trait is generic over the Self type as well
-            // let the_trait = resolver.interner.get_trait(trait_id);
-            // generics.push((the_trait.self_type_typevar_id, the_trait.self_type_typevar.clone()));
+            let generics = vecmap(resolver.get_generics(), |(_, type_var, _)| type_var.clone());
 
             let default_impl_list: Vec<_> = unresolved_trait
                 .fns_with_default_impl
@@ -465,8 +454,7 @@ pub(crate) fn resolve_trait_impls(
                 methods: vecmap(&impl_methods, |(_, func_id)| *func_id),
             });
 
-            let impl_generics =
-                vecmap(impl_generics, |(_, type_variable, _)| (type_variable.id(), type_variable));
+            let impl_generics = vecmap(impl_generics, |(_, type_variable, _)| type_variable);
 
             if let Err((prev_span, prev_file)) = interner.add_trait_implementation(
                 self_type.clone(),

compiler/noirc_frontend/src/hir/type_check/expr.rs

@@ -595,7 +595,7 @@ impl<'interner> TypeChecker<'interner> {
                     .generics
                     .iter()
                     .zip(generics)
-                    .map(|((id, var), arg)| (*id, (var.clone(), arg)))
+                    .map(|(var, arg)| (var.id(), (var.clone(), arg)))
                     .collect();
 
                 (method.typ.clone(), method.arguments().len(), generic_bindings)

compiler/noirc_frontend/src/hir_def/traits.rs

@@ -147,7 +147,7 @@ impl TraitFunction {
         }
     }
 
-    pub fn generics(&self) -> &[(TypeVariableId, TypeVariable)] {
+    pub fn generics(&self) -> &[TypeVariable] {
         match &self.typ {
             Type::Function(..) => &[],
             Type::Forall(generics, _) => generics,

compiler/noirc_frontend/src/hir_def/types.rs

@@ -64,7 +64,7 @@
     TypeVariable(TypeVariable, TypeVariableKind),
 
     /// `impl Trait` when used in a type position.
     /// These are only matched based on the TraitId. The trait name paramer is only
     /// used for displaying error messages using the name of the trait.
     TraitAsType(TraitId, /*name:*/ Rc<String>, /*generics:*/ Vec<Type>),
 
@@ -170,11 +170,8 @@
     pub location: Location,
 }
 
-/// Corresponds to generic lists such as `<T, U>` in the source
-/// program. The `TypeVariableId` portion is used to match two
-/// type variables to check for equality, while the `TypeVariable` is
-/// the actual part that can be mutated to bind it to another type.
-pub type Generics = Vec<(TypeVariableId, TypeVariable)>;
+/// Corresponds to generic lists such as `<T, U>` in the source program.
+pub type Generics = Vec<TypeVariable>;
 
 impl std::hash::Hash for StructType {
     fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
@@ -223,7 +220,7 @@
                     .generics
                     .iter()
                     .zip(generic_args)
-                    .map(|((old_id, old_var), new)| (*old_id, (old_var.clone(), new.clone())))
+                    .map(|(old, new)| (old.id(), (old.clone(), new.clone())))
                     .collect();
 
                 (typ.substitute(&substitutions), i)
@@ -239,7 +236,7 @@
             .generics
             .iter()
             .zip(generic_args)
-            .map(|((old_id, old_var), new)| (*old_id, (old_var.clone(), new.clone())))
+            .map(|(old, new)| (old.id(), (old.clone(), new.clone())))
             .collect();
 
         vecmap(&self.fields, |(name, typ)| {
@@ -300,7 +297,7 @@
         write!(f, "{}", self.name)?;
 
         if !self.generics.is_empty() {
-            let generics = vecmap(&self.generics, |(_, binding)| binding.borrow().to_string());
+            let generics = vecmap(&self.generics, |binding| binding.borrow().to_string());
             write!(f, "{}", generics.join(", "))?;
         }
 
@@ -332,7 +329,7 @@
             .generics
             .iter()
             .zip(generic_args)
-            .map(|((old_id, old_var), new)| (*old_id, (old_var.clone(), new.clone())))
+            .map(|(old, new)| (old.id(), (old.clone(), new.clone())))
             .collect();
 
         self.typ.substitute(&substitutions)
@@ -670,7 +667,7 @@
     /// Takes a monomorphic type and generalizes it over each of the type variables in the
     /// given type bindings, ignoring what each type variable is bound to in the TypeBindings.
     pub(crate) fn generalize_from_substitutions(self, type_bindings: TypeBindings) -> Type {
-        let polymorphic_type_vars = vecmap(type_bindings, |(id, (type_var, _))| (id, type_var));
+        let polymorphic_type_vars = vecmap(type_bindings, |(_, (type_var, _))| type_var);
         Type::Forall(polymorphic_type_vars, Box::new(self))
     }
 
@@ -764,7 +761,7 @@
             },
             Type::Constant(x) => x.fmt(f),
             Type::Forall(typevars, typ) => {
-                let typevars = vecmap(typevars, |(var, _)| var.to_string());
+                let typevars = vecmap(typevars, |var| var.id().to_string());
                 write!(f, "forall {}. {}", typevars.join(" "), typ)
             }
             Type::Function(args, ret, env) => {
@@ -1270,9 +1267,9 @@
     ) -> (Type, TypeBindings) {
         match self {
             Type::Forall(typevars, typ) => {
-                for (id, var) in typevars {
+                for var in typevars {
                     bindings
-                        .entry(*id)
+                        .entry(var.id())
                         .or_insert_with(|| (var.clone(), interner.next_type_variable()));
                 }
 
@@ -1291,9 +1288,9 @@
             Type::Forall(typevars, typ) => {
                 let replacements = typevars
                     .iter()
-                    .map(|(id, var)| {
+                    .map(|var| {
                         let new = interner.next_type_variable();
-                        (*id, (var.clone(), new))
+                        (var.id(), (var.clone(), new))
                     })
                     .collect();
 
@@ -1391,8 +1388,8 @@
             Type::Forall(typevars, typ) => {
                 // Trying to substitute_helper a variable de, substitute_bound_typevarsfined within a nested Forall
                 // is usually impossible and indicative of an error in the type checker somewhere.
-                for (var, _) in typevars {
-                    assert!(!type_bindings.contains_key(var));
+                for var in typevars {
+                    assert!(!type_bindings.contains_key(&var.id()));
                 }
                 let typ = Box::new(typ.substitute_helper(type_bindings, substitute_bound_typevars));
                 Type::Forall(typevars.clone(), typ)
@@ -1411,7 +1408,7 @@
 
             Type::FieldElement
             | Type::Integer(_, _)
             | Type::Bool
             | Type::Constant(_)
             | Type::TraitAsType(..)
             | Type::Error
@@ -1426,7 +1423,7 @@
             Type::Array(len, elem) => len.occurs(target_id) || elem.occurs(target_id),
             Type::String(len) => len.occurs(target_id),
             Type::FmtString(len, fields) => {
                 let len_occurs = len.occurs(target_id);
                 let field_occurs = fields.occurs(target_id);
                 len_occurs || field_occurs
             }
@@ -1439,7 +1436,7 @@
                 }
             }
             Type::Forall(typevars, typ) => {
-                !typevars.iter().any(|(id, _)| *id == target_id) && typ.occurs(target_id)
+                !typevars.iter().any(|var| var.id() == target_id) && typ.occurs(target_id)
             }
             Type::Function(args, ret, env) => {
                 args.iter().any(|arg| arg.occurs(target_id))
@@ -1512,7 +1509,7 @@
     }
 
     pub fn from_generics(generics: &Generics) -> Vec<Type> {
-        vecmap(generics, |(_, var)| Type::TypeVariable(var.clone(), TypeVariableKind::Normal))
+        vecmap(generics, |var| Type::TypeVariable(var.clone(), TypeVariableKind::Normal))
     }
 }
 

compiler/noirc_frontend/src/monomorphization/mod.rs

@@ -27,7 +27,7 @@ use crate::{
     node_interner::{self, DefinitionKind, NodeInterner, StmtId, TraitImplKind, TraitMethodId},
     token::FunctionAttribute,
     ContractFunctionType, FunctionKind, Type, TypeBinding, TypeBindings, TypeVariable,
-    TypeVariableId, TypeVariableKind, UnaryOp, Visibility,
+    TypeVariableKind, UnaryOp, Visibility,
 };
 
 use self::ast::{Definition, FuncId, Function, LocalId, Program};
@@ -1533,8 +1533,8 @@ impl<'interner> Monomorphizer<'interner> {
             let (generics, impl_method_type) =
                 self.interner.function_meta(&impl_method).typ.unwrap_forall();
 
-            let replace_type_variable = |(id, var): &(TypeVariableId, TypeVariable)| {
-                (*id, (var.clone(), Type::TypeVariable(var.clone(), TypeVariableKind::Normal)))
+            let replace_type_variable = |var: &TypeVariable| {
+                (var.id(), (var.clone(), Type::TypeVariable(var.clone(), TypeVariableKind::Normal)))
             };
 
             // Replace each NamedGeneric with a TypeVariable containing the same internal type variable

compiler/noirc_frontend/src/node_interner.rs

@@ -499,8 +499,7 @@ impl NodeInterner {
                 // This lets us record how many arguments the type expects so that other types
                 // can refer to it with generic arguments before the generic parameters themselves
                 // are resolved.
-                let id = TypeVariableId(0);
-                (id, TypeVariable::unbound(id))
+                TypeVariable::unbound(TypeVariableId(0))
             }),
             self_type_typevar_id,
             self_type_typevar: TypeVariable::unbound(self_type_typevar_id),
@@ -530,8 +529,7 @@ impl NodeInterner {
             // This lets us record how many arguments the type expects so that other types
             // can refer to it with generic arguments before the generic parameters themselves
             // are resolved.
-            let id = TypeVariableId(0);
-            (id, TypeVariable::unbound(id))
+            TypeVariable::unbound(TypeVariableId(0))
         });
 
         let location = Location::new(typ.struct_def.span, file_id);
@@ -549,10 +547,7 @@ impl NodeInterner {
             typ.type_alias_def.name.clone(),
             typ.type_alias_def.span,
             Type::Error,
-            vecmap(&typ.type_alias_def.generics, |_| {
-                let id = TypeVariableId(0);
-                (id, TypeVariable::unbound(id))
-            }),
+            vecmap(&typ.type_alias_def.generics, |_| TypeVariable::unbound(TypeVariableId(0))),
         ));
 
         type_id
@@ -1194,19 +1189,18 @@ impl NodeInterner {
 
         self.trait_implementations.push(trait_impl.clone());
 
-        // Ignoring overlapping TraitImplKind::Assumed impls here is perfectly fine.
-        // It should never happen since impls are defined at global scope, but even
-        // if they were, we should never prevent defining a new impl because a where
-        // clause already assumes it exists.
-
         // Replace each generic with a fresh type variable
         let substitutions = impl_generics
             .into_iter()
-            .map(|(id, typevar)| (id, (typevar, self.next_type_variable())))
+            .map(|typevar| (typevar.id(), (typevar, self.next_type_variable())))
             .collect();
 
         let instantiated_object_type = object_type.substitute(&substitutions);
 
+        // Ignoring overlapping `TraitImplKind::Assumed` impls here is perfectly fine.
+        // It should never happen since impls are defined at global scope, but even
+        // if they were, we should never prevent defining a new impl because a 'where'
+        // clause already assumes it exists.
         if let Ok((TraitImplKind::Normal(existing), _)) = self.try_lookup_trait_implementation(
             &instantiated_object_type,
             trait_id,