Skip to content

Commit 1cb25cb

Browse files
authored
Refactor reference iterator to use struct instead of tuple (#636)
While working on #635, I noticed that we were instantiating the reference iterator using tuples of `(id, kind)`. I think we can standardize on the approach we used for declarations with a `CDeclaration` struct to back this up. If we end up needing to add more data to it, the changes to the FFI are minimized because we're not passing more tuple items. This PR creates `CReference` to be used across the FFI.
2 parents f9f3205 + 8d57e7e commit 1cb25cb

4 files changed

Lines changed: 35 additions & 29 deletions

File tree

ext/rubydex/utils.c

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,10 @@ VALUE rdxi_references_yield(VALUE args) {
5757
VALUE graph_obj = rb_ary_entry(args, 0);
5858
void *iter = (void *)(uintptr_t)NUM2ULL(rb_ary_entry(args, 1));
5959

60-
uint64_t id = 0;
61-
ReferenceKind kind;
62-
63-
while (rdx_references_iter_next(iter, &id, &kind)) {
64-
VALUE ref_class = rdxi_reference_class_for_kind(kind);
65-
VALUE argv[] = {graph_obj, ULL2NUM(id)};
60+
CReference cref;
61+
while (rdx_references_iter_next(iter, &cref)) {
62+
VALUE ref_class = rdxi_reference_class_for_kind(cref.kind);
63+
VALUE argv[] = {graph_obj, ULL2NUM(cref.id)};
6664
VALUE obj = rb_class_new_instance(2, argv, ref_class);
6765
rb_yield(obj);
6866
}

rust/rubydex-sys/src/declaration_api.rs

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

88
use crate::definition_api::{DefinitionsIter, rdx_definitions_iter_new_from_ids};
99
use crate::graph_api::{GraphPointer, with_graph};
10-
use crate::reference_api::{ReferenceKind, ReferencesIter};
10+
use crate::reference_api::{CReference, ReferenceKind, ReferencesIter};
1111
use crate::utils;
1212
use rubydex::model::ids::{DeclarationId, StringId};
1313

@@ -429,7 +429,11 @@ pub unsafe extern "C" fn rdx_declaration_references_iter_new(
429429
}
430430
};
431431

432-
let entries: Vec<_> = decl.references().iter().map(|ref_id| (**ref_id, kind)).collect();
432+
let entries: Vec<_> = decl
433+
.references()
434+
.iter()
435+
.map(|ref_id| CReference::new(**ref_id, kind))
436+
.collect();
433437
ReferencesIter::new(entries.into_boxed_slice())
434438
})
435439
}

rust/rubydex-sys/src/graph_api.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
33
use crate::declaration_api::CDeclaration;
44
use crate::declaration_api::DeclarationsIter;
5-
use crate::reference_api::{ReferenceKind, ReferencesIter};
5+
use crate::reference_api::{CReference, ReferenceKind, ReferencesIter};
66
use crate::{name_api, utils};
77
use libc::{c_char, c_void};
88
use rubydex::indexing::LanguageId;
@@ -452,7 +452,7 @@ pub unsafe extern "C" fn rdx_graph_constant_references_iter_new(pointer: GraphPo
452452
let refs: Vec<_> = graph
453453
.constant_references()
454454
.keys()
455-
.map(|id| (**id, ReferenceKind::Constant))
455+
.map(|id| CReference::new(**id, ReferenceKind::Constant))
456456
.collect();
457457

458458
ReferencesIter::new(refs.into_boxed_slice())
@@ -469,7 +469,7 @@ pub unsafe extern "C" fn rdx_graph_method_references_iter_new(pointer: GraphPoin
469469
let refs: Vec<_> = graph
470470
.method_references()
471471
.keys()
472-
.map(|id| (**id, ReferenceKind::Method))
472+
.map(|id| CReference::new(**id, ReferenceKind::Method))
473473
.collect();
474474

475475
ReferencesIter::new(refs.into_boxed_slice())

rust/rubydex-sys/src/reference_api.rs

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,29 @@ pub enum ReferenceKind {
1515
Method = 1,
1616
}
1717

18-
/// Shared iterator over reference (id, kind) pairs
18+
#[repr(C)]
19+
#[derive(Debug, Clone, Copy)]
20+
pub struct CReference {
21+
id: u64,
22+
kind: ReferenceKind,
23+
}
24+
25+
impl CReference {
26+
#[must_use]
27+
pub fn new(id: u64, kind: ReferenceKind) -> Self {
28+
Self { id, kind }
29+
}
30+
}
31+
1932
#[derive(Debug)]
2033
pub struct ReferencesIter {
21-
pub entries: Box<[(u64, ReferenceKind)]>,
34+
pub entries: Box<[CReference]>,
2235
pub index: usize,
2336
}
2437

2538
impl ReferencesIter {
2639
#[must_use]
27-
pub fn new(entries: Box<[(u64, ReferenceKind)]>) -> *mut ReferencesIter {
40+
pub fn new(entries: Box<[CReference]>) -> *mut ReferencesIter {
2841
Box::into_raw(Box::new(ReferencesIter { entries, index: 0 }))
2942
}
3043
}
@@ -41,23 +54,15 @@ pub unsafe extern "C" fn rdx_references_iter_len(iter: *const ReferencesIter) ->
4154
unsafe { (&*iter).entries.len() }
4255
}
4356

44-
/// Advances the iterator and writes the next entry into the provided out params.
57+
/// Advances the iterator and writes the next entry into `out_ref`.
4558
/// Returns `true` if an entry was written, or `false` if the iterator is exhausted or inputs are invalid.
4659
///
4760
/// # Safety
4861
/// - `iter` must be a valid pointer previously returned by `ReferencesIter::new`.
49-
/// - `out_id` and `out_kind` must be valid, writable pointers.
50-
///
51-
/// # Panics
52-
/// - If the iterator is exhausted or inputs are invalid.
53-
/// - If the name, URI, start, or end pointers are invalid.
62+
/// - `out_ref` must be a valid, writable pointer.
5463
#[unsafe(no_mangle)]
55-
pub unsafe extern "C" fn rdx_references_iter_next(
56-
iter: *mut ReferencesIter,
57-
out_id: *mut u64,
58-
out_kind: *mut ReferenceKind,
59-
) -> bool {
60-
if iter.is_null() || out_id.is_null() || out_kind.is_null() {
64+
pub unsafe extern "C" fn rdx_references_iter_next(iter: *mut ReferencesIter, out_ref: *mut CReference) -> bool {
65+
if iter.is_null() || out_ref.is_null() {
6166
return false;
6267
}
6368

@@ -66,11 +71,10 @@ pub unsafe extern "C" fn rdx_references_iter_next(
6671
return false;
6772
}
6873

69-
let (id, kind) = it.entries[it.index];
74+
let entry = it.entries[it.index];
7075
it.index += 1;
7176
unsafe {
72-
*out_id = id;
73-
*out_kind = kind;
77+
*out_ref = entry;
7478
}
7579
true
7680
}

0 commit comments

Comments
 (0)