Skip to content

Commit d9f7717

Browse files
committed
Pass filesystem paths as OsStr instead of str
1 parent 96c7e79 commit d9f7717

14 files changed

Lines changed: 176 additions & 84 deletions

File tree

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ winsplit = "0.1"
5252
[dev-dependencies]
5353
# Switch to std's assert_matches when MSRV is 1.96
5454
matches = "0.1"
55+
tempdir = "0.3"
5556
# copy of build-dependencies because we need to test methods of the build script
5657
opencv-binding-generator = { version = "0.103.0", path = "binding-generator" }
5758
cc = { version = "1.0.83", features = ["parallel"] }

binding-generator/src/func.rs

Lines changed: 59 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -491,31 +491,21 @@ impl<'tu, 'ge> Func<'tu, 'ge> {
491491
.into_iter()
492492
.enumerate()
493493
.map(|(idx, a)| {
494+
let arg_name = a.get_name();
494495
if let Some(func_arg_override) = arg_overrides
495-
&& let Some(type_hint) = a.get_name().and_then(|arg_name| func_arg_override.get(arg_name.as_str()))
496+
&& let Some(type_hint) = arg_name.as_deref().and_then(|arg_name| func_arg_override.get(arg_name))
496497
{
497498
return Field::new_ext(a, type_hint.clone(), gen_env);
498499
}
499-
let out = Field::new(a, gen_env);
500-
slice_arg_finder.feed(idx, &out);
501-
out
500+
let mut arg = Field::new(a, gen_env);
501+
if let Some(arg_name) = arg_name.as_deref() {
502+
update_path_argument(&mut arg, arg_name);
503+
}
504+
slice_arg_finder.feed(idx, &arg);
505+
arg
502506
})
503507
.collect::<Vec<_>>();
504-
for (slice_arg_indices, slice_len_arg_idx) in slice_arg_finder.finish() {
505-
let mut slice_arg_names = Vec::with_capacity(slice_arg_indices.len());
506-
for &slice_arg_idx in &slice_arg_indices {
507-
let slice_arg = &mut out[slice_arg_idx];
508-
slice_arg_names.push(slice_arg.rust_name(NameStyle::ref_()).into_owned());
509-
slice_arg.set_type_ref_type_hint(TypeRefTypeHint::Slice);
510-
}
511-
let slice_len_arg = &mut out[slice_len_arg_idx];
512-
let divisor = if slice_len_arg.cpp_name(CppNameStyle::Declaration).contains("pair") {
513-
2
514-
} else {
515-
1
516-
};
517-
slice_len_arg.set_type_ref_type_hint(TypeRefTypeHint::LenForSlice(slice_arg_names.into(), divisor));
518-
}
508+
update_slice_arguments(&mut out, slice_arg_finder);
519509
Owned(out)
520510
}
521511
Self::Desc(desc) => Borrowed(desc.arguments.as_ref()),
@@ -874,3 +864,53 @@ impl InheritConfig {
874864
self.kind || self.name || self.arguments || self.doc_comment || self.return_type_ref || self.definition_location
875865
}
876866
}
867+
868+
/// Checks whether the `arg_name` is a name of the argument that's expected to receive a filesystem path, and adds a corresponding
869+
/// type hint to `field` if so.
870+
fn update_path_argument(field: &mut Field, arg_name: &str) {
871+
const CHECK_SUFFIXES: [&str; 5] = ["file", "filename", "file_name", "path", "pathorname"];
872+
const CHECK_PREFIXES: [&str; 2] = ["pathto", "path_to"];
873+
874+
let len = arg_name.len();
875+
let arg_name_matched = CHECK_SUFFIXES
876+
.into_iter()
877+
.filter(|suf| suf.len() <= len)
878+
.flat_map(|suf| {
879+
arg_name
880+
.get(len - suf.len()..)
881+
.filter(|suf_check| suf_check.eq_ignore_ascii_case(suf))
882+
})
883+
.chain(CHECK_PREFIXES.into_iter().flat_map(|pref| {
884+
arg_name
885+
.get(..pref.len())
886+
.filter(|pref_check| pref_check.eq_ignore_ascii_case(pref))
887+
}))
888+
.next()
889+
.is_some();
890+
if arg_name_matched {
891+
field.set_type_ref_type_hint(TypeRefTypeHint::StringAsPath);
892+
return;
893+
}
894+
895+
if arg_name == "filenames" || arg_name == "paths" {
896+
// todo: add support for Vector<impl Into<OsStr>>
897+
}
898+
}
899+
900+
fn update_slice_arguments(arguments: &mut [Field], slice_arg_finder: SliceArgFinder) {
901+
for (slice_arg_indices, slice_len_arg_idx) in slice_arg_finder.finish() {
902+
let mut slice_arg_names = Vec::with_capacity(slice_arg_indices.len());
903+
for &slice_arg_idx in &slice_arg_indices {
904+
let slice_arg = &mut arguments[slice_arg_idx];
905+
slice_arg_names.push(slice_arg.rust_name(NameStyle::ref_()).into_owned());
906+
slice_arg.set_type_ref_type_hint(TypeRefTypeHint::Slice);
907+
}
908+
let slice_len_arg = &mut arguments[slice_len_arg_idx];
909+
let divisor = if slice_len_arg.cpp_name(CppNameStyle::Declaration).contains("pair") {
910+
2
911+
} else {
912+
1
913+
};
914+
slice_len_arg.set_type_ref_type_hint(TypeRefTypeHint::LenForSlice(slice_arg_names.into(), divisor));
915+
}
916+
}

