Skip to content

Commit b25368e

Browse files
molpopgenmolpopgen
authored
style: generic fns for ragged column access (#423)
Replace (several) macros with fns.
1 parent 1a80144 commit b25368e

11 files changed

+98
-240
lines changed

‎src/_macros.rs

Lines changed: 4 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -27,130 +27,12 @@ macro_rules! panic_on_tskit_error {
2727
};
2828
}
2929

30-
macro_rules! unsafe_tsk_ragged_column_access {
31-
($i: expr, $lo: expr, $hi: expr, $owner: expr, $array: ident, $offset_array: ident, $offset_array_len: ident, $output_id_type: ty) => {{
32-
let i = $crate::SizeType::try_from($i).ok()?;
33-
if $i < $lo || i >= $hi {
34-
None
35-
} else if $owner.$offset_array_len == 0 {
36-
None
37-
} else {
38-
debug_assert!(!$owner.$array.is_null());
39-
if $owner.$array.is_null() {
40-
return None;
41-
}
42-
let offset = i.as_usize() as isize;
43-
// SAFETY: we have checked bounds and ensured not null
44-
let start = unsafe { *$owner.$offset_array.offset(offset) };
45-
let stop = if i < $hi {
46-
unsafe { *$owner.$offset_array.offset((offset + 1) as isize) }
47-
} else {
48-
$owner.$offset_array_len as tsk_size_t
49-
};
50-
if start == stop {
51-
None
52-
} else {
53-
Some(unsafe {
54-
std::slice::from_raw_parts(
55-
$owner.$array.offset(start as isize) as *const $output_id_type,
56-
stop as usize - start as usize,
57-
)
58-
})
59-
}
60-
}
61-
}};
62-
}
63-
64-
// Allow this to be unused for default features
65-
// to pass clippy checks
66-
#[allow(unused_macros)]
67-
macro_rules! unsafe_tsk_ragged_char_column_access {
68-
($i: expr, $lo: expr, $hi: expr, $owner: expr, $array: ident, $offset_array: ident, $offset_array_len: ident) => {{
69-
let i = $crate::SizeType::try_from($i).ok()?;
70-
if $i < $lo || i >= $hi {
71-
None
72-
} else if $owner.$offset_array_len == 0 {
73-
None
74-
} else {
75-
assert!(!$owner.$array.is_null());
76-
assert!(!$owner.$offset_array.is_null());
77-
let start = unsafe { *$owner.$offset_array.offset($i as isize) };
78-
let stop = if i < $hi {
79-
unsafe { *$owner.$offset_array.offset(($i + 1) as isize) }
80-
} else {
81-
$owner.$offset_array_len as tsk_size_t
82-
};
83-
if start == stop {
84-
None
85-
} else {
86-
let mut buffer = String::new();
87-
for i in start..stop {
88-
buffer.push(unsafe { *$owner.$array.offset(i as isize) as u8 as char });
89-
}
90-
Some(buffer)
91-
}
92-
}
93-
}};
94-
}
95-
96-
#[cfg(feature = "provenance")]
97-
macro_rules! unsafe_tsk_ragged_char_column_access_to_slice_u8 {
98-
($i: expr, $lo: expr, $hi: expr, $owner: expr, $array: ident, $offset_array: ident, $offset_array_len: ident) => {{
99-
let i = match $crate::SizeType::try_from($i).ok() {
100-
Some(j) => j,
101-
None => $crate::SizeType::from(u64::MAX),
102-
};
103-
if $i < $lo || i >= $hi {
104-
None
105-
} else if $owner.$offset_array_len == 0 {
106-
None
107-
} else {
108-
assert!(!$owner.$array.is_null());
109-
assert!(!$owner.$offset_array.is_null());
110-
let offset = i.as_usize() as isize;
111-
let start = unsafe { *$owner.$offset_array.offset(offset) };
112-
let stop = if i < $hi {
113-
unsafe { *$owner.$offset_array.offset((offset + 1) as isize) }
114-
} else {
115-
$owner.$offset_array_len as tsk_size_t
116-
};
117-
if start == stop {
118-
None
119-
} else {
120-
let ptr = unsafe { $owner.$array.offset(start as isize) as *const u8 };
121-
let len = (stop - start) as usize;
122-
let slice = unsafe { std::slice::from_raw_parts(ptr, len) };
123-
Some(slice)
124-
}
125-
}
126-
}};
127-
}
128-
129-
macro_rules! metadata_to_vector {
130-
($outer: ident, $table: expr, $row: expr) => {
131-
$crate::metadata::char_column_to_slice(
132-
$outer,
133-
$table.metadata,
134-
$table.metadata_offset,
135-
$row,
136-
$table.num_rows,
137-
$table.metadata_length,
138-
)
139-
};
140-
}
141-
14230
macro_rules! decode_metadata_row {
14331
($T: ty, $buffer: expr) => {
14432
<$T as $crate::metadata::MetadataRoundtrip>::decode($buffer)
14533
};
14634
}
14735

