dtolnay / cxx

Safe interop between Rust and C++
https://cxx.rs
Apache License 2.0
5.68k stars 322 forks source link

Implement VectorElement for more types #1297

Closed adamchalmers closed 6 months ago

adamchalmers commented 6 months ago

Background

I have an enum EntityType, which in Rust is

#[repr(u8)]
enum EntityType {
  Entity,
  Object,
  Path,
}

and in C++ is

enum class EntityType : uint8_t
{
    Entity,
    Object,
    Path
};

The Rust type implements cxx::ExternType appropriately bridging these two.

Problem

I cannot put this type into a CxxVector because EntityType does not implement cxx::VectorElement. I think I should be able to, though, because really it's just a u8 and that u8 does impl VectorElement.

Possible solution

Allow users to implement VectorElement if it's an enum/newtype around some T: VectorElement.

Workaround

Just use CxxVector<u8> and convert between u8 and EntityType where appropriate in either Rust or C++.

dtolnay commented 6 months ago

I think this is already supported. You need to use https://cxx.rs/extern-c++.html#explicit-shim-trait-impls.

impl CxxVector<EntityType> {}

in the same cxx::bridge that defines EntityType.

adamchalmers commented 6 months ago

Thank you, I'll open a PR shortly to add that suggestion to the docs. But in this particular case it isn't working for me. So, what I actually have is closer to this:

pub mod cpp {

    #[namespace = "Enums"]
    unsafe extern "C++" {
        include!("enums.h");

        type _EntityType = kittycad_modeling_cmds::shared::EntityType;
    }

}

If I add impl CxxVector<_EntityType> {} I get

error[E0117]: only traits defined in the current crate can be implemented for types defined outside of the crate
  --> bridge/src/engine.rs:71:5
   |
71 |     impl CxxVector<_EntityType> {}
   |     ^^^^^^^^^^^^^^^-----------
   |     |              |
   |     |              `EntityType` is not defined in the current crate
   |     impl doesn't use only types from inside the current crate
   |
   = note: define and implement a trait or new type instead

For more information about this error, try `rustc --explain E0117`.

Otherwise, if I try impl CxxVector<kittycad_modeling_cmds::shared::EntityType> {} I get


  error[cxxbridge]: unsupported type
     ┌─ src/engine.rs:71:20
     │
  71 │     impl CxxVector<kittycad_modeling_cmds::shared::EntityType> {}
     │                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unsupported type
dtolnay commented 6 months ago

It needs to be in the same crate that defines kittycad_modeling_cmds::shared::EntityType, i.e. in kittycad_modeling_cmds. Just like a Clone impl or Debug impl or ExternType impl would also need to be in that crate.

dtolnay commented 6 months ago
#[cxx::bridge]
mod ffi {
    unsafe extern "C++" {
        include!("enums.h");

        type _EntityType = crate::shared::EntityType;
    }

    impl CxxVector<_EntityType> {}
}
adamchalmers commented 6 months ago

OK, I see, thanks. In this particular case it won't be possible, as the kittycad_modeling_cmds crate is open-source but our C++ code is closed-source (and the bridge is defined in a separate, closed-source crate).

The workarounds will be enough in this case though, thanks. If I decide the workarounds aren't enough, I'll make a second enum in the closed-source crate with exactly matching variants to the closed-source one.

I'll open a PR to clarify the VectorElement docs.