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

Cannot instantiate C++ std::vec of a simple C++ type due to lack of cxx VectorElement trait implementation. #1360

Open
johnoneil opened this issue Jan 22, 2024 · 3 comments

Comments

@johnoneil
Copy link

Description

Returning a C++ Vector back to rust only seems to work for basic types (int, float) as they have the trivial VectorElement trait defined.

For more complex types, the VectorElement trait is not defined, despite the fact that (in my reading) cxx describes it as trivial. See here..

We can't define an implementation for VectorElement in external code, as I believe it has to be defined in generated bindings.

I guess this boils down to emitting impl CxxVector<MyType> {} in generated code for bound types. I don't know if this would be trivial or not.

Reproduce

We have a C++ class called TextRun which is exposed to Rust via autocxx:

generate!("TextRun")

I'd like to return a C++ vector of this type to Rust (maybe to manipulate via other bound C++ functions), so we have a helper like what follows:

inline std::vector<TextRun> MakeTextRunList() {
  return {};
}

We also provide a binding to this simple function:

generate!("MakeTextRunList")

The above approach works for basic types, but for this example I get the error:

error[E0277]: the trait bound `root::impeller::TextRun: VectorElement` is not satisfied
  --> /home/johnoneil/code/display-safety/target/debug/build/impeller-rs-ca3ef0ac788db410/out/autocxx-build-dir/rs/autocxx-ffi-default-gen.rs:1:249233
   |
1  | ...94ac7 () -> UniquePtr < CxxVector < TextRun > > ; # [doc = "Synthesized move constructor."] pub unsafe fn impeller_Radians_new_synthet...
   |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `VectorElement` is not implemented for `root::impeller::TextRun`
   |
   = help: the following other types implement trait `VectorElement`:
             isize
             i8
             i16
             i32
             i64
             usize
             u8
             u16
           and 38 others
   = note: required for `CxxVector<root::impeller::TextRun>` to implement `UniquePtrTarget`
note: required by a bound in `UniquePtr`
  --> /home/johnoneil/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cxx-1.0.107/src/unique_ptr.rs:17:8
   |
15 | pub struct UniquePtr<T>
   |            --------- required by a bound in this struct
16 | where
17 |     T: UniquePtrTarget,
   |        ^^^^^^^^^^^^^^^ required by this bound in `UniquePtr`

Expected Behavior

For simple types i would believe the test code above should build and be useful out of the box. Not sure what the limiting factors would be, but I think for at least simple C++ structs, std::vector should be easily supported.

Additional Context

I can try to provide a test case for this if necessary.

I believe emitting the correct trait might be relatively simple, but I haven't yet delved into autocxx code, so I'm unsure if I could deliver a fix as yet. It might take me some time.

@BabaBert
Copy link

It depends on what type of class TextRun is. It is possible that autocxx is unable to generate this for virtual or templated classes. I don't know the exact restrictions, but you might want to write a simple wrapper for TextRun instead.

@johnoneil
Copy link
Author

Thanks for your advice @BabaBert . I'll try that approach when possible.

If I have success I'll update this bug and close.

@BabaBert
Copy link

Hey @johnoneil, my initial response wasn't quite correct. Your C++ type needs ta Move Constructor. Otherwise, autocxxcan't safely implement VectorElement for you as described here: #1170.
If you can't do that, then a simple wrapper may suffice.
If you ignore this, you will most likely get a segfault down the line.

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

2 participants