148-
macro_rules! table_row_decode_metadata {
149-
($owner: ident, $table: ident, $pos: ident) => {
150-
metadata_to_vector!($owner, $table, $pos)
151-
};
152-
}
153-
15436
macro_rules! process_state_input {
15537
($state: expr) => {
15638
match $state {
@@ -1093,14 +975,13 @@ macro_rules! build_owned_table_type {
1093975
}
1094976

1095977
macro_rules! raw_metadata_getter_for_tables {
1096-
($idtype: ident) => {
978+
($idtype: ty) => {
1097979
fn raw_metadata(&self, row: $idtype) -> Option<&[u8]> {
1098-
$crate::metadata::char_column_to_slice(
1099-
self,
980+
$crate::sys::tsk_ragged_column_access::<'_, u8, $idtype, _, _>(
981+
row.into(),
1100982
self.as_ref().metadata,
983+
self.num_rows(),
1101984
self.as_ref().metadata_offset,
1102-
row.into(),
1103-
self.num_rows().into(),
1104985
self.as_ref().metadata_length,
1105986
)
1106987
}

‎src/edge_table.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,13 @@ impl PartialEq for EdgeTableRow {
3131
}
3232

3333
fn make_edge_table_row(table: &EdgeTable, pos: tsk_id_t) -> Option<EdgeTableRow> {
34-
let table_ref = table.as_ref();
3534
Some(EdgeTableRow {
3635
id: pos.into(),
3736
left: table.left(pos)?,
3837
right: table.right(pos)?,
3938
parent: table.parent(pos)?,
4039
child: table.child(pos)?,
41-
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
40+
metadata: table.raw_metadata(pos.into()).map(|m| m.to_vec()),
4241
})
4342
}
4443

@@ -238,8 +237,7 @@ impl EdgeTable {
238237
&self,
239238
row: EdgeId,
240239
) -> Option<Result<T, TskitError>> {
241-
let table_ref = self.as_ref();
242-
let buffer = metadata_to_vector!(self, table_ref, row.into())?;
240+
let buffer = self.raw_metadata(row)?;
243241
Some(decode_metadata_row!(T, buffer).map_err(|e| e.into()))
244242
}
245243

‎src/individual_table.rs

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::sys;
66
use crate::IndividualFlags;
77
use crate::IndividualId;
88
use crate::Location;
9-
use crate::{tsk_id_t, tsk_size_t, TskitError};
9+
use crate::{tsk_id_t, TskitError};
1010
use ll_bindings::{tsk_individual_table_free, tsk_individual_table_init};
1111

1212
/// Row of a [`IndividualTable`]
@@ -109,13 +109,12 @@ pub struct IndividualTable {
109109
}
110110

111111
fn make_individual_table_row(table: &IndividualTable, pos: tsk_id_t) -> Option<IndividualTableRow> {
112-
let table_ref = table.as_ref();
113112
Some(IndividualTableRow {
114113
id: pos.into(),
115114
flags: table.flags(pos)?,
116115
location: table.location(pos).map(|s| s.to_vec()),
117116
parents: table.parents(pos).map(|s| s.to_vec()),
118-
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
117+
metadata: table.raw_metadata(pos.into()).map(|m| m.to_vec()),
119118
})
120119
}
121120

@@ -186,15 +185,12 @@ impl IndividualTable {
186185
/// * `Some(location)` if `row` is valid.
187186
/// * `None` otherwise.
188187
pub fn location<I: Into<IndividualId> + Copy>(&self, row: I) -> Option<&[Location]> {
189-
unsafe_tsk_ragged_column_access!(
188+
sys::tsk_ragged_column_access(
190189
row.into(),
191-
0,
190+
self.as_ref().location,
192191
self.num_rows(),
193-
self.as_ref(),
194-
location,
195-
location_offset,
196-
location_length,
197-
Location
192+
self.as_ref().location_offset,
193+
self.as_ref().location_length,
198194
)
199195
}
200196

@@ -205,15 +201,12 @@ impl IndividualTable {
205201
/// * `Some(parents)` if `row` is valid.
206202
/// * `None` otherwise.
207203
pub fn parents<I: Into<IndividualId> + Copy>(&self, row: I) -> Option<&[IndividualId]> {
208-
unsafe_tsk_ragged_column_access!(
204+
sys::tsk_ragged_column_access(
209205
row.into(),
210-
0,
206+
self.as_ref().parents,
211207
self.num_rows(),
212-
self.as_ref(),
213-
parents,
214-
parents_offset,
215-
parents_length,
216-
IndividualId
208+
self.as_ref().parents_offset,
209+
self.as_ref().parents_length,
217210
)
218211
}
219212

@@ -391,8 +384,7 @@ match tables.individuals().metadata::<MutationMetadata>(0.into())
391384
&self,
392385
row: IndividualId,
393386
) -> Option<Result<T, TskitError>> {
394-
let table_ref = self.as_ref();
395-
let buffer = metadata_to_vector!(self, table_ref, row.into())?;
387+
let buffer = self.raw_metadata(row)?;
396388
Some(decode_metadata_row!(T, buffer).map_err(|e| e.into()))
397389
}
398390

‎src/metadata.rs

Lines changed: 0 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,6 @@
163163
//! * We have not yet tested importing metadata encoded using `rust`
164164
//! into `Python` via the `tskit` `Python API`.
165165
166-
use crate::bindings::{tsk_id_t, tsk_size_t};
167166
use crate::SizeType;
168167
use thiserror::Error;
169168

@@ -250,59 +249,6 @@ pub enum MetadataError {
250249
},
251250
}
252251

253-
pub(crate) fn char_column_to_slice<T: Sized>(
254-
_lifetime: &T,
255-
column: *const libc::c_char,
256-
column_offset: *const tsk_size_t,
257-
row: tsk_id_t,
258-
num_rows: tsk_size_t,
259-
column_length: tsk_size_t,
260-
) -> Option<&[u8]> {
261-
let row = match tsk_size_t::try_from(row).ok() {
262-
Some(r) if r < num_rows => r,
263-
_ => return None,
264-
};
265-
if column_length == 0 {
266-
return None;
267-
}
268-
let row_isize = match isize::try_from(row).ok() {
269-
Some(x) => x,
270-
None => return None,
271-
};
272-
debug_assert!(!column.is_null());
273-
debug_assert!(!column_offset.is_null());
274-
if column.is_null() {
275-
return None;
276-
}
277-
if column_offset.is_null() {
278-
return None;
279-
}
280-
// SAFETY: not null and best effort bounds check
281-
let start = unsafe { *column_offset.offset(row_isize) };
282-
let stop = if (row as tsk_size_t) < num_rows {
283-
unsafe { *column_offset.offset(row_isize + 1) }
284-
} else {
285-
column_length
286-
};
287-
if start >= stop {
288-
return None;
289-
}
290-
if column_length == 0 {
291-
return None;
292-
}
293-
let istart = match isize::try_from(start).ok() {
294-
Some(v) => v,
295-
None => return None,
296-
};
297-
let ustop = match usize::try_from(stop).ok() {
298-
Some(v) => v,
299-
None => return None,
300-
};
301-
Some(unsafe {
302-
std::slice::from_raw_parts(column.offset(istart) as *const u8, ustop - istart as usize)
303-
})
304-
}
305-
306252
#[cfg(test)]
307253
mod tests {
308254
use super::*;

‎src/migration_table.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ impl PartialEq for MigrationTableRow {
3737
}
3838

3939
fn make_migration_table_row(table: &MigrationTable, pos: tsk_id_t) -> Option<MigrationTableRow> {
40-
let table_ref = table.as_ref();
4140
Some(MigrationTableRow {
4241
id: pos.into(),
4342
left: table.left(pos)?,
@@ -46,7 +45,7 @@ fn make_migration_table_row(table: &MigrationTable, pos: tsk_id_t) -> Option<Mig
4645
source: table.source(pos)?,
4746
dest: table.dest(pos)?,
4847
time: table.time(pos)?,
49-
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
48+
metadata: table.raw_metadata(pos.into()).map(|m| m.to_vec()),
5049
})
5150
}
5251

@@ -285,8 +284,7 @@ impl MigrationTable {
285284
&self,
286285
row: MigrationId,
287286
) -> Option<Result<T, TskitError>> {
288-
let table_ref = self.as_ref();
289-
let buffer = metadata_to_vector!(self, table_ref, row.into())?;
287+
let buffer = self.raw_metadata(row)?;
290288
Some(decode_metadata_row!(T, buffer).map_err(|e| e.into()))
291289
}
292290

‎src/mutation_table.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ fn make_mutation_table_row(table: &MutationTable, pos: tsk_id_t) -> Option<Mutat
3737
let index = ll_bindings::tsk_size_t::try_from(pos).ok()?;
3838
match index {
3939
i if i < table.num_rows() => {
40-
let table_ref = table.as_ref();
4140
let derived_state = table.derived_state(pos).map(|s| s.to_vec());
4241
Some(MutationTableRow {
4342
id: pos.into(),
@@ -46,7 +45,7 @@ fn make_mutation_table_row(table: &MutationTable, pos: tsk_id_t) -> Option<Mutat
4645
parent: table.parent(pos)?,
4746
time: table.time(pos)?,
4847
derived_state,
49-
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
48+
metadata: table.raw_metadata(pos.into()).map(|m| m.to_vec()),
5049
})
5150
}
5251
_ => None,
@@ -245,12 +244,11 @@ impl MutationTable {
245244
/// Will return [``IndexError``](crate::TskitError::IndexError)
246245
/// if ``row`` is out of range.
247246
pub fn derived_state<M: Into<MutationId>>(&self, row: M) -> Option<&[u8]> {
248-
metadata::char_column_to_slice(
249-
self,
247+
sys::tsk_ragged_column_access(
248+
row.into(),
250249
self.as_ref().derived_state,
250+
self.num_rows(),
251251
self.as_ref().derived_state_offset,
252-
row.into().into(),
253-
self.as_ref().num_rows,
254252
self.as_ref().derived_state_length,
255253
)
256254
}
@@ -275,8 +273,7 @@ impl MutationTable {
275273
&self,
276274
row: MutationId,
277275
) -> Option<Result<T, TskitError>> {
278-
let table_ref = self.as_ref();
279-
let buffer = metadata_to_vector!(self, table_ref, row.into())?;
276+
let buffer = self.raw_metadata(row)?;
280277
Some(decode_metadata_row!(T, buffer).map_err(|e| e.into()))
281278
}
282279

0 commit comments

Comments
 (0)