Add API to get slice from CxxVector #321
Conversation
| @@ -1270,6 +1270,13 @@ fn write_cxx_vector(out: &mut OutFile, vector_ty: &Type, element: &Ident, types: | |||
| ); | |||
| writeln!(out, " return s.size();"); | |||
| writeln!(out, "}}"); | |||
| writeln!( | |||
There was a problem hiding this comment.
Wondering whether I can or should generate src/cxx.h|cc from this. Need to look into that...
| @@ -401,6 +401,12 @@ extern "C" const char *cxx_run_test() noexcept { | |||
| std::unique_ptr<std::string>(new std::string("2020"))); | |||
| r_take_enum(Enum::AVal); | |||
|
|
|||
| // Caller owns data handed into rust APIs | |||
There was a problem hiding this comment.
Test for that did not really fit into the "take" or "return" tests.
I initially had a c_take...() test that would create a std::vector and then call into the r_access_...() test function, but I found that overly complex as the c_take...() test did not add anything. Hence, directly testing the API in r_access..() here.
macro/src/expand.rs
Outdated
| #[link_name = #link_data] | ||
| fn __vector_data(_: &::cxx::CxxVector<#elem>) -> *const #elem; | ||
| } | ||
| let ret = unsafe { std::slice::from_raw_parts(__vector_data(v), __vector_size(v)) }; |
There was a problem hiding this comment.
I believe this is undefined behavior in the case that size is 0. The data ptr of a &[T] has to be nonnull and properly aligned regardless of size, which C++ does not guarantee.
See https://doc.rust-lang.org/std/slice/fn.from_raw_parts.html#safety.
Somewhere there will need to be special handling of the empty vector case.
There was a problem hiding this comment.
I was actually wondering about that. ;)
Let me add the special casing of nullptr/0 size
There was a problem hiding this comment.
Have not looked into how rust handles alignment. That could be a biggie if rust has different alignment reqs...
Having said that, we could enforce that in the __vector_data() implementation. As in, if wrongly aligned, "do something". that "do something" needs looking into/discussion.
There was a problem hiding this comment.
Fixed handling of null data() ptr.
UB only happens if std::vector's capacity() is 0 if I got the from_raw_parts documentation right. There does not seem to be a need to specifically handle 0 size. Famous last words?
Ignore my going off the tangent with non-alignment. I was thinking off T, whereas you were talking about the data ptr if null.
Map std::vector without capacity, i.e. data() == nullptr, to empty slice.
| { | ||
| // std::vector with capacity() has valid data ptr, even if | ||
| // size() == 0. | ||
| let ret = unsafe { slice::from_raw_parts(data, __vector_size(v)) }; |
There was a problem hiding this comment.
This is still undefined behavior, I think. As far as I can tell nothing in https://en.cppreference.com/w/cpp/container/vector/data would guarantee that data produces a sufficiently aligned pointer in the empty case. Can you find that in the standard, or implement a different way?
There was a problem hiding this comment.
You are right. Went after the implementation behaviour that always has been instead of reading the standard.
Fix would be to check the size() == 0 instead the ptr, which I see you have done in your implementation. Thanks for fixing my code up. ;)
Implementing #320
Adds a get_slice() to CxxVector so that one can use slices in the rust code and std::vector on the C++ side.
Bear with me, new to rust.