Skip to content

remove table struct lifetimes #266

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

Closed
wants to merge 36 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
af22df6
remove lifetime from PopulationTable
molpopgen Jul 17, 2022
63ed6c4
all tests pass
molpopgen Jul 17, 2022
2d6d6d3
add compile fail
molpopgen Jul 17, 2022
30a4a17
breaking: change fn populations in TableAccess
molpopgen Jul 17, 2022
31f3c08
refactor init for TableCollection
molpopgen Jul 17, 2022
2241afe
and now the treeseq type
molpopgen Jul 17, 2022
61dfa4e
do provenance table now
molpopgen Jul 17, 2022
fb77705
the problem is record
molpopgen Jul 17, 2022
fb5763e
aha...
molpopgen Jul 17, 2022
7d5f60f
bit of refactor. mem error in treeseq.provenances()
molpopgen Jul 17, 2022
ced2104
yep, definitely the treeseq
molpopgen Jul 17, 2022
f798aa6
fmt
molpopgen Jul 17, 2022
6ef5959
treeseq add provenance seems to be the culprit
molpopgen Jul 18, 2022
7b8fedf
this is so odd...
molpopgen Jul 18, 2022
42cf541
simplify
molpopgen Jul 18, 2022
21c1b59
not clear what is up
molpopgen Jul 18, 2022
6821e81
more streamline
molpopgen Jul 18, 2022
d37a4f3
the problem is after transfer of ownership.
molpopgen Jul 18, 2022
3b014fc
streamline
molpopgen Jul 18, 2022
ba847c7
gotta be careful when setting inside the treeseq
molpopgen Jul 18, 2022
8a16319
remove lifetime annotation from SiteTable
molpopgen Jul 18, 2022
6e8249b
done w/site table
molpopgen Jul 18, 2022
26c5f95
edge tables done
molpopgen Jul 18, 2022
ca4d944
migrations
molpopgen Jul 18, 2022
8e3fbee
set that pointer...
molpopgen Jul 18, 2022
daad1f7
individual table
molpopgen Jul 18, 2022
bf0b7b8
mutation tables
molpopgen Jul 18, 2022
90e073a
node table
molpopgen Jul 18, 2022
badbb09
BREAKING:
molpopgen Jul 18, 2022
7db074a
Delete removed fns.
molpopgen Jul 18, 2022
93ddd19
Delete test of removed fns
molpopgen Jul 18, 2022
f9fc11d
tidy up conditional compilation
molpopgen Jul 18, 2022
863b95b
add failing test of unimplemented fn
molpopgen Jul 18, 2022
7800353
encapsulate table references
molpopgen Jul 18, 2022
d03fa01
same for tree seq
molpopgen Jul 18, 2022
74ed640
Tree now stores NodeTable
molpopgen Jul 18, 2022
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
68 changes: 48 additions & 20 deletions src/edge_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ fn make_edge_table_row(table: &EdgeTable, pos: tsk_id_t) -> Option<EdgeTableRow>
// 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 = EdgeTableRow {
id: pos.into(),
left: table.left(pos).unwrap(),
Expand All @@ -46,8 +46,8 @@ fn make_edge_table_row(table: &EdgeTable, pos: tsk_id_t) -> Option<EdgeTableRow>
}
}

pub(crate) type EdgeTableRefIterator<'a> = crate::table_iterator::TableIterator<&'a EdgeTable<'a>>;
pub(crate) type EdgeTableIterator<'a> = crate::table_iterator::TableIterator<EdgeTable<'a>>;
pub(crate) type EdgeTableRefIterator<'a> = crate::table_iterator::TableIterator<&'a EdgeTable>;
pub(crate) type EdgeTableIterator<'a> = crate::table_iterator::TableIterator<EdgeTable>;