binding-generator/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
// todo public static properties like opencv2/core/base.hpp:384 Hamming::normType
77
// todo test returning reference to array like cv_MatStep_buf
88
// todo, allow extension of simple classes for e.g. Elliptic_KeyPoint
9+
// todo OCRTesseract::create should have nullable params
910
// fixme vector<Mat*> get's interpreted as Vector<Mat> which should be wrong (e.g. Layer::forward and Layer::apply_halide_scheduler)
1011
// fixme MatConstIterator::m return Mat**, is it handled correctly?
1112
// fixme VectorOfMat::get allows mutation

binding-generator/src/type_ref.rs

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -493,26 +493,17 @@ impl fmt::Debug for TypeRef<'_, '_> {
493493
str_type
494494
}
495495
};
496-
match str_type {
497-
StrType::StdString(StrEnc::Text) => {
498-
props.push("std_string");
499-
}
500-
StrType::CvString(StrEnc::Text) => {
501-
props.push("cv_string");
502-
}
503-
StrType::CharPtr(StrEnc::Text) => {
504-
props.push("char_ptr_string");
505-
}
506-
StrType::StdString(StrEnc::Binary) => {
507-
props.push("byte_std_string");
508-
}
509-
StrType::CvString(StrEnc::Binary) => {
510-
props.push("byte_cv_string");
511-
}
512-
StrType::CharPtr(StrEnc::Binary) => {
513-
props.push("byte_ptr_string");
514-
}
515-
}
496+
props.push(match str_type {
497+
StrType::StdString(StrEnc::Text) => "std_string",
498+
StrType::CvString(StrEnc::Text) => "cv_string",
499+
StrType::CharPtr(StrEnc::Text) => "char_ptr_string",
500+
StrType::StdString(StrEnc::Binary) => "byte_std_string",
501+
StrType::CvString(StrEnc::Binary) => "byte_cv_string",
502+
StrType::CharPtr(StrEnc::Binary) => "byte_ptr_string",
503+
StrType::StdString(StrEnc::OsStr) => "osstr_std_string",
504+
StrType::CvString(StrEnc::OsStr) => "osstr_cv_string",
505+
StrType::CharPtr(StrEnc::OsStr) => "osstr_ptr_string",
506+
})
516507
}
517508
if kind.as_by_move().is_some() {
518509
props.push("by_move");

binding-generator/src/type_ref/kind.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -260,10 +260,12 @@ impl<'tu, 'ge> TypeRefKind<'tu, 'ge> {
260260
TypeRefKind::Typedef(tdef) => tdef.underlying_type_ref().kind().as_string(type_hint),
261261
_ => None,
262262
};
263-
if let Some((_, str_type)) = out.as_mut()
264-
&& matches!(type_hint, TypeRefTypeHint::StringAsBytes(_))
265-
{
266-
str_type.set_encoding(StrEnc::Binary)
263+
if let Some((_, str_type)) = out.as_mut() {
264+
match type_hint {
265+
TypeRefTypeHint::StringAsBytes(_) => str_type.set_encoding(StrEnc::Binary),
266+
TypeRefTypeHint::StringAsPath => str_type.set_encoding(StrEnc::OsStr),
267+
_ => {}
268+
}
267269
}
268270
out
269271
}

binding-generator/src/type_ref/types.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ pub enum TypeRefTypeHint {
2121
LenForSlice(Arc<[String]>, usize),
2222
/// Treat C++ string as a byte buffer (`Vec<u8>`) instead of an actual string, argument is optional cpp_arg_name of the argument that specifies the buffer byte length
2323
StringAsBytes(Option<Arc<str>>),
24+
/// Treat C++ string as a file path, done heuristically based on the argument name
25+
StringAsPath,
2426
/// String len is passed in an additional argument (cpp_arg_name)
2527
StringWithLen(Rc<str>),
2628
/// when C++ char needs to be represented as Rust char
@@ -60,6 +62,7 @@ impl TypeRefTypeHint {
6062
| Self::Slice
6163
| Self::LenForSlice(_, _)
6264
| Self::StringAsBytes(_)
65+
| Self::StringAsPath
6366
| Self::StringWithLen(_)
6467
| Self::CharAsRustChar
6568
| Self::CharPtrSingleChar
@@ -78,6 +81,7 @@ impl TypeRefTypeHint {
7881
| Self::Slice
7982
| Self::LenForSlice(_, _)
8083
| Self::StringAsBytes(_)
84+
| Self::StringAsPath
8185
| Self::StringWithLen(_)
8286
| Self::CharAsRustChar
8387
| Self::CharPtrSingleChar
@@ -244,23 +248,24 @@ pub enum StrType {
244248
impl StrType {
245249
pub fn set_encoding(&mut self, enc: StrEnc) {
246250
match self {
247-
StrType::StdString(old_enc) | StrType::CvString(old_enc) | StrType::CharPtr(old_enc) => *old_enc = enc,
251+
Self::StdString(old_enc) | Self::CvString(old_enc) | Self::CharPtr(old_enc) => *old_enc = enc,
248252
}
249253
}
250254

251-
pub fn is_binary(&self) -> bool {
255+
pub fn encoding(&self) -> StrEnc {
252256
match self {
253-
StrType::StdString(StrEnc::Binary) | StrType::CvString(StrEnc::Binary) | StrType::CharPtr(StrEnc::Binary) => true,
254-
StrType::StdString(StrEnc::Text) | StrType::CvString(StrEnc::Text) | StrType::CharPtr(StrEnc::Text) => false,
257+
Self::StdString(enc) | Self::CvString(enc) | Self::CharPtr(enc) => *enc,
255258
}
256259
}
257260
}
258261

259262
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
260263
pub enum StrEnc {
261264
Text,
262-
/// string with binary data, e.g. can contain 0 byte
265+
/// string with binary data (=== [Vec<u8>]), can contain 0 byte
263266
Binary,
267+
/// OS-specific encoding (== [std::ffi::OsString]), used to pass filesystem paths for example
268+
OsStr,
264269
}
265270

266271
#[derive(Clone, Copy, Debug, PartialEq, Eq)]

binding-generator/src/writer/rust_native/func.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -789,15 +789,16 @@ pub fn cpp_return_map<'f>(return_type: &TypeRef, name: &'f str, is_constructor:
789789
("".into(), false)
790790
} else if let Some((_, string_type)) = return_kind.as_string(return_type.type_hint()) {
791791
let str_mk = match string_type {
792-
StrType::StdString(StrEnc::Text) | StrType::CvString(StrEnc::Text) => {
793-
format!("ocvrs_create_string({name}.c_str())").into()
794-
}
795-
StrType::StdString(StrEnc::Binary) => format!("ocvrs_create_byte_string({name}.data(), {name}.size())").into(),
796-
StrType::CvString(StrEnc::Binary) => format!("ocvrs_create_byte_string({name}.begin(), {name}.size())").into(),
797-
StrType::CharPtr(StrEnc::Text) => format!("ocvrs_create_string({name})").into(),
792+
StrType::StdString(StrEnc::Text) | StrType::CvString(StrEnc::Text) => format!("ocvrs_create_string({name}.c_str())"),
793+
StrType::StdString(StrEnc::Binary) => format!("ocvrs_create_byte_string({name}.data(), {name}.size())"),
794+
StrType::CvString(StrEnc::Binary) => format!("ocvrs_create_byte_string({name}.begin(), {name}.size())"),
795+
StrType::CharPtr(StrEnc::Text) => format!("ocvrs_create_string({name})"),
798796
StrType::CharPtr(StrEnc::Binary) => panic!("Returning a byte string via char* is not supported yet"),
797+
StrType::StdString(StrEnc::OsStr) | StrType::CvString(StrEnc::OsStr) | StrType::CharPtr(StrEnc::OsStr) => {
798+
panic!("Returning a path string is not supported yet")
799+
}
799800
};
800-
(str_mk, false)
801+
(str_mk.into(), false)
801802
} else if return_kind.extern_pass_kind().is_by_void_ptr() && !is_constructor {
802803
let ret_source = return_type.source();
803804
let out = ret_source.kind().as_class().filter(|cls| cls.is_abstract()).map_or_else(

binding-generator/src/writer/rust_native/renderer.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::fmt::Write;
33

44
use crate::renderer::TypeRefRenderer;
55
use crate::type_ref::{
6-
Constness, CppNameStyle, Dir, ExternDir, FishStyle, NameStyle, StrType, TemplateArg, TypeRef, TypeRefDesc, TypeRefKind,
6+
Constness, CppNameStyle, Dir, ExternDir, FishStyle, NameStyle, StrEnc, StrType, TemplateArg, TypeRef, TypeRefDesc, TypeRefKind,
77
};
88
use crate::writer::rust_native::class::ClassExt;
99
use crate::writer::rust_native::element::RustElement;
@@ -72,10 +72,10 @@ impl TypeRefRenderer<'_> for RustRenderer {
7272
fn render<'t>(self, type_ref: &'t TypeRef) -> Cow<'t, str> {
7373
let kind = type_ref.kind();
7474
if let Some((_, str_type)) = kind.as_string(type_ref.type_hint()) {
75-
if str_type.is_binary() {
76-
format!("Vec{fish}<u8>", fish = self.name_style.turbo_fish_style().rust_qual()).into()
77-
} else {
78-
"String".into()
75+
match str_type.encoding() {
76+
StrEnc::Text => "String".into(),
77+
StrEnc::Binary => format!("Vec{fish}<u8>", fish = self.name_style.turbo_fish_style().rust_qual()).into(),
78+
StrEnc::OsStr => "PathBuf".into(),
7979
}
8080
} else {
8181
kind.map_borrowed(|kind| match kind {

binding-generator/src/writer/rust_native/type_ref/render_lane/string.rs

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,23 @@ impl RenderLaneTrait for InStringRenderLane<'_, '_> {
2121
}
2222

2323
fn rust_arg_func_decl(&self, name: &str, _lifetime: Lifetime) -> String {
24-
let typ = if self.str_type.is_binary() {
25-
"&[u8]"
26-
} else {
27-
"&str"
24+
let typ = match self.str_type.encoding() {
25+
StrEnc::Text => "&str",
26+
StrEnc::Binary => "&[u8]",
27+
StrEnc::OsStr => "impl AsRef<OsStr>",
2828
};
2929
rust_arg_func_decl(name, Constness::Const, typ)
3030
}
3131

3232
fn rust_arg_pre_call(&self, name: &str, function_props: &FunctionProps) -> String {
33-
if function_props.is_infallible {
34-
format!("extern_container_arg!(nofail {name})")
33+
let fail_spec = if function_props.is_infallible {
34+
"nofail "
3535
} else {
36-
format!("extern_container_arg!({name})")
36+
""
37+
};
38+
match self.str_type.encoding() {
39+
StrEnc::Text | StrEnc::Binary => format!("extern_container_arg!({fail_spec}{name})"),
40+
StrEnc::OsStr => format!("path_arg!({fail_spec}{name})"),
3741
}
3842
}
3943

@@ -75,10 +79,10 @@ impl RenderLaneTrait for OutStringRenderLane<'_, '_> {
7579
}
7680

7781
fn rust_arg_func_decl(&self, name: &str, _lifetime: Lifetime) -> String {
78-
let typ = if self.str_type.is_binary() {
79-
"&mut Vec<u8>"
80-
} else {
81-
"&mut String"
82+
let typ = match self.str_type.encoding() {
83+
StrEnc::Text => "&mut String",
84+
StrEnc::Binary => "&mut Vec<u8>",
85+
StrEnc::OsStr => "&mut OsString",
8286
};
8387
rust_arg_func_decl(name, Constness::Const, typ)
8488
}
@@ -117,6 +121,7 @@ impl RenderLaneTrait for OutStringRenderLane<'_, '_> {
117121
| TypeRefTypeHint::Slice
118122
| TypeRefTypeHint::LenForSlice(_, _)
119123
| TypeRefTypeHint::StringAsBytes(_)
124+
| TypeRefTypeHint::StringAsPath
120125
| TypeRefTypeHint::CharAsRustChar
121126
| TypeRefTypeHint::CharPtrSingleChar
122127
| TypeRefTypeHint::PrimitivePtrAsRaw
@@ -149,21 +154,23 @@ impl RenderLaneTrait for OutStringRenderLane<'_, '_> {
149154
StrType::StdString(StrEnc::Text) | StrType::CvString(StrEnc::Text) => {
150155
format!("*{name} = ocvrs_create_string({name}_out.c_str())")
151156
}
157+
StrType::CharPtr(StrEnc::Text) => {
158+
format!("*{name} = ocvrs_create_string({name}_out.get())")
159+
}
152160
StrType::StdString(StrEnc::Binary) => {
153161
format!("*{name} = ocvrs_create_byte_string({name}_out.data(), {name}_out.size())")
154162
}
155163
StrType::CvString(StrEnc::Binary) => {
156164
format!("*{name} = ocvrs_create_byte_string({name}_out.begin(), {name}_out.size())")
157165
}
158-
StrType::CharPtr(StrEnc::Text) => {
159-
format!("*{name} = ocvrs_create_string({name}_out.get())")
160-
}
161166
StrType::CharPtr(StrEnc::Binary) => {
162-
if let TypeRefTypeHint::StringAsBytes(Some(len_arg_name)) = self.canonical.type_hint() {
163-
format!("*{name} = ocvrs_create_byte_string({name}_out.get(), {len_arg_name})")
164-
} else {
167+
let TypeRefTypeHint::StringAsBytes(Some(len_arg_name)) = self.canonical.type_hint() else {
165168
panic!("Output argument of type `char*` with binary encoding must have `len` argument specified")
166-
}
169+
};
170+
format!("*{name} = ocvrs_create_byte_string({name}_out.get(), {len_arg_name})")
171+
}
172+
StrType::StdString(StrEnc::OsStr) | StrType::CvString(StrEnc::OsStr) | StrType::CharPtr(_) => {
173+
panic!("Output string argument with OsStr encoding is not supported")
167174
}
168175
}
169176
}

src/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,15 @@ pub mod platform_types {
3333

3434
/// Prelude for sys (externs) module and types
3535
pub mod mod_prelude_sys {
36-
pub use std::ffi::{c_char, c_void};
36+
pub use core::ffi::{c_char, c_void};
3737

3838
pub use crate::platform_types::*;
3939
pub use crate::traits::{Boxed, OpenCVFromExtern, OpenCVIntoExternContainer, OpenCVTypeExternContainer};
4040
}
4141

4242
/// Prelude for generated modules and types
4343
pub mod mod_prelude {
44+
pub use std::ffi::OsStr;
4445
pub use std::marker::PhantomData;
4546

4647
pub use crate::boxed_ref::{BoxedRef, BoxedRefMut};

0 commit comments

Comments
 (0)