Skip to content

Add safe wrappers for owning tables #433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 20 additions & 41 deletions src/_macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,24 +375,16 @@ macro_rules! handle_metadata_return {
}

macro_rules! build_owned_tables {
($name: ty, $deref: ident, $llname: ty, $init: ident, $free: ident, $clear: expr) => {
($name: ty, $deref: ident, $lltype: ty, $tsktable: ty) => {
impl $name {
fn new() -> Self {
let temp = unsafe { libc::malloc(std::mem::size_of::<$llname>()) as *mut $llname };
let nonnull = match std::ptr::NonNull::<$llname>::new(temp) {
Some(x) => x,
None => panic!("out of memory"),
};
let mut table = unsafe { mbox::MBox::from_non_null_raw(nonnull) };
let rv = unsafe { $init(&mut (*table), 0) };
assert_eq!(rv, 0);
let table = <$lltype>::new();
Self { table }
}

/// Clear the table.
pub fn clear(&mut self) -> $crate::TskReturnValue {
let rv = unsafe { $clear(self.as_mut_ptr()) };
handle_tsk_return_value!(rv)
self.table.clear().map_err(|e| e.into())
}
}

Expand Down Expand Up @@ -420,22 +412,13 @@ macro_rules! build_owned_tables {
}
}

impl Drop for $name {
fn drop(&mut self) {
let rv = unsafe { $free(&mut (*self.table)) };
if rv != 0 {
panic!("error when calling {}: {}", stringify!(free), rv);
}
}
}

impl $name {
pub fn as_ptr(&self) -> *const $llname {
&*self.table
pub fn as_ptr(&self) -> *const $tsktable {
self.table.as_ptr()
}

pub fn as_mut_ptr(&mut self) -> *mut $llname {
&mut *self.table as *mut $llname
pub fn as_mut_ptr(&mut self) -> *mut $tsktable {
self.table.as_mut_ptr()
}
}
};
Expand All @@ -451,7 +434,7 @@ macro_rules! node_table_add_row_details {
$table: expr) => {{
let rv = unsafe {
$crate::bindings::tsk_node_table_add_row(
&mut $table,
$table,
$flags.into().bits(),
$time.into().into(),
$population.into().into(),
Expand Down Expand Up @@ -532,7 +515,7 @@ macro_rules! edge_table_add_row_details {
$table: expr) => {{
let rv = unsafe {
$crate::bindings::tsk_edge_table_add_row(
&mut $table,
$table,
$left.into().into(),
$right.into().into(),
$parent.into().into(),
Expand Down Expand Up @@ -605,7 +588,7 @@ macro_rules! edge_table_add_row_with_metadata {
macro_rules! population_table_add_row_details {
($metadata: expr, $metadata_len: expr, $table: expr) => {{
let rv = unsafe {
$crate::bindings::tsk_population_table_add_row(&mut $table, $metadata, $metadata_len)
$crate::bindings::tsk_population_table_add_row($table, $metadata, $metadata_len)
};
handle_tsk_return_value!(rv, rv.into())
}};
Expand Down Expand Up @@ -640,7 +623,7 @@ macro_rules! individual_table_add_row_details {
$table: expr) => {{
let rv = unsafe {
$crate::bindings::tsk_individual_table_add_row(
&mut $table,
$table,
$flags.into().bits(),
$location.get_slice().as_ptr().cast::<f64>(),
$location.get_slice().len() as $crate::bindings::tsk_size_t,
Expand Down Expand Up @@ -715,7 +698,7 @@ macro_rules! mutation_table_add_row_details {
let dstate = process_state_input!($derived_state);
let rv = unsafe {
$crate::bindings::tsk_mutation_table_add_row(
&mut $table,
$table,
$site.into().into(),
$node.into().into(),
$parent.into().into(),
Expand Down Expand Up @@ -796,7 +779,7 @@ macro_rules! site_table_add_row_details {
let astate = process_state_input!($ancestral_state);
let rv = unsafe {
$crate::bindings::tsk_site_table_add_row(
&mut $table,
$table,
$position.into().into(),
astate.0,
astate.1,
Expand Down Expand Up @@ -853,7 +836,7 @@ macro_rules! migration_table_add_row_details {
$table: expr) => {{
let rv = unsafe {
$crate::bindings::tsk_migration_table_add_row(
&mut $table,
$table,
$span.0.into().into(),
$span.1.into().into(),
$node.into().into(),
Expand Down Expand Up @@ -928,7 +911,7 @@ macro_rules! provenance_table_add_row {
let timestamp = humantime::format_rfc3339(std::time::SystemTime::now()).to_string();
let rv = unsafe {
$crate::bindings::tsk_provenance_table_add_row(
&mut $table,
$table,
timestamp.as_ptr() as *mut i8,
timestamp.len() as tsk_size_t,
record.as_ptr() as *mut i8,
Expand All @@ -943,22 +926,18 @@ macro_rules! provenance_table_add_row {
macro_rules! build_owned_table_type {
($(#[$attr:meta])* => $name: ident,
$deref_type: ident,
$tskname: ident,
$tskinit: ident,
$tskfree: ident,
$tskclear: expr) => {
$lltype: ty,
$tsktable: ty) => {
$(#[$attr])*
pub struct $name {
table: mbox::MBox<$crate::bindings::$tskname>,
table: $lltype
}

build_owned_tables!(
$name,
$deref_type,
$crate::bindings::$tskname,
$tskinit,
$tskfree,
$tskclear
$lltype,
$tsktable
);
};
}
Expand Down
11 changes: 4 additions & 7 deletions src/edge_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use crate::sys;
use crate::Position;
use crate::{tsk_id_t, TskitError};
use crate::{EdgeId, NodeId};
use ll_bindings::{tsk_edge_table_free, tsk_edge_table_init};

/// Row of an [`EdgeTable`]
#[derive(Debug)]
Expand Down Expand Up @@ -360,13 +359,11 @@ build_owned_table_type!(
/// ```
=> OwningEdgeTable,
EdgeTable,
tsk_edge_table_t,
tsk_edge_table_init,
tsk_edge_table_free,
crate::bindings::tsk_edge_table_clear
crate::sys::LLOwningEdgeTable,
crate::bindings::tsk_edge_table_t
);

impl OwningEdgeTable {
edge_table_add_row!(=> add_row, self, *self.table);
edge_table_add_row_with_metadata!(=> add_row_with_metadata, self, *self.table);
edge_table_add_row!(=> add_row, self, self.as_mut_ptr());
edge_table_add_row_with_metadata!(=> add_row_with_metadata, self, self.as_mut_ptr());
}
11 changes: 4 additions & 7 deletions src/individual_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use crate::IndividualFlags;
use crate::IndividualId;
use crate::Location;
use crate::{tsk_id_t, TskitError};
use ll_bindings::{tsk_individual_table_free, tsk_individual_table_init};

/// Row of a [`IndividualTable`]
#[derive(Debug)]
Expand Down Expand Up @@ -488,13 +487,11 @@ build_owned_table_type!(
/// ```
=> OwningIndividualTable,
IndividualTable,
tsk_individual_table_t,
tsk_individual_table_init,
tsk_individual_table_free,
crate::bindings::tsk_individual_table_clear
crate::sys::LLOwningIndividualTable,
crate::bindings::tsk_individual_table_t
);

impl OwningIndividualTable {
individual_table_add_row!(=> add_row, self, *self.table);
individual_table_add_row_with_metadata!(=> add_row_with_metadata, self, *self.table);
individual_table_add_row!(=> add_row, self, self.as_mut_ptr());
individual_table_add_row_with_metadata!(=> add_row_with_metadata, self, self.as_mut_ptr());
}
11 changes: 4 additions & 7 deletions src/migration_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use crate::SizeType;
use crate::Time;
use crate::{tsk_id_t, TskitError};
use crate::{MigrationId, NodeId, PopulationId};
use ll_bindings::{tsk_migration_table_free, tsk_migration_table_init};

/// Row of a [`MigrationTable`]
#[derive(Debug)]
Expand Down Expand Up @@ -419,13 +418,11 @@ build_owned_table_type!(
/// ```
=> OwningMigrationTable,
MigrationTable,
tsk_migration_table_t,
tsk_migration_table_init,
tsk_migration_table_free,
ll_bindings::tsk_migration_table_clear
crate::sys::LLOwningMigrationTable,
crate::bindings::tsk_migration_table_t
);

impl OwningMigrationTable {
migration_table_add_row!(=> add_row, self, *self.table);
migration_table_add_row_with_metadata!(=> add_row_with_metadata, self, *self.table);
migration_table_add_row!(=> add_row, self, self.as_mut_ptr());
migration_table_add_row_with_metadata!(=> add_row_with_metadata, self, self.as_mut_ptr());
}
11 changes: 4 additions & 7 deletions src/mutation_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use crate::SizeType;
use crate::Time;
use crate::{tsk_id_t, TskitError};
use crate::{MutationId, NodeId, SiteId};
use ll_bindings::{tsk_mutation_table_free, tsk_mutation_table_init};

/// Row of a [`MutationTable`]
#[derive(Debug)]
Expand Down Expand Up @@ -394,13 +393,11 @@ build_owned_table_type!(
/// ```
=> OwningMutationTable,
MutationTable,
tsk_mutation_table_t,
tsk_mutation_table_init,
tsk_mutation_table_free,
ll_bindings::tsk_mutation_table_clear
crate::sys::LLOwningMutationTable,
crate::bindings::tsk_mutation_table_t
);

impl OwningMutationTable {
mutation_table_add_row!(=> add_row, self, *self.table);
mutation_table_add_row_with_metadata!(=> add_row_with_metadata, self, *self.table);
mutation_table_add_row!(=> add_row, self, self.as_mut_ptr());
mutation_table_add_row_with_metadata!(=> add_row_with_metadata, self, self.as_mut_ptr());
}
11 changes: 4 additions & 7 deletions src/node_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use crate::SizeType;
use crate::Time;
use crate::{tsk_id_t, TskitError};
use crate::{IndividualId, NodeId, PopulationId};
use ll_bindings::{tsk_node_table_free, tsk_node_table_init};

/// Row of a [`NodeTable`]
#[derive(Debug)]
Expand Down Expand Up @@ -583,15 +582,13 @@ build_owned_table_type!(
/// ```
=> OwningNodeTable,
NodeTable,
tsk_node_table_t,
tsk_node_table_init,
tsk_node_table_free,
ll_bindings::tsk_node_table_clear
crate::sys::LLOwningNodeTable,
crate::bindings::tsk_node_table_t
);

impl OwningNodeTable {
node_table_add_row!(=> add_row, self, (*self.table));
node_table_add_row_with_metadata!(=> add_row_with_metadata, self, (*self.table));
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());
}

#[cfg(test)]
Expand Down
11 changes: 4 additions & 7 deletions src/population_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use crate::tsk_id_t;
use crate::PopulationId;
use crate::SizeType;
use crate::TskitError;
use ll_bindings::{tsk_population_table_free, tsk_population_table_init};

/// Row of a [`PopulationTable`]
#[derive(Eq, Debug)]
Expand Down Expand Up @@ -258,13 +257,11 @@ build_owned_table_type!(
/// ```
=> OwningPopulationTable,
PopulationTable,
tsk_population_table_t,
tsk_population_table_init,
tsk_population_table_free,
ll_bindings::tsk_population_table_clear
crate::sys::LLOwningPopulationTable,
crate::bindings::tsk_population_table_t
);

impl OwningPopulationTable {
population_table_add_row!(=> add_row, self, *self.table);
population_table_add_row_with_metadata!(=> add_row_with_metadata, self, *self.table);
population_table_add_row!(=> add_row, self, self.as_mut_ptr());
population_table_add_row_with_metadata!(=> add_row_with_metadata, self, self.as_mut_ptr());
}
9 changes: 3 additions & 6 deletions src/provenance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use crate::bindings as ll_bindings;
use crate::sys;
use crate::SizeType;
use crate::{tsk_id_t, tsk_size_t, ProvenanceId};
use ll_bindings::{tsk_provenance_table_free, tsk_provenance_table_init};

#[derive(Eq, Debug)]
/// Row of a [`ProvenanceTable`].
Expand Down Expand Up @@ -287,14 +286,12 @@ build_owned_table_type!(
/// ```
=> OwningProvenanceTable,
ProvenanceTable,
tsk_provenance_table_t,
tsk_provenance_table_init,
tsk_provenance_table_free,
ll_bindings::tsk_provenance_table_clear
crate::sys::LLOwningProvenanceTable,
crate::bindings::tsk_provenance_table_t
);

impl OwningProvenanceTable {
provenance_table_add_row!(=> add_row, self, *self.table);
provenance_table_add_row!(=> add_row, self, self.as_mut_ptr());
}

#[cfg(test)]
Expand Down
11 changes: 4 additions & 7 deletions src/site_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use crate::Position;
use crate::SiteId;
use crate::SizeType;
use crate::TskitError;
use ll_bindings::{tsk_site_table_free, tsk_site_table_init};

/// Row of a [`SiteTable`]
#[derive(Debug)]
Expand Down Expand Up @@ -309,13 +308,11 @@ build_owned_table_type!(
/// ```
=> OwningSiteTable,
SiteTable,
tsk_site_table_t,
tsk_site_table_init,
tsk_site_table_free,
ll_bindings::tsk_site_table_clear
crate::sys::LLOwningSiteTable,
crate::bindings::tsk_site_table_t
);

impl OwningSiteTable {
site_table_add_row!(=> add_row, self, *self.table);
site_table_add_row_with_metadata!(=> add_row_with_metadata, self, *self.table);
site_table_add_row!(=> add_row, self, self.as_mut_ptr());
site_table_add_row_with_metadata!(=> add_row_with_metadata, self, self.as_mut_ptr());
}
Loading