impl<'a> Iterator for EdgeTableRefIterator<'a> {
type Item = EdgeTableRow;
Expand All @@ -74,18 +74,34 @@ impl<'a> Iterator for EdgeTableIterator<'a> {
/// These are not created directly.
/// Instead, use [`TableAccess::edges`](crate::TableAccess::edges)
/// to get a reference to an existing edge table;
pub struct EdgeTable<'a> {
table_: &'a ll_bindings::tsk_edge_table_t,
pub struct EdgeTable {
table_: *const ll_bindings::tsk_edge_table_t,
}

impl<'a> EdgeTable<'a> {
pub(crate) fn new_from_table(edges: &'a ll_bindings::tsk_edge_table_t) -> Self {
impl EdgeTable {
pub(crate) fn as_ll_ref(&self) -> &ll_bindings::tsk_edge_table_t {
// SAFETY: cannot be constructed with null pointer
unsafe { &(*self.table_) }
}

pub(crate) fn new_from_table(edges: &ll_bindings::tsk_edge_table_t) -> Self {
EdgeTable { table_: edges }
}

pub(crate) fn new_null() -> Self {
Self {
table_: std::ptr::null(),
}
}

pub(crate) fn set_ptr(&mut self, ptr: *const ll_bindings::tsk_edge_table_t) {
assert!(!ptr.is_null());
self.table_ = ptr;
}

/// Return the number of rows
pub fn num_rows(&'a self) -> crate::SizeType {
self.table_.num_rows.into()
pub fn num_rows(&self) -> crate::SizeType {
self.as_ll_ref().num_rows.into()
}

/// Return the ``parent`` value from row ``row`` of the table.
Expand All @@ -94,8 +110,14 @@ impl<'a> EdgeTable<'a> {
///
/// Will return [``IndexError``](crate::TskitError::IndexError)
/// if ``row`` is out of range.
pub fn parent<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Result<NodeId, TskitError> {
unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.parent, NodeId)
pub fn parent<E: Into<EdgeId> + Copy>(&self, row: E) -> Result<NodeId, TskitError> {
unsafe_tsk_column_access!(
row.into().0,
0,
self.num_rows(),
self.as_ll_ref().parent,
NodeId
)
}

/// Return the ``child`` value from row ``row`` of the table.
Expand All @@ -104,8 +126,14 @@ impl<'a> EdgeTable<'a> {
///
/// Will return [``IndexError``](crate::TskitError::IndexError)
/// if ``row`` is out of range.
pub fn child<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Result<NodeId, TskitError> {
unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.child, NodeId)
pub fn child<E: Into<EdgeId> + Copy>(&self, row: E) -> Result<NodeId, TskitError> {
unsafe_tsk_column_access!(
row.into().0,
0,
self.num_rows(),
self.as_ll_ref().child,
NodeId
)
}

/// Return the ``left`` value from row ``row`` of the table.
Expand All @@ -114,8 +142,8 @@ impl<'a> EdgeTable<'a> {
///
/// Will return [``IndexError``](crate::TskitError::IndexError)
/// if ``row`` is out of range.
pub fn left<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Result<Position, TskitError> {
match unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.left) {
pub fn left<E: Into<EdgeId> + Copy>(&self, row: E) -> Result<Position, TskitError> {
match unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.as_ll_ref().left) {
Ok(p) => Ok(p.into()),
Err(e) => Err(e),
}
Expand All @@ -127,18 +155,18 @@ impl<'a> EdgeTable<'a> {
///
/// Will return [``IndexError``](crate::TskitError::IndexError)
/// if ``row`` is out of range.
pub fn right<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Result<Position, TskitError> {
match unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.right) {
pub fn right<E: Into<EdgeId> + Copy>(&self, row: E) -> Result<Position, TskitError> {
match unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.as_ll_ref().right) {
Ok(p) => Ok(p.into()),
Err(e) => Err(e),
}
}

pub fn metadata<T: metadata::MetadataRoundtrip>(
&'a self,
&self,
row: EdgeId,
) -> 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)
}
Expand All @@ -147,7 +175,7 @@ impl<'a> EdgeTable<'a> {
/// The value of the iterator is [`EdgeTableRow`].
///
pub fn iter(&self) -> impl Iterator<Item = EdgeTableRow> + '_ {
crate::table_iterator::make_table_iterator::<&EdgeTable<'a>>(self)
crate::table_iterator::make_table_iterator::<&EdgeTable>(self)
}

/// Return row `r` of the table.
Expand Down
54 changes: 34 additions & 20 deletions src/individual_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ impl PartialEq for IndividualTableRow {
/// These are not created directly.
/// Instead, use [`TableAccess::individuals`](crate::TableAccess::individuals)
/// to get a reference to an existing node table;
pub struct IndividualTable<'a> {
table_: &'a ll_bindings::tsk_individual_table_t,
pub struct IndividualTable {
table_: *const ll_bindings::tsk_individual_table_t,
}

fn make_individual_table_row(table: &IndividualTable, pos: tsk_id_t) -> Option<IndividualTableRow> {
Expand All @@ -56,7 +56,7 @@ fn make_individual_table_row(table: &IndividualTable, pos: tsk_id_t) -> Option<I
// 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 = IndividualTableRow {
id: pos.into(),
flags: table.flags(pos).unwrap(),
Expand All @@ -71,9 +71,8 @@ fn make_individual_table_row(table: &IndividualTable, pos: tsk_id_t) -> Option<I
}

pub(crate) type IndividualTableRefIterator<'a> =
crate::table_iterator::TableIterator<&'a IndividualTable<'a>>;
pub(crate) type IndividualTableIterator<'a> =
crate::table_iterator::TableIterator<IndividualTable<'a>>;
crate::table_iterator::TableIterator<&'a IndividualTable>;
pub(crate) type IndividualTableIterator<'a> = crate::table_iterator::TableIterator<IndividualTable>;

impl<'a> Iterator for IndividualTableRefIterator<'a> {
type Item = IndividualTableRow;
Expand All @@ -95,16 +94,31 @@ impl<'a> Iterator for IndividualTableIterator<'a> {
}
}

impl<'a> IndividualTable<'a> {
pub(crate) fn new_from_table(individuals: &'a ll_bindings::tsk_individual_table_t) -> Self {
impl IndividualTable {
pub(crate) fn as_ll_ref(&self) -> &ll_bindings::tsk_individual_table_t {
unsafe { &(*self.table_) }
}

pub(crate) fn new_from_table(individuals: &ll_bindings::tsk_individual_table_t) -> Self {
IndividualTable {
table_: individuals,
}
}

pub(crate) fn new_null() -> Self {
Self {
table_: std::ptr::null(),
}
}

pub(crate) fn set_ptr(&mut self, ptr: *const ll_bindings::tsk_individual_table_t) {
assert!(!ptr.is_null());
self.table_ = ptr;
}

/// Return the number of rows
pub fn num_rows(&'a self) -> crate::SizeType {
self.table_.num_rows.into()
pub fn num_rows(&self) -> crate::SizeType {
self.as_ll_ref().num_rows.into()
}

/// Return the flags for a given row.
Expand All @@ -116,7 +130,7 @@ impl<'a> IndividualTable<'a> {
&self,
row: I,
) -> Result<IndividualFlags, TskitError> {
match unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.flags) {
match unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.as_ll_ref().flags) {
Ok(f) => Ok(IndividualFlags::from(f)),
Err(e) => Err(e),
}
Expand All @@ -135,9 +149,9 @@ impl<'a> IndividualTable<'a> {
row.into().0,
0,
self.num_rows(),
self.table_.location,
self.table_.location_offset,
self.table_.location_length,
self.as_ll_ref().location,
self.as_ll_ref().location_offset,
self.as_ll_ref().location_length,
Location
)
}
Expand All @@ -155,9 +169,9 @@ impl<'a> IndividualTable<'a> {
row.into().0,
0,
self.num_rows(),
self.table_.parents,
self.table_.parents_offset,
self.table_.parents_length,
self.as_ll_ref().parents,
self.as_ll_ref().parents_offset,
self.as_ll_ref().parents_length,
IndividualId
)
}
Expand Down Expand Up @@ -243,10 +257,10 @@ impl<'a> IndividualTable<'a> {
/// # }
/// ```
pub fn metadata<T: metadata::MetadataRoundtrip>(
&'a self,
&self,
row: IndividualId,
) -> 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)
}
Expand All @@ -255,7 +269,7 @@ impl<'a> IndividualTable<'a> {
/// The value of the iterator is [`IndividualTableRow`].
///
pub fn iter(&self) -> impl Iterator<Item = IndividualTableRow> + '_ {
crate::table_iterator::make_table_iterator::<&IndividualTable<'a>>(self)
crate::table_iterator::make_table_iterator::<&IndividualTable>(self)
}

/// Return row `r` of the table.
Expand Down
Loading