Skip to content

Commit a7efd22

Browse files
committed
refactor: add low-level table wrappers to sys
* transparent newtypes over NonNull * moves lots of unsafe from table modules to sys.
1 parent c6a50d2 commit a7efd22

10 files changed

+82
-75
lines changed

src/edge_table.rs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
use std::ptr::NonNull;
2-
31
use crate::bindings as ll_bindings;
42
use crate::metadata;
53
use crate::sys;
@@ -148,22 +146,19 @@ impl<'a> streaming_iterator::StreamingIterator for EdgeTableRowView<'a> {
148146
#[repr(transparent)]
149147
#[derive(Debug)]
150148
pub struct EdgeTable {
151-
pub(crate) table_: NonNull<ll_bindings::tsk_edge_table_t>,
149+
pub(crate) table_: sys::LLEdgeTableRef,
152150
}
153151

154152
impl EdgeTable {
155153
pub(crate) fn new_from_table(
156154
edges: *mut ll_bindings::tsk_edge_table_t,
157155
) -> Result<Self, TskitError> {
158-
let n = NonNull::new(edges).ok_or_else(|| {
159-
TskitError::LibraryError("null pointer to tsk_edge_table_t".to_string())
160-
})?;
161-
Ok(EdgeTable { table_: n })
156+
let table_ = sys::LLEdgeTableRef::new_from_table(edges)?;
157+
Ok(EdgeTable { table_ })
162158
}
163159

164160
pub(crate) fn as_ref(&self) -> &ll_bindings::tsk_edge_table_t {
165-
// SAFETY: NonNull
166-
unsafe { self.table_.as_ref() }
161+
self.table_.as_ref()
167162
}
168163

169164
/// Return the number of rows

src/individual_table.rs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
use std::ptr::NonNull;
2-
31
use crate::bindings as ll_bindings;
42
use crate::metadata;
53
use crate::sys;
@@ -105,7 +103,7 @@ impl<'a> streaming_iterator::StreamingIterator for IndividualTableRowView<'a> {
105103
/// [`crate::table_views::TableViews`]
106104
#[derive(Debug)]
107105
pub struct IndividualTable {
108-
table_: NonNull<ll_bindings::tsk_individual_table_t>,
106+
table_: sys::LLIndividualTableRef,
109107
}
110108

111109
fn make_individual_table_row(table: &IndividualTable, pos: tsk_id_t) -> Option<IndividualTableRow> {
@@ -146,15 +144,12 @@ impl IndividualTable {
146144
pub(crate) fn new_from_table(
147145
individuals: *mut ll_bindings::tsk_individual_table_t,
148146
) -> Result<Self, TskitError> {
149-
let n = NonNull::new(individuals).ok_or_else(|| {
150-
TskitError::LibraryError("null pointer to tsk_individual_table_t".to_string())
151-
})?;
152-
Ok(IndividualTable { table_: n })
147+
let table_ = sys::LLIndividualTableRef::new_from_table(individuals)?;
148+
Ok(IndividualTable { table_ })
153149
}
154150

155151
pub(crate) fn as_ref(&self) -> &ll_bindings::tsk_individual_table_t {
156-
// SAFETY: NonNull
157-
unsafe { self.table_.as_ref() }
152+
self.table_.as_ref()
158153
}
159154

160155
raw_metadata_getter_for_tables!(IndividualId);

src/migration_table.rs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
use std::ptr::NonNull;
2-
31
use crate::bindings as ll_bindings;
42
use crate::metadata;
53
use crate::sys;
@@ -167,22 +165,19 @@ impl<'a> streaming_iterator::StreamingIterator for MigrationTableRowView<'a> {
167165
/// [`crate::table_views::TableViews`]
168166
#[derive(Debug)]
169167
pub struct MigrationTable {
170-
table_: NonNull<ll_bindings::tsk_migration_table_t>,
168+
table_: sys::LLMigrationTableRef,
171169
}
172170

173171
impl MigrationTable {
174172
pub(crate) fn new_from_table(
175173
migrations: *mut ll_bindings::tsk_migration_table_t,
176174
) -> Result<Self, TskitError> {
177-
let n = NonNull::new(migrations).ok_or_else(|| {
178-
TskitError::LibraryError("null pointer to tsk_migration_table_t".to_string())
179-
})?;
180-
Ok(MigrationTable { table_: n })
175+
let table_ = sys::LLMigrationTableRef::new_from_table(migrations)?;
176+
Ok(MigrationTable { table_ })
181177
}
182178

183179
pub(crate) fn as_ref(&self) -> &ll_bindings::tsk_migration_table_t {
184-
// SAFETY: NonNull
185-
unsafe { self.table_.as_ref() }
180+
self.table_.as_ref()
186181
}
187182

188183
/// Return the number of rows

src/mutation_table.rs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
use std::ptr::NonNull;
2-
31
use crate::bindings as ll_bindings;
42
use crate::metadata;
53
use crate::sys;
@@ -164,22 +162,19 @@ impl<'a> streaming_iterator::StreamingIterator for MutationTableRowView<'a> {
164162
/// [`crate::table_views::TableViews`]
165163
#[derive(Debug)]
166164
pub struct MutationTable {
167-
table_: NonNull<ll_bindings::tsk_mutation_table_t>,
165+
table_: sys::LLMutationTableRef,
168166
}
169167

170168
impl MutationTable {
171169
pub(crate) fn new_from_table(
172170
mutations: *mut ll_bindings::tsk_mutation_table_t,
173171
) -> Result<Self, TskitError> {
174-
let n = NonNull::new(mutations).ok_or_else(|| {
175-
TskitError::LibraryError("null pointer to tsk_mutation_table_t".to_string())
176-
})?;
177-
Ok(MutationTable { table_: n })
172+
let table_ = sys::LLMutationTableRef::new_from_table(mutations)?;
173+
Ok(MutationTable { table_ })
178174
}
179175

180176
pub(crate) fn as_ref(&self) -> &ll_bindings::tsk_mutation_table_t {
181-
// SAFETY: NonNull
182-
unsafe { self.table_.as_ref() }
177+
self.table_.as_ref()
183178
}
184179

185180
/// Return the number of rows.

src/node_table.rs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
use std::ptr::NonNull;
2-
31
use crate::bindings as ll_bindings;
42
use crate::metadata;
53
use crate::sys;
@@ -148,22 +146,19 @@ impl<'a> streaming_iterator::StreamingIterator for NodeTableRowView<'a> {
148146
/// [`crate::table_views::TableViews`]
149147
#[derive(Debug)]
150148
pub struct NodeTable {
151-
table_: NonNull<ll_bindings::tsk_node_table_t>,
149+
table_: sys::LLNodeTableRef,
152150
}
153151

154152
impl NodeTable {
155153
pub(crate) fn new_from_table(
156154
nodes: *mut ll_bindings::tsk_node_table_t,
157155
) -> Result<Self, TskitError> {
158-
let n = NonNull::new(nodes).ok_or_else(|| {
159-
TskitError::LibraryError("null pointer to tsk_node_table_t".to_string())
160-
})?;
161-
Ok(NodeTable { table_: n })
156+
let table_ = sys::LLNodeTableRef::new_from_table(nodes)?;
157+
Ok(NodeTable { table_ })
162158
}
163159

164160
pub(crate) fn as_ref(&self) -> &ll_bindings::tsk_node_table_t {
165-
// SAFETY: NonNull
166-
unsafe { self.table_.as_ref() }
161+
self.table_.as_ref()
167162
}
168163

169164
/// Return the number of rows

src/population_table.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
1-
use std::ptr::NonNull;
2-
31
use crate::bindings as ll_bindings;
42
use crate::metadata;
3+
use crate::sys;
54
use crate::tsk_id_t;
65
use crate::PopulationId;
76
use crate::SizeType;
@@ -116,22 +115,19 @@ impl<'a> streaming_iterator::StreamingIterator for PopulationTableRowView<'a> {
116115
#[repr(transparent)]
117116
#[derive(Debug)]
118117
pub struct PopulationTable {
119-
table_: NonNull<ll_bindings::tsk_population_table_t>,
118+
table_: sys::LLPopulationTableRef,
120119
}
121120

122121
impl PopulationTable {
123122
pub(crate) fn new_from_table(
124123
populations: *mut ll_bindings::tsk_population_table_t,
125124
) -> Result<Self, TskitError> {
126-
let n = NonNull::new(populations).ok_or_else(|| {
127-
TskitError::LibraryError("null pointer to tsk_population_table_t".to_string())
128-
})?;
129-
Ok(PopulationTable { table_: n })
125+
let table_ = sys::LLPopulationTableRef::new_from_table(populations)?;
126+
Ok(PopulationTable { table_ })
130127
}
131128

132129
pub(crate) fn as_ref(&self) -> &ll_bindings::tsk_population_table_t {
133-
// SAFETY: NonNull
134-
unsafe { self.table_.as_ref() }
130+
self.table_.as_ref()
135131
}
136132

137133
raw_metadata_getter_for_tables!(PopulationId);

src/provenance.rs

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@
1010
//! [`ProvenanceTable::iter`].
1111
//!
1212
13-
use std::ptr::NonNull;
14-
1513
use crate::bindings as ll_bindings;
1614
use crate::sys;
1715
use crate::SizeType;
@@ -141,23 +139,19 @@ impl<'a> streaming_iterator::StreamingIterator for ProvenanceTableRowView<'a> {
141139
///
142140
#[derive(Debug)]
143141
pub struct ProvenanceTable {
144-
table_: NonNull<ll_bindings::tsk_provenance_table_t>,
142+
table_: sys::LLProvenanceTableRef,
145143
}
146144

147145
impl ProvenanceTable {
148146
pub(crate) fn new_from_table(
149147
provenances: *mut ll_bindings::tsk_provenance_table_t,
150148
) -> Result<Self, crate::TskitError> {
151-
// FIXME: unwrap
152-
let n = NonNull::new(provenances).ok_or_else(|| {
153-
crate::TskitError::LibraryError("null pointer to tsk_provenance_table_t".to_string())
154-
})?;
155-
Ok(ProvenanceTable { table_: n })
149+
let table_ = sys::LLProvenanceTableRef::new_from_table(provenances)?;
150+
Ok(ProvenanceTable { table_ })
156151
}
157152

158153
pub(crate) fn as_ref(&self) -> &ll_bindings::tsk_provenance_table_t {
159-
// SAFETY: NonNull
160-
unsafe { self.table_.as_ref() }
154+
self.table_.as_ref()
161155
}
162156

163157
/// Return the number of rows

src/site_table.rs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
use std::ptr::NonNull;
2-
31
use crate::bindings as ll_bindings;
42
use crate::metadata;
53
use crate::sys;
@@ -134,22 +132,19 @@ impl<'a> streaming_iterator::StreamingIterator for SiteTableRowView<'a> {
134132
/// [`crate::table_views::TableViews`]
135133
#[derive(Debug)]
136134
pub struct SiteTable {
137-
table_: NonNull<ll_bindings::tsk_site_table_t>,
135+
table_: sys::LLSiteTableRef,
138136
}
139137

140138
impl SiteTable {
141139
pub(crate) fn new_from_table(
142140
sites: *mut ll_bindings::tsk_site_table_t,
143141
) -> Result<Self, TskitError> {
144-
let n = NonNull::new(sites).ok_or_else(|| {
145-
TskitError::LibraryError("null pointer to tsk_site_table_t".to_string())
146-
})?;
147-
Ok(SiteTable { table_: n })
142+
let table_ = sys::LLSiteTableRef::new_from_table(sites)?;
143+
Ok(SiteTable { table_ })
148144
}
149145

150146
pub(crate) fn as_ref(&self) -> &ll_bindings::tsk_site_table_t {
151-
// SAFETY: NonNull
152-
unsafe { self.table_.as_ref() }
147+
self.table_.as_ref()
153148
}
154149

155150
raw_metadata_getter_for_tables!(SiteId);

src/sys.rs

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,51 @@
1-
use crate::bindings;
1+
use crate::{bindings, TskitError};
2+
use bindings::tsk_edge_table_t;
3+
use bindings::tsk_individual_table_t;
4+
use bindings::tsk_migration_table_t;
5+
use bindings::tsk_mutation_table_t;
6+
use bindings::tsk_node_table_t;
7+
use bindings::tsk_population_table_t;
8+
use bindings::tsk_site_table_t;
9+
use std::ptr::NonNull;
10+
11+
#[cfg(feature = "provenance")]
12+
use bindings::tsk_provenance_table_t;
13+
14+
macro_rules! basic_lltableref_impl {
15+
($lltable: ident, $tsktable: ident) => {
16+
#[repr(transparent)]
17+
#[derive(Debug)]
18+
pub struct $lltable(NonNull<bindings::$tsktable>);
19+
20+
impl $lltable {
21+
pub fn new_from_table(table: *mut $tsktable) -> Result<Self, TskitError> {
22+
let internal = NonNull::new(table).ok_or_else(|| {
23+
let msg = format!("null pointer to {}", stringify!($tsktable));
24+
TskitError::LibraryError(msg)
25+
})?;
26+
Ok(Self(internal))
27+
}
28+
29+
pub fn as_ref(&self) -> &$tsktable {
30+
// SAFETY: we cannot get this far w/o
31+
// going through new_from_table and that
32+
// fn protects us from null ptrs
33+
unsafe { self.0.as_ref() }
34+
}
35+
}
36+
};
37+
}
38+
39+
basic_lltableref_impl!(LLEdgeTableRef, tsk_edge_table_t);
40+
basic_lltableref_impl!(LLNodeTableRef, tsk_node_table_t);
41+
basic_lltableref_impl!(LLMutationTableRef, tsk_mutation_table_t);
42+
basic_lltableref_impl!(LLSiteTableRef, tsk_site_table_t);
43+
basic_lltableref_impl!(LLMigrationTableRef, tsk_migration_table_t);
44+
basic_lltableref_impl!(LLPopulationTableRef, tsk_population_table_t);
45+
basic_lltableref_impl!(LLIndividualTableRef, tsk_individual_table_t);
46+
47+
#[cfg(feature = "provenance")]
48+
basic_lltableref_impl!(LLProvenanceTableRef, tsk_provenance_table_t);
249

350
fn tsk_column_access_detail<R: Into<bindings::tsk_id_t>, L: Into<bindings::tsk_size_t>, T: Copy>(
451
row: R,

src/table_collection.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ impl TableCollection {
109109
// AHA?
110110
assert!(std::ptr::eq(
111111
&mbox.as_ref().edges as *const ll_bindings::tsk_edge_table_t,
112-
views.edges().table_.as_ptr() as *const ll_bindings::tsk_edge_table_t
112+
views.edges().table_.as_ref() as *const ll_bindings::tsk_edge_table_t
113113
));
114114
let mut tables = Self {
115115
inner: mbox,

0 commit comments

Comments
 (0)