Implement PortableRegistryBuilder to manage ids

src/interner.rs

@@ -130,7 +130,7 @@ pub struct Interner<T> {
     ///
     /// This is used to efficiently provide access to the cached elements and
     /// to establish a strict ordering upon them since each is uniquely
-    /// idenfitied later by its position in the vector.
+    /// identified later by its position in the vector.
     vec: Vec<T>,
 }
 
@@ -198,6 +198,11 @@ where
         }
         self.vec.get(idx)
     }
+
+    /// Returns the ordered sequence of interned elements.
+    pub fn elements(&self) -> &[T] {
+        &self.vec
+    }
 }
 
 #[cfg(test)]

src/portable.rs

@@ -25,6 +25,7 @@
 
 use crate::{
     form::PortableForm,
+    interner::Interner,
     prelude::{
         fmt::Debug,
         vec::Vec,
@@ -60,11 +61,6 @@ impl From<Registry> for PortableRegistry {
 }
 
 impl PortableRegistry {
-    /// Construct a new `PortableRegistry` from custom types.
-    pub fn new_from_types(types: Vec<PortableType>) -> Self {
-        Self { types }
-    }
-
     /// Returns the type definition for the given identifier, `None` if no type found for that ID.
     pub fn resolve(&self, id: u32) -> Option<&Type<PortableForm>> {
         self.types.get(id as usize).map(|ty| ty.ty())
@@ -106,18 +102,60 @@ impl PortableType {
 }
 
 /// Construct a [`PortableRegistry`].
+///
+/// Guarantees that the resulting [`PortableRegistry`] has the list of types in the correct order,
+/// since downstream libs assume that a `u32` type id corresponds to the index of the type
+/// definition type table.
 pub struct PortableRegistryBuilder {
+    types: Interner<Type<PortableForm>>,
+}
+
+impl PortableRegistryBuilder {
+    /// Create a new [`PortableRegistryBuilder`].
+    pub fn new() -> Self {
+        Self {
+            types: Default::default(),
+        }
+    }
+
+    /// Register a type, returning the assigned ID.
+    ///
+    /// If the type is already registered it will return the existing ID.
+    pub fn register_type(&mut self, ty: Type<PortableForm>) -> u32 {
+        self.types.intern_or_get(ty).1.into_untracked().id()
+    }
+
+    /// Returns the type id that would be assigned to a newly registered type.
+    pub fn next_type_id(&self) -> u32 {
+        self.types.elements().len() as u32
+    }
 
+    /// Finalize and return a valid [`PortableRegistry`] instance.
+    pub fn finish(&self) -> PortableRegistry {
+        let types = self
+            .types
+            .elements()
+            .iter()
+            .enumerate()
+            .map(|(i, ty)| {
+                PortableType {
+                    id: i as u32,
+                    ty: ty.clone(),
+                }
+            })
+            .collect();
+        PortableRegistry { types }
+    }
 }
 
 #[cfg(test)]
 mod tests {
+    use super::*;
     use crate::{
         build::*,
         prelude::vec,
         *,
     };
-    use super::*;
 
     #[test]
     fn type_ids_are_sequential() {
@@ -137,39 +175,39 @@ mod tests {
 
     #[test]
     fn construct_portable_registry() {
-        let mut types = Vec::new();
-
-        let u32_type_id = types.len() as u32;
-        types.push(PortableType::new(
-            u32_type_id,
-            Type::new(Path::default(), vec![], TypeDefPrimitive::U32, vec![]),
-        ));
-
-        let vec_u32_type_id = types.len() as u32;
-        types.push(PortableType::new(
-            vec_u32_type_id,
-            Type::new(
-                Path::default(),
-                vec![],
-                TypeDefSequence::new(u32_type_id.into()),
-                vec![],
-            ),
-        ));
-
-        let composite_type_id = types.len() as u32;
-        let composite = Type::builder_portable()
+        let mut builder = PortableRegistryBuilder::new();
+        let u32_type = Type::new(Path::default(), vec![], TypeDefPrimitive::U32, vec![]);
+        let u32_type_id = builder.register_type(u32_type.clone());
+
+        let vec_u32_type = Type::new(
+            Path::default(),
+            vec![],
+            TypeDefSequence::new(u32_type_id.into()),
+            vec![],
+        );
+        let vec_u32_type_id = builder.register_type(vec_u32_type.clone());
+
+        let self_referential_type_id = builder.next_type_id();
+
+        let composite_type = Type::builder_portable()
             .path(Path::new_custom(vec!["MyStruct".into()]))
             .composite(
                 Fields::named()
                     .field_portable(|f| f.name("primitive".into()).ty(u32_type_id))
                     .field_portable(|f| f.name("vec_of_u32".into()).ty(vec_u32_type_id))
                     .field_portable(|f| {
-                        f.name("self_referential".into()).ty(composite_type_id)
+                        f.name("self_referential".into())
+                            .ty(self_referential_type_id)
                     }),
             );
-        types.push(PortableType::new(composite_type_id, composite));
+        let composite_type_id = builder.register_type(composite_type.clone());
+
+        assert_eq!(self_referential_type_id, composite_type_id);
 
-        let _registry = PortableRegistry::new_from_types(types);
+        let registry = builder.finish();
+
+        assert_eq!(Some(&u32_type), registry.resolve(u32_type_id));
+        assert_eq!(Some(&vec_u32_type), registry.resolve(vec_u32_type_id));
+        assert_eq!(Some(&composite_type), registry.resolve(composite_type_id));
     }
 }
-

src/registry.rs

@@ -146,7 +146,9 @@ impl Registry {
     }
 
     /// Returns an iterator over the types with their keys
-    pub fn types(&self) -> impl Iterator<Item = (&UntrackedSymbol<TypeId>, &Type<PortableForm>)> {
+    pub fn types(
+        &self,
+    ) -> impl Iterator<Item = (&UntrackedSymbol<TypeId>, &Type<PortableForm>)> {
         self.types.iter()
     }
 }