Skip to content

Commit a2e9b15

Browse files
twistedfalltwistedfall
committed
Improve slice detection
1 parent d7fdc81 commit a2e9b15

File tree

10 files changed

+257
-71
lines changed

10 files changed

+257
-71
lines changed

‎binding-generator/src/field.rs

+40-33
Original file line numberDiff line numberDiff line change
@@ -190,38 +190,44 @@ impl<'tu, 'ge> Field<'tu, 'ge> {
190190
}
191191

192192
pub fn slice_arg_eligibility(&self) -> SliceArgEligibility {
193-
self
194-
.type_ref()
195-
.kind()
193+
let type_ref = self.type_ref();
194+
let kind = type_ref.kind();
195+
kind
196196
.as_pointer()
197197
.filter(|inner| inner.kind().is_copy(inner.type_hint()))
198-
.map_or(SliceArgEligibility::NotEligible, |_| {
199-
let name = self.cpp_name(CppNameStyle::Declaration);
200-
let name = name.as_ref();
201-
if ARGUMENT_NAMES_NOT_SLICE.contains(name) {
202-
SliceArgEligibility::NotEligible
203-
} else if ARGUMENT_NAMES_MULTIPLE_SLICE.contains(name) {
204-
SliceArgEligibility::EligibleWithMultiple
205-
} else {
206-
SliceArgEligibility::Eligible
207-
}
208-
})
209-
}
210-
211-
pub fn can_be_slice_arg_len(&self) -> bool {
212-
let type_ref = self.type_ref();
213-
type_ref.kind().as_primitive().map_or(false, |(_, cpp)| {
214-
if cpp == "int" || cpp == "size_t" {
215-
let name = self.cpp_name(CppNameStyle::Declaration);
216-
name.ends_with('s') && name.contains('n') && name != "thickness" // fixme: have to exclude thickness
217-
|| name.contains("dims")
218-
|| name == "size"
219-
|| name.ends_with("Size")
220-
|| name == "len"
221-
} else {
222-
false
223-
}
224-
})
198+
.or_else(|| kind.as_variable_array())
199+
.map_or_else(
200+
|| {
201+
// check if still can be a slice arg length
202+
let can_be_slice_arg_len = kind.as_primitive().map_or(false, |(_, cpp)| {
203+
if cpp == "int" || cpp == "size_t" {
204+
let name = self.cpp_name(CppNameStyle::Declaration);
205+
name.ends_with('s') && name.contains('n') && name != "thickness" // fixme: have to exclude thickness
206+
|| ["size", "len", "argc"].contains(&name.as_ref())
207+
|| name.contains("dims")
208+
|| name.ends_with("Size")
209+
} else {
210+
false
211+
}
212+
});
213+
if can_be_slice_arg_len {
214+
SliceArgEligibility::SliceArgLength
215+
} else {
216+
SliceArgEligibility::None
217+
}
218+
},
219+
|_| {
220+
let name = self.cpp_name(CppNameStyle::Declaration);
221+
let name = name.as_ref();
222+
if ARGUMENT_NAMES_NOT_SLICE.contains(name) {
223+
SliceArgEligibility::None
224+
} else if ARGUMENT_NAMES_MULTIPLE_SLICE.contains(name) {
225+
SliceArgEligibility::SliceArgMultiple
226+
} else {
227+
SliceArgEligibility::SliceArgSingle
228+
}
229+
},
230+
)
225231
}
226232
}
227233

@@ -307,7 +313,8 @@ impl<'tu, 'ge> FieldDesc<'tu, 'ge> {
307313
}
308314

309315
pub enum SliceArgEligibility {
310-
NotEligible,
311-
Eligible,
312-
EligibleWithMultiple,
316+
None,
317+
SliceArgLength,
318+
SliceArgSingle,
319+
SliceArgMultiple,
313320
}

‎binding-generator/src/func/slice_arg_finder.rs

