From e3e23127b386af6ceeed81cff4983ca5ee49a08e Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 8 Mar 2023 17:39:47 +0200 Subject: [PATCH 01/30] ty: Make type fields public Signed-off-by: Alexandru Vasile --- src/ty/composite.rs | 2 +- src/ty/fields.rs | 8 ++++---- src/ty/mod.rs | 26 +++++++++++++------------- src/ty/variant.rs | 2 +- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/ty/composite.rs b/src/ty/composite.rs index faeaef68..8815966b 100644 --- a/src/ty/composite.rs +++ b/src/ty/composite.rs @@ -76,7 +76,7 @@ pub struct TypeDefComposite { feature = "serde", serde(skip_serializing_if = "Vec::is_empty", default) )] - fields: Vec>, + pub fields: Vec>, } impl IntoPortable for TypeDefComposite { diff --git a/src/ty/fields.rs b/src/ty/fields.rs index e024a421..7b71faeb 100644 --- a/src/ty/fields.rs +++ b/src/ty/fields.rs @@ -78,22 +78,22 @@ pub struct Field { feature = "serde", serde(skip_serializing_if = "Option::is_none", default) )] - name: Option, + pub name: Option, /// The type of the field. #[cfg_attr(feature = "serde", serde(rename = "type"))] - ty: T::Type, + pub ty: T::Type, /// The name of the type of the field as it appears in the source code. #[cfg_attr( feature = "serde", serde(skip_serializing_if = "Option::is_none", default) )] - type_name: Option, + pub type_name: Option, /// Documentation #[cfg_attr( feature = "serde", serde(skip_serializing_if = "Vec::is_empty", default) )] - docs: Vec, + pub docs: Vec, } impl IntoPortable for Field { diff --git a/src/ty/mod.rs b/src/ty/mod.rs index 20215a22..2f79a7d0 100644 --- a/src/ty/mod.rs +++ b/src/ty/mod.rs @@ -68,22 +68,22 @@ pub struct Type { feature = "serde", serde(skip_serializing_if = "Path::is_empty", default) )] - path: Path, + pub path: Path, /// The generic type parameters of the type in use. Empty for non generic types #[cfg_attr( feature = "serde", serde(rename = "params", skip_serializing_if = "Vec::is_empty", default) )] - type_params: Vec>, + pub type_params: Vec>, /// The actual type definition #[cfg_attr(feature = "serde", serde(rename = "def"))] - type_def: TypeDef, + pub type_def: TypeDef, /// Documentation #[cfg_attr( feature = "serde", serde(skip_serializing_if = "Vec::is_empty", default) )] - docs: Vec, + pub docs: Vec, } impl IntoPortable for Type { @@ -194,12 +194,12 @@ where #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, From, Debug, Encode)] pub struct TypeParameter { /// The name of the generic type parameter e.g. "T". - name: T::String, + pub name: T::String, /// The concrete type for the type parameter. /// /// `None` if the type parameter is skipped. #[cfg_attr(feature = "serde", serde(rename = "type"))] - ty: Option, + pub ty: Option, } impl IntoPortable for TypeParameter { @@ -400,10 +400,10 @@ pub enum TypeDefPrimitive { #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Encode, Debug)] pub struct TypeDefArray { /// The length of the array type. - len: u32, + pub len: u32, /// The element type of the array type. #[cfg_attr(feature = "serde", serde(rename = "type"))] - type_param: T::Type, + pub type_param: T::Type, } impl IntoPortable for TypeDefArray { @@ -452,7 +452,7 @@ where #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Encode, Debug)] pub struct TypeDefTuple { /// The types of the tuple fields. - fields: Vec, + pub fields: Vec, } impl IntoPortable for TypeDefTuple { @@ -514,7 +514,7 @@ where pub struct TypeDefSequence { /// The element type of the sequence type. #[cfg_attr(feature = "serde", serde(rename = "type"))] - type_param: T::Type, + pub type_param: T::Type, } impl IntoPortable for TypeDefSequence { @@ -564,7 +564,7 @@ where pub struct TypeDefCompact { /// The type wrapped in [`Compact`], i.e. the `T` in `Compact`. #[cfg_attr(feature = "serde", serde(rename = "type"))] - type_param: T::Type, + pub type_param: T::Type, } impl IntoPortable for TypeDefCompact { @@ -603,9 +603,9 @@ where #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Encode, Debug)] pub struct TypeDefBitSequence { /// The type implementing [`bitvec::store::BitStore`]. - bit_store_type: T::Type, + pub bit_store_type: T::Type, /// The type implementing [`bitvec::order::BitOrder`]. - bit_order_type: T::Type, + pub bit_order_type: T::Type, } impl IntoPortable for TypeDefBitSequence { diff --git a/src/ty/variant.rs b/src/ty/variant.rs index eb791b24..b76b435b 100644 --- a/src/ty/variant.rs +++ b/src/ty/variant.rs @@ -88,7 +88,7 @@ pub struct TypeDefVariant { feature = "serde", serde(skip_serializing_if = "Vec::is_empty", default) )] - variants: Vec>, + pub variants: Vec>, } impl IntoPortable for TypeDefVariant { From 8143c658c9d347562bf40f8c3e6efd6eab9e9778 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 8 Mar 2023 17:46:54 +0200 Subject: [PATCH 02/30] portable: Add recursive type ID resolver Signed-off-by: Alexandru Vasile --- src/portable.rs | 103 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/src/portable.rs b/src/portable.rs index 96978bf2..558af500 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -27,12 +27,18 @@ use crate::{ form::PortableForm, interner::Interner, prelude::{ + collections::{ + HashMap, + HashSet, + }, fmt::Debug, vec::Vec, }, Registry, Type, + TypeDef, }; +use core::sync::atomic::AtomicU32; use scale::Encode; /// A read-only registry containing types in their portable form for serialization. @@ -152,6 +158,103 @@ impl PortableRegistryBuilder { } } +/// Recursive resolver for the type IDs needed to express a given type ID. +struct TypeIdResolver<'a> { + registry: &'a PortableRegistry, + result: HashMap, + next_id: AtomicU32, +} + +impl<'a> TypeIdResolver<'a> { + /// Construct a new [`TypeIdResolver`]. + fn new(registry: &'a PortableRegistry) -> Self { + TypeIdResolver { + registry, + result: Default::default(), + next_id: Default::default(), + } + } + + /// Get the next unique ID. + fn next_id(&mut self) -> u32 { + self.next_id + .fetch_add(1, core::sync::atomic::Ordering::Relaxed) + } + + /// Recursively add all type ids needed to express the given identifier. + fn visit_type_id(&mut self, id: u32) -> Result<(), ()> { + if self.result.get(&id).is_some() { + return Ok(()) + } + + let ty = self.registry.resolve(id).ok_or(())?; + + let new_id = self.next_id(); + self.result.insert(id, new_id); + + // Add generic type params. + for param in ty.type_params() { + if let Some(ty) = param.ty() { + self.visit_type_id(ty.id())?; + } + } + + // Recursively visit any other type ids needed to represent this type. + match ty.type_def() { + TypeDef::Composite(composite) => { + for field in composite.fields() { + self.visit_type_id(field.ty().id())?; + } + } + TypeDef::Variant(variant) => { + for var in variant.variants() { + for field in var.fields() { + self.visit_type_id(field.ty().id())?; + } + } + } + TypeDef::Sequence(sequence) => { + self.visit_type_id(sequence.type_param().id())?; + } + TypeDef::Array(array) => { + self.visit_type_id(array.type_param().id())?; + } + TypeDef::Tuple(tuple) => { + for ty in tuple.fields() { + self.visit_type_id(ty.id())?; + } + } + TypeDef::Primitive(_) => {} + TypeDef::Compact(compact) => { + self.visit_type_id(compact.type_param().id())?; + } + TypeDef::BitSequence(bit_sequence) => { + self.visit_type_id(bit_sequence.bit_store_type().id())?; + self.visit_type_id(bit_sequence.bit_order_type().id())?; + } + } + + Ok(()) + } + + /// Resolve all the type IDs needed to express the given type IDs. + /// + /// The type IDs are returned as key to the `HashMap`. + /// The value of the `HashMap` represents the new ID of that type + /// if only the resolved types are expressed in the [`PortableRegistry`]. + fn resolve( + mut self, + ids: impl IntoIterator, + ) -> Result, ()> { + let ids: HashSet<_> = ids.into_iter().collect(); + for id in ids { + self.visit_type_id(id)?; + } + + Ok(self.result) + } +} + #[cfg(test)] mod tests { use super::*; From f18bafa1c04b59b2e6e2ec86e61b3e3e3a1eb354 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 8 Mar 2023 17:48:22 +0200 Subject: [PATCH 03/30] tests: Check recursive type ID correctness Signed-off-by: Alexandru Vasile --- src/portable.rs | 77 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/src/portable.rs b/src/portable.rs index 558af500..834750a3 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -321,4 +321,81 @@ mod tests { assert_eq!(Some(&vec_u32_type), registry.resolve(vec_u32_type_id)); assert_eq!(Some(&composite_type), registry.resolve(composite_type_id)); } + + #[test] + fn retain_ids() { + 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 u64_type = Type::new(Path::default(), vec![], TypeDefPrimitive::U64, vec![]); + let u64_type_id = builder.register_type(u64_type.clone()); + + let registry = builder.finish(); + assert_eq!(registry.types.len(), 2); + + // Resolve just u64. + let resolver = TypeIdResolver::new(®istry); + let result = resolver.resolve(vec![u64_type_id]).unwrap(); + assert_eq!(result.len(), 1); + // Make sure `u32_type_id` is not present. + assert!(!result.contains_key(&u32_type_id)); + + // `u64_type_id` should be mapped on id `0`. + assert_eq!(result.get(&u64_type_id).unwrap(), &0); + } + + #[test] + fn retain_recursive_ids() { + 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 u64_type = Type::new(Path::default(), vec![], TypeDefPrimitive::U64, vec![]); + let u64_type_id = builder.register_type(u64_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 composite_type = Type::builder_portable() + .path(Path::from_segments_unchecked(["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)), + ); + let composite_type_id = builder.register_type(composite_type.clone()); + + let composite_type_second = Type::builder_portable() + .path(Path::from_segments_unchecked(["MyStructSecond".into()])) + .composite( + Fields::named() + .field_portable(|f| f.name("vec_of_u32".into()).ty(vec_u32_type_id)) + .field_portable(|f| f.name("second".into()).ty(composite_type_id)), + ); + let composite_type_second_id = + builder.register_type(composite_type_second.clone()); + + let registry = builder.finish(); + + assert_eq!(registry.types.len(), 5); + + // Resolve just `MyStruct`. + let resolver = TypeIdResolver::new(®istry); + let result = resolver.resolve(vec![composite_type_second_id]).unwrap(); + assert_eq!(result.len(), 4); + // Make sure `u64_type_id` is not present. + assert!(!result.contains_key(&u64_type_id)); + + // `MyStructSecond` is the first id visited. + assert_eq!(result.get(&composite_type_second_id).unwrap(), &0); + assert_eq!(result.get(&vec_u32_type_id).unwrap(), &1); + assert_eq!(result.get(&u32_type_id).unwrap(), &2); + assert_eq!(result.get(&composite_type_id).unwrap(), &3); + } } From e5dc1e293d5127c23919dcbad2ed9f02e51d39d0 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 8 Mar 2023 18:00:06 +0200 Subject: [PATCH 04/30] portable: Implement `retain` on the `PortableRegistry` Signed-off-by: Alexandru Vasile --- src/portable.rs | 124 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 124 insertions(+) diff --git a/src/portable.rs b/src/portable.rs index 834750a3..44b843ae 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -76,6 +76,130 @@ impl PortableRegistry { pub fn types(&self) -> &[PortableType] { &self.types } + + /// Retains only the portable types needed to express the provided ids. + /// + /// The type IDs retained are returned as key to the `HashMap`. + /// The value of the `HashMap` represents the new ID of that type. + /// + /// # Note + /// + /// A given type ID can be expressed by nesting type IDs, such is the case + /// of a [`TypeDef::Composite`] and others. To retain a valid [`PortableRegistry`] + /// all the types needed to express an ID are retained. Therefore, the number of + /// elements expressed by the result is equal or greater than the number of + /// provided IDs. + pub fn retain( + &mut self, + ids: impl IntoIterator, + ) -> Result, ()> { + // Recursively visit all type ids needed to express the list of provided ids. + let resolver = TypeIdResolver::new(&self); + // Map of "old id" to "new id". + let ids_map = resolver.resolve(ids)?; + + // Sort the ids by their order in the new registry. + let mut ids_order: Vec<_> = ids_map.clone().into_iter().collect(); + ids_order.sort_by(|(_, lhs_new), (_, rhs_new)| lhs_new.cmp(rhs_new)); + + // We cannot construct directly a new `PortableRegistry` by registering + // the current types because they may contain recursive type ids + // that must be updated. + let mut types = Vec::with_capacity(ids_order.len()); + for (old_id, new_id) in ids_order.iter() { + let Some(ty) = self.types.get_mut(*old_id as usize) else { + return Err(()) + }; + + let mut ty = ty.clone(); + ty.id = *new_id; + self.update_type(&ids_map, &mut ty.ty)?; + types.push(ty); + } + + self.types = types; + + Ok(ids_map) + } + + /// Update all the type IDs composting the given type. + fn update_type( + &self, + ids_map: &HashMap, + ty: &mut Type, + ) -> Result<(), ()> { + for param in ty.type_params.iter_mut() { + let Some(ty) = param.ty() else { + continue + }; + + let Some(new_id) = ids_map.get(&ty.id()) else { + return Err(()) + }; + param.ty = Some(*new_id).map(Into::into); + } + + match &mut ty.type_def { + TypeDef::Composite(composite) => { + for field in composite.fields.iter_mut() { + let Some(new_id) = ids_map.get(&field.ty().id()) else { + return Err(()) + }; + field.ty = (*new_id).into(); + } + } + TypeDef::Variant(variant) => { + for var in variant.variants.iter_mut() { + for field in var.fields.iter_mut() { + let Some(new_id) = ids_map.get(&field.ty().id()) else { + return Err(()) + }; + field.ty = (*new_id).into(); + } + } + } + TypeDef::Sequence(sequence) => { + let Some(new_id) = ids_map.get(&sequence.type_param().id()) else { + return Err(()) + }; + sequence.type_param = (*new_id).into(); + } + TypeDef::Array(array) => { + let Some(new_id) = ids_map.get(&array.type_param().id()) else { + return Err(()) + }; + array.type_param = (*new_id).into(); + } + TypeDef::Tuple(tuple) => { + for ty in tuple.fields.iter_mut() { + let Some(new_id) = ids_map.get(&ty.id()) else { + return Err(()) + }; + *ty = (*new_id).into(); + } + } + TypeDef::Primitive(_) => (), + TypeDef::Compact(compact) => { + let Some(new_id) = ids_map.get(&compact.type_param().id()) else { + return Err(()) + }; + compact.type_param = (*new_id).into(); + } + TypeDef::BitSequence(bit_seq) => { + let Some(new_id) = ids_map.get(&bit_seq.bit_order_type().id()) else { + return Err(()) + }; + bit_seq.bit_order_type = (*new_id).into(); + + let Some(new_id) = ids_map.get(&bit_seq.bit_store_type().id()) else { + return Err(()) + }; + bit_seq.bit_store_type = (*new_id).into(); + } + }; + + Ok(()) + } } /// Represent a type in it's portable form. From 965579e483efadfa2fd59f60ab785baa16fdb972 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 8 Mar 2023 18:01:37 +0200 Subject: [PATCH 05/30] tests: Check `retain` method Signed-off-by: Alexandru Vasile --- src/portable.rs | 45 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/src/portable.rs b/src/portable.rs index 44b843ae..20d6f1cf 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -94,7 +94,7 @@ impl PortableRegistry { ids: impl IntoIterator, ) -> Result, ()> { // Recursively visit all type ids needed to express the list of provided ids. - let resolver = TypeIdResolver::new(&self); + let resolver = TypeIdResolver::new(self); // Map of "old id" to "new id". let ids_map = resolver.resolve(ids)?; @@ -455,7 +455,7 @@ mod tests { let u64_type = Type::new(Path::default(), vec![], TypeDefPrimitive::U64, vec![]); let u64_type_id = builder.register_type(u64_type.clone()); - let registry = builder.finish(); + let mut registry = builder.finish(); assert_eq!(registry.types.len(), 2); // Resolve just u64. @@ -467,6 +467,12 @@ mod tests { // `u64_type_id` should be mapped on id `0`. assert_eq!(result.get(&u64_type_id).unwrap(), &0); + + let expected_result = registry.retain(vec![u64_type_id]).unwrap(); + assert_eq!(expected_result, result); + assert_eq!(registry.types.len(), 1); + + assert_eq!(registry.resolve(0).unwrap(), &u64_type); } #[test] @@ -505,8 +511,7 @@ mod tests { let composite_type_second_id = builder.register_type(composite_type_second.clone()); - let registry = builder.finish(); - + let mut registry = builder.finish(); assert_eq!(registry.types.len(), 5); // Resolve just `MyStruct`. @@ -521,5 +526,37 @@ mod tests { assert_eq!(result.get(&vec_u32_type_id).unwrap(), &1); assert_eq!(result.get(&u32_type_id).unwrap(), &2); assert_eq!(result.get(&composite_type_id).unwrap(), &3); + + let expected_result = registry.retain(vec![composite_type_second_id]).unwrap(); + assert_eq!(expected_result, result); + assert_eq!(registry.types.len(), 4); + + // New type IDs are generated in DFS manner. + let expected_type = Type::builder_portable() + .path(Path::from_segments_unchecked(["MyStructSecond".into()])) + .composite( + Fields::named() + .field_portable(|f| f.name("vec_of_u32".into()).ty(1)) + .field_portable(|f| f.name("second".into()).ty(3)), + ); + assert_eq!(registry.resolve(0).unwrap(), &expected_type); + + let expected_type = Type::new( + Path::default(), + vec![], + TypeDefSequence::new(2.into()), + vec![], + ); + assert_eq!(registry.resolve(1).unwrap(), &expected_type); + assert_eq!(registry.resolve(2).unwrap(), &u32_type); + + let expected_type = Type::builder_portable() + .path(Path::from_segments_unchecked(["MyStruct".into()])) + .composite( + Fields::named() + .field_portable(|f| f.name("primitive".into()).ty(2)) + .field_portable(|f| f.name("vec_of_u32".into()).ty(1)), + ); + assert_eq!(registry.resolve(3).unwrap(), &expected_type); } } From d1e77735a7544557562f262cc7b3847fbc743251 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 8 Mar 2023 18:14:40 +0200 Subject: [PATCH 06/30] portable: Add error type Signed-off-by: Alexandru Vasile --- src/portable.rs | 99 +++++++++++++++++++++++++++++++------------------ 1 file changed, 63 insertions(+), 36 deletions(-) diff --git a/src/portable.rs b/src/portable.rs index 20d6f1cf..c58678bd 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -92,7 +92,7 @@ impl PortableRegistry { pub fn retain( &mut self, ids: impl IntoIterator, - ) -> Result, ()> { + ) -> Result, PortableRegistryError> { // Recursively visit all type ids needed to express the list of provided ids. let resolver = TypeIdResolver::new(self); // Map of "old id" to "new id". @@ -107,10 +107,10 @@ impl PortableRegistry { // that must be updated. let mut types = Vec::with_capacity(ids_order.len()); for (old_id, new_id) in ids_order.iter() { - let Some(ty) = self.types.get_mut(*old_id as usize) else { - return Err(()) - }; - + let ty = self + .types + .get_mut(*old_id as usize) + .ok_or(PortableRegistryError::MissingTypeID { id: *old_id })?; let mut ty = ty.clone(); ty.id = *new_id; self.update_type(&ids_map, &mut ty.ty)?; @@ -127,73 +127,87 @@ impl PortableRegistry { &self, ids_map: &HashMap, ty: &mut Type, - ) -> Result<(), ()> { + ) -> Result<(), PortableRegistryError> { for param in ty.type_params.iter_mut() { let Some(ty) = param.ty() else { continue }; - let Some(new_id) = ids_map.get(&ty.id()) else { - return Err(()) - }; + let new_id = ids_map + .get(&ty.id()) + .ok_or(PortableRegistryError::MissingTypeID { id: ty.id() })?; param.ty = Some(*new_id).map(Into::into); } match &mut ty.type_def { TypeDef::Composite(composite) => { for field in composite.fields.iter_mut() { - let Some(new_id) = ids_map.get(&field.ty().id()) else { - return Err(()) - }; + let new_id = ids_map.get(&field.ty().id()).ok_or( + PortableRegistryError::MissingTypeID { + id: field.ty().id(), + }, + )?; field.ty = (*new_id).into(); } } TypeDef::Variant(variant) => { for var in variant.variants.iter_mut() { for field in var.fields.iter_mut() { - let Some(new_id) = ids_map.get(&field.ty().id()) else { - return Err(()) - }; + let new_id = ids_map.get(&field.ty().id()).ok_or( + PortableRegistryError::MissingTypeID { + id: field.ty().id(), + }, + )?; field.ty = (*new_id).into(); } } } TypeDef::Sequence(sequence) => { - let Some(new_id) = ids_map.get(&sequence.type_param().id()) else { - return Err(()) - }; + let new_id = ids_map.get(&sequence.type_param().id()).ok_or( + PortableRegistryError::MissingTypeID { + id: sequence.type_param().id(), + }, + )?; sequence.type_param = (*new_id).into(); } TypeDef::Array(array) => { - let Some(new_id) = ids_map.get(&array.type_param().id()) else { - return Err(()) - }; + let new_id = ids_map.get(&array.type_param().id()).ok_or( + PortableRegistryError::MissingTypeID { + id: array.type_param().id(), + }, + )?; array.type_param = (*new_id).into(); } TypeDef::Tuple(tuple) => { for ty in tuple.fields.iter_mut() { - let Some(new_id) = ids_map.get(&ty.id()) else { - return Err(()) - }; + let new_id = ids_map + .get(&ty.id()) + .ok_or(PortableRegistryError::MissingTypeID { id: ty.id() })?; *ty = (*new_id).into(); } } TypeDef::Primitive(_) => (), TypeDef::Compact(compact) => { - let Some(new_id) = ids_map.get(&compact.type_param().id()) else { - return Err(()) - }; + let new_id = ids_map.get(&compact.type_param().id()).ok_or( + PortableRegistryError::MissingTypeID { + id: compact.type_param().id(), + }, + )?; compact.type_param = (*new_id).into(); } TypeDef::BitSequence(bit_seq) => { - let Some(new_id) = ids_map.get(&bit_seq.bit_order_type().id()) else { - return Err(()) - }; + let new_id = ids_map.get(&bit_seq.bit_order_type().id()).ok_or( + PortableRegistryError::MissingTypeID { + id: bit_seq.bit_order_type().id(), + }, + )?; bit_seq.bit_order_type = (*new_id).into(); - let Some(new_id) = ids_map.get(&bit_seq.bit_store_type().id()) else { - return Err(()) - }; + let new_id = ids_map.get(&bit_seq.bit_store_type().id()).ok_or( + PortableRegistryError::MissingTypeID { + id: bit_seq.bit_store_type().id(), + }, + )?; bit_seq.bit_store_type = (*new_id).into(); } }; @@ -202,6 +216,16 @@ impl PortableRegistry { } } +/// An error that may be encountered upon using the portable registry. +#[derive(PartialEq, Eq, Debug)] +pub enum PortableRegistryError { + /// The type ID is not present in the registry. + MissingTypeID { + /// The type ID that is missing. + id: u32, + }, +} + /// Represent a type in it's portable form. #[cfg_attr(feature = "serde", derive(serde::Serialize))] #[cfg_attr(all(feature = "serde", feature = "decode"), derive(serde::Deserialize))] @@ -306,12 +330,15 @@ impl<'a> TypeIdResolver<'a> { } /// Recursively add all type ids needed to express the given identifier. - fn visit_type_id(&mut self, id: u32) -> Result<(), ()> { + fn visit_type_id(&mut self, id: u32) -> Result<(), PortableRegistryError> { if self.result.get(&id).is_some() { return Ok(()) } - let ty = self.registry.resolve(id).ok_or(())?; + let ty = self + .registry + .resolve(id) + .ok_or(PortableRegistryError::MissingTypeID { id })?; let new_id = self.next_id(); self.result.insert(id, new_id); @@ -369,7 +396,7 @@ impl<'a> TypeIdResolver<'a> { fn resolve( mut self, ids: impl IntoIterator, - ) -> Result, ()> { + ) -> Result, PortableRegistryError> { let ids: HashSet<_> = ids.into_iter().collect(); for id in ids { self.visit_type_id(id)?; From 49be10dfca90134593e3f620710513f3fd2b2d3f Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 8 Mar 2023 18:36:05 +0200 Subject: [PATCH 07/30] derive: Fix clippy and improve documentation Signed-off-by: Alexandru Vasile --- derive/src/trait_bounds.rs | 2 +- src/portable.rs | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/derive/src/trait_bounds.rs b/derive/src/trait_bounds.rs index 08fb1ec0..da28f486 100644 --- a/derive/src/trait_bounds.rs +++ b/derive/src/trait_bounds.rs @@ -104,7 +104,7 @@ pub fn make_where_clause<'a>( } }); - generics.type_params().into_iter().for_each(|type_param| { + generics.type_params().for_each(|type_param| { let ident = type_param.ident.clone(); let mut bounds = type_param.bounds.clone(); if attrs diff --git a/src/portable.rs b/src/portable.rs index c58678bd..14e29ba1 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -84,11 +84,10 @@ impl PortableRegistry { /// /// # Note /// - /// A given type ID can be expressed by nesting type IDs, such is the case + /// A given type ID can be defined by nesting type IDs, such as the case /// of a [`TypeDef::Composite`] and others. To retain a valid [`PortableRegistry`] - /// all the types needed to express an ID are retained. Therefore, the number of - /// elements expressed by the result is equal or greater than the number of - /// provided IDs. + /// all the types needed to express an ID are included. Therefore, the number of + /// elements defined by the result equals or exceeds the number of provided IDs. pub fn retain( &mut self, ids: impl IntoIterator, From f1a378b1ea6849de7fd5dd47c485738d19d89bec Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 9 Mar 2023 13:09:00 +0200 Subject: [PATCH 08/30] Fix clippy Signed-off-by: Alexandru Vasile --- src/utils.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/utils.rs b/src/utils.rs index 88f76cf2..ba3d54e0 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -23,14 +23,13 @@ pub fn is_rust_identifier(s: &str) -> bool { let trimmed = s.trim_start_matches("r#"); if let Some((&head, tail)) = trimmed.as_bytes().split_first() { // Check if head and tail make up a proper Rust identifier. - let head_ok = head == b'_' - || (b'a'..=b'z').contains(&head) - || (b'A'..=b'Z').contains(&head); + let head_ok = + head == b'_' || head.is_ascii_lowercase() || head.is_ascii_uppercase(); let tail_ok = tail.iter().all(|&ch| { ch == b'_' - || (b'a'..=b'z').contains(&ch) - || (b'A'..=b'Z').contains(&ch) - || (b'0'..=b'9').contains(&ch) + || ch.is_ascii_lowercase() + || ch.is_ascii_uppercase() + || ch.is_ascii_digit() }); head_ok && tail_ok } else { From 3fdf492dd2ad5729edaec5c85dcbb90c706d194f Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 9 Mar 2023 13:44:11 +0200 Subject: [PATCH 09/30] rustfmt: Change deprecated fmt Signed-off-by: Alexandru Vasile --- .rustfmt.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.rustfmt.toml b/.rustfmt.toml index 536703e6..33407627 100644 --- a/.rustfmt.toml +++ b/.rustfmt.toml @@ -34,7 +34,7 @@ struct_field_align_threshold = 0 enum_discrim_align_threshold = 0 match_arm_blocks = true force_multiline_blocks = true # changed -fn_args_layout = "Tall" +fn_params_layout = "Tall" brace_style = "SameLineWhere" control_brace_style = "AlwaysSameLine" trailing_semicolon = false # changed From 867b926ad5e23bbb31c93dea0bbfceda1f137a5f Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 9 Mar 2023 13:47:29 +0200 Subject: [PATCH 10/30] portable: Ensure no-std works Signed-off-by: Alexandru Vasile --- src/portable.rs | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/src/portable.rs b/src/portable.rs index 14e29ba1..1270efe3 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -27,10 +27,7 @@ use crate::{ form::PortableForm, interner::Interner, prelude::{ - collections::{ - HashMap, - HashSet, - }, + collections::BTreeMap, fmt::Debug, vec::Vec, }, @@ -79,8 +76,13 @@ impl PortableRegistry { /// Retains only the portable types needed to express the provided ids. /// - /// The type IDs retained are returned as key to the `HashMap`. - /// The value of the `HashMap` represents the new ID of that type. + /// Returns the type IDs that have been retained. + /// The order of the type IDs in the returned vector corresponds to their + /// new positions in the type registry after filtering. + /// + /// For instance, if the function returns the vector [30, 10, 20], it means that + /// type ID 30 is now at position 0 and has the type ID 0 in the registry, + /// type ID 10 is now type ID 1, and type ID 20 is now type ID 2. /// /// # Note /// @@ -91,10 +93,8 @@ impl PortableRegistry { pub fn retain( &mut self, ids: impl IntoIterator, - ) -> Result, PortableRegistryError> { - // Recursively visit all type ids needed to express the list of provided ids. + ) -> Result, PortableRegistryError> { let resolver = TypeIdResolver::new(self); - // Map of "old id" to "new id". let ids_map = resolver.resolve(ids)?; // Sort the ids by their order in the new registry. @@ -116,15 +116,16 @@ impl PortableRegistry { types.push(ty); } + let ids_order: Vec<_> = ids_order.into_iter().map(|(old, _)| old).collect(); self.types = types; - Ok(ids_map) + Ok(ids_order) } /// Update all the type IDs composting the given type. fn update_type( &self, - ids_map: &HashMap, + ids_map: &BTreeMap, ty: &mut Type, ) -> Result<(), PortableRegistryError> { for param in ty.type_params.iter_mut() { @@ -308,7 +309,7 @@ impl PortableRegistryBuilder { /// Recursive resolver for the type IDs needed to express a given type ID. struct TypeIdResolver<'a> { registry: &'a PortableRegistry, - result: HashMap, + result: BTreeMap, next_id: AtomicU32, } @@ -395,9 +396,8 @@ impl<'a> TypeIdResolver<'a> { fn resolve( mut self, ids: impl IntoIterator, - ) -> Result, PortableRegistryError> { - let ids: HashSet<_> = ids.into_iter().collect(); - for id in ids { + ) -> Result, PortableRegistryError> { + for id in ids.into_iter() { self.visit_type_id(id)?; } @@ -495,6 +495,10 @@ mod tests { assert_eq!(result.get(&u64_type_id).unwrap(), &0); let expected_result = registry.retain(vec![u64_type_id]).unwrap(); + let mut result: Vec<_> = result.into_iter().collect(); + result.sort_by(|(_, lhs_new), (_, rhs_new)| lhs_new.cmp(rhs_new)); + let result: Vec<_> = result.into_iter().map(|(old, _)| old).collect(); + assert_eq!(expected_result, result); assert_eq!(registry.types.len(), 1); @@ -554,7 +558,11 @@ mod tests { assert_eq!(result.get(&composite_type_id).unwrap(), &3); let expected_result = registry.retain(vec![composite_type_second_id]).unwrap(); + let mut result: Vec<_> = result.into_iter().collect(); + result.sort_by(|(_, lhs_new), (_, rhs_new)| lhs_new.cmp(rhs_new)); + let result: Vec<_> = result.into_iter().map(|(old, _)| old).collect(); assert_eq!(expected_result, result); + assert_eq!(registry.types.len(), 4); // New type IDs are generated in DFS manner. From 7241cc879e6ce3191d3b33440144b6e7757ea517 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 9 Mar 2023 13:52:18 +0200 Subject: [PATCH 11/30] test_suite: Adjust UI tests to latest Signed-off-by: Alexandru Vasile --- test_suite/tests/ui/fail_duplicate_bounds_params.stderr | 2 +- test_suite/tests/ui/fail_with_invalid_codec_attrs.stderr | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test_suite/tests/ui/fail_duplicate_bounds_params.stderr b/test_suite/tests/ui/fail_duplicate_bounds_params.stderr index cd76cddf..7c3d8be9 100644 --- a/test_suite/tests/ui/fail_duplicate_bounds_params.stderr +++ b/test_suite/tests/ui/fail_duplicate_bounds_params.stderr @@ -2,4 +2,4 @@ error: Duplicate `bounds` attributes --> tests/ui/fail_duplicate_bounds_params.rs:6:1 | 6 | #[scale_info(bounds(), bounds())] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^ diff --git a/test_suite/tests/ui/fail_with_invalid_codec_attrs.stderr b/test_suite/tests/ui/fail_with_invalid_codec_attrs.stderr index 90d52022..6246d1c9 100644 --- a/test_suite/tests/ui/fail_with_invalid_codec_attrs.stderr +++ b/test_suite/tests/ui/fail_with_invalid_codec_attrs.stderr @@ -2,13 +2,13 @@ error: Invalid attribute on field, only `#[codec(skip)]`, `#[codec(compact)]` an --> tests/ui/fail_with_invalid_codec_attrs.rs:8:7 | 8 | #[codec(skip, compact)] - | ^^^^^^^^^^^^^^^^^^^^ + | ^^^^^ error: Invalid attribute on field, only `#[codec(skip)]`, `#[codec(compact)]` and `#[codec(encoded_as = "$EncodeAs")]` are accepted. --> tests/ui/fail_with_invalid_codec_attrs.rs:14:19 | 14 | Thing(#[codec(index = 3)] u32), - | ^^^^^^^^^ + | ^^^^^ error: expected literal --> tests/ui/fail_with_invalid_codec_attrs.rs:19:21 From 8ee93dbf378757febbbd9a040a29e7372bbbd68d Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 9 Mar 2023 13:56:26 +0200 Subject: [PATCH 12/30] Adjust UI tests with nightly Signed-off-by: Alexandru Vasile --- test_suite/tests/ui/fail_duplicate_bounds_params.stderr | 2 +- test_suite/tests/ui/fail_missing_derive.stderr | 4 ++-- test_suite/tests/ui/fail_with_invalid_codec_attrs.stderr | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test_suite/tests/ui/fail_duplicate_bounds_params.stderr b/test_suite/tests/ui/fail_duplicate_bounds_params.stderr index 7c3d8be9..cd76cddf 100644 --- a/test_suite/tests/ui/fail_duplicate_bounds_params.stderr +++ b/test_suite/tests/ui/fail_duplicate_bounds_params.stderr @@ -2,4 +2,4 @@ error: Duplicate `bounds` attributes --> tests/ui/fail_duplicate_bounds_params.rs:6:1 | 6 | #[scale_info(bounds(), bounds())] - | ^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/test_suite/tests/ui/fail_missing_derive.stderr b/test_suite/tests/ui/fail_missing_derive.stderr index 9207cb25..4183c20e 100644 --- a/test_suite/tests/ui/fail_missing_derive.stderr +++ b/test_suite/tests/ui/fail_missing_derive.stderr @@ -13,12 +13,12 @@ error[E0277]: the trait bound `PawType: TypeInfo` is not satisfied (A, B, C, D) (A, B, C, D, E) (A, B, C, D, E, F) - and 54 others + and $N others note: required for `Cat` to implement `TypeInfo` --> tests/ui/fail_missing_derive.rs:8:10 | 8 | #[derive(TypeInfo)] - | ^^^^^^^^ + | ^^^^^^^^ unsatisfied trait bound introduced in this `derive` macro 9 | #[scale_info(crate = info)] 10 | struct Cat { | ^^^^^^^^^^^^^^^^^^^ diff --git a/test_suite/tests/ui/fail_with_invalid_codec_attrs.stderr b/test_suite/tests/ui/fail_with_invalid_codec_attrs.stderr index 6246d1c9..90d52022 100644 --- a/test_suite/tests/ui/fail_with_invalid_codec_attrs.stderr +++ b/test_suite/tests/ui/fail_with_invalid_codec_attrs.stderr @@ -2,13 +2,13 @@ error: Invalid attribute on field, only `#[codec(skip)]`, `#[codec(compact)]` an --> tests/ui/fail_with_invalid_codec_attrs.rs:8:7 | 8 | #[codec(skip, compact)] - | ^^^^^ + | ^^^^^^^^^^^^^^^^^^^^ error: Invalid attribute on field, only `#[codec(skip)]`, `#[codec(compact)]` and `#[codec(encoded_as = "$EncodeAs")]` are accepted. --> tests/ui/fail_with_invalid_codec_attrs.rs:14:19 | 14 | Thing(#[codec(index = 3)] u32), - | ^^^^^ + | ^^^^^^^^^ error: expected literal --> tests/ui/fail_with_invalid_codec_attrs.rs:19:21 From 42649694f2cede938c3e27a8d7ae96062927211b Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 13 Mar 2023 15:25:00 +0200 Subject: [PATCH 13/30] portable: Remove AtomicU32 Signed-off-by: Alexandru Vasile --- src/portable.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/portable.rs b/src/portable.rs index 1270efe3..b7828107 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -35,7 +35,6 @@ use crate::{ Type, TypeDef, }; -use core::sync::atomic::AtomicU32; use scale::Encode; /// A read-only registry containing types in their portable form for serialization. @@ -310,7 +309,7 @@ impl PortableRegistryBuilder { struct TypeIdResolver<'a> { registry: &'a PortableRegistry, result: BTreeMap, - next_id: AtomicU32, + next_id: u32, } impl<'a> TypeIdResolver<'a> { @@ -325,8 +324,9 @@ impl<'a> TypeIdResolver<'a> { /// Get the next unique ID. fn next_id(&mut self) -> u32 { - self.next_id - .fetch_add(1, core::sync::atomic::Ordering::Relaxed) + let id = self.next_id; + self.next_id += 1; + id } /// Recursively add all type ids needed to express the given identifier. From 45495552e6a2ea49131b5b5c8587ec1835534fc0 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 13 Mar 2023 16:18:45 +0200 Subject: [PATCH 14/30] portable: Use DFS for recursive ID assignment Signed-off-by: Alexandru Vasile --- src/portable.rs | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/portable.rs b/src/portable.rs index b7828107..565fb514 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -340,9 +340,6 @@ impl<'a> TypeIdResolver<'a> { .resolve(id) .ok_or(PortableRegistryError::MissingTypeID { id })?; - let new_id = self.next_id(); - self.result.insert(id, new_id); - // Add generic type params. for param in ty.type_params() { if let Some(ty) = param.ty() { @@ -385,6 +382,9 @@ impl<'a> TypeIdResolver<'a> { } } + let new_id = self.next_id(); + self.result.insert(id, new_id); + Ok(()) } @@ -551,11 +551,10 @@ mod tests { // Make sure `u64_type_id` is not present. assert!(!result.contains_key(&u64_type_id)); - // `MyStructSecond` is the first id visited. - assert_eq!(result.get(&composite_type_second_id).unwrap(), &0); + assert_eq!(result.get(&u32_type_id).unwrap(), &0); assert_eq!(result.get(&vec_u32_type_id).unwrap(), &1); - assert_eq!(result.get(&u32_type_id).unwrap(), &2); - assert_eq!(result.get(&composite_type_id).unwrap(), &3); + assert_eq!(result.get(&composite_type_id).unwrap(), &2); + assert_eq!(result.get(&composite_type_second_id).unwrap(), &3); let expected_result = registry.retain(vec![composite_type_second_id]).unwrap(); let mut result: Vec<_> = result.into_iter().collect(); @@ -566,31 +565,32 @@ mod tests { assert_eq!(registry.types.len(), 4); // New type IDs are generated in DFS manner. - let expected_type = Type::builder_portable() - .path(Path::from_segments_unchecked(["MyStructSecond".into()])) - .composite( - Fields::named() - .field_portable(|f| f.name("vec_of_u32".into()).ty(1)) - .field_portable(|f| f.name("second".into()).ty(3)), - ); - assert_eq!(registry.resolve(0).unwrap(), &expected_type); + assert_eq!(registry.resolve(0).unwrap(), &u32_type); let expected_type = Type::new( Path::default(), vec![], - TypeDefSequence::new(2.into()), + TypeDefSequence::new(0.into()), vec![], ); assert_eq!(registry.resolve(1).unwrap(), &expected_type); - assert_eq!(registry.resolve(2).unwrap(), &u32_type); let expected_type = Type::builder_portable() .path(Path::from_segments_unchecked(["MyStruct".into()])) .composite( Fields::named() - .field_portable(|f| f.name("primitive".into()).ty(2)) + .field_portable(|f| f.name("primitive".into()).ty(0)) .field_portable(|f| f.name("vec_of_u32".into()).ty(1)), ); + assert_eq!(registry.resolve(2).unwrap(), &expected_type); + + let expected_type = Type::builder_portable() + .path(Path::from_segments_unchecked(["MyStructSecond".into()])) + .composite( + Fields::named() + .field_portable(|f| f.name("vec_of_u32".into()).ty(1)) + .field_portable(|f| f.name("second".into()).ty(2)), + ); assert_eq!(registry.resolve(3).unwrap(), &expected_type); } } From 5d70abb81014ce343ebeb09c992340d96c013364 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 17 Mar 2023 13:30:36 +0200 Subject: [PATCH 15/30] ty: Implement zero-alloc Default::default() for Type Signed-off-by: Alexandru Vasile --- src/ty/mod.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/ty/mod.rs b/src/ty/mod.rs index 2f79a7d0..77914f81 100644 --- a/src/ty/mod.rs +++ b/src/ty/mod.rs @@ -86,6 +86,22 @@ pub struct Type { pub docs: Vec, } +impl Default for Type +where + T: Form, +{ + // Zero-allocation default implementation that assumes + // the type def to be `TypeDefPrimitive::Bool`. + fn default() -> Type { + Type { + type_def: TypeDef::Primitive(TypeDefPrimitive::Bool), + path: Path::default(), + type_params: vec![], + docs: vec![], + } + } +} + impl IntoPortable for Type { type Output = Type; From 5071adfd7e620d8840f80e06b46f035c475255c8 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 17 Mar 2023 13:43:22 +0200 Subject: [PATCH 16/30] portable: Change `retain` signature with FnMut Signed-off-by: Alexandru Vasile --- src/portable.rs | 200 ++++++++++++++++++++---------------------------- 1 file changed, 85 insertions(+), 115 deletions(-) diff --git a/src/portable.rs b/src/portable.rs index 565fb514..59f6e513 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -89,12 +89,17 @@ impl PortableRegistry { /// of a [`TypeDef::Composite`] and others. To retain a valid [`PortableRegistry`] /// all the types needed to express an ID are included. Therefore, the number of /// elements defined by the result equals or exceeds the number of provided IDs. - pub fn retain( - &mut self, - ids: impl IntoIterator, - ) -> Result, PortableRegistryError> { - let resolver = TypeIdResolver::new(self); - let ids_map = resolver.resolve(ids)?; + pub fn retain(&mut self, mut filter: F, mut on_retained: R) + where + F: FnMut(&u32) -> bool, + R: FnMut(&u32, &u32), + { + let ids = self + .types + .iter() + .filter_map(|ty| filter(&ty.id).then_some(ty.id)); + + let ids_map = TypeIdResolver::new(self).resolve(ids); // Sort the ids by their order in the new registry. let mut ids_order: Vec<_> = ids_map.clone().into_iter().collect(); @@ -105,131 +110,97 @@ impl PortableRegistry { // that must be updated. let mut types = Vec::with_capacity(ids_order.len()); for (old_id, new_id) in ids_order.iter() { - let ty = self - .types - .get_mut(*old_id as usize) - .ok_or(PortableRegistryError::MissingTypeID { id: *old_id })?; - let mut ty = ty.clone(); + let Some(ty) = self.types.get_mut(*old_id as usize) else { + continue; + }; + on_retained(old_id, new_id); + + let mut ty = std::mem::take(ty); ty.id = *new_id; - self.update_type(&ids_map, &mut ty.ty)?; + self.update_type(&ids_map, &mut ty.ty); types.push(ty); } - let ids_order: Vec<_> = ids_order.into_iter().map(|(old, _)| old).collect(); self.types = types; - - Ok(ids_order) } /// Update all the type IDs composting the given type. - fn update_type( - &self, - ids_map: &BTreeMap, - ty: &mut Type, - ) -> Result<(), PortableRegistryError> { + fn update_type(&self, ids_map: &BTreeMap, ty: &mut Type) { for param in ty.type_params.iter_mut() { let Some(ty) = param.ty() else { continue }; - - let new_id = ids_map - .get(&ty.id()) - .ok_or(PortableRegistryError::MissingTypeID { id: ty.id() })?; + let Some(new_id) = ids_map.get(&ty.id()) else { + continue + }; param.ty = Some(*new_id).map(Into::into); } match &mut ty.type_def { TypeDef::Composite(composite) => { for field in composite.fields.iter_mut() { - let new_id = ids_map.get(&field.ty().id()).ok_or( - PortableRegistryError::MissingTypeID { - id: field.ty().id(), - }, - )?; + let Some(new_id) = ids_map.get(&field.ty().id()) else { + return; + }; field.ty = (*new_id).into(); } } TypeDef::Variant(variant) => { for var in variant.variants.iter_mut() { for field in var.fields.iter_mut() { - let new_id = ids_map.get(&field.ty().id()).ok_or( - PortableRegistryError::MissingTypeID { - id: field.ty().id(), - }, - )?; + let Some(new_id) = ids_map.get(&field.ty().id()) else { + return; + }; field.ty = (*new_id).into(); } } } TypeDef::Sequence(sequence) => { - let new_id = ids_map.get(&sequence.type_param().id()).ok_or( - PortableRegistryError::MissingTypeID { - id: sequence.type_param().id(), - }, - )?; + let Some(new_id) = ids_map.get(&sequence.type_param().id()) else { + return; + }; sequence.type_param = (*new_id).into(); } TypeDef::Array(array) => { - let new_id = ids_map.get(&array.type_param().id()).ok_or( - PortableRegistryError::MissingTypeID { - id: array.type_param().id(), - }, - )?; + let Some(new_id) = ids_map.get(&array.type_param().id()) else { + return; + }; array.type_param = (*new_id).into(); } TypeDef::Tuple(tuple) => { for ty in tuple.fields.iter_mut() { - let new_id = ids_map - .get(&ty.id()) - .ok_or(PortableRegistryError::MissingTypeID { id: ty.id() })?; + let Some(new_id) = ids_map.get(&ty.id()) else { + return; + }; *ty = (*new_id).into(); } } TypeDef::Primitive(_) => (), TypeDef::Compact(compact) => { - let new_id = ids_map.get(&compact.type_param().id()).ok_or( - PortableRegistryError::MissingTypeID { - id: compact.type_param().id(), - }, - )?; + let Some(new_id) = ids_map.get(&compact.type_param().id()) else { + return; + }; compact.type_param = (*new_id).into(); } TypeDef::BitSequence(bit_seq) => { - let new_id = ids_map.get(&bit_seq.bit_order_type().id()).ok_or( - PortableRegistryError::MissingTypeID { - id: bit_seq.bit_order_type().id(), - }, - )?; - bit_seq.bit_order_type = (*new_id).into(); - - let new_id = ids_map.get(&bit_seq.bit_store_type().id()).ok_or( - PortableRegistryError::MissingTypeID { - id: bit_seq.bit_store_type().id(), - }, - )?; - bit_seq.bit_store_type = (*new_id).into(); + let Some(bit_order_id) = ids_map.get(&bit_seq.bit_order_type().id()) else { + return; + }; + let Some(bit_store_id) = ids_map.get(&bit_seq.bit_store_type().id()) else { + return; + }; + bit_seq.bit_order_type = (*bit_order_id).into(); + bit_seq.bit_store_type = (*bit_store_id).into(); } }; - - Ok(()) } } -/// An error that may be encountered upon using the portable registry. -#[derive(PartialEq, Eq, Debug)] -pub enum PortableRegistryError { - /// The type ID is not present in the registry. - MissingTypeID { - /// The type ID that is missing. - id: u32, - }, -} - /// Represent a type in it's portable form. #[cfg_attr(feature = "serde", derive(serde::Serialize))] #[cfg_attr(all(feature = "serde", feature = "decode"), derive(serde::Deserialize))] #[cfg_attr(any(feature = "std", feature = "decode"), derive(scale::Decode))] -#[derive(Clone, Debug, PartialEq, Eq, Encode)] +#[derive(Clone, Debug, PartialEq, Eq, Encode, Default)] pub struct PortableType { #[codec(compact)] id: u32, @@ -330,20 +301,19 @@ impl<'a> TypeIdResolver<'a> { } /// Recursively add all type ids needed to express the given identifier. - fn visit_type_id(&mut self, id: u32) -> Result<(), PortableRegistryError> { + fn visit_type_id(&mut self, id: u32) { if self.result.get(&id).is_some() { - return Ok(()) + return } - let ty = self - .registry - .resolve(id) - .ok_or(PortableRegistryError::MissingTypeID { id })?; + let Some(ty) = self.registry.resolve(id) else { + return + }; // Add generic type params. for param in ty.type_params() { if let Some(ty) = param.ty() { - self.visit_type_id(ty.id())?; + self.visit_type_id(ty.id()); } } @@ -351,41 +321,39 @@ impl<'a> TypeIdResolver<'a> { match ty.type_def() { TypeDef::Composite(composite) => { for field in composite.fields() { - self.visit_type_id(field.ty().id())?; + self.visit_type_id(field.ty().id()); } } TypeDef::Variant(variant) => { for var in variant.variants() { for field in var.fields() { - self.visit_type_id(field.ty().id())?; + self.visit_type_id(field.ty().id()); } } } TypeDef::Sequence(sequence) => { - self.visit_type_id(sequence.type_param().id())?; + self.visit_type_id(sequence.type_param().id()); } TypeDef::Array(array) => { - self.visit_type_id(array.type_param().id())?; + self.visit_type_id(array.type_param().id()); } TypeDef::Tuple(tuple) => { for ty in tuple.fields() { - self.visit_type_id(ty.id())?; + self.visit_type_id(ty.id()); } } TypeDef::Primitive(_) => {} TypeDef::Compact(compact) => { - self.visit_type_id(compact.type_param().id())?; + self.visit_type_id(compact.type_param().id()); } TypeDef::BitSequence(bit_sequence) => { - self.visit_type_id(bit_sequence.bit_store_type().id())?; - self.visit_type_id(bit_sequence.bit_order_type().id())?; + self.visit_type_id(bit_sequence.bit_store_type().id()); + self.visit_type_id(bit_sequence.bit_order_type().id()); } } let new_id = self.next_id(); self.result.insert(id, new_id); - - Ok(()) } /// Resolve all the type IDs needed to express the given type IDs. @@ -393,15 +361,12 @@ impl<'a> TypeIdResolver<'a> { /// The type IDs are returned as key to the `HashMap`. /// The value of the `HashMap` represents the new ID of that type /// if only the resolved types are expressed in the [`PortableRegistry`]. - fn resolve( - mut self, - ids: impl IntoIterator, - ) -> Result, PortableRegistryError> { + fn resolve(mut self, ids: impl IntoIterator) -> BTreeMap { for id in ids.into_iter() { - self.visit_type_id(id)?; + self.visit_type_id(id); } - Ok(self.result) + self.result } } @@ -485,8 +450,7 @@ mod tests { assert_eq!(registry.types.len(), 2); // Resolve just u64. - let resolver = TypeIdResolver::new(®istry); - let result = resolver.resolve(vec![u64_type_id]).unwrap(); + let result = TypeIdResolver::new(®istry).resolve(vec![u64_type_id]); assert_eq!(result.len(), 1); // Make sure `u32_type_id` is not present. assert!(!result.contains_key(&u32_type_id)); @@ -494,12 +458,15 @@ mod tests { // `u64_type_id` should be mapped on id `0`. assert_eq!(result.get(&u64_type_id).unwrap(), &0); - let expected_result = registry.retain(vec![u64_type_id]).unwrap(); - let mut result: Vec<_> = result.into_iter().collect(); - result.sort_by(|(_, lhs_new), (_, rhs_new)| lhs_new.cmp(rhs_new)); - let result: Vec<_> = result.into_iter().map(|(old, _)| old).collect(); + let mut ids_result = BTreeMap::new(); + registry.retain( + |id| *id == u64_type_id, + |old, new| { + ids_result.insert(*old, *new); + }, + ); - assert_eq!(expected_result, result); + assert_eq!(ids_result, result); assert_eq!(registry.types.len(), 1); assert_eq!(registry.resolve(0).unwrap(), &u64_type); @@ -545,8 +512,8 @@ mod tests { assert_eq!(registry.types.len(), 5); // Resolve just `MyStruct`. - let resolver = TypeIdResolver::new(®istry); - let result = resolver.resolve(vec![composite_type_second_id]).unwrap(); + let result = + TypeIdResolver::new(®istry).resolve(vec![composite_type_second_id]); assert_eq!(result.len(), 4); // Make sure `u64_type_id` is not present. assert!(!result.contains_key(&u64_type_id)); @@ -556,11 +523,14 @@ mod tests { assert_eq!(result.get(&composite_type_id).unwrap(), &2); assert_eq!(result.get(&composite_type_second_id).unwrap(), &3); - let expected_result = registry.retain(vec![composite_type_second_id]).unwrap(); - let mut result: Vec<_> = result.into_iter().collect(); - result.sort_by(|(_, lhs_new), (_, rhs_new)| lhs_new.cmp(rhs_new)); - let result: Vec<_> = result.into_iter().map(|(old, _)| old).collect(); - assert_eq!(expected_result, result); + let mut ids_result = BTreeMap::new(); + registry.retain( + |id| *id == composite_type_second_id, + |old, new| { + ids_result.insert(*old, *new); + }, + ); + assert_eq!(ids_result, result); assert_eq!(registry.types.len(), 4); From 863f06ba24b450ed57fcd98d9b541c64585af784 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 17 Mar 2023 13:44:17 +0200 Subject: [PATCH 17/30] portable/tests: Apply clippy Signed-off-by: Alexandru Vasile --- src/portable.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/portable.rs b/src/portable.rs index 59f6e513..e21ec876 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -441,7 +441,7 @@ mod tests { fn retain_ids() { 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 u32_type_id = builder.register_type(u32_type); let u64_type = Type::new(Path::default(), vec![], TypeDefPrimitive::U64, vec![]); let u64_type_id = builder.register_type(u64_type.clone()); @@ -479,7 +479,7 @@ mod tests { let u32_type_id = builder.register_type(u32_type.clone()); let u64_type = Type::new(Path::default(), vec![], TypeDefPrimitive::U64, vec![]); - let u64_type_id = builder.register_type(u64_type.clone()); + let u64_type_id = builder.register_type(u64_type); let vec_u32_type = Type::new( Path::default(), @@ -487,7 +487,7 @@ mod tests { TypeDefSequence::new(u32_type_id.into()), vec![], ); - let vec_u32_type_id = builder.register_type(vec_u32_type.clone()); + let vec_u32_type_id = builder.register_type(vec_u32_type); let composite_type = Type::builder_portable() .path(Path::from_segments_unchecked(["MyStruct".into()])) @@ -496,7 +496,7 @@ mod tests { .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)), ); - let composite_type_id = builder.register_type(composite_type.clone()); + let composite_type_id = builder.register_type(composite_type); let composite_type_second = Type::builder_portable() .path(Path::from_segments_unchecked(["MyStructSecond".into()])) @@ -505,8 +505,7 @@ mod tests { .field_portable(|f| f.name("vec_of_u32".into()).ty(vec_u32_type_id)) .field_portable(|f| f.name("second".into()).ty(composite_type_id)), ); - let composite_type_second_id = - builder.register_type(composite_type_second.clone()); + let composite_type_second_id = builder.register_type(composite_type_second); let mut registry = builder.finish(); assert_eq!(registry.types.len(), 5); From d5123ea06699622b50d469c3bc5c0206f88c0f1f Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 17 Mar 2023 13:47:30 +0200 Subject: [PATCH 18/30] git: Use clippy with `--all-targets` Signed-off-by: Alexandru Vasile --- .github/workflows/rust.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index d88de063..b875a59a 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -31,7 +31,7 @@ jobs: - name: clippy run: | cargo clippy --version - cargo clippy --all -- -D warnings + cargo clippy --all-targets -- -D warnings - name: check-all-features run: | From ee3ae6de980eebfa9ec0da698396abd0d59afd4a Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 17 Mar 2023 14:00:47 +0200 Subject: [PATCH 19/30] ty: Make fields public to this crate `pub(crate)` only Signed-off-by: Alexandru Vasile --- src/ty/composite.rs | 2 +- src/ty/fields.rs | 8 ++++---- src/ty/mod.rs | 26 +++++++++++++------------- src/ty/variant.rs | 2 +- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/ty/composite.rs b/src/ty/composite.rs index 8815966b..26d8852a 100644 --- a/src/ty/composite.rs +++ b/src/ty/composite.rs @@ -76,7 +76,7 @@ pub struct TypeDefComposite { feature = "serde", serde(skip_serializing_if = "Vec::is_empty", default) )] - pub fields: Vec>, + pub(crate) fields: Vec>, } impl IntoPortable for TypeDefComposite { diff --git a/src/ty/fields.rs b/src/ty/fields.rs index 7b71faeb..5d8d0029 100644 --- a/src/ty/fields.rs +++ b/src/ty/fields.rs @@ -78,22 +78,22 @@ pub struct Field { feature = "serde", serde(skip_serializing_if = "Option::is_none", default) )] - pub name: Option, + pub(crate) name: Option, /// The type of the field. #[cfg_attr(feature = "serde", serde(rename = "type"))] - pub ty: T::Type, + pub(crate) ty: T::Type, /// The name of the type of the field as it appears in the source code. #[cfg_attr( feature = "serde", serde(skip_serializing_if = "Option::is_none", default) )] - pub type_name: Option, + pub(crate) type_name: Option, /// Documentation #[cfg_attr( feature = "serde", serde(skip_serializing_if = "Vec::is_empty", default) )] - pub docs: Vec, + pub(crate) docs: Vec, } impl IntoPortable for Field { diff --git a/src/ty/mod.rs b/src/ty/mod.rs index 77914f81..9969c33e 100644 --- a/src/ty/mod.rs +++ b/src/ty/mod.rs @@ -68,22 +68,22 @@ pub struct Type { feature = "serde", serde(skip_serializing_if = "Path::is_empty", default) )] - pub path: Path, + pub(crate) path: Path, /// The generic type parameters of the type in use. Empty for non generic types #[cfg_attr( feature = "serde", serde(rename = "params", skip_serializing_if = "Vec::is_empty", default) )] - pub type_params: Vec>, + pub(crate) type_params: Vec>, /// The actual type definition #[cfg_attr(feature = "serde", serde(rename = "def"))] - pub type_def: TypeDef, + pub(crate) type_def: TypeDef, /// Documentation #[cfg_attr( feature = "serde", serde(skip_serializing_if = "Vec::is_empty", default) )] - pub docs: Vec, + pub(crate) docs: Vec, } impl Default for Type @@ -210,12 +210,12 @@ where #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, From, Debug, Encode)] pub struct TypeParameter { /// The name of the generic type parameter e.g. "T". - pub name: T::String, + pub(crate) name: T::String, /// The concrete type for the type parameter. /// /// `None` if the type parameter is skipped. #[cfg_attr(feature = "serde", serde(rename = "type"))] - pub ty: Option, + pub(crate) ty: Option, } impl IntoPortable for TypeParameter { @@ -416,10 +416,10 @@ pub enum TypeDefPrimitive { #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Encode, Debug)] pub struct TypeDefArray { /// The length of the array type. - pub len: u32, + pub(crate) len: u32, /// The element type of the array type. #[cfg_attr(feature = "serde", serde(rename = "type"))] - pub type_param: T::Type, + pub(crate) type_param: T::Type, } impl IntoPortable for TypeDefArray { @@ -468,7 +468,7 @@ where #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Encode, Debug)] pub struct TypeDefTuple { /// The types of the tuple fields. - pub fields: Vec, + pub(crate) fields: Vec, } impl IntoPortable for TypeDefTuple { @@ -530,7 +530,7 @@ where pub struct TypeDefSequence { /// The element type of the sequence type. #[cfg_attr(feature = "serde", serde(rename = "type"))] - pub type_param: T::Type, + pub(crate) type_param: T::Type, } impl IntoPortable for TypeDefSequence { @@ -580,7 +580,7 @@ where pub struct TypeDefCompact { /// The type wrapped in [`Compact`], i.e. the `T` in `Compact`. #[cfg_attr(feature = "serde", serde(rename = "type"))] - pub type_param: T::Type, + pub(crate) type_param: T::Type, } impl IntoPortable for TypeDefCompact { @@ -619,9 +619,9 @@ where #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Encode, Debug)] pub struct TypeDefBitSequence { /// The type implementing [`bitvec::store::BitStore`]. - pub bit_store_type: T::Type, + pub(crate) bit_store_type: T::Type, /// The type implementing [`bitvec::order::BitOrder`]. - pub bit_order_type: T::Type, + pub(crate) bit_order_type: T::Type, } impl IntoPortable for TypeDefBitSequence { diff --git a/src/ty/variant.rs b/src/ty/variant.rs index b76b435b..fb4944c2 100644 --- a/src/ty/variant.rs +++ b/src/ty/variant.rs @@ -88,7 +88,7 @@ pub struct TypeDefVariant { feature = "serde", serde(skip_serializing_if = "Vec::is_empty", default) )] - pub variants: Vec>, + pub(crate) variants: Vec>, } impl IntoPortable for TypeDefVariant { From 47859aef170c6344b79484d089d8143e7bc8ad74 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 17 Mar 2023 14:07:50 +0200 Subject: [PATCH 20/30] portable: Use `mem` from `prelude` and not from `std` Signed-off-by: Alexandru Vasile --- src/portable.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/portable.rs b/src/portable.rs index e21ec876..33370eb2 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -29,6 +29,7 @@ use crate::{ prelude::{ collections::BTreeMap, fmt::Debug, + mem, vec::Vec, }, Registry, @@ -115,7 +116,7 @@ impl PortableRegistry { }; on_retained(old_id, new_id); - let mut ty = std::mem::take(ty); + let mut ty = mem::take(ty); ty.id = *new_id; self.update_type(&ids_map, &mut ty.ty); types.push(ty); From b04048bc9a811618fc760b786c9f193a7a201f13 Mon Sep 17 00:00:00 2001 From: James Wilson Date: Fri, 17 Mar 2023 14:56:34 +0000 Subject: [PATCH 21/30] retain --- src/portable.rs | 314 ++++++++++++++---------------------------------- 1 file changed, 88 insertions(+), 226 deletions(-) diff --git a/src/portable.rs b/src/portable.rs index 33370eb2..b7eda04b 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -27,9 +27,9 @@ use crate::{ form::PortableForm, interner::Interner, prelude::{ + mem, collections::BTreeMap, fmt::Debug, - mem, vec::Vec, }, Registry, @@ -90,110 +90,98 @@ impl PortableRegistry { /// of a [`TypeDef::Composite`] and others. To retain a valid [`PortableRegistry`] /// all the types needed to express an ID are included. Therefore, the number of /// elements defined by the result equals or exceeds the number of provided IDs. - pub fn retain(&mut self, mut filter: F, mut on_retained: R) + pub fn retain(&mut self, mut filter: F) -> BTreeMap where - F: FnMut(&u32) -> bool, - R: FnMut(&u32, &u32), + F: FnMut(u32) -> bool, { - let ids = self - .types - .iter() - .filter_map(|ty| filter(&ty.id).then_some(ty.id)); - - let ids_map = TypeIdResolver::new(self).resolve(ids); - - // Sort the ids by their order in the new registry. - let mut ids_order: Vec<_> = ids_map.clone().into_iter().collect(); - ids_order.sort_by(|(_, lhs_new), (_, rhs_new)| lhs_new.cmp(rhs_new)); - - // We cannot construct directly a new `PortableRegistry` by registering - // the current types because they may contain recursive type ids - // that must be updated. - let mut types = Vec::with_capacity(ids_order.len()); - for (old_id, new_id) in ids_order.iter() { - let Some(ty) = self.types.get_mut(*old_id as usize) else { - continue; - }; - on_retained(old_id, new_id); - - let mut ty = mem::take(ty); - ty.id = *new_id; - self.update_type(&ids_map, &mut ty.ty); - types.push(ty); - } + let mut retained_mappings = BTreeMap::new(); + let mut new_types = vec![]; + + fn retain_type( + id: u32, + types: &mut [PortableType], + new_types: &mut Vec, + retained_mappings: &mut BTreeMap + ) -> u32 { + // Type already retained; just return the new ID for it: + if let Some(id) = retained_mappings.get(&id) { + return *id; + } - self.types = types; - } + // Take the type out of the registry that we'll be retaining: + let mut ty = mem::take(&mut types[id as usize]); - /// Update all the type IDs composting the given type. - fn update_type(&self, ids_map: &BTreeMap, ty: &mut Type) { - for param in ty.type_params.iter_mut() { - let Some(ty) = param.ty() else { - continue - }; - let Some(new_id) = ids_map.get(&ty.id()) else { - continue - }; - param.ty = Some(*new_id).map(Into::into); - } + // Make sure any type params are also retained: + for param in ty.ty.type_params.iter_mut() { + let Some(ty) = param.ty() else { + continue + }; + let new_id = retain_type(ty.id(), types, new_types, retained_mappings); + param.ty = Some(new_id).map(Into::into); + } - match &mut ty.type_def { - TypeDef::Composite(composite) => { - for field in composite.fields.iter_mut() { - let Some(new_id) = ids_map.get(&field.ty().id()) else { - return; - }; - field.ty = (*new_id).into(); + // make sure any types inside this type are also retained and update the IDs: + match &mut ty.ty.type_def { + TypeDef::Composite(composite) => { + for field in composite.fields.iter_mut() { + let new_id = retain_type(field.ty.id(), types, new_types, retained_mappings); + field.ty = new_id.into(); + } + }, + TypeDef::Variant(variant) => { + for var in variant.variants.iter_mut() { + for field in var.fields.iter_mut() { + let new_id = retain_type(field.ty.id(), types, new_types, retained_mappings); + field.ty = new_id.into(); + } + } + }, + TypeDef::Sequence(sequence) => { + let new_id = retain_type(sequence.type_param.id(), types, new_types, retained_mappings); + sequence.type_param = new_id.into(); } - } - TypeDef::Variant(variant) => { - for var in variant.variants.iter_mut() { - for field in var.fields.iter_mut() { - let Some(new_id) = ids_map.get(&field.ty().id()) else { - return; - }; - field.ty = (*new_id).into(); + TypeDef::Array(array) => { + let new_id = retain_type(array.type_param.id(), types, new_types, retained_mappings); + array.type_param = new_id.into(); + } + TypeDef::Tuple(tuple) => { + for ty in tuple.fields.iter_mut() { + let new_id = retain_type(ty.id(), types, new_types, retained_mappings); + *ty = new_id.into(); } } - } - TypeDef::Sequence(sequence) => { - let Some(new_id) = ids_map.get(&sequence.type_param().id()) else { - return; - }; - sequence.type_param = (*new_id).into(); - } - TypeDef::Array(array) => { - let Some(new_id) = ids_map.get(&array.type_param().id()) else { - return; - }; - array.type_param = (*new_id).into(); - } - TypeDef::Tuple(tuple) => { - for ty in tuple.fields.iter_mut() { - let Some(new_id) = ids_map.get(&ty.id()) else { - return; - }; - *ty = (*new_id).into(); + TypeDef::Primitive(_) => (), + TypeDef::Compact(compact) => { + let new_id = retain_type(compact.type_param().id(), types, new_types, retained_mappings); + compact.type_param = new_id.into(); + } + TypeDef::BitSequence(bit_seq) => { + let bit_order_id = retain_type(bit_seq.bit_order_type().id(), types, new_types, retained_mappings); + let bit_store_id = retain_type(bit_seq.bit_store_type().id(), types, new_types, retained_mappings); + + bit_seq.bit_order_type = bit_order_id.into(); + bit_seq.bit_store_type = bit_store_id.into(); } } - TypeDef::Primitive(_) => (), - TypeDef::Compact(compact) => { - let Some(new_id) = ids_map.get(&compact.type_param().id()) else { - return; - }; - compact.type_param = (*new_id).into(); - } - TypeDef::BitSequence(bit_seq) => { - let Some(bit_order_id) = ids_map.get(&bit_seq.bit_order_type().id()) else { - return; - }; - let Some(bit_store_id) = ids_map.get(&bit_seq.bit_store_type().id()) else { - return; - }; - bit_seq.bit_order_type = (*bit_order_id).into(); - bit_seq.bit_store_type = (*bit_store_id).into(); + + // Retain this type, having updated any inner IDs: + let new_id = new_types.len() as u32; + new_types.push(ty); + retained_mappings.insert(id, new_id); + new_id + } + + for id in 0..self.types.len() as u32 { + // We don't care about the type; move on: + if !filter(id) { + continue; } - }; + + retain_type(id, &mut self.types, &mut new_types, &mut retained_mappings); + } + + self.types = new_types; + retained_mappings } } @@ -277,100 +265,6 @@ impl PortableRegistryBuilder { } } -/// Recursive resolver for the type IDs needed to express a given type ID. -struct TypeIdResolver<'a> { - registry: &'a PortableRegistry, - result: BTreeMap, - next_id: u32, -} - -impl<'a> TypeIdResolver<'a> { - /// Construct a new [`TypeIdResolver`]. - fn new(registry: &'a PortableRegistry) -> Self { - TypeIdResolver { - registry, - result: Default::default(), - next_id: Default::default(), - } - } - - /// Get the next unique ID. - fn next_id(&mut self) -> u32 { - let id = self.next_id; - self.next_id += 1; - id - } - - /// Recursively add all type ids needed to express the given identifier. - fn visit_type_id(&mut self, id: u32) { - if self.result.get(&id).is_some() { - return - } - - let Some(ty) = self.registry.resolve(id) else { - return - }; - - // Add generic type params. - for param in ty.type_params() { - if let Some(ty) = param.ty() { - self.visit_type_id(ty.id()); - } - } - - // Recursively visit any other type ids needed to represent this type. - match ty.type_def() { - TypeDef::Composite(composite) => { - for field in composite.fields() { - self.visit_type_id(field.ty().id()); - } - } - TypeDef::Variant(variant) => { - for var in variant.variants() { - for field in var.fields() { - self.visit_type_id(field.ty().id()); - } - } - } - TypeDef::Sequence(sequence) => { - self.visit_type_id(sequence.type_param().id()); - } - TypeDef::Array(array) => { - self.visit_type_id(array.type_param().id()); - } - TypeDef::Tuple(tuple) => { - for ty in tuple.fields() { - self.visit_type_id(ty.id()); - } - } - TypeDef::Primitive(_) => {} - TypeDef::Compact(compact) => { - self.visit_type_id(compact.type_param().id()); - } - TypeDef::BitSequence(bit_sequence) => { - self.visit_type_id(bit_sequence.bit_store_type().id()); - self.visit_type_id(bit_sequence.bit_order_type().id()); - } - } - - let new_id = self.next_id(); - self.result.insert(id, new_id); - } - - /// Resolve all the type IDs needed to express the given type IDs. - /// - /// The type IDs are returned as key to the `HashMap`. - /// The value of the `HashMap` represents the new ID of that type - /// if only the resolved types are expressed in the [`PortableRegistry`]. - fn resolve(mut self, ids: impl IntoIterator) -> BTreeMap { - for id in ids.into_iter() { - self.visit_type_id(id); - } - - self.result - } -} - #[cfg(test)] mod tests { use super::*; @@ -442,7 +336,7 @@ mod tests { fn retain_ids() { 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); + let _u32_type_id = builder.register_type(u32_type); let u64_type = Type::new(Path::default(), vec![], TypeDefPrimitive::U64, vec![]); let u64_type_id = builder.register_type(u64_type.clone()); @@ -450,26 +344,11 @@ mod tests { let mut registry = builder.finish(); assert_eq!(registry.types.len(), 2); - // Resolve just u64. - let result = TypeIdResolver::new(®istry).resolve(vec![u64_type_id]); - assert_eq!(result.len(), 1); - // Make sure `u32_type_id` is not present. - assert!(!result.contains_key(&u32_type_id)); - - // `u64_type_id` should be mapped on id `0`. - assert_eq!(result.get(&u64_type_id).unwrap(), &0); - - let mut ids_result = BTreeMap::new(); - registry.retain( - |id| *id == u64_type_id, - |old, new| { - ids_result.insert(*old, *new); - }, + let _ids_result = registry.retain( + |id| id == u64_type_id, ); - assert_eq!(ids_result, result); assert_eq!(registry.types.len(), 1); - assert_eq!(registry.resolve(0).unwrap(), &u64_type); } @@ -480,7 +359,7 @@ mod tests { let u32_type_id = builder.register_type(u32_type.clone()); let u64_type = Type::new(Path::default(), vec![], TypeDefPrimitive::U64, vec![]); - let u64_type_id = builder.register_type(u64_type); + let _u64_type_id = builder.register_type(u64_type); let vec_u32_type = Type::new( Path::default(), @@ -511,26 +390,9 @@ mod tests { let mut registry = builder.finish(); assert_eq!(registry.types.len(), 5); - // Resolve just `MyStruct`. - let result = - TypeIdResolver::new(®istry).resolve(vec![composite_type_second_id]); - assert_eq!(result.len(), 4); - // Make sure `u64_type_id` is not present. - assert!(!result.contains_key(&u64_type_id)); - - assert_eq!(result.get(&u32_type_id).unwrap(), &0); - assert_eq!(result.get(&vec_u32_type_id).unwrap(), &1); - assert_eq!(result.get(&composite_type_id).unwrap(), &2); - assert_eq!(result.get(&composite_type_second_id).unwrap(), &3); - - let mut ids_result = BTreeMap::new(); - registry.retain( - |id| *id == composite_type_second_id, - |old, new| { - ids_result.insert(*old, *new); - }, + let _ids_result = registry.retain( + |id| id == composite_type_second_id, ); - assert_eq!(ids_result, result); assert_eq!(registry.types.len(), 4); From dabb64099529058bcbaa2e921bdd88ca365df66f Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 17 Mar 2023 18:12:31 +0200 Subject: [PATCH 22/30] portable: Modify docs and apply fmt Signed-off-by: Alexandru Vasile --- src/portable.rs | 81 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 54 insertions(+), 27 deletions(-) diff --git a/src/portable.rs b/src/portable.rs index b7eda04b..d0c0cf59 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -27,9 +27,9 @@ use crate::{ form::PortableForm, interner::Interner, prelude::{ - mem, collections::BTreeMap, fmt::Debug, + mem, vec::Vec, }, Registry, @@ -76,13 +76,8 @@ impl PortableRegistry { /// Retains only the portable types needed to express the provided ids. /// - /// Returns the type IDs that have been retained. - /// The order of the type IDs in the returned vector corresponds to their - /// new positions in the type registry after filtering. - /// - /// For instance, if the function returns the vector [30, 10, 20], it means that - /// type ID 30 is now at position 0 and has the type ID 0 in the registry, - /// type ID 10 is now type ID 1, and type ID 20 is now type ID 2. + /// The type IDs retained are returned as key to the `HashMap`. + /// The value of the map represents the new ID of that type. /// /// # Note /// @@ -101,11 +96,11 @@ impl PortableRegistry { id: u32, types: &mut [PortableType], new_types: &mut Vec, - retained_mappings: &mut BTreeMap + retained_mappings: &mut BTreeMap, ) -> u32 { // Type already retained; just return the new ID for it: if let Some(id) = retained_mappings.get(&id) { - return *id; + return *id } // Take the type out of the registry that we'll be retaining: @@ -124,40 +119,76 @@ impl PortableRegistry { match &mut ty.ty.type_def { TypeDef::Composite(composite) => { for field in composite.fields.iter_mut() { - let new_id = retain_type(field.ty.id(), types, new_types, retained_mappings); + let new_id = retain_type( + field.ty.id(), + types, + new_types, + retained_mappings, + ); field.ty = new_id.into(); } - }, + } TypeDef::Variant(variant) => { for var in variant.variants.iter_mut() { for field in var.fields.iter_mut() { - let new_id = retain_type(field.ty.id(), types, new_types, retained_mappings); + let new_id = retain_type( + field.ty.id(), + types, + new_types, + retained_mappings, + ); field.ty = new_id.into(); } } - }, + } TypeDef::Sequence(sequence) => { - let new_id = retain_type(sequence.type_param.id(), types, new_types, retained_mappings); + let new_id = retain_type( + sequence.type_param.id(), + types, + new_types, + retained_mappings, + ); sequence.type_param = new_id.into(); } TypeDef::Array(array) => { - let new_id = retain_type(array.type_param.id(), types, new_types, retained_mappings); + let new_id = retain_type( + array.type_param.id(), + types, + new_types, + retained_mappings, + ); array.type_param = new_id.into(); } TypeDef::Tuple(tuple) => { for ty in tuple.fields.iter_mut() { - let new_id = retain_type(ty.id(), types, new_types, retained_mappings); + let new_id = + retain_type(ty.id(), types, new_types, retained_mappings); *ty = new_id.into(); } } TypeDef::Primitive(_) => (), TypeDef::Compact(compact) => { - let new_id = retain_type(compact.type_param().id(), types, new_types, retained_mappings); + let new_id = retain_type( + compact.type_param().id(), + types, + new_types, + retained_mappings, + ); compact.type_param = new_id.into(); } TypeDef::BitSequence(bit_seq) => { - let bit_order_id = retain_type(bit_seq.bit_order_type().id(), types, new_types, retained_mappings); - let bit_store_id = retain_type(bit_seq.bit_store_type().id(), types, new_types, retained_mappings); + let bit_order_id = retain_type( + bit_seq.bit_order_type().id(), + types, + new_types, + retained_mappings, + ); + let bit_store_id = retain_type( + bit_seq.bit_store_type().id(), + types, + new_types, + retained_mappings, + ); bit_seq.bit_order_type = bit_order_id.into(); bit_seq.bit_store_type = bit_store_id.into(); @@ -174,7 +205,7 @@ impl PortableRegistry { for id in 0..self.types.len() as u32 { // We don't care about the type; move on: if !filter(id) { - continue; + continue } retain_type(id, &mut self.types, &mut new_types, &mut retained_mappings); @@ -344,9 +375,7 @@ mod tests { let mut registry = builder.finish(); assert_eq!(registry.types.len(), 2); - let _ids_result = registry.retain( - |id| id == u64_type_id, - ); + let _ids_result = registry.retain(|id| id == u64_type_id); assert_eq!(registry.types.len(), 1); assert_eq!(registry.resolve(0).unwrap(), &u64_type); @@ -390,9 +419,7 @@ mod tests { let mut registry = builder.finish(); assert_eq!(registry.types.len(), 5); - let _ids_result = registry.retain( - |id| id == composite_type_second_id, - ); + let _ids_result = registry.retain(|id| id == composite_type_second_id); assert_eq!(registry.types.len(), 4); From 17a98f284ea995bc3e5b172e2577ddc26f722f20 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 17 Mar 2023 18:18:50 +0200 Subject: [PATCH 23/30] portable/test: Check result BTreeMap Signed-off-by: Alexandru Vasile --- src/portable.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/portable.rs b/src/portable.rs index d0c0cf59..fb607612 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -375,7 +375,9 @@ mod tests { let mut registry = builder.finish(); assert_eq!(registry.types.len(), 2); - let _ids_result = registry.retain(|id| id == u64_type_id); + let ids_result = registry.retain(|id| id == u64_type_id); + assert_eq!(ids_result.len(), 1); + assert_eq!(ids_result.get(&u64_type_id), Some(&0)); assert_eq!(registry.types.len(), 1); assert_eq!(registry.resolve(0).unwrap(), &u64_type); @@ -419,7 +421,12 @@ mod tests { let mut registry = builder.finish(); assert_eq!(registry.types.len(), 5); - let _ids_result = registry.retain(|id| id == composite_type_second_id); + let ids_result = registry.retain(|id| id == composite_type_second_id); + assert_eq!(ids_result.len(), 4); + assert_eq!(ids_result.get(&u32_type_id), Some(&0)); + assert_eq!(ids_result.get(&vec_u32_type_id), Some(&1)); + assert_eq!(ids_result.get(&composite_type_id), Some(&2)); + assert_eq!(ids_result.get(&composite_type_second_id), Some(&3)); assert_eq!(registry.types.len(), 4); From 9bcb199da1fa72c58ece33c6e852942143671ac2 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 17 Mar 2023 18:20:37 +0200 Subject: [PATCH 24/30] Fix cargo check Signed-off-by: Alexandru Vasile --- src/portable.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/portable.rs b/src/portable.rs index fb607612..896b8f89 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -90,7 +90,7 @@ impl PortableRegistry { F: FnMut(u32) -> bool, { let mut retained_mappings = BTreeMap::new(); - let mut new_types = vec![]; + let mut new_types = crate::prelude::vec![]; fn retain_type( id: u32, From 16011f50c1534b6f1f9c93da4964e6ce52da0942 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 21 Mar 2023 14:18:45 +0200 Subject: [PATCH 25/30] tests: Recursive retain for all type-def types Signed-off-by: Alexandru Vasile --- src/portable.rs | 315 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 310 insertions(+), 5 deletions(-) diff --git a/src/portable.rs b/src/portable.rs index 896b8f89..35575a61 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -177,21 +177,21 @@ impl PortableRegistry { compact.type_param = new_id.into(); } TypeDef::BitSequence(bit_seq) => { - let bit_order_id = retain_type( - bit_seq.bit_order_type().id(), + let bit_store_id = retain_type( + bit_seq.bit_store_type().id(), types, new_types, retained_mappings, ); - let bit_store_id = retain_type( - bit_seq.bit_store_type().id(), + let bit_order_id = retain_type( + bit_seq.bit_order_type().id(), types, new_types, retained_mappings, ); - bit_seq.bit_order_type = bit_order_id.into(); bit_seq.bit_store_type = bit_store_id.into(); + bit_seq.bit_order_type = bit_order_id.into(); } } @@ -305,6 +305,311 @@ mod tests { *, }; + // Type IDs generated by `build_registry`. + const U32_TY_ID: u32 = 0; + const U64_TY_ID: u32 = 1; + const VEC_U32_TY_ID: u32 = 2; + const ARRAY_U32_TY_ID: u32 = 3; + const TUPLE_TY_ID: u32 = 4; + const COMPACT_TY_ID: u32 = 5; + const BIT_SEQ_TY_ID: u32 = 6; + const COMPOSITE_TY_ID: u32 = 7; + const VARIANT_TY_ID: u32 = 8; + + fn build_registry() -> PortableRegistry { + let mut builder = PortableRegistryBuilder::new(); + // Primitives + let u32_type = Type::new(Path::default(), vec![], TypeDefPrimitive::U32, vec![]); + let u32_type_id = builder.register_type(u32_type); + assert_eq!(U32_TY_ID, u32_type_id); + + let u64_type = Type::new(Path::default(), vec![], TypeDefPrimitive::U64, vec![]); + let u64_type_id = builder.register_type(u64_type); + assert_eq!(U64_TY_ID, u64_type_id); + + // Sequence + 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); + assert_eq!(VEC_U32_TY_ID, vec_u32_type_id); + + // Array + let array_u32_type = Type::new( + Path::default(), + vec![], + TypeDefArray::new(3, u32_type_id.into()), + vec![], + ); + let array_u32_type_id = builder.register_type(array_u32_type); + assert_eq!(ARRAY_U32_TY_ID, array_u32_type_id); + + // Tuple + let tuple_type = Type::new( + Path::default(), + vec![], + TypeDefTuple::new_portable(vec![u32_type_id.into(), u64_type_id.into()]), + vec![], + ); + let tuple_type_id = builder.register_type(tuple_type); + assert_eq!(TUPLE_TY_ID, tuple_type_id); + + // Compact + let compact_type = Type::new( + Path::default(), + vec![], + TypeDefCompact::new(tuple_type_id.into()), + vec![], + ); + let compact_type_id = builder.register_type(compact_type); + assert_eq!(COMPACT_TY_ID, compact_type_id); + + // BitSequence + let bit_seq_type = Type::new( + Path::default(), + vec![], + TypeDefBitSequence::new_portable(u32_type_id.into(), u64_type_id.into()), + vec![], + ); + let bit_seq_type_id = builder.register_type(bit_seq_type); + assert_eq!(BIT_SEQ_TY_ID, bit_seq_type_id); + + // Composite + let composite_type = Type::builder_portable() + .path(Path::from_segments_unchecked(["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)), + ); + let composite_type_id = builder.register_type(composite_type); + assert_eq!(COMPOSITE_TY_ID, composite_type_id); + + // Variant + let enum_type = Type::builder_portable() + .path(Path::from_segments_unchecked(["MyEnum".into()])) + .variant( + Variants::new() + .variant("A".into(), |v| { + v.index(0).fields( + 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) + }), + ) + }) + .variant_unit("B".into(), 1), + ); + let enum_type_id = builder.register_type(enum_type); + assert_eq!(VARIANT_TY_ID, enum_type_id); + + builder.finish() + } + + #[test] + fn retain_recursive_seq() { + let mut registry = build_registry(); + + let ids_result = registry.retain(|id| id == VEC_U32_TY_ID); + assert_eq!(ids_result.len(), 2); + assert_eq!(ids_result.get(&U32_TY_ID), Some(&0)); + assert_eq!(ids_result.get(&VEC_U32_TY_ID), Some(&1)); + + assert_eq!(registry.types.len(), 2); + let expected_ty = + Type::new(Path::default(), vec![], TypeDefPrimitive::U32, vec![]); + assert_eq!(registry.resolve(0).unwrap(), &expected_ty); + let expected_ty = Type::new( + Path::default(), + vec![], + TypeDefSequence::new(0.into()), + vec![], + ); + assert_eq!(registry.resolve(1).unwrap(), &expected_ty); + } + + #[test] + fn retain_recursive_array() { + let mut registry = build_registry(); + + let ids_result = registry.retain(|id| id == ARRAY_U32_TY_ID); + assert_eq!(ids_result.len(), 2); + assert_eq!(ids_result.get(&U32_TY_ID), Some(&0)); + assert_eq!(ids_result.get(&ARRAY_U32_TY_ID), Some(&1)); + + assert_eq!(registry.types.len(), 2); + let expected_ty = + Type::new(Path::default(), vec![], TypeDefPrimitive::U32, vec![]); + assert_eq!(registry.resolve(0).unwrap(), &expected_ty); + let expected_ty = Type::new( + Path::default(), + vec![], + TypeDefArray::new(3, 0.into()), + vec![], + ); + assert_eq!(registry.resolve(1).unwrap(), &expected_ty); + } + + #[test] + fn retain_recursive_tuple() { + let mut registry = build_registry(); + + let ids_result = registry.retain(|id| id == TUPLE_TY_ID); + assert_eq!(ids_result.len(), 3); + assert_eq!(ids_result.get(&U32_TY_ID), Some(&0)); + assert_eq!(ids_result.get(&U64_TY_ID), Some(&1)); + assert_eq!(ids_result.get(&TUPLE_TY_ID), Some(&2)); + + assert_eq!(registry.types.len(), 3); + let expected_ty = + Type::new(Path::default(), vec![], TypeDefPrimitive::U32, vec![]); + assert_eq!(registry.resolve(0).unwrap(), &expected_ty); + let expected_ty = + Type::new(Path::default(), vec![], TypeDefPrimitive::U64, vec![]); + assert_eq!(registry.resolve(1).unwrap(), &expected_ty); + let expected_ty = Type::new( + Path::default(), + vec![], + TypeDefTuple::new_portable(vec![0.into(), 1.into()]), + vec![], + ); + assert_eq!(registry.resolve(2).unwrap(), &expected_ty); + } + + #[test] + fn retain_recursive_compact() { + let mut registry = build_registry(); + + let ids_result = registry.retain(|id| id == COMPACT_TY_ID); + assert_eq!(ids_result.len(), 4); + assert_eq!(ids_result.get(&U32_TY_ID), Some(&0)); + assert_eq!(ids_result.get(&U64_TY_ID), Some(&1)); + assert_eq!(ids_result.get(&TUPLE_TY_ID), Some(&2)); + assert_eq!(ids_result.get(&COMPACT_TY_ID), Some(&3)); + + assert_eq!(registry.types.len(), 4); + let expected_ty = + Type::new(Path::default(), vec![], TypeDefPrimitive::U32, vec![]); + assert_eq!(registry.resolve(0).unwrap(), &expected_ty); + let expected_ty = + Type::new(Path::default(), vec![], TypeDefPrimitive::U64, vec![]); + assert_eq!(registry.resolve(1).unwrap(), &expected_ty); + let expected_ty = Type::new( + Path::default(), + vec![], + TypeDefTuple::new_portable(vec![0.into(), 1.into()]), + vec![], + ); + assert_eq!(registry.resolve(2).unwrap(), &expected_ty); + let expected_ty = Type::new( + Path::default(), + vec![], + TypeDefCompact::new(2.into()), + vec![], + ); + assert_eq!(registry.resolve(3).unwrap(), &expected_ty); + } + + #[test] + fn retain_recursive_bit_seq() { + let mut registry = build_registry(); + + let ids_result = registry.retain(|id| id == BIT_SEQ_TY_ID); + assert_eq!(ids_result.len(), 3); + assert_eq!(ids_result.get(&U32_TY_ID), Some(&0)); + assert_eq!(ids_result.get(&U64_TY_ID), Some(&1)); + assert_eq!(ids_result.get(&BIT_SEQ_TY_ID), Some(&2)); + + assert_eq!(registry.types.len(), 3); + let expected_ty = + Type::new(Path::default(), vec![], TypeDefPrimitive::U32, vec![]); + assert_eq!(registry.resolve(0).unwrap(), &expected_ty); + let expected_ty = + Type::new(Path::default(), vec![], TypeDefPrimitive::U64, vec![]); + assert_eq!(registry.resolve(1).unwrap(), &expected_ty); + let expected_ty = Type::new( + Path::default(), + vec![], + TypeDefBitSequence::new_portable(0.into(), 1.into()), + vec![], + ); + assert_eq!(registry.resolve(2).unwrap(), &expected_ty); + } + + #[test] + fn retain_recursive_composite() { + let mut registry = build_registry(); + + let ids_result = registry.retain(|id| id == COMPOSITE_TY_ID); + assert_eq!(ids_result.len(), 3); + assert_eq!(ids_result.get(&U32_TY_ID), Some(&0)); + assert_eq!(ids_result.get(&VEC_U32_TY_ID), Some(&1)); + assert_eq!(ids_result.get(&COMPOSITE_TY_ID), Some(&2)); + + assert_eq!(registry.types.len(), 3); + let expected_ty = + Type::new(Path::default(), vec![], TypeDefPrimitive::U32, vec![]); + assert_eq!(registry.resolve(0).unwrap(), &expected_ty); + let expected_ty = Type::new( + Path::default(), + vec![], + TypeDefSequence::new(0.into()), + vec![], + ); + assert_eq!(registry.resolve(1).unwrap(), &expected_ty); + let expected_ty = Type::builder_portable() + .path(Path::from_segments_unchecked(["MyStruct".into()])) + .composite( + Fields::named() + .field_portable(|f| f.name("primitive".into()).ty(0)) + .field_portable(|f| f.name("vec_of_u32".into()).ty(1)), + ); + assert_eq!(registry.resolve(2).unwrap(), &expected_ty); + } + + #[test] + fn retain_recursive_variant() { + let mut registry = build_registry(); + + let ids_result = registry.retain(|id| id == VARIANT_TY_ID); + assert_eq!(ids_result.len(), 3); + assert_eq!(ids_result.get(&U32_TY_ID), Some(&0)); + assert_eq!(ids_result.get(&VEC_U32_TY_ID), Some(&1)); + assert_eq!(ids_result.get(&VARIANT_TY_ID), Some(&2)); + + assert_eq!(registry.types.len(), 3); + let expected_ty = + Type::new(Path::default(), vec![], TypeDefPrimitive::U32, vec![]); + assert_eq!(registry.resolve(0).unwrap(), &expected_ty); + let expected_ty = Type::new( + Path::default(), + vec![], + TypeDefSequence::new(0.into()), + vec![], + ); + assert_eq!(registry.resolve(1).unwrap(), &expected_ty); + let expected_ty = Type::builder_portable() + .path(Path::from_segments_unchecked(["MyEnum".into()])) + .variant( + Variants::new() + .variant("A".into(), |v| { + v.index(0).fields( + Fields::::named() + .field_portable(|f| f.name("primitive".into()).ty(0)) + .field_portable(|f| f.name("vec_of_u32".into()).ty(1)), + ) + }) + .variant_unit("B".into(), 1), + ); + assert_eq!(registry.resolve(2).unwrap(), &expected_ty); + } + #[test] fn type_ids_are_sequential() { let mut registry = Registry::new(); From 5701df5952484dd20f5f73128afacc87d260a52b Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 21 Mar 2023 14:48:42 +0200 Subject: [PATCH 26/30] ty: Use placeholder instead of implementing dummy Default::default() Signed-off-by: Alexandru Vasile --- src/portable.rs | 17 +++++++++++++++-- src/ty/mod.rs | 16 ---------------- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/portable.rs b/src/portable.rs index 35575a61..c20e2aeb 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -32,9 +32,11 @@ use crate::{ mem, vec::Vec, }, + Path, Registry, Type, TypeDef, + TypeDefPrimitive, }; use scale::Encode; @@ -103,8 +105,19 @@ impl PortableRegistry { return *id } + // Zero-allocation default implementation that assumes + // the type def to be `TypeDefPrimitive::Bool`. + let placeholder = PortableType { + id: 0, + ty: Type { + type_def: TypeDef::Primitive(TypeDefPrimitive::Bool), + path: Path::default(), + type_params: vec![], + docs: vec![], + }, + }; // Take the type out of the registry that we'll be retaining: - let mut ty = mem::take(&mut types[id as usize]); + let mut ty = mem::replace(&mut types[id as usize], placeholder); // Make sure any type params are also retained: for param in ty.ty.type_params.iter_mut() { @@ -220,7 +233,7 @@ impl PortableRegistry { #[cfg_attr(feature = "serde", derive(serde::Serialize))] #[cfg_attr(all(feature = "serde", feature = "decode"), derive(serde::Deserialize))] #[cfg_attr(any(feature = "std", feature = "decode"), derive(scale::Decode))] -#[derive(Clone, Debug, PartialEq, Eq, Encode, Default)] +#[derive(Clone, Debug, PartialEq, Eq, Encode)] pub struct PortableType { #[codec(compact)] id: u32, diff --git a/src/ty/mod.rs b/src/ty/mod.rs index 9969c33e..56bf4c71 100644 --- a/src/ty/mod.rs +++ b/src/ty/mod.rs @@ -86,22 +86,6 @@ pub struct Type { pub(crate) docs: Vec, } -impl Default for Type -where - T: Form, -{ - // Zero-allocation default implementation that assumes - // the type def to be `TypeDefPrimitive::Bool`. - fn default() -> Type { - Type { - type_def: TypeDef::Primitive(TypeDefPrimitive::Bool), - path: Path::default(), - type_params: vec![], - docs: vec![], - } - } -} - impl IntoPortable for Type { type Output = Type; From 3000e1884b428c96a5b592eee4d9dcc3d59aadd8 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 21 Mar 2023 14:51:15 +0200 Subject: [PATCH 27/30] portable: use crate::prelude::vec![] insted of vec![] Signed-off-by: Alexandru Vasile --- src/portable.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/portable.rs b/src/portable.rs index c20e2aeb..444964c0 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -112,8 +112,8 @@ impl PortableRegistry { ty: Type { type_def: TypeDef::Primitive(TypeDefPrimitive::Bool), path: Path::default(), - type_params: vec![], - docs: vec![], + type_params: crate::prelude::vec![], + docs: crate::prelude::vec![], }, }; // Take the type out of the registry that we'll be retaining: From 95fa6cb677e5aeb50c9f211aa8d53f9b8cf9d990 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> Date: Tue, 21 Mar 2023 15:01:14 +0200 Subject: [PATCH 28/30] Update src/portable.rs Co-authored-by: James Wilson --- src/portable.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/portable.rs b/src/portable.rs index 444964c0..c057e8bf 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -105,8 +105,8 @@ impl PortableRegistry { return *id } - // Zero-allocation default implementation that assumes - // the type def to be `TypeDefPrimitive::Bool`. + // Zero-allocation default implementation that is used as + // a placeholder and never accessed. let placeholder = PortableType { id: 0, ty: Type { From 72ac00db9d83b4357ac2e64e456802a73f91ef51 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 21 Mar 2023 15:02:28 +0200 Subject: [PATCH 29/30] portable: Fix clippy Signed-off-by: Alexandru Vasile --- src/portable.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/portable.rs b/src/portable.rs index c057e8bf..4ce64108 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -106,7 +106,7 @@ impl PortableRegistry { } // Zero-allocation default implementation that is used as - // a placeholder and never accessed. + // a placeholder and never accessed. let placeholder = PortableType { id: 0, ty: Type { From e4018f2028a809bc3dc0d32836f8ef73b806fd95 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> Date: Tue, 21 Mar 2023 16:27:09 +0200 Subject: [PATCH 30/30] Update src/portable.rs Co-authored-by: Andrew Jones --- src/portable.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/portable.rs b/src/portable.rs index 4ce64108..74aae9e0 100644 --- a/src/portable.rs +++ b/src/portable.rs @@ -78,7 +78,7 @@ impl PortableRegistry { /// Retains only the portable types needed to express the provided ids. /// - /// The type IDs retained are returned as key to the `HashMap`. + /// The type IDs retained are returned as key to the [`BTreeMap`]. /// The value of the map represents the new ID of that type. /// /// # Note