Skip to content
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

[docs] Improve CxxString example with various usage patterns #1190

Open
nyurik opened this issue Mar 2, 2023 · 5 comments
Open

[docs] Improve CxxString example with various usage patterns #1190

nyurik opened this issue Mar 2, 2023 · 5 comments

Comments

@nyurik
Copy link

nyurik commented Mar 2, 2023

I am new to the Rust-C++ FFI, and have been trying to adapt it to one of our projects. I have been going over the CxxString docs, trying to understand the best usage pattern. I think it could be improved with a few more code examples. I would love to contribute to the docs/book, but will need to understand it first. I do think that the Json example gets a little in the way of answering a simple question of "given this existing C++ API that uses std::string, this is how to call it", but that could be just me.

Given this C++ function, I need to either add some extra C++ code to work around the limitations of no-by-value passing, or some other steps:

// key is passed by value, it is not modified by the function
// value is passed by ref, it receives some output
int32_t GetValue(std::string key, std::string& value);

By-value limitation

Apparently @schreter is working on this in #1185 (thank you!!! I would love to participate), so might be relevant here.
To work around the by-value limitation, my understanding is that i may need to create a wrapper C++ function. This seems a bit strange that C++ allows me to silently cast by ref to by val, but I haven't worked with it for a while, might be a bit ... "rusty".

int32_t GetValue2(const std::string& key, std::string& value) {
  return GetValue(key, value);
}

Calling C++ function from Rust

This is what I would have expected to use, but it does not even begin to compile, so a bit lost with pinning and other magic.

let_cxx_string!(key = "example");
let_cxx_string!(value = "");
let res = crate::ffi::GetValue2(key, value);  // may need some modifiers like &key ?
println!("res = {res}, key = {key}, value = {value}");  // may need some key/value unwrapping

Thank you!!!

@schreter
Copy link
Contributor

schreter commented Mar 2, 2023

@nyurik Just to set the expectations straight, the #1185 is for now meant for Rust objects to be passed to C++ by value. The other way around is fairly easy as well (it can be done in a very similar manner, even somewhat simpler), BUT the implementor must guarantee that the C++ object passed by-value is not self-referential. Handling self-referential objects by value is naturally not possible.

However, before I post the by-value patch (we already have a working solution in our private fork) I wanted to clarify the API (especially, the explicit layout specification). Similarly, I want to clarify the API for Rust type aliases/imports (#1187 with a minimal implementation in #1181), because this is directly related (we need to declare the same layout on the import in another bridge).

For these two and also the exception change, I'm waiting for reaction from @dtolnay, so I can address potential maintainer feedback to get those finished and merged before publishing anything more complex.

But back to your issue:

As already mentioned in the documentation, C++ std::string can't be supported for by-value passing due to potential self-referential members.

let_cxx_string! basically builds an opaque std::string on the stack in some reserved space by calling back to C++. Since the object is then pinned, you can't move it, which makes it safe to use, even if it is self-referential. However, the CxxString being !Unpin, you can only get an immutable reference. So even if you'd write it "right" like this:

let_cxx_string!(key = "example");
let_cxx_string!(value = "");
let res = crate::ffi::GetValue2(&key, &mut value);

it wouldn't work, since you can't get the mutable reference from a Pin<T: !Unpin> reference (at least not without unsafe trickery I don't want to get into) and the CxxString is intentionally !Unpin. This could be fixed, though, then it would work as expected here (the fix would likely entail using a wrapper different from Pin, which would provide also DerefMut; but the soundness needs to be clarified - I didn't put much thought into it).

However, there already is explicit support for passing Rust String and &str to/from C++ as well as passing C++ std::string to/from Rust, so you might consider rewriting your C++ function to something like:

int32_t GetValue(rust::Str key, rust::String &value_out);

and then on the Rust side, you could call it via

let key: &str = "some_key";
let mut value_out = String::new();
let res = crate::ffi::GetValue(key, &mut value_out);

Disclaimer: I didn't try it.

Alternatively, if you don't care about the performance (additional allocations/copies), you can construct rust::String on the C++ side from an std::string and return it from a wrapper in C++ to Rust by value (and vice versa).

BTW, you should not pass the non-modifiable key as an std::string by value, since that's typically an unnecessary memory allocation and copy. On the C++ side, use std::string_view (if you use C++17 or newer) or at least const std::string& or similar as the key (or rust::Str, which is just a borrow).

@nyurik
Copy link
Author

nyurik commented Mar 3, 2023

@schreter thank you for such an amazingly detailed answer!!! I will go into more details in subsequent posts, but one thing to reply to right away -- sadly the C++ part of the code is not under my "full" control -- I can get C++ side to add more functions, but they must use standard C++ things, not Rust::Str. Which means everything I need must live in a separate wrapper lib (which can have any needed code)

@izolyomi
Copy link

izolyomi commented Mar 7, 2023

I'm also experimenting with exposing a Rust library to C++ and just found this interesting discussion.

I'd like to use something like a CxxString argument in an exposed Rust function so that the C++ side could look like native C++ just passing std::string (references) arguments instead of rust::String.

Unfortunately, the Rust side of the cxx bridge consequently reports that cxx::CxxString: unsupported type no matter how I try to pass it by pointer, reference, etc.

As far as I understand this is exactly what @nyurik is trying to do as well and I agree that it would really make sense. Is there a technical reason why using CxxString in Rust signatures must be explicitly unsupported or just a feature maybe unimplemented yet?

Thank you for the answers and your work on this awesome project.

@schreter
Copy link
Contributor

schreter commented Mar 7, 2023

Unfortunately, the Rust side of the cxx bridge consequently reports that cxx::CxxString: unsupported type no matter how I try to pass it by pointer, reference, etc.

@izolyomi Did you try to write the signature without cxx:: prefix, i.e., only name the type CxxString in the extern block?

(I didn't experiment with std::string so far, but the parser is expecting a simple Ident for the type name)

@izolyomi
Copy link

izolyomi commented Mar 8, 2023

@izolyomi Did you try to write the signature without cxx:: prefix, i.e., only name the type CxxString in the extern block?

(I didn't experiment with std::string so far, but the parser is expecting a simple Ident for the type name)

Thank you, you're right, it compiled without the cxx:: prefix, only a use cxx::CxxString; was needed to enable it. 👍

Maybe I have missed this detail mentioned reading the documentation, but the parser is not very intuitive with this behavior though. Would it need a significant amount of work to enable parsing these namespace qualifiers?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants