From 286a92513127dc617589471a9a3d8281b19cfae2 Mon Sep 17 00:00:00 2001 From: "Kevin R. Thornton" Date: Thu, 1 Dec 2022 11:53:55 -0800 Subject: [PATCH] refactor: usize to SizeType conversion is now fallible. * New behavior reflects underlying types. * Update book example to use new instance methods. BREAKING CHANGE: From is now TryFrom --- src/_macros.rs | 15 ++++++------- src/lib.rs | 51 ++++++++++++++++++++++++++++++++++++++++++--- src/metadata.rs | 8 +++---- tests/book_trees.rs | 2 +- 4 files changed, 61 insertions(+), 15 deletions(-) diff --git a/src/_macros.rs b/src/_macros.rs index a29a59b6f..7d4f71c1a 100644 --- a/src/_macros.rs +++ b/src/_macros.rs @@ -676,12 +676,13 @@ macro_rules! node_table_add_row_with_metadata { M: $crate::metadata::NodeMetadata, { let md = $crate::metadata::EncodedMetadata::new(metadata)?; + let mdlen = md.len()?; node_table_add_row_details!(flags, time, population, individual, md.as_ptr(), - md.len().into(), + mdlen.into(), $table) } }; @@ -761,7 +762,7 @@ macro_rules! edge_table_add_row_with_metadata { parent, child, md.as_ptr(), - md.len().into(), + md.len()?.into(), $table) } }; @@ -791,7 +792,7 @@ macro_rules! population_table_add_row_with_metadata { pub fn $name(&mut $self, metadata: &M) -> Result<$crate::PopulationId, $crate::TskitError> where M: $crate::metadata::PopulationMetadata { let md = $crate::metadata::EncodedMetadata::new(metadata)?; - population_table_add_row_details!(md.as_ptr(), md.len().into(), $table) + population_table_add_row_details!(md.as_ptr(), md.len()?.into(), $table) } }; } @@ -865,7 +866,7 @@ macro_rules! individual_table_add_row_with_metadata { location, parents, md.as_ptr(), - md.len().into(), + md.len()?.into(), $table) } }; @@ -946,7 +947,7 @@ macro_rules! mutation_table_add_row_with_metadata { time, derived_state, md.as_ptr(), - md.len().into(), + md.len()?.into(), $table) } }; @@ -1002,7 +1003,7 @@ macro_rules! site_table_add_row_with_metadata { let md = $crate::metadata::EncodedMetadata::new(metadata)?; site_table_add_row_details!(position, ancestral_state, md.as_ptr(), - md.len().into(), + md.len()?.into(), $table) } }; @@ -1076,7 +1077,7 @@ macro_rules! migration_table_add_row_with_metadata { { let md = $crate::metadata::EncodedMetadata::new(metadata)?; migration_table_add_row_details!(span, node, source_dest, time, - md.as_ptr(), md.len().into(), $table) + md.as_ptr(), md.len()?.into(), $table) } }; } diff --git a/src/lib.rs b/src/lib.rs index 8c5cf0455..9bcf13f2d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -263,6 +263,27 @@ impl_size_type_comparisons_for_row_ids!(MigrationId); #[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, std::hash::Hash)] pub struct SizeType(tsk_size_t); +impl SizeType { + /// Convenience function to convert to usize. + /// + /// Works via [`TryFrom`]. + /// + /// # Returns + /// + /// * `None` if the underlying value is negative. + /// * `Some(usize)` otherwise. + pub fn to_usize(&self) -> Option { + (*self).try_into().ok() + } + + /// Convenience function to convert to usize. + /// Implemented via `as`. + /// Negative values with therefore wrap. + pub fn as_usize(&self) -> usize { + self.0 as usize + } +} + impl std::fmt::Display for SizeType { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{}", self.0) @@ -297,9 +318,17 @@ impl TryFrom for usize { } } -impl From for SizeType { - fn from(value: usize) -> Self { - Self(value as tsk_size_t) +impl TryFrom for SizeType { + type Error = TskitError; + + fn try_from(value: usize) -> Result { + match tsk_size_t::try_from(value) { + Ok(x) => Ok(Self(x)), + Err(_) => Err(TskitError::RangeError(format!( + "could not convert usize {} to SizeType", + value + ))), + } } } @@ -539,6 +568,22 @@ mod tests { } } +#[test] +fn test_usize_to_size_type() { + let x = usize::MAX; + let s = SizeType::try_from(x).ok(); + + #[cfg(target_pointer_width = "64")] + assert_eq!(s, Some(bindings::tsk_size_t::MAX.into())); + + #[cfg(target_pointer_width = "32")] + assert_eq!(s, Some((usize::MAX as bindings::tsk_size_t).into())); + + let x = usize::MIN; + let s = SizeType::try_from(x).ok(); + assert_eq!(s, Some(0.into())); +} + // Testing modules mod test_fixtures; mod test_simplification; diff --git a/src/metadata.rs b/src/metadata.rs index 3ff081a08..b62cf8016 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -234,8 +234,8 @@ impl EncodedMetadata { } } - pub(crate) fn len(&self) -> SizeType { - self.encoded.len().into() + pub(crate) fn len(&self) -> Result { + SizeType::try_from(self.encoded.len()) } } @@ -365,7 +365,7 @@ mod tests { let enc = EncodedMetadata::new(&f).unwrap(); let p = enc.as_ptr(); let mut d = vec![]; - for i in 0..usize::try_from(enc.len()).unwrap() { + for i in 0..usize::try_from(enc.len().unwrap()).unwrap() { d.push(unsafe { *p.add(i) as u8 }); } let df = F::decode(&d).unwrap(); @@ -399,7 +399,7 @@ mod test_serde { let enc = EncodedMetadata::new(&f).unwrap(); let p = enc.as_ptr(); let mut d = vec![]; - for i in 0..usize::try_from(enc.len()).unwrap() { + for i in 0..usize::try_from(enc.len().unwrap()).unwrap() { d.push(unsafe { *p.add(i) as u8 }); } let df = F::decode(&d).unwrap(); diff --git a/tests/book_trees.rs b/tests/book_trees.rs index 6ffbf4469..88baa31a1 100644 --- a/tests/book_trees.rs +++ b/tests/book_trees.rs @@ -159,7 +159,7 @@ fn initialize_from_table_collection() { // ANCHOR_END: iterate_edge_differences // ANCHOR: iterate_edge_differences_update_parents - let num_nodes: usize = treeseq.nodes().num_rows().try_into().unwrap(); + let num_nodes = treeseq.nodes().num_rows().as_usize(); // num_nodes + 1 to reflect a "virtual root" present in // the tree arrays let mut parents = vec![NodeId::NULL; num_nodes + 1];