Audit pedantic lints re: integer casts

src/error.rs

@@ -7,7 +7,7 @@ use thiserror::Error;
 pub enum TskitError {
     /// Returned when conversion attempts fail
     #[error("range error: {}", *.0)]
-    RangeError(&'static str),
+    RangeError(String),
     /// Used when bad input is encountered.
     #[error("we received {} but expected {}",*got, *expected)]
     ValueError { got: String, expected: String },

src/lib.rs

@@ -276,7 +276,7 @@ impl From<SizeType> for tsk_size_t {
 
 impl From<SizeType> for usize {
     fn from(value: SizeType) -> Self {
-        value.0 as usize
+        crate::util::handle_u64_to_usize(value.0)
     }
 }
 
@@ -290,9 +290,11 @@ impl TryFrom<tsk_id_t> for SizeType {
     type Error = crate::TskitError;
 
     fn try_from(value: tsk_id_t) -> Result<Self, Self::Error> {
-        match value >= 0 {
-            true => Ok(Self(value as crate::bindings::tsk_size_t)),
-            false => Err(crate::TskitError::RangeError(stringify!(value.0))),
+        match tsk_size_t::try_from(value) {
+            Ok(v) => Ok(Self(v)),
+            Err(_) => Err(crate::TskitError::RangeError(
+                stringify!(value.0).to_string(),
+            )),
         }
     }
 }
@@ -301,10 +303,9 @@ impl TryFrom<SizeType> for tsk_id_t {
     type Error = crate::TskitError;
 
     fn try_from(value: SizeType) -> Result<Self, Self::Error> {
-        if value.0 > tsk_id_t::MAX as tsk_size_t {
-            Err(TskitError::RangeError(stringify!(value.0)))
-        } else {
-            Ok(value.0 as tsk_id_t)
+        match tsk_id_t::try_from(value.0) {
+            Ok(v) => Ok(v),
+            Err(_) => Err(TskitError::RangeError(stringify!(value.0).to_string())),
         }
     }
 }

src/metadata.rs

@@ -257,15 +257,28 @@ pub(crate) fn char_column_to_vector(
     num_rows: tsk_size_t,
     column_length: tsk_size_t,
 ) -> Result<Option<Vec<u8>>, crate::TskitError> {
-    if row < 0 || (row as tsk_size_t) >= num_rows {
+    let row = match tsk_size_t::try_from(row) {
+        Ok(r) => r,
+        Err(_) => return Err(TskitError::IndexError),
+    };
+    if row >= num_rows {
         return Err(crate::TskitError::IndexError {});
     }
     if column_length == 0 {
         return Ok(None);
     }
-    let start = unsafe { *column_offset.offset(row as isize) };
+    let row_isize = match isize::try_from(row) {
+        Ok(r) => r,
+        Err(_) => {
+            return Err(TskitError::RangeError(format!(
+                "could not convert u64 value {} to isize",
+                stringify!(row)
+            )))
+        }
+    };
+    let start = unsafe { *column_offset.offset(row_isize) };
     let stop = if (row as tsk_size_t) < num_rows {
-        unsafe { *column_offset.offset((row + 1) as isize) }
+        unsafe { *column_offset.offset(row_isize + 1) }
     } else {
         column_length
     };
@@ -278,8 +291,17 @@ pub(crate) fn char_column_to_vector(
     let mut buffer = vec![];
     for i in start..stop {
         match isize::try_from(i) {
+            // NOTE: cast_sign_loss pedantic lint is a false +ve here.
+            // The metadata live as C strings on the tskit-C side, so
+            // the integer cast exists as part of the round trip.
+            #[allow(clippy::cast_sign_loss)]
             Ok(o) => buffer.push(unsafe { *column.offset(o) } as u8),
-            Err(_) => return Err(TskitError::RangeError("could not convert value to isize")),
+            Err(_) => {
+                return Err(TskitError::RangeError(format!(
+                    "cauld not convert value {} to isize",
+                    stringify!(i)
+                )))
+            }
         };
     }
     Ok(Some(buffer))

src/node_table.rs

@@ -112,15 +112,20 @@ impl<'a> NodeTable<'a> {
 
     /// Mutable access to node flags.
     pub fn flags_array_mut(&mut self) -> &mut [tsk_flags_t] {
-        unsafe { std::slice::from_raw_parts_mut(self.table_.flags, self.table_.num_rows as usize) }
+        unsafe {
+            std::slice::from_raw_parts_mut(
+                self.table_.flags,
+                crate::util::handle_u64_to_usize(self.table_.num_rows),
+            )
+        }
     }
 
     /// Mutable access to node times.
     pub fn time_array_mut(&mut self) -> &mut [Time] {
         unsafe {
             std::slice::from_raw_parts_mut(
                 self.table_.time.cast::<Time>(),
-                self.table_.num_rows as usize,
+                crate::util::handle_u64_to_usize(self.table_.num_rows),
             )
         }
     }

src/table_collection.rs

@@ -574,7 +574,7 @@ impl TableCollection {
             Some(unsafe {
                 std::slice::from_raw_parts(
                     (*self.as_ptr()).indexes.edge_insertion_order as *const EdgeId,
-                    (*self.as_ptr()).indexes.num_edges as usize,
+                    crate::util::handle_u64_to_usize((*self.as_ptr()).indexes.num_edges),
                 )
             })
         } else {
@@ -590,7 +590,7 @@ impl TableCollection {
             Some(unsafe {
                 std::slice::from_raw_parts(
                     (*self.as_ptr()).indexes.edge_removal_order as *const EdgeId,
-                    (*self.as_ptr()).indexes.num_edges as usize,
+                    crate::util::handle_u64_to_usize((*self.as_ptr()).indexes.num_edges),
                 )
             })
         } else {

src/trees.rs

@@ -708,20 +708,22 @@ struct PostorderNodeIterator<'a> {
 
 impl<'a> PostorderNodeIterator<'a> {
     fn new(tree: &'a Tree) -> Self {
-        let mut num_nodes_current_tree: usize = 0;
+        let mut num_nodes_current_tree: tsk_size_t = 0;
         let ptr = std::ptr::addr_of_mut!(num_nodes_current_tree);
         let mut nodes = vec![
-                NodeId::NULL;
-                // NOTE: this fn does not return error codes
-                unsafe { ll_bindings::tsk_tree_get_size_bound(tree.as_ptr()) } as usize
-            ];
+            NodeId::NULL;
+            // NOTE: this fn does not return error codes
+            crate::util::handle_u64_to_usize(unsafe {
+                ll_bindings::tsk_tree_get_size_bound(tree.as_ptr())
+            })
+        ];
 
         let rv = unsafe {
             ll_bindings::tsk_tree_postorder(
                 tree.as_ptr(),
                 NodeId::NULL.into(), // start from virtual root
                 nodes.as_mut_ptr().cast::<tsk_id_t>(),
-                ptr.cast::<tsk_size_t>(),
+                ptr,
             )
         };
 
@@ -736,7 +738,7 @@ impl<'a> PostorderNodeIterator<'a> {
         Self {
             nodes,
             current_node_index: 0,
-            num_nodes_current_tree,
+            num_nodes_current_tree: crate::util::handle_u64_to_usize(num_nodes_current_tree),
             tree: std::marker::PhantomData,
         }
     }
@@ -838,7 +840,16 @@ struct ParentsIterator<'a> {
 
 impl<'a> ParentsIterator<'a> {
     fn new(tree: &'a Tree, u: NodeId) -> Result<Self, TskitError> {
-        match u.0 >= tree.num_nodes as tsk_id_t {
+        let num_nodes = match tsk_id_t::try_from(tree.num_nodes) {
+            Ok(n) => n,
+            Err(_) => {
+                return Err(TskitError::RangeError(format!(
+                    "could not convert {} into tsk_id_t",
+                    stringify!(num_nodes)
+                )))
+            }
+        };
+        match u.0 >= num_nodes {
             true => Err(TskitError::IndexError),
             false => Ok(ParentsIterator {
                 current_node: None,

src/util.rs

@@ -1,3 +1,33 @@
 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),
+    }
+}