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

Multiple FFI blocks referencing types from one another #99

Closed
adetaylor opened this issue Apr 10, 2020 · 6 comments · Fixed by #190
Closed

Multiple FFI blocks referencing types from one another #99

adetaylor opened this issue Apr 10, 2020 · 6 comments · Fixed by #190

Comments

@adetaylor
Copy link
Collaborator

I have two files requiring FFI, json.rs and basevalue.rs. I'd like to keep the FFI blocks local to those files, instead of having a central ffi.rs, but right now this is not possible.

My FFI block in basevalue.rs.

#[cxx::bridge(namespace = base)]
pub mod ffi {
    extern "C" {
        include!("base/values_rust.h");
        type RawOptionalBaseValue;
        fn RustOptionalBaseValueSetString(v: &mut RawOptionalBaseValue, value: &str);
        fn RustOptionalBaseValueSetBool(v: &mut RawOptionalBaseValue, val: bool);
        // etc.
    }
}

My FFI block in json.rs:

#[cxx::bridge(namespace = base)]
pub mod ffi {
    extern "Rust" {
        pub fn decode_json(
            bv: UniquePtr<RawOptionalBaseValue>,
            json: &str
        ) -> bool;
    }
}       

Note that the json.rs block references a type from the basevalue.rs block.

This gives

error[cxxbridge]: unsupported type

which is unsurprising, and I imagine is really hard to solve.

(The exact code above is significantly simplified so obviously don't try to build it; if this issue isn't as "known" as I expect, let me know and I'll make a minimal test case).

@dtolnay
Copy link
Owner

dtolnay commented Apr 10, 2020

I've encountered this in my work codebase as well. What we've done for now is declare RawOptionalBaseValue as an opaque C type in both files, and then do unsafe pointer casts from one to the other where necessary:

// :(
fn convert(bv: &mut first::ffi::BaseValue) -> &mut second::ffi::BaseValue {
    unsafe {
        &mut *(bv as *mut first::ffi::BaseValue as *mut second::ffi::BaseValue)
    }
}

Obviously I'd like to support this better. It needs some more design work but I have some ideas for how to make it work safely and seamlessly, based on treating opaque C types in a way that doesn't produce a distinguishable representation in different places when using the same header and type name:

// effectively this, but encoded without relying on const generics
type RawOptionalBaseValue = OpaqueC<"base/values_rust.h", "RawOptionalBaseValue">;

@dtolnay
Copy link
Owner

dtolnay commented Apr 10, 2020

The part that needs design work is how to deal with codebases in which the same import path might not always refer to the same exact file; in that case allowing the interconversion would be bad. I'm confident it's solvable but just needs some attention.

@adetaylor
Copy link
Collaborator Author

Thanks, your workaround worked nicely for me.

@dtolnay
Copy link
Owner

dtolnay commented Apr 17, 2020

Does anyone know how bindgen deals with the same issue? If we do two bindgen invocations in two different crates but some of the types are in common (something like folly::StringPiece which both crates might use) then do they also have the problem of those becoming two incompatible StringPiece types on the Rust side?

I guess they provide a workaround of --opaque-type folly::StringPiece which turns it into pub type folly_StringPiece = [u64; 2usize] which is the same type in every crate, but it's not great because it's also the same type as lots of other non-StringPiece types.

@dtolnay
Copy link
Owner

dtolnay commented Apr 28, 2020

I got a chance to think about this a bit today. I think what I'd like looks more or less like this:

// basevalue.rs

#[cxx::bridge(namespace = base)]
pub mod ffi {
    extern "C" {
        include!("base/values_rust.h");

        type RawOptionalBaseValue;

        fn /* ... */
    }
}
// json.rs

#[cxx::bridge(namespace = base)]
pub mod ffi {
    extern "C" {
        type RawOptionalBaseValue = basevalue::ffi::RawOptionalBaseValue;

        fn /* ... */
    }
}

where the first type expands to something like:

// same as today
#[repr(C)]
pub struct RawOptionalBaseValue {
    _private: ::cxx::private::Opaque,
}

// new
unsafe impl ::cxx::private::ExternType for RawOptionalBaseValue {
    type Id = /*type-encoding of "base::RawOptionalBaseValue"*/;
}

while the second type expands to something like:

// assume that they're the same
pub use basevalue::ffi::RawOptionalBaseValue as RawOptionalBaseValue;

// but also enforce that they're the same at compile time
const _: fn() = ::cxx::private::enforce_extern_type_id::<
    RawOptionalBaseValue,
    /*type-encoding of "base::RawOptionalBaseValue"*/,
>;

and in CXX we provide:

#[doc(hidden)]
pub fn enforce_extern_type_id<T: ExternType<Id = Id>, Id>() {}

The exact type-encoding there for the strings isn't important and I am not particularly worried about figuring out something that will work. Longer term we'll use a "non-type template parameter" a.k.a. "const generic" with the actual string literal, but for now you can conceptualize a possible encoding as:

  • "ABC" <-> ch::A<ch::B<ch::C>>

where each of those is a distinct struct A<Rest = ()>(PhantomData<Rest>). This wouldn't necessarily be how we do it for various reasons but it gives the same effect.

@dtolnay
Copy link
Owner

dtolnay commented Apr 29, 2020

In fact we'd likely end up making the trait ExternType public (but still unsafe) so that the user can write their own impls for any bindgen-generated types to let those interop seamlessly with cxx bridge extern functions.

unsafe impl cxx::ExternType for folly_sys::StringPiece {
    type Id = cxx::type_id!("folly::StringPiece");
}

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

Successfully merging a pull request may close this issue.

2 participants