+154-25
Original file line numberDiff line numberDiff line change
@@ -16,56 +16,87 @@ impl SliceArgFinder {
1616
pub fn feed(&mut self, idx: usize, arg: &Field) {
1717
self.state = match self.state.take() {
1818
None => match arg.slice_arg_eligibility() {
19-
SliceArgEligibility::NotEligible => {
20-
if arg.can_be_slice_arg_len() {
21-
Some(SliceArgFinderState::LenArgFound(idx))
22-
} else {
23-
None
24-
}
25-
}
26-
SliceArgEligibility::Eligible => Some(SliceArgFinderState::SliceArgFound(idx)),
27-
SliceArgEligibility::EligibleWithMultiple => Some(SliceArgFinderState::SliceArgWithPotentialMultipleFound(vec![idx])),
19+
SliceArgEligibility::None => None,
20+
SliceArgEligibility::SliceArgLength => Some(SliceArgFinderState::LenArgFound(idx)),
21+
SliceArgEligibility::SliceArgSingle => Some(SliceArgFinderState::SliceArgFound(idx)),
22+
SliceArgEligibility::SliceArgMultiple => Some(SliceArgFinderState::SliceArgWithPotentialMultipleFound(vec![idx])),
2823
},
29-
Some(SliceArgFinderState::SliceArgFound(slice_arg_idx)) => {
30-
if arg.can_be_slice_arg_len() {
24+
Some(SliceArgFinderState::SliceArgFound(slice_arg_idx)) => match arg.slice_arg_eligibility() {
25+
SliceArgEligibility::None => Some(SliceArgFinderState::SliceArgFound(slice_arg_idx)),
26+
SliceArgEligibility::SliceArgLength => {
3127
self.slice_args.push((vec![slice_arg_idx], idx));
3228
None
33-
} else {
34-
Some(SliceArgFinderState::SliceArgFound(slice_arg_idx))
3529
}
36-
}
30+
SliceArgEligibility::SliceArgSingle => Some(SliceArgFinderState::SliceArgFound(idx)),
31+
SliceArgEligibility::SliceArgMultiple => Some(SliceArgFinderState::SliceArgWithPotentialMultipleFound(vec![
32+
slice_arg_idx,
33+
idx,
34+
])),
35+
},
3736
Some(SliceArgFinderState::SliceArgWithPotentialMultipleFound(mut slice_arg_indices)) => {
3837
match arg.slice_arg_eligibility() {
39-
SliceArgEligibility::NotEligible if arg.can_be_slice_arg_len() => {
38+
SliceArgEligibility::SliceArgLength => {
4039
self.slice_args.push((slice_arg_indices, idx));
4140
None
4241
}
4342
// multiple slice arguments are consecutive
44-
SliceArgEligibility::NotEligible | SliceArgEligibility::Eligible => {
43+
SliceArgEligibility::None | SliceArgEligibility::SliceArgSingle => {
4544
slice_arg_indices.into_iter().next().map(SliceArgFinderState::SliceArgFound)
4645
} // todo: try `idx`
47-
SliceArgEligibility::EligibleWithMultiple => {
46+
SliceArgEligibility::SliceArgMultiple => {
4847
slice_arg_indices.push(idx);
4948
Some(SliceArgFinderState::SliceArgWithPotentialMultipleFound(slice_arg_indices))
5049
}
5150
}
5251
}
53-
Some(SliceArgFinderState::LenArgFound(slice_len_arg_idx)) => match arg.slice_arg_eligibility() {
54-
SliceArgEligibility::NotEligible => Some(SliceArgFinderState::LenArgFound(slice_len_arg_idx)),
55-
SliceArgEligibility::Eligible => {
56-
self.slice_args.push((vec![idx], slice_len_arg_idx));
57-
None
52+
Some(SliceArgFinderState::SliceArgWithPotentialMultipleAndLenFound(mut slice_arg_indices, slice_len_arg_idx)) => {
53+
match arg.slice_arg_eligibility() {
54+
SliceArgEligibility::None => {
55+
self.slice_args.push((slice_arg_indices, slice_len_arg_idx));
56+
None
57+
}
58+
SliceArgEligibility::SliceArgLength => {
59+
self.slice_args.push((slice_arg_indices, slice_len_arg_idx));
60+
Some(SliceArgFinderState::LenArgFound(idx))
61+
}
62+
SliceArgEligibility::SliceArgSingle => {
63+
self.slice_args.push((slice_arg_indices, slice_len_arg_idx));
64+
Some(SliceArgFinderState::SliceArgFound(idx))
65+
}
66+
SliceArgEligibility::SliceArgMultiple => {
67+
slice_arg_indices.push(idx);
68+
Some(SliceArgFinderState::SliceArgWithPotentialMultipleAndLenFound(
69+
slice_arg_indices,
70+
slice_len_arg_idx,
71+
))
72+
}
5873
}
59-
SliceArgEligibility::EligibleWithMultiple => {
60-
// fixme: case where slice len argument comes before 2 slice arguments is not handled
74+
}
75+
Some(SliceArgFinderState::LenArgFound(slice_len_arg_idx)) => match arg.slice_arg_eligibility() {
76+
SliceArgEligibility::None => Some(SliceArgFinderState::LenArgFound(slice_len_arg_idx)),
77+
SliceArgEligibility::SliceArgLength => Some(SliceArgFinderState::LenArgFound(idx)),
78+
SliceArgEligibility::SliceArgSingle => {
6179
self.slice_args.push((vec![idx], slice_len_arg_idx));
6280
None
6381
}
82+
SliceArgEligibility::SliceArgMultiple => Some(SliceArgFinderState::SliceArgWithPotentialMultipleAndLenFound(
83+
vec![idx],
84+
slice_len_arg_idx,
85+
)),
6486
},
6587
};
6688
}
6789

68-
pub fn finish(self) -> Vec<(Vec<usize>, usize)> {
90+
pub fn finish(mut self) -> Vec<(Vec<usize>, usize)> {
91+
match self.state {
92+
None
93+
| Some(SliceArgFinderState::LenArgFound(_))
94+
| Some(SliceArgFinderState::SliceArgFound(_))
95+
| Some(SliceArgFinderState::SliceArgWithPotentialMultipleFound(_)) => {}
96+
Some(SliceArgFinderState::SliceArgWithPotentialMultipleAndLenFound(slice_arg_indices, slice_len_arg_idx)) => {
97+
self.slice_args.push((slice_arg_indices, slice_len_arg_idx));
98+
}
99+
}
69100
self.slice_args
70101
}
71102
}
@@ -76,5 +107,103 @@ enum SliceArgFinderState {
76107
SliceArgFound(usize),
77108
/// Found a slice argument which might have an additional connected slice arg (e.g. `src`/`dst` which share the same slice length argument)
78109
SliceArgWithPotentialMultipleFound(Vec<usize>),
110+
/// Same as [SliceArgFinderState::SliceArgWithPotentialMultipleFound], but also a length for those arg has also been found
111+
SliceArgWithPotentialMultipleAndLenFound(Vec<usize>, usize),
79112
LenArgFound(usize),
80113
}
114+
115+
#[cfg(test)]
116+
mod test {
117+
use super::SliceArgFinder;
118+
use crate::field::{Field, FieldDesc};
119+
use crate::type_ref::{TypeRef, TypeRefDesc};
120+
121+
#[test]
122+
fn test_slice_arg_finder() {
123+
// one arg, not slice
124+
{
125+
let mut finder = SliceArgFinder::with_capacity(0);
126+
finder.feed(0, &Field::new_desc(FieldDesc::new("src", TypeRefDesc::int())));
127+
assert_eq!(finder.finish(), vec![]);
128+
}
129+
130+
// slice len after
131+
{
132+
let mut finder = SliceArgFinder::with_capacity(0);
133+
finder.feed(
134+
0,
135+
&Field::new_desc(FieldDesc::new("dims", TypeRef::new_pointer(TypeRefDesc::float()))),
136+
);
137+
finder.feed(1, &Field::new_desc(FieldDesc::new("ndims", TypeRefDesc::size_t())));
138+
assert_eq!(finder.finish(), vec![(vec![0], 1)]);
139+
}
140+
141+
// slice len first
142+
{
143+
let mut finder = SliceArgFinder::with_capacity(0);
144+
finder.feed(0, &Field::new_desc(FieldDesc::new("argc", TypeRefDesc::int())));
145+
finder.feed(
146+
1,
147+
&Field::new_desc(FieldDesc::new("argv", TypeRef::new_array(TypeRefDesc::char_ptr(), None))),
148+
);
149+
assert_eq!(finder.finish(), vec![(vec![1], 0)]);
150+
}
151+
152+
// slice name prevents eligibility
153+
{
154+
let mut finder = SliceArgFinder::with_capacity(0);
155+
finder.feed(
156+
0,
157+
&Field::new_desc(FieldDesc::new("rmsd", TypeRef::new_pointer(TypeRefDesc::float()))),
158+
);
159+
finder.feed(1, &Field::new_desc(FieldDesc::new("ndims", TypeRefDesc::size_t())));
160+
assert_eq!(finder.finish(), vec![]);
161+
}
162+
163+
// 2 slices with 1 length, size first
164+
{
165+
let mut finder = SliceArgFinder::with_capacity(0);
166+
finder.feed(0, &Field::new_desc(FieldDesc::new("abSize", TypeRefDesc::int())));
167+
finder.feed(
168+
1,
169+
&Field::new_desc(FieldDesc::new("a", TypeRef::new_pointer(TypeRefDesc::float()))),
170+
);
171+
finder.feed(
172+
2,
173+
&Field::new_desc(FieldDesc::new("b", TypeRef::new_pointer(TypeRefDesc::int()))),
174+
);
175+
assert_eq!(finder.finish(), vec![(vec![1, 2], 0)]);
176+
}
177+
178+
// 2 slices with 1 length, size last
179+
{
180+
let mut finder = SliceArgFinder::with_capacity(0);
181+
finder.feed(
182+
0,
183+
&Field::new_desc(FieldDesc::new("a", TypeRef::new_pointer(TypeRefDesc::float()))),
184+
);
185+
finder.feed(
186+
1,
187+
&Field::new_desc(FieldDesc::new("b", TypeRef::new_pointer(TypeRefDesc::int()))),
188+
);
189+
finder.feed(2, &Field::new_desc(FieldDesc::new("abSize", TypeRefDesc::int())));
190+
assert_eq!(finder.finish(), vec![(vec![0, 1], 2)]);
191+
}
192+
193+
// 2 sets of slices with lengths
194+
{
195+
let mut finder = SliceArgFinder::with_capacity(0);
196+
finder.feed(
197+
0,
198+
&Field::new_desc(FieldDesc::new("a", TypeRef::new_pointer(TypeRefDesc::float()))),
199+
);
200+
finder.feed(1, &Field::new_desc(FieldDesc::new("aSize", TypeRefDesc::int())));
201+
finder.feed(
202+
2,
203+
&Field::new_desc(FieldDesc::new("b", TypeRef::new_pointer(TypeRefDesc::uint64_t()))),
204+
);
205+
finder.feed(3, &Field::new_desc(FieldDesc::new("len", TypeRefDesc::int())));
206+
assert_eq!(finder.finish(), vec![(vec![0], 1), (vec![2], 3)]);
207+
}
208+
}
209+
}

‎binding-generator/src/settings/argument_names.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,5 @@ pub static ARGUMENT_NAMES_USERDATA: Lazy<HashSet<&str>> =
1212
pub static ARGUMENT_NAMES_NOT_SLICE: Lazy<HashSet<&str>> = Lazy::new(|| HashSet::from(["rmsd"]));
1313

1414
/// List of C++ argument names that can hint on multiple connected slice arguments in a function
15-
pub static ARGUMENT_NAMES_MULTIPLE_SLICE: Lazy<HashSet<&str>> = Lazy::new(|| HashSet::from(["a", "b", "src", "dst", "lut"]));
15+
pub static ARGUMENT_NAMES_MULTIPLE_SLICE: Lazy<HashSet<&str>> =
16+
Lazy::new(|| HashSet::from(["a", "b", "src", "dst", "lut", "globalsize", "localsize"]));

‎binding-generator/src/type_ref/desc.rs

+9-4
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,14 @@ impl<'tu, 'ge> TypeRefDesc<'tu, 'ge> {
8282
Self::try_primitive("char").expect("Static primitive type")
8383
}
8484

85+
pub fn char_ptr() -> TypeRef<'tu, 'ge> {
86+
TypeRef::new_pointer(Self::char())
87+
}
88+
89+
pub fn char_const_ptr() -> TypeRef<'tu, 'ge> {
90+
TypeRef::new_pointer(Self::char().with_inherent_constness(Constness::Const))
91+
}
92+
8593
pub fn uchar() -> TypeRef<'tu, 'ge> {
8694
Self::try_primitive("unsigned char").expect("Static primitive type")
8795
}
@@ -375,10 +383,7 @@ impl<'tu> ClangTypeExt<'tu> for Type<'tu> {
375383
let pointee_kind = pointee_typeref.kind();
376384
if pointee_kind.is_function() {
377385
pointee_kind.into_owned()
378-
} else if matches!(
379-
pointee_typeref.type_hint(),
380-
TypeRefTypeHint::Slice | TypeRefTypeHint::NullableSlice
381-
) {
386+
} else if matches!(pointee_typeref.type_hint(), TypeRefTypeHint::Slice) {
382387
TypeRefKind::Array(pointee_typeref, None)
383388
} else {
384389
TypeRefKind::Pointer(pointee_typeref)

‎binding-generator/src/type_ref/kind.rs

+45
Original file line numberDiff line numberDiff line change
@@ -486,3 +486,48 @@ pub enum InputOutputArrayKind {
486486
Output,
487487
InputOutput,
488488
}
489+
490+
#[cfg(test)]
491+
mod tests {
492+
use crate::type_ref::{Dir, StrEnc, StrType, TypeRef, TypeRefDesc, TypeRefTypeHint};
493+
494+
fn as_string(type_ref: TypeRef) -> Option<(Dir, StrType)> {
495+
type_ref.kind().as_string(type_ref.type_hint())
496+
}
497+
498+
#[test]
499+
fn test_as_string_char_ptr() {
500+
{
501+
let char = TypeRefDesc::char();
502+
assert_eq!(None, as_string(char));
503+
}
504+
505+
{
506+
let char_ptr = TypeRefDesc::char_ptr();
507+
assert_eq!(Some((Dir::Out, StrType::CharPtr(StrEnc::Text))), as_string(char_ptr));
508+
}
509+
510+
{
511+
let char_ptr_const = TypeRefDesc::char_const_ptr();
512+
assert_eq!(Some((Dir::In, StrType::CharPtr(StrEnc::Text))), as_string(char_ptr_const));
513+
}
514+
515+
{
516+
let char_ptr_const_slice = TypeRefDesc::char_const_ptr().with_type_hint(TypeRefTypeHint::StringAsBytes(None));
517+
assert_eq!(
518+
Some((Dir::In, StrType::CharPtr(StrEnc::Binary))),
519+
as_string(char_ptr_const_slice)
520+
);
521+
}
522+
523+
{
524+
let single_char_ptr = TypeRefDesc::char_ptr().with_type_hint(TypeRefTypeHint::CharPtrSingleChar);
525+
assert_eq!(None, as_string(single_char_ptr));
526+
}
527+
528+
{
529+
let char_array = TypeRef::new_array(TypeRefDesc::char(), None);
530+
assert_eq!(Some((Dir::In, StrType::CharPtr(StrEnc::Text))), as_string(char_array));
531+
}
532+
}
533+
}

0 commit comments

Comments
 (0)