Replace From<SizeType> for usize with TryFrom. (#234)

SizeType wraps tsk_size_t, a.k.a. u64.
The From conversion was fallible on 32-bit systems.

Author: molpopgen

src/lib.rs

@@ -274,9 +274,19 @@ impl From<SizeType> for tsk_size_t {
     }
 }
 
-impl From<SizeType> for usize {
-    fn from(value: SizeType) -> Self {
-        crate::util::handle_u64_to_usize(value.0)
+// SizeType is u64, so converstion
+// can fail on systems with smaller pointer widths.
+impl TryFrom<SizeType> for usize {
+    type Error = TskitError;
+
+    fn try_from(value: SizeType) -> Result<Self, Self::Error> {
+        match usize::try_from(value.0) {
+            Ok(x) => Ok(x),
+            Err(_) => Err(TskitError::RangeError(format!(
+                "could not convert {} to usize",
+                value
+            ))),
+        }
     }
 }
 

src/metadata.rs

@@ -369,7 +369,7 @@ mod tests {
         let enc = EncodedMetadata::new(&f).unwrap();
         let p = enc.as_ptr();
         let mut d = vec![];
-        for i in 0..usize::from(enc.len()) {
+        for i in 0..usize::try_from(enc.len()).unwrap() {
             d.push(unsafe { *p.add(i) as u8 });
         }
         let df = F::decode(&d).unwrap();
@@ -403,7 +403,7 @@ mod test_serde {
         let enc = EncodedMetadata::new(&f).unwrap();
         let p = enc.as_ptr();
         let mut d = vec![];
-        for i in 0..usize::from(enc.len()) {
+        for i in 0..usize::try_from(enc.len()).unwrap() {
             d.push(unsafe { *p.add(i) as u8 });
         }
         let df = F::decode(&d).unwrap();

src/node_table.rs

@@ -119,7 +119,7 @@ impl<'a> NodeTable<'a> {
         unsafe {
             std::slice::from_raw_parts_mut(
                 self.table_.flags.cast::<NodeFlags>(),
-                crate::util::handle_u64_to_usize(self.table_.num_rows),
+                usize::try_from(self.table_.num_rows).unwrap(),
             )
         }
     }
@@ -129,7 +129,7 @@ impl<'a> NodeTable<'a> {
         unsafe {
             std::slice::from_raw_parts_mut(
                 self.table_.time.cast::<Time>(),
-                crate::util::handle_u64_to_usize(self.table_.num_rows),
+                usize::try_from(self.table_.num_rows).unwrap(),
             )
         }
     }

src/provenance.rs

@@ -212,7 +212,10 @@ mod test_provenances {
             assert!(row_id == ProvenanceId(i as crate::tsk_id_t));
             assert_eq!(tables.provenances().record(row_id).unwrap(), *r);
         }
-        assert_eq!(usize::from(tables.provenances().num_rows()), records.len());
+        assert_eq!(
+            usize::try_from(tables.provenances().num_rows()).unwrap(),
+            records.len()
+        );
         for (i, row) in tables.provenances_iter().enumerate() {
             assert_eq!(records[i], row.record);
         }

src/table_collection.rs

@@ -583,7 +583,7 @@ impl TableCollection {
             Some(unsafe {
                 std::slice::from_raw_parts(
                     (*self.as_ptr()).indexes.edge_insertion_order as *const EdgeId,
-                    crate::util::handle_u64_to_usize((*self.as_ptr()).indexes.num_edges),
+                    usize::try_from((*self.as_ptr()).indexes.num_edges).unwrap(),
                 )
             })
         } else {
@@ -599,7 +599,7 @@ impl TableCollection {
             Some(unsafe {
                 std::slice::from_raw_parts(
                     (*self.as_ptr()).indexes.edge_removal_order as *const EdgeId,
-                    crate::util::handle_u64_to_usize((*self.as_ptr()).indexes.num_edges),
+                    usize::try_from((*self.as_ptr()).indexes.num_edges).unwrap(),
                 )
             })
         } else {
@@ -798,7 +798,7 @@ impl TableCollection {
     ) -> Result<Option<Vec<NodeId>>, TskitError> {
         let mut output_node_map: Vec<NodeId> = vec![];
         if idmap {
-            output_node_map.resize(usize::from(self.nodes().num_rows()), NodeId::NULL);
+            output_node_map.resize(usize::try_from(self.nodes().num_rows())?, NodeId::NULL);
         }
         let rv = unsafe {
             ll_bindings::tsk_table_collection_simplify(
@@ -1105,11 +1105,11 @@ mod test {
         assert!(tables.is_indexed());
         assert_eq!(
             tables.edge_insertion_order().unwrap().len(),
-            tables.edges().num_rows().into()
+            tables.edges().num_rows().try_into().unwrap()
         );
         assert_eq!(
             tables.edge_removal_order().unwrap().len(),
-            tables.edges().num_rows().into()
+            tables.edges().num_rows().try_into().unwrap()
         );
 
         for i in tables.edge_insertion_order().unwrap() {
@@ -1362,7 +1362,7 @@ mod test {
 
         let mut num_with_metadata = 0;
         let mut num_without_metadata = 0;
-        for i in 0..usize::from(tables.mutations().num_rows()) {
+        for i in 0..usize::try_from(tables.mutations().num_rows()).unwrap() {
             match tables
                 .mutations()
                 .metadata::<F>((i as tsk_id_t).into())
@@ -1837,7 +1837,7 @@ mod test_adding_site {
                 Some(metadata[mi])
             );
         }
-        for i in 0..usize::from(tables.sites().num_rows()) {
+        for i in 0..usize::try_from(tables.sites().num_rows()).unwrap() {
             assert!(
                 tables.sites().row(SiteId::from(i as tsk_id_t)).unwrap()
                     == tables.sites().row(SiteId::from(i as tsk_id_t)).unwrap()

src/trees.rs

@@ -714,9 +714,9 @@ impl<'a> PostorderNodeIterator<'a> {
         let mut nodes = vec![
             NodeId::NULL;
             // NOTE: this fn does not return error codes
-            crate::util::handle_u64_to_usize(unsafe {
+            usize::try_from(unsafe {
                 ll_bindings::tsk_tree_get_size_bound(tree.as_ptr())
-            })
+            }).unwrap()
         ];
 
         let rv = unsafe {
@@ -738,7 +738,7 @@ impl<'a> PostorderNodeIterator<'a> {
         Self {
             nodes,
             current_node_index: 0,
-            num_nodes_current_tree: crate::util::handle_u64_to_usize(num_nodes_current_tree),
+            num_nodes_current_tree: usize::try_from(num_nodes_current_tree).unwrap(),
             tree: std::marker::PhantomData,
         }
     }
@@ -1187,7 +1187,7 @@ impl TreeSequence {
         let mut ts = tables.tree_sequence(TreeSequenceFlags::default())?;
         let mut output_node_map: Vec<NodeId> = vec![];
         if idmap {
-            output_node_map.resize(usize::from(self.nodes().num_rows()), NodeId::NULL);
+            output_node_map.resize(usize::try_from(self.nodes().num_rows())?, NodeId::NULL);
         }
         let rv = unsafe {
             ll_bindings::tsk_treeseq_simplify(
@@ -1430,7 +1430,7 @@ pub(crate) mod test_trees {
             assert_eq!(s.len(), 2);
             assert_eq!(
                 s.len(),
-                usize::from(tree.num_tracked_samples(0.into()).unwrap())
+                usize::try_from(tree.num_tracked_samples(0.into()).unwrap()).unwrap()
             );
             assert_eq!(s[0], 1);
             assert_eq!(s[1], 2);
@@ -1444,7 +1444,7 @@ pub(crate) mod test_trees {
                 assert_eq!(s[0], u);
                 assert_eq!(
                     s.len(),
-                    usize::from(tree.num_tracked_samples(u.into()).unwrap())
+                    usize::try_from(tree.num_tracked_samples(u.into()).unwrap()).unwrap()
                 );
             }
         } else {

src/util.rs

@@ -1,33 +1,3 @@
 pub(crate) fn partial_cmp_equal<T: PartialOrd>(lhs: &T, rhs: &T) -> bool {
     matches!(lhs.partial_cmp(rhs), Some(std::cmp::Ordering::Equal))
 }
-
-// If the system is not 16 or 32 bit, then u64 fits safely in the native
-// pointer width, so we can suppress the clippy pedantic lints
-//
-// To test 32 bit builds:
-//
-// 1. sudo apt install gcc-multilib
-// 2. rustup target install i686-unknown-linux-gnu
-// 3. cargo clean
-// 4. cargo test --target=i686-unknown-linux-gnu
-
-#[inline]
-#[cfg(not(any(target_pointer_width = "16", target_pointer_width = "32")))]
-#[allow(clippy::cast_possible_truncation)]
-/// Safely handle u64 -> usize on 16/32 vs 64 bit systems.
-/// On 16/32-bit systems, panic! if the conversion cannot happen.
-pub(crate) fn handle_u64_to_usize(v: u64) -> usize {
-    v as usize
-}
-
-#[inline]
-#[cfg(any(target_pointer_width = "16", target_pointer_width = "32"))]
-/// Safely handle u64 -> usize on 16/32 vs 64 bit systems.
-/// On 16/32-bit systems, panic! if the conversion cannot happen.
-pub(crate) fn handle_u64_to_usize(v: u64) -> usize {
-    match usize::try_from(v) {
-        Ok(u) => u,
-        Err(_) => panic!("could not convert {} to usize", v),
-    }
-}