-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The define ENCODING_RS_ENCODING is unusual way of wrapping #5
Comments
Yeah, this gets messy if the same app 1) uses it both from C and C++ and 2) it passes what's returned from Rust to C further from C to C++.
The dereferences happen on the Rust side, so the dereferencing isn't a problem.
Exposing value semantics without actually wrapping a pointer would require passing size and alignment information from the Rust compiler to the C++ compiler, which would mean using a nightly-only Rust feature and parsing its output in the C++ build system. Exposing value semantics in the way you suggest seems like making
It was done this way to allow the pointer to be a pointer to a namespaced thing in C++ in a C++ codebase 1) that doesn't care about also using the raw C API directly and 2) doesn't treat heap-allocated objects pointed to by unique pointer as unidiomatic and to allow the pointer to be directly a pointer to the underlying Rust object. I'd like to understand more about your use case that ends up wishing to call the API both via the raw C API and the C++ wrapper. |
I understand, but I don't require that.
You may say that, it is a good point of view, but I would say that that CppEncoding is a proper C++ class that follows the RAII pattern, like any other class in the C++ standard library, e.g. Now, the struct/class Encoding does not need RAII because it doesn't need to be destroyed (from C/C++ side), but To achieve RAII with the
That way, you still can circumvent RAII by calling |
Oops. Sorry, my previous reply was bogus. Let's try again. In my previous reply, I what thinking of The reason why For Once they are heap-allocated, wrapping them in C++ such that the wrappers would be smart pointers in disguise would be a lie. Letting these look like heap-allocated objects managed by
In C++, you can always circumvent things if you try hard enough. |
Noticed that this library does some tricks that are unidiomatic for C++. I'll post a minimal example.
The effect is that if
library_function()
is called from C code it returns pointer toCEncoding
, and from C++ code it returns pointer toCppEncoding
. This is unusual and it gets very close to the land of undefined behaviour (breaking strict aliasing), although probably is not as long as you don't dereference those pointers.The greater concern to me is that the default C++ wrapper exposes pointer semantics, and not value semantics. The way I would write the C++ wrapper is the usual way by wrapping the (pointer to) C-struct inside the C++ class.
Why was this done the way it is? Maybe to avoid name clashes? Would you consider to update to the idiomatic C++?
The text was updated successfully, but these errors were encountered: