refactor: replace macros with fn for adding node table rows (#489)

tskit-dev · Mar 16, 2023 · 0ccaa7a · 0ccaa7a
1 parent e342718
commit 0ccaa7a
Show file tree

Hide file tree

Showing 3 changed files with 157 additions and 87 deletions.
diff --git a/src/_macros.rs b/src/_macros.rs
@@ -430,87 +430,6 @@ macro_rules! build_owned_tables {
     };
 }
 
-macro_rules! node_table_add_row_details {
-    ($flags: ident,
-     $time: ident,
-     $population: ident,
-     $individual: ident,
-     $metadata: expr,
-     $metadata_len: expr,
-     $table: expr) => {{
-        let rv = unsafe {
-            $crate::bindings::tsk_node_table_add_row(
-                $table,
-                $flags.into().bits(),
-                $time.into().into(),
-                $population.into().into(),
-                $individual.into().into(),
-                $metadata,
-                $metadata_len,
-            )
-        };
-        handle_tsk_return_value!(rv, rv.into())
-    }};
-}
-
-macro_rules! node_table_add_row {
-    ($(#[$attr:meta])* => $name: ident, $self: ident, $table: expr) => {
-        $(#[$attr])*
-        pub fn $name<F,T,P,I>(
-            &mut $self,
-            flags: F ,
-            time: T ,
-            population: P ,
-            individual: I ,
-        ) -> Result<$crate::NodeId, $crate::TskitError>
-        where
-            F: Into<$crate::NodeFlags>,
-            T:Into<$crate::Time>,
-            P:Into<$crate::PopulationId>,
-            I:Into<$crate::IndividualId>
-        {
-            node_table_add_row_details!(flags,
-                                        time,
-                                        population,
-                                        individual,
-                                        std::ptr::null(),
-                                        0,
-                                        $table)
-        }
-    };
-}
-
-macro_rules! node_table_add_row_with_metadata {
-    ($(#[$attr:meta])* => $name: ident, $self: ident, $table: expr) => {
-        $(#[$attr])*
-        pub fn $name<F,T,P,I,M>(
-            &mut $self,
-            flags: F,
-            time: T,
-            population: P,
-            individual: I,
-            metadata: &M,
-        ) -> Result<$crate::NodeId, $crate::TskitError>
-        where
-            F: Into<$crate::NodeFlags>,
-            T:Into<$crate::Time>,
-            P:Into<$crate::PopulationId>,
-            I:Into<$crate::IndividualId>,
-            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(),
-                                        mdlen.into(),
-                                        $table)
-        }
-    };
-}
-
 macro_rules! edge_table_add_row_details {
     ($left: ident,
      $right: ident,

diff --git a/src/node_table.rs b/src/node_table.rs
@@ -1,12 +1,88 @@
 use crate::bindings as ll_bindings;
 use crate::metadata;
+use crate::metadata::NodeMetadata;
 use crate::sys;
 use crate::NodeFlags;
 use crate::SizeType;
 use crate::Time;
 use crate::{tsk_id_t, TskitError};
 use crate::{IndividualId, NodeId, PopulationId};
 
+pub(crate) fn add_row_details(
+    flags: ll_bindings::tsk_flags_t,
+    time: f64,
+    population: ll_bindings::tsk_id_t,
+    individual: ll_bindings::tsk_id_t,
+    metadata: *const std::os::raw::c_char,
+    metadata_length: ll_bindings::tsk_size_t,
+    table: *mut ll_bindings::tsk_node_table_t,
+) -> Result<NodeId, TskitError> {
+    let rv = unsafe {
+        ll_bindings::tsk_node_table_add_row(
+            table,
+            flags,
+            time,
+            population,
+            individual,
+            metadata,
+            metadata_length,
+        )
+    };
+    handle_tsk_return_value!(rv, rv.into())
+}
+
+pub(crate) fn add_row<F, T, P, I>(
+    flags: F,
+    time: T,
+    population: P,
+    individual: I,
+    table: *mut ll_bindings::tsk_node_table_t,
+) -> Result<NodeId, TskitError>
+where
+    F: Into<NodeFlags>,
+    T: Into<Time>,
+    P: Into<PopulationId>,
+    I: Into<IndividualId>,
+{
+    add_row_details(
+        flags.into().bits(),
+        time.into().into(),
+        population.into().into(),
+        individual.into().into(),
+        std::ptr::null(),
+        0,
+        table,
+    )
+}
+
+pub(crate) fn add_row_with_metadata<F, T, P, I, N>(
+    flags: F,
+    time: T,
+    population: P,
+    individual: I,
+    metadata: &N,
+    table: *mut ll_bindings::tsk_node_table_t,
+) -> Result<NodeId, TskitError>
+where
+    F: Into<NodeFlags>,
+    T: Into<Time>,
+    P: Into<PopulationId>,
+    I: Into<IndividualId>,
+    N: NodeMetadata,
+{
+    let md = crate::metadata::EncodedMetadata::new(metadata)?;
+    let mdlen = md.len()?;
+    add_row_details(
+        flags.into().bits(),
+        time.into().into(),
+        population.into().into(),
+        individual.into().into(),
+        md.as_ptr(),
+        mdlen.into(),
+        table,
+    )
+}
+
 /// Row of a [`NodeTable`]
 #[derive(Debug)]
 pub struct NodeTableRow {
@@ -584,8 +660,46 @@ build_owned_table_type!(
 );
 
 impl OwningNodeTable {
-    node_table_add_row!(=> add_row, self, self.as_mut_ptr());
-    node_table_add_row_with_metadata!(=> add_row_with_metadata, self, self.as_mut_ptr());
+    pub fn add_row<F, T, P, I>(
+        &mut self,
+        flags: F,
+        time: T,
+        population: P,
+        individual: I,
+    ) -> Result<NodeId, TskitError>
+    where
+        F: Into<NodeFlags>,
+        T: Into<Time>,
+        P: Into<PopulationId>,
+        I: Into<IndividualId>,
+    {
+        add_row(flags, time, population, individual, self.as_mut_ptr())
+    }
+
+    pub fn add_row_with_metadata<F, T, P, I, N>(
+        &mut self,
+        flags: F,
+        time: T,
+        population: P,
+        individual: I,
+        metadata: &N,
+    ) -> Result<NodeId, TskitError>
+    where
+        F: Into<NodeFlags>,
+        T: Into<Time>,
+        P: Into<PopulationId>,
+        I: Into<IndividualId>,
+        N: NodeMetadata,
+    {
+        add_row_with_metadata(
+            flags,
+            time,
+            population,
+            individual,
+            metadata,
+            self.as_mut_ptr(),
+        )
+    }
 }
 
 #[cfg(test)]

diff --git a/src/table_collection.rs b/src/table_collection.rs
@@ -399,11 +399,25 @@ impl TableCollection {
     /// by tree sequence simplification.
     => add_migration_with_metadata, self, &mut (*self.as_mut_ptr()).migrations);
 
-    node_table_add_row!(
     /// Add a row to the node table
-    => add_node, self, &mut (*self.as_mut_ptr()).nodes);
+    pub fn add_node<F, T, P, I>(
+        &mut self,
+        flags: F,
+        time: T,
+        population: P,
+        individual: I,
+    ) -> Result<NodeId, TskitError>
+    where
+        F: Into<crate::NodeFlags>,
+        T: Into<crate::Time>,
+        P: Into<crate::PopulationId>,
+        I: Into<crate::IndividualId>,
+    {
+        crate::node_table::add_row(flags, time, population, individual, unsafe {
+            &mut (*self.as_mut_ptr()).nodes
+        })
+    }
 
-    node_table_add_row_with_metadata!(
     /// Add a row with optional metadata to the node table
     ///
     /// # Examples
@@ -424,7 +438,30 @@ impl TableCollection {
     /// assert!(tables.add_node_with_metadata(0, 0.0, -1, -1, &metadata).is_ok());
     /// # }
     /// ```
-    => add_node_with_metadata, self, &mut (*self.as_mut_ptr()).nodes);
+    pub fn add_node_with_metadata<F, T, P, I, N>(
+        &mut self,
+        flags: F,
+        time: T,
+        population: P,
+        individual: I,
+        metadata: &N,
+    ) -> Result<NodeId, TskitError>
+    where
+        F: Into<crate::NodeFlags>,
+        T: Into<crate::Time>,
+        P: Into<crate::PopulationId>,
+        I: Into<crate::IndividualId>,
+        N: crate::metadata::NodeMetadata,
+    {
+        crate::node_table::add_row_with_metadata(
+            flags,
+            time,
+            population,
+            individual,
+            metadata,
+            unsafe { &mut (*self.as_mut_ptr()).nodes },
+        )
+    }
 
     site_table_add_row!(
     /// Add a row to the site table