Skip to content

Commit 489630c

Browse files
molpopgenmolpopgen
committed
style: replace column access macro with generic fn
This change improves readability and declutters some internal details.
1 parent 4c2c949 commit 489630c

File tree

10 files changed

+89
-106
lines changed

10 files changed

+89
-106
lines changed

‎src/_macros.rs‎

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -27,47 +27,6 @@ macro_rules! panic_on_tskit_error {
2727
};
2828
}
2929

30-
macro_rules! unsafe_tsk_column_access {
31-
($i: expr, $lo: expr, $hi: expr, $owner: expr, $array: ident) => {{
32-
let x = $crate::tsk_id_t::from($i);
33-
if x < $lo || (x as $crate::tsk_size_t) >= $hi {
34-
None
35-
} else {
36-
debug_assert!(!($owner).$array.is_null());
37-
if !$owner.$array.is_null() {
38-
// SAFETY: array is not null
39-
// and we did our best effort
40-
// on bounds checking
41-
Some(unsafe { *$owner.$array.offset(x as isize) })
42-
} else {
43-
None
44-
}
45-
}
46-
}};
47-
($i: expr, $lo: expr, $hi: expr, $owner: expr, $array: ident, $output_id_type: ty) => {{
48-
let x = $crate::tsk_id_t::from($i);
49-
if x < $lo || (x as $crate::tsk_size_t) >= $hi {
50-
None
51-
} else {
52-
debug_assert!(!($owner).$array.is_null());
53-
if !$owner.$array.is_null() {
54-
// SAFETY: array is not null
55-
// and we did our best effort
56-
// on bounds checking
57-
unsafe { Some(<$output_id_type>::from(*($owner.$array.offset(x as isize)))) }
58-
} else {
59-
None
60-
}
61-
}
62-
}};
63-
}
64-
65-
macro_rules! unsafe_tsk_column_access_and_map_into {
66-
($i: expr, $lo: expr, $hi: expr, $owner: expr, $array: ident) => {{
67-
unsafe_tsk_column_access!($i, $lo, $hi, $owner, $array).map(|v| v.into())
68-
}};
69-
}
70-
7130
macro_rules! unsafe_tsk_ragged_column_access {
7231
($i: expr, $lo: expr, $hi: expr, $owner: expr, $array: ident, $offset_array: ident, $offset_array_len: ident, $output_id_type: ty) => {{
7332
let i = $crate::SizeType::try_from($i).ok()?;

‎src/edge_table.rs‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use std::ptr::NonNull;
22

33
use crate::bindings as ll_bindings;
44
use crate::metadata;
5+
use crate::sys;
56
use crate::Position;
67
use crate::{tsk_id_t, TskitError};
78
use crate::{EdgeId, NodeId};
@@ -180,14 +181,7 @@ impl EdgeTable {
180181
/// * `Some(parent)` if `u` is valid.
181182
/// * `None` otherwise.
182183
pub fn parent<E: Into<EdgeId> + Copy>(&self, row: E) -> Option<NodeId> {
183-
unsafe_tsk_column_access!(
184-
row.into(),
185-
0,
186-
self.num_rows(),
187-
self.as_ref(),
188-
parent,
189-
NodeId
190-
)
184+
sys::tsk_column_access::<NodeId, _, _, _>(row.into(), self.as_ref().parent, self.num_rows())
191185
}
192186

193187
/// Return the ``child`` value from row ``row`` of the table.
@@ -197,7 +191,7 @@ impl EdgeTable {
197191
/// * `Some(child)` if `u` is valid.
198192
/// * `None` otherwise.
199193
pub fn child<E: Into<EdgeId> + Copy>(&self, row: E) -> Option<NodeId> {
200-
unsafe_tsk_column_access!(row.into(), 0, self.num_rows(), self.as_ref(), child, NodeId)
194+
sys::tsk_column_access::<NodeId, _, _, _>(row.into(), self.as_ref().child, self.num_rows())
201195
}
202196

203197
/// Return the ``left`` value from row ``row`` of the table.
@@ -207,14 +201,7 @@ impl EdgeTable {
207201
/// * `Some(position)` if `u` is valid.
208202
/// * `None` otherwise.
209203
pub fn left<E: Into<EdgeId> + Copy>(&self, row: E) -> Option<Position> {
210-
unsafe_tsk_column_access!(
211-
row.into(),
212-
0,
213-
self.num_rows(),
214-
self.as_ref(),
215-
left,
216-
Position
217-
)
204+
sys::tsk_column_access::<Position, _, _, _>(row.into(), self.as_ref().left, self.num_rows())
218205
}
219206

220207
/// Return the ``right`` value from row ``row`` of the table.
@@ -224,7 +211,11 @@ impl EdgeTable {
224211
/// * `Some(position)` if `u` is valid.
225212
/// * `None` otherwise.
226213
pub fn right<E: Into<EdgeId> + Copy>(&self, row: E) -> Option<Position> {
227-
unsafe_tsk_column_access_and_map_into!(row.into(), 0, self.num_rows(), self.as_ref(), right)
214+
sys::tsk_column_access::<Position, _, _, _>(
215+
row.into(),
216+
self.as_ref().right,
217+
self.num_rows(),
218+
)
228219
}
229220

230221
/// Retrieve decoded metadata for a `row`.

‎src/individual_table.rs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use std::ptr::NonNull;
22

33
use crate::bindings as ll_bindings;
44
use crate::metadata;
5+
use crate::sys;
56
use crate::IndividualFlags;
67
use crate::IndividualId;
78
use crate::Location;
@@ -171,7 +172,11 @@ impl IndividualTable {
171172
/// * `Some(flags)` if `row` is valid.
172173
/// * `None` otherwise.
173174
pub fn flags<I: Into<IndividualId> + Copy>(&self, row: I) -> Option<IndividualFlags> {
174-
unsafe_tsk_column_access_and_map_into!(row.into(), 0, self.num_rows(), self.as_ref(), flags)
175+
sys::tsk_column_access::<IndividualFlags, _, _, _>(
176+
row.into(),
177+
self.as_ref().flags,
178+
self.num_rows(),
179+
)
175180
}
176181

177182
/// Return the locations for a given row.

‎src/lib.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ mod node_table;
9292
mod population_table;
9393
pub mod prelude;
9494
mod site_table;
95+
mod sys;
9596
mod table_collection;
9697
mod table_iterator;
9798
pub mod table_views;

‎src/migration_table.rs‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use std::ptr::NonNull;
22

33
use crate::bindings as ll_bindings;
44
use crate::metadata;
5+
use crate::sys;
56
use crate::Position;
67
use crate::SizeType;
78
use crate::Time;
@@ -199,7 +200,7 @@ impl MigrationTable {
199200
/// * `Some(position)` if `row` is valid.
200201
/// * `None` otherwise.
201202
pub fn left<M: Into<MigrationId> + Copy>(&self, row: M) -> Option<Position> {
202-
unsafe_tsk_column_access_and_map_into!(row.into(), 0, self.num_rows(), self.as_ref(), left)
203+
sys::tsk_column_access::<Position, _, _, _>(row.into(), self.as_ref().left, self.num_rows())
203204
}
204205

205206
/// Return the right coordinate for a given row.
@@ -209,7 +210,11 @@ impl MigrationTable {
209210
/// * `Some(positions)` if `row` is valid.
210211
/// * `None` otherwise.
211212
pub fn right<M: Into<MigrationId> + Copy>(&self, row: M) -> Option<Position> {
212-
unsafe_tsk_column_access_and_map_into!(row.into(), 0, self.num_rows(), self.as_ref(), right)
213+
sys::tsk_column_access::<Position, _, _, _>(
214+
row.into(),
215+
self.as_ref().right,
216+
self.num_rows(),
217+
)
213218
}
214219

215220
/// Return the node for a given row.
@@ -219,7 +224,7 @@ impl MigrationTable {
219224
/// * `Some(node)` if `row` is valid.
220225
/// * `None` otherwise.
221226
pub fn node<M: Into<MigrationId> + Copy>(&self, row: M) -> Option<NodeId> {
222-
unsafe_tsk_column_access!(row.into(), 0, self.num_rows(), self.as_ref(), node, NodeId)
227+
sys::tsk_column_access::<NodeId, _, _, _>(row.into(), self.as_ref().node, self.num_rows())
223228
}
224229

225230
/// Return the source population for a given row.
@@ -229,13 +234,10 @@ impl MigrationTable {
229234
/// * `Some(population)` if `row` is valid.
230235
/// * `None` otherwise.
231236
pub fn source<M: Into<MigrationId> + Copy>(&self, row: M) -> Option<PopulationId> {
232-
unsafe_tsk_column_access!(
237+
sys::tsk_column_access::<PopulationId, _, _, _>(
233238
row.into(),
234-
0,
239+
self.as_ref().source,
235240
self.num_rows(),
236-
self.as_ref(),
237-
source,
238-
PopulationId
239241
)
240242
}
241243

@@ -246,13 +248,10 @@ impl MigrationTable {
246248
/// * `Some(population)` if `row` is valid.
247249
/// * `None` otherwise.
248250
pub fn dest<M: Into<MigrationId> + Copy>(&self, row: M) -> Option<PopulationId> {
249-
unsafe_tsk_column_access!(
251+
sys::tsk_column_access::<PopulationId, _, _, _>(
250252
row.into(),
251-
0,
253+
self.as_ref().dest,
252254
self.num_rows(),
253-
self.as_ref(),
254-
dest,
255-
PopulationId
256255
)
257256
}
258257

@@ -263,7 +262,7 @@ impl MigrationTable {
263262
/// * `Some(time)` if `row` is valid.
264263
/// * `None` otherwise.
265264
pub fn time<M: Into<MigrationId> + Copy>(&self, row: M) -> Option<Time> {
266-
unsafe_tsk_column_access_and_map_into!(row.into(), 0, self.num_rows(), self.as_ref(), time)
265+
sys::tsk_column_access::<Time, _, _, _>(row.into(), self.as_ref().time, self.num_rows())
267266
}
268267

269268
/// Retrieve decoded metadata for a `row`.

‎src/mutation_table.rs‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use std::ptr::NonNull;
22

33
use crate::bindings as ll_bindings;
44
use crate::metadata;
5+
use crate::sys;
56
use crate::SizeType;
67
use crate::Time;
78
use crate::{tsk_id_t, TskitError};
@@ -196,7 +197,7 @@ impl MutationTable {
196197
/// Will return [``IndexError``](crate::TskitError::IndexError)
197198
/// if ``row`` is out of range.
198199
pub fn site<M: Into<MutationId> + Copy>(&self, row: M) -> Option<SiteId> {
199-
unsafe_tsk_column_access!(row.into(), 0, self.num_rows(), self.as_ref(), site, SiteId)
200+
sys::tsk_column_access::<SiteId, _, _, _>(row.into(), self.as_ref().site, self.num_rows())
200201
}
201202

202203
/// Return the ``node`` value from row ``row`` of the table.
@@ -206,7 +207,7 @@ impl MutationTable {
206207
/// Will return [``IndexError``](crate::TskitError::IndexError)
207208
/// if ``row`` is out of range.
208209
pub fn node<M: Into<MutationId> + Copy>(&self, row: M) -> Option<NodeId> {
209-
unsafe_tsk_column_access!(row.into(), 0, self.num_rows(), self.as_ref(), node, NodeId)
210+
sys::tsk_column_access::<NodeId, _, _, _>(row.into(), self.as_ref().node, self.num_rows())
210211
}
211212

212213
/// Return the ``parent`` value from row ``row`` of the table.
@@ -216,13 +217,10 @@ impl MutationTable {
216217
/// Will return [``IndexError``](crate::TskitError::IndexError)
217218
/// if ``row`` is out of range.
218219
pub fn parent<M: Into<MutationId> + Copy>(&self, row: M) -> Option<MutationId> {
219-
unsafe_tsk_column_access!(
220+
sys::tsk_column_access::<MutationId, _, _, _>(
220221
row.into(),
221-
0,
222+
self.as_ref().parent,
222223
self.num_rows(),
223-
self.as_ref(),
224-
parent,
225-
MutationId
226224
)
227225
}
228226

@@ -233,7 +231,7 @@ impl MutationTable {
233231
/// Will return [``IndexError``](crate::TskitError::IndexError)
234232
/// if ``row`` is out of range.
235233
pub fn time<M: Into<MutationId> + Copy>(&self, row: M) -> Option<Time> {
236-
unsafe_tsk_column_access!(row.into(), 0, self.num_rows(), self.as_ref(), time, Time)
234+
sys::tsk_column_access::<Time, _, _, _>(row.into(), self.as_ref().time, self.num_rows())
237235
}
238236

239237
/// Get the ``derived_state`` value from row ``row`` of the table.

‎src/node_table.rs‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use std::ptr::NonNull;
22

33
use crate::bindings as ll_bindings;
44
use crate::metadata;
5+
use crate::sys;
56
use crate::NodeFlags;
67
use crate::SizeType;
78
use crate::Time;
@@ -195,7 +196,7 @@ impl NodeTable {
195196
/// # }
196197
/// ```
197198
pub fn time<N: Into<NodeId> + Copy>(&self, row: N) -> Option<Time> {
198-
unsafe_tsk_column_access!(row.into(), 0, self.num_rows(), self.as_ref(), time, Time)
199+
sys::tsk_column_access::<Time, _, _, _>(row.into(), self.as_ref().time, self.num_rows())
199200
}
200201

201202
/// Return the ``flags`` value from row ``row`` of the table.
@@ -220,7 +221,11 @@ impl NodeTable {
220221
/// # }
221222
/// ```
222223
pub fn flags<N: Into<NodeId> + Copy>(&self, row: N) -> Option<NodeFlags> {
223-
unsafe_tsk_column_access_and_map_into!(row.into(), 0, self.num_rows(), self.as_ref(), flags)
224+
sys::tsk_column_access::<NodeFlags, _, _, _>(
225+
row.into(),
226+
self.as_ref().flags,
227+
self.num_rows(),
228+
)
224229
}
225230

226231
#[deprecated(since = "0.12.0", note = "use flags_slice_mut instead")]
@@ -265,13 +270,10 @@ impl NodeTable {
265270
/// * `Some(population)` if `row` is valid.
266271
/// * `None` otherwise.
267272
pub fn population<N: Into<NodeId> + Copy>(&self, row: N) -> Option<PopulationId> {
268-
unsafe_tsk_column_access!(
273+
sys::tsk_column_access::<PopulationId, _, _, _>(
269274
row.into(),
270-
0,
275+
self.as_ref().population,
271276
self.num_rows(),
272-
self.as_ref(),
273-
population,
274-
PopulationId
275277
)
276278
}
277279

@@ -311,13 +313,10 @@ impl NodeTable {
311313
/// * `Some(individual)` if `row` is valid.
312314
/// * `None` otherwise.
313315
pub fn individual<N: Into<NodeId> + Copy>(&self, row: N) -> Option<IndividualId> {
314-
unsafe_tsk_column_access!(
316+
sys::tsk_column_access::<IndividualId, _, _, _>(
315317
row.into(),
316-
0,
318+
self.as_ref().individual,
317319
self.num_rows(),
318-
self.as_ref(),
319-
individual,
320-
IndividualId
321320
)
322321
}
323322

‎src/site_table.rs‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use std::ptr::NonNull;
22

33
use crate::bindings as ll_bindings;
44
use crate::metadata;
5+
use crate::sys;
56
use crate::tsk_id_t;
67
use crate::Position;
78
use crate::SiteId;
@@ -166,13 +167,10 @@ impl SiteTable {
166167
/// * `Some(position)` if `row` is valid.
167168
/// * `None` otherwise.
168169
pub fn position<S: Into<SiteId> + Copy>(&self, row: S) -> Option<Position> {
169-
unsafe_tsk_column_access!(
170+
sys::tsk_column_access::<Position, _, _, _>(
170171
row.into(),
171-
0,
172+
self.as_ref().position,
172173
self.num_rows(),
173-
self.as_ref(),
174-
position,
175-
Position
176174
)
177175
}
178176

‎src/sys.rs‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
use crate::bindings;
2+
3+
fn tsk_column_access_detail<R: Into<bindings::tsk_id_t>, L: Into<bindings::tsk_size_t>, T: Copy>(
4+
row: R,
5+
column: *const T,
6+
column_length: L,
7+
) -> Option<T> {
8+
let row = row.into();
9+
let column_length = column_length.into();
10+
if row < 0 || (row as crate::tsk_size_t) >= column_length {
11+
None
12+
} else {
13+
assert!(!column.is_null());
14+
// SAFETY: pointer is not null.
15+
// column_length is assumed to come directly
16+
// from a table.
17+
Some(unsafe { *column.offset(row as isize) })
18+
}
19+
}
20+
21+
pub fn tsk_column_access<
22+
O: From<T>,
23+
R: Into<bindings::tsk_id_t>,
24+
L: Into<bindings::tsk_size_t>,
25+
T: Copy,
26+
>(
27+
row: R,
28+
column: *const T,
29+
column_length: L,
30+
) -> Option<O> {
31+
tsk_column_access_detail(row, column, column_length).map(|v| v.into())
32+
}

0 commit comments

Comments
 (0)