refactor: add low-level table wrappers to sys

* transparent newtypes over NonNull
* moves lots of unsafe from table modules to sys.

Author: molpopgen

src/edge_table.rs

@@ -1,5 +1,3 @@
-use std::ptr::NonNull;
-
 use crate::bindings as ll_bindings;
 use crate::metadata;
 use crate::sys;
@@ -148,22 +146,19 @@ impl<'a> streaming_iterator::StreamingIterator for EdgeTableRowView<'a> {
 #[repr(transparent)]
 #[derive(Debug)]
 pub struct EdgeTable {
-    pub(crate) table_: NonNull<ll_bindings::tsk_edge_table_t>,
+    pub(crate) table_: sys::LLEdgeTableRef,
 }
 
 impl EdgeTable {
     pub(crate) fn new_from_table(
         edges: *mut ll_bindings::tsk_edge_table_t,
     ) -> Result<Self, TskitError> {
-        let n = NonNull::new(edges).ok_or_else(|| {
-            TskitError::LibraryError("null pointer to tsk_edge_table_t".to_string())
-        })?;
-        Ok(EdgeTable { table_: n })
+        let table_ = sys::LLEdgeTableRef::new_from_table(edges)?;
+        Ok(EdgeTable { table_ })
     }
 
     pub(crate) fn as_ref(&self) -> &ll_bindings::tsk_edge_table_t {
-        // SAFETY: NonNull
-        unsafe { self.table_.as_ref() }
+        self.table_.as_ref()
     }
 
     /// Return the number of rows

src/individual_table.rs

@@ -1,5 +1,3 @@
-use std::ptr::NonNull;
-
 use crate::bindings as ll_bindings;
 use crate::metadata;
 use crate::sys;
@@ -105,7 +103,7 @@ impl<'a> streaming_iterator::StreamingIterator for IndividualTableRowView<'a> {
 /// [`crate::table_views::TableViews`]
 #[derive(Debug)]
 pub struct IndividualTable {
-    table_: NonNull<ll_bindings::tsk_individual_table_t>,
+    table_: sys::LLIndividualTableRef,
 }
 
 fn make_individual_table_row(table: &IndividualTable, pos: tsk_id_t) -> Option<IndividualTableRow> {
@@ -146,15 +144,12 @@ impl IndividualTable {
     pub(crate) fn new_from_table(
         individuals: *mut ll_bindings::tsk_individual_table_t,
     ) -> Result<Self, TskitError> {
-        let n = NonNull::new(individuals).ok_or_else(|| {
-            TskitError::LibraryError("null pointer to tsk_individual_table_t".to_string())
-        })?;
-        Ok(IndividualTable { table_: n })
+        let table_ = sys::LLIndividualTableRef::new_from_table(individuals)?;
+        Ok(IndividualTable { table_ })
     }
 
     pub(crate) fn as_ref(&self) -> &ll_bindings::tsk_individual_table_t {
-        // SAFETY: NonNull
-        unsafe { self.table_.as_ref() }
+        self.table_.as_ref()
     }
 
     raw_metadata_getter_for_tables!(IndividualId);

src/migration_table.rs

@@ -1,5 +1,3 @@
-use std::ptr::NonNull;
-
 use crate::bindings as ll_bindings;
 use crate::metadata;
 use crate::sys;
@@ -167,22 +165,19 @@ impl<'a> streaming_iterator::StreamingIterator for MigrationTableRowView<'a> {
 /// [`crate::table_views::TableViews`]
 #[derive(Debug)]
 pub struct MigrationTable {
-    table_: NonNull<ll_bindings::tsk_migration_table_t>,
+    table_: sys::LLMigrationTableRef,
 }
 
 impl MigrationTable {
     pub(crate) fn new_from_table(
         migrations: *mut ll_bindings::tsk_migration_table_t,
     ) -> Result<Self, TskitError> {
-        let n = NonNull::new(migrations).ok_or_else(|| {
-            TskitError::LibraryError("null pointer to tsk_migration_table_t".to_string())
-        })?;
-        Ok(MigrationTable { table_: n })
+        let table_ = sys::LLMigrationTableRef::new_from_table(migrations)?;
+        Ok(MigrationTable { table_ })
     }
 
     pub(crate) fn as_ref(&self) -> &ll_bindings::tsk_migration_table_t {
-        // SAFETY: NonNull
-        unsafe { self.table_.as_ref() }
+        self.table_.as_ref()
     }
 
     /// Return the number of rows

src/mutation_table.rs

@@ -1,5 +1,3 @@
-use std::ptr::NonNull;
-
 use crate::bindings as ll_bindings;
 use crate::metadata;
 use crate::sys;
@@ -164,22 +162,19 @@ impl<'a> streaming_iterator::StreamingIterator for MutationTableRowView<'a> {
 /// [`crate::table_views::TableViews`]
 #[derive(Debug)]
 pub struct MutationTable {
-    table_: NonNull<ll_bindings::tsk_mutation_table_t>,
+    table_: sys::LLMutationTableRef,
 }
 
 impl MutationTable {
     pub(crate) fn new_from_table(
         mutations: *mut ll_bindings::tsk_mutation_table_t,
     ) -> Result<Self, TskitError> {
-        let n = NonNull::new(mutations).ok_or_else(|| {
-            TskitError::LibraryError("null pointer to tsk_mutation_table_t".to_string())
-        })?;
-        Ok(MutationTable { table_: n })
+        let table_ = sys::LLMutationTableRef::new_from_table(mutations)?;
+        Ok(MutationTable { table_ })
     }
 
     pub(crate) fn as_ref(&self) -> &ll_bindings::tsk_mutation_table_t {
-        // SAFETY: NonNull
-        unsafe { self.table_.as_ref() }
+        self.table_.as_ref()
     }
 
     /// Return the number of rows.

src/node_table.rs

@@ -1,5 +1,3 @@
-use std::ptr::NonNull;
-
 use crate::bindings as ll_bindings;
 use crate::metadata;
 use crate::sys;
@@ -148,22 +146,19 @@ impl<'a> streaming_iterator::StreamingIterator for NodeTableRowView<'a> {
 /// [`crate::table_views::TableViews`]
 #[derive(Debug)]
 pub struct NodeTable {
-    table_: NonNull<ll_bindings::tsk_node_table_t>,
+    table_: sys::LLNodeTableRef,
 }
 
 impl NodeTable {
     pub(crate) fn new_from_table(
         nodes: *mut ll_bindings::tsk_node_table_t,
     ) -> Result<Self, TskitError> {
-        let n = NonNull::new(nodes).ok_or_else(|| {
-            TskitError::LibraryError("null pointer to tsk_node_table_t".to_string())
-        })?;
-        Ok(NodeTable { table_: n })
+        let table_ = sys::LLNodeTableRef::new_from_table(nodes)?;
+        Ok(NodeTable { table_ })
     }
 
     pub(crate) fn as_ref(&self) -> &ll_bindings::tsk_node_table_t {
-        // SAFETY: NonNull
-        unsafe { self.table_.as_ref() }
+        self.table_.as_ref()
     }
 
     /// Return the number of rows

src/population_table.rs

@@ -1,7 +1,6 @@
-use std::ptr::NonNull;
-
 use crate::bindings as ll_bindings;
 use crate::metadata;
+use crate::sys;
 use crate::tsk_id_t;
 use crate::PopulationId;
 use crate::SizeType;
@@ -116,22 +115,19 @@ impl<'a> streaming_iterator::StreamingIterator for PopulationTableRowView<'a> {
 #[repr(transparent)]
 #[derive(Debug)]
 pub struct PopulationTable {
-    table_: NonNull<ll_bindings::tsk_population_table_t>,
+    table_: sys::LLPopulationTableRef,
 }
 
 impl PopulationTable {
     pub(crate) fn new_from_table(
         populations: *mut ll_bindings::tsk_population_table_t,
     ) -> Result<Self, TskitError> {
-        let n = NonNull::new(populations).ok_or_else(|| {
-            TskitError::LibraryError("null pointer to tsk_population_table_t".to_string())
-        })?;
-        Ok(PopulationTable { table_: n })
+        let table_ = sys::LLPopulationTableRef::new_from_table(populations)?;
+        Ok(PopulationTable { table_ })
     }
 
     pub(crate) fn as_ref(&self) -> &ll_bindings::tsk_population_table_t {
-        // SAFETY: NonNull
-        unsafe { self.table_.as_ref() }
+        self.table_.as_ref()
     }
 
     raw_metadata_getter_for_tables!(PopulationId);

src/provenance.rs

@@ -10,8 +10,6 @@
 //!   [`ProvenanceTable::iter`].
 //!
 
-use std::ptr::NonNull;
-
 use crate::bindings as ll_bindings;
 use crate::sys;
 use crate::SizeType;
@@ -141,23 +139,19 @@ impl<'a> streaming_iterator::StreamingIterator for ProvenanceTableRowView<'a> {
 ///
 #[derive(Debug)]
 pub struct ProvenanceTable {
-    table_: NonNull<ll_bindings::tsk_provenance_table_t>,
+    table_: sys::LLProvenanceTableRef,
 }
 
 impl ProvenanceTable {
     pub(crate) fn new_from_table(
         provenances: *mut ll_bindings::tsk_provenance_table_t,
     ) -> Result<Self, crate::TskitError> {
-        // FIXME: unwrap
-        let n = NonNull::new(provenances).ok_or_else(|| {
-            crate::TskitError::LibraryError("null pointer to tsk_provenance_table_t".to_string())
-        })?;
-        Ok(ProvenanceTable { table_: n })
+        let table_ = sys::LLProvenanceTableRef::new_from_table(provenances)?;
+        Ok(ProvenanceTable { table_ })
     }
 
     pub(crate) fn as_ref(&self) -> &ll_bindings::tsk_provenance_table_t {
-        // SAFETY: NonNull
-        unsafe { self.table_.as_ref() }
+        self.table_.as_ref()
     }
 
     /// Return the number of rows

src/site_table.rs

@@ -1,5 +1,3 @@
-use std::ptr::NonNull;
-
 use crate::bindings as ll_bindings;
 use crate::metadata;
 use crate::sys;
@@ -134,22 +132,19 @@ impl<'a> streaming_iterator::StreamingIterator for SiteTableRowView<'a> {
 /// [`crate::table_views::TableViews`]
 #[derive(Debug)]
 pub struct SiteTable {
-    table_: NonNull<ll_bindings::tsk_site_table_t>,
+    table_: sys::LLSiteTableRef,
 }
 
 impl SiteTable {
     pub(crate) fn new_from_table(
         sites: *mut ll_bindings::tsk_site_table_t,
     ) -> Result<Self, TskitError> {
-        let n = NonNull::new(sites).ok_or_else(|| {
-            TskitError::LibraryError("null pointer to tsk_site_table_t".to_string())
-        })?;
-        Ok(SiteTable { table_: n })
+        let table_ = sys::LLSiteTableRef::new_from_table(sites)?;
+        Ok(SiteTable { table_ })
     }
 
     pub(crate) fn as_ref(&self) -> &ll_bindings::tsk_site_table_t {
-        // SAFETY: NonNull
-        unsafe { self.table_.as_ref() }
+        self.table_.as_ref()
     }
 
     raw_metadata_getter_for_tables!(SiteId);

src/sys.rs

@@ -1,4 +1,51 @@
-use crate::bindings;
+use crate::{bindings, TskitError};
+use bindings::tsk_edge_table_t;
+use bindings::tsk_individual_table_t;
+use bindings::tsk_migration_table_t;
+use bindings::tsk_mutation_table_t;
+use bindings::tsk_node_table_t;
+use bindings::tsk_population_table_t;
+use bindings::tsk_site_table_t;
+use std::ptr::NonNull;
+
+#[cfg(feature = "provenance")]
+use bindings::tsk_provenance_table_t;
+
+macro_rules! basic_lltableref_impl {
+    ($lltable: ident, $tsktable: ident) => {
+        #[repr(transparent)]
+        #[derive(Debug)]
+        pub struct $lltable(NonNull<bindings::$tsktable>);
+
+        impl $lltable {
+            pub fn new_from_table(table: *mut $tsktable) -> Result<Self, TskitError> {
+                let internal = NonNull::new(table).ok_or_else(|| {
+                    let msg = format!("null pointer to {}", stringify!($tsktable));
+                    TskitError::LibraryError(msg)
+                })?;
+                Ok(Self(internal))
+            }
+
+            pub fn as_ref(&self) -> &$tsktable {
+                // SAFETY: we cannot get this far w/o
+                // going through new_from_table and that
+                // fn protects us from null ptrs
+                unsafe { self.0.as_ref() }
+            }
+        }
+    };
+}
+
+basic_lltableref_impl!(LLEdgeTableRef, tsk_edge_table_t);
+basic_lltableref_impl!(LLNodeTableRef, tsk_node_table_t);
+basic_lltableref_impl!(LLMutationTableRef, tsk_mutation_table_t);
+basic_lltableref_impl!(LLSiteTableRef, tsk_site_table_t);
+basic_lltableref_impl!(LLMigrationTableRef, tsk_migration_table_t);
+basic_lltableref_impl!(LLPopulationTableRef, tsk_population_table_t);
+basic_lltableref_impl!(LLIndividualTableRef, tsk_individual_table_t);
+
+#[cfg(feature = "provenance")]
+basic_lltableref_impl!(LLProvenanceTableRef, tsk_provenance_table_t);
 
 fn tsk_column_access_detail<R: Into<bindings::tsk_id_t>, L: Into<bindings::tsk_size_t>, T: Copy>(
     row: R,

src/table_collection.rs

@@ -109,7 +109,7 @@ impl TableCollection {
         // AHA?
         assert!(std::ptr::eq(
             &mbox.as_ref().edges as *const ll_bindings::tsk_edge_table_t,
-            views.edges().table_.as_ptr() as *const ll_bindings::tsk_edge_table_t
+            views.edges().table_.as_ref() as *const ll_bindings::tsk_edge_table_t
         ));
         let mut tables = Self {
             inner: mbox,