refactor population table storage

src/population_table.rs

@@ -24,7 +24,7 @@ fn make_population_table_row(table: &PopulationTable, pos: tsk_id_t) -> Option<P
     // set up the iterator
     let p = crate::SizeType::try_from(pos).unwrap();
     if p < table.num_rows() {
-        let table_ref = table.table_;
+        let table_ref = unsafe { *table.table_ };
         let rv = PopulationTableRow {
             id: pos.into(),
             metadata: table_row_decode_metadata!(table_ref, pos),
@@ -36,9 +36,8 @@ fn make_population_table_row(table: &PopulationTable, pos: tsk_id_t) -> Option<P
 }
 
 pub(crate) type PopulationTableRefIterator<'a> =
-    crate::table_iterator::TableIterator<&'a PopulationTable<'a>>;
-pub(crate) type PopulationTableIterator<'a> =
-    crate::table_iterator::TableIterator<PopulationTable<'a>>;
+    crate::table_iterator::TableIterator<&'a PopulationTable>;
+pub(crate) type PopulationTableIterator<'a> = crate::table_iterator::TableIterator<PopulationTable>;
 
 impl<'a> Iterator for PopulationTableRefIterator<'a> {
     type Item = PopulationTableRow;
@@ -65,33 +64,40 @@ impl<'a> Iterator for PopulationTableIterator<'a> {
 /// These are not created directly.
 /// Instead, use [`TableAccess::populations`](crate::TableAccess::populations)
 /// to get a reference to an existing population table;
-pub struct PopulationTable<'a> {
-    table_: &'a ll_bindings::tsk_population_table_t,
+pub struct PopulationTable {
+    pub(crate) table_: *const ll_bindings::tsk_population_table_t,
 }
 
-impl<'a> PopulationTable<'a> {
-    pub(crate) fn new_from_table(mutations: &'a ll_bindings::tsk_population_table_t) -> Self {
-        PopulationTable { table_: mutations }
+impl PopulationTable {
+    fn ll_table_ref(&self) -> &ll_bindings::tsk_population_table_t {
+        unsafe { &(*self.table_) }
+    }
+
+    pub(crate) fn new_from_table(populations: *const ll_bindings::tsk_population_table_t) -> Self {
+        // assert!(!populations.is_null());
+        PopulationTable {
+            table_: populations,
+        }
     }
 
     /// Return the number of rows.
-    pub fn num_rows(&'a self) -> SizeType {
-        self.table_.num_rows.into()
+    pub fn num_rows(&self) -> SizeType {
+        self.ll_table_ref().num_rows.into()
     }
 
     pub fn metadata<T: metadata::MetadataRoundtrip>(
-        &'a self,
+        &self,
         row: PopulationId,
     ) -> Result<Option<T>, TskitError> {
-        let table_ref = self.table_;
+        let table_ref = unsafe { *self.table_ };
         let buffer = metadata_to_vector!(table_ref, row.0)?;
         decode_metadata_row!(T, buffer)
     }
 
     /// Return an iterator over rows of the table.
     /// The value of the iterator is [`PopulationTableRow`].
     pub fn iter(&self) -> impl Iterator<Item = PopulationTableRow> + '_ {
-        crate::table_iterator::make_table_iterator::<&PopulationTable<'a>>(self)
+        crate::table_iterator::make_table_iterator::<&PopulationTable>(self)
     }
 
     /// Return row `r` of the table.
@@ -114,3 +120,74 @@ impl<'a> PopulationTable<'a> {
         table_row_access!(ri.0, self, make_population_table_row)
     }
 }
+
+pub struct OwningPopulationTable {
+    pointer: mbox::MBox<ll_bindings::tsk_population_table_t>,
+    deref_target: PopulationTable,
+}
+
+impl OwningPopulationTable {
+    fn new() -> Self {
+        let pointer = unsafe {
+            libc::malloc(std::mem::size_of::<ll_bindings::tsk_population_table_t>())
+                as *mut ll_bindings::tsk_population_table_t
+        };
+        // Gotta validate this code
+        let code = unsafe { ll_bindings::tsk_population_table_init(pointer, 0) };
+        assert!(!pointer.is_null()); // Should Err here if true!
+        let deref_target = PopulationTable::new_from_table(std::ptr::null());
+        let nonnull = match std::ptr::NonNull::<ll_bindings::tsk_population_table_t>::new(pointer) {
+            Some(x) => x,
+            None => panic!("out of memory"),
+        };
+        let mbox = unsafe { mbox::MBox::from_non_null_raw(nonnull) };
+        let mut rv = Self {
+            pointer: mbox,
+            deref_target,
+        };
+        rv.deref_target.table_ = &(*rv.pointer) as *const ll_bindings::tsk_population_table_t;
+        rv
+    }
+}
+
+impl OwningPopulationTable {
+    fn add_row(&mut self) -> Result<PopulationId, TskitError> {
+        let rv = unsafe {
+            ll_bindings::tsk_population_table_add_row(&mut (*self.pointer), std::ptr::null(), 0)
+        };
+        handle_tsk_return_value!(rv, rv.into())
+    }
+}
+
+impl std::ops::Deref for OwningPopulationTable {
+    type Target = PopulationTable;
+    fn deref(&self) -> &Self::Target {
+        &self.deref_target
+    }
+}
+
+impl Drop for OwningPopulationTable {
+    fn drop(&mut self) {
+        let rv = unsafe {
+            ll_bindings::tsk_population_table_free(
+                &mut (*self.pointer) as *mut ll_bindings::tsk_population_table_t,
+            )
+        };
+        assert_eq!(rv, 0);
+        //assert!(!self.pointer.is_null());
+        //unsafe { libc::free(self.pointer as *mut libc::c_void) };
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn add_population() {
+        let mut p = OwningPopulationTable::new();
+        let x = p.add_row().unwrap();
+        assert_eq!(x, 0);
+        assert_eq!(p.num_rows(), 1);
+    }
+}

src/table_collection.rs

@@ -67,13 +67,36 @@ use mbox::MBox;
 ///
 pub struct TableCollection {
     pub(crate) inner: MBox<ll_bindings::tsk_table_collection_t>,
+    populations: PopulationTable,
 }
 
-build_tskit_type!(
-    TableCollection,
-    ll_bindings::tsk_table_collection_t,
-    tsk_table_collection_free
-);
+//build_tskit_type!(
+//    TableCollection,
+//    ll_bindings::tsk_table_collection_t,
+//    tsk_table_collection_free
+//);
+impl crate::ffi::WrapTskitType<ll_bindings::tsk_table_collection_t> for TableCollection {
+    fn wrap() -> Self {
+        let temp = unsafe {
+            libc::malloc(std::mem::size_of::<ll_bindings::tsk_table_collection_t>())
+                as *mut ll_bindings::tsk_table_collection_t
+        };
+        let nonnull = match std::ptr::NonNull::<ll_bindings::tsk_table_collection_t>::new(temp) {
+            Some(x) => x,
+            None => panic!("out of memory"),
+        };
+        let mbox = unsafe { MBox::from_non_null_raw(nonnull) };
+        let populations = PopulationTable::new_from_table(std::ptr::null());
+        let rv = Self {
+            inner: mbox,
+            populations,
+        };
+        rv
+    }
+}
+
+drop_for_tskit_type!(TableCollection, tsk_table_collection_free);
+tskit_type_access!(TableCollection, ll_bindings::tsk_table_collection_t);
 
 impl TableCollection {
     /// Create a new table collection with a sequence length.
@@ -102,6 +125,9 @@ impl TableCollection {
         if rv < 0 {
             return Err(crate::error::TskitError::ErrorCode { code: rv });
         }
+
+        // NOTE: gotta be like this
+        tables.populations.table_ = &(*tables.inner).populations;
         unsafe {
             (*tables.as_mut_ptr()).sequence_length = sequence_length.0;
         }
@@ -1222,8 +1248,8 @@ impl TableAccess for TableCollection {
         MutationTable::new_from_table(&(*self.inner).mutations)
     }
 
-    fn populations(&self) -> PopulationTable {
-        PopulationTable::new_from_table(&(*self.inner).populations)
+    fn populations(&self) -> &PopulationTable {
+        &self.populations
     }
 
     #[cfg(any(feature = "provenance", doc))]

src/traits.rs

@@ -84,13 +84,13 @@ pub trait TableAccess {
     }
 
     /// Get reference to the [``PopulationTable``](crate::PopulationTable).
-    fn populations(&self) -> PopulationTable;
+    fn populations(&self) -> &PopulationTable;
 
     /// Return an iterator over the populations.
     fn populations_iter(
         &self,
     ) -> Box<dyn Iterator<Item = crate::population_table::PopulationTableRow> + '_> {
-        Box::new(make_table_iterator::<PopulationTable>(self.populations()))
+        Box::new(make_table_iterator::<&PopulationTable>(self.populations()))
     }
 
     /// Get reference to the [``MigrationTable``](crate::MigrationTable).

src/trees.rs

@@ -959,9 +959,32 @@ iterator_for_nodeiterator!(SamplesIterator<'_>);
 /// ```
 pub struct TreeSequence {
     pub(crate) inner: MBox<ll_bindings::tsk_treeseq_t>,
+    populations: PopulationTable,
 }
+impl crate::ffi::WrapTskitType<ll_bindings::tsk_treeseq_t> for TreeSequence {
+    fn wrap() -> Self {
+        let temp = unsafe {
+            libc::malloc(std::mem::size_of::<ll_bindings::tsk_treeseq_t >())
+                as *mut ll_bindings::tsk_treeseq_t  
+        };
+        let nonnull = match std::ptr::NonNull::<ll_bindings::tsk_treeseq_t  >::new(temp) {
+            Some(x) => x,
+            None => panic!("out of memory"),
+        };
+        let mbox = unsafe { MBox::from_non_null_raw(nonnull) };
+        let populations = PopulationTable::new_from_table(std::ptr::null());
+        let mut rv = Self {
+            inner: mbox,
+            populations,
+        };
+        rv
+    }
+}
+
+drop_for_tskit_type!(TreeSequence, tsk_treeseq_free);
+tskit_type_access!(TreeSequence, ll_bindings::tsk_treeseq_t);
 
-build_tskit_type!(TreeSequence, ll_bindings::tsk_treeseq_t, tsk_treeseq_free);
+//build_tskit_type!(TreeSequence, ll_bindings::tsk_treeseq_t, tsk_treeseq_free);
 
 impl TreeSequence {
     /// Create a tree sequence from a [`TableCollection`].
@@ -1017,6 +1040,7 @@ impl TreeSequence {
         let raw_tables_ptr = tables.into_raw()?;
         let rv =
             unsafe { ll_bindings::tsk_treeseq_init(treeseq.as_mut_ptr(), raw_tables_ptr, flags) };
+        treeseq.populations.table_ = &unsafe { *(*treeseq.inner).tables }.populations;
         handle_tsk_return_value!(rv, treeseq)
     }
 
@@ -1308,8 +1332,8 @@ impl TableAccess for TreeSequence {
         MutationTable::new_from_table(unsafe { &(*(*self.inner).tables).mutations })
     }
 
-    fn populations(&self) -> PopulationTable {
-        PopulationTable::new_from_table(unsafe { &(*(*self.inner).tables).populations })
+    fn populations(&self) -> &PopulationTable {
+        &self.populations
     }
 
     #[cfg(any(feature = "provenance", doc))]