dtolnay / cxx

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

Question about defining types that are simple structs on the C++ side #1270

Closed scott-wilson closed 10 months ago

scott-wilson commented 10 months ago

Hey all, I have a library that I'm wrapping that contains a struct that looks like this:

OpenImageIO/imageio.h

struct ROI {
    int xbegin, xend;
    int ybegin, yend;
    int zbegin, zend;
    int chbegin, chend;

    // Functions defined here...
};

I want to be able to treat this as a shared type, so I have some code that looks like this:

ffi_imageio.h

#pragma once

#include <OpenImageIO/imageio.h>
#include <memory>
#include <vector>
#include <string>
#include <rust/cxx.h>

namespace oiio
{
    using ROI = OIIO::ROI;

    // ROI
    ROI roi_default() noexcept;

} // namespace oiio

ffi_imageio.cpp

#include <OpenImageIO/imageio.h>
#include <OpenImageIO/string_view.h>
#include <memory>
#include <stdexcept>
#include <stdio.h>
#include "oiio-sys/include/ffi_imageio.h"

namespace oiio
{
    // ROI
    ROI roi_default() noexcept { return ROI(); }
} // namespace oiio

imageio.rs

pub use ffi::*;

#[cxx::bridge(namespace = oiio)]
mod ffi {
    // ROI
    pub struct ROI {
        pub xbegin: i32,
        pub xend: i32,
        pub ybegin: i32,
        pub yend: i32,
        pub zbegin: i32,
        pub zend: i32,
        pub chbegin: i32,
        pub chend: i32,
    }

    unsafe extern "C++" {
        include!("oiio-sys/include/ffi_imageio.h");

        // ROI
        pub fn roi_default() -> ROI;
    }
}

Yet, I'm getting the following compile error:

warning: cxxbridge/sources/oiio-sys/src/imageio.rs.cc: At global scope:
warning: cxxbridge/sources/oiio-sys/src/imageio.rs.cc:205:10: error: using typedef-name ‘using ROI = struct OpenImageIO_v2_6_0::ROI’ after ‘struct’
warning:   205 |   struct ROI;
warning:       |          ^~~
warning: cxxbridge/crate/oiio-sys/include/ffi_imageio.h:11:11: note: ‘using ROI = struct OpenImageIO_v2_6_0::ROI’ has a previous declaration here
warning:    11 |     using ROI = OIIO::ROI;
warning:       |           ^~~
warning: cxxbridge/sources/oiio-sys/src/imageio.rs.cc:213:12: error: using typedef-name ‘using ROI = struct OpenImageIO_v2_6_0::ROI’ after ‘struct’
warning:   213 | struct ROI final {
warning:       |            ^~~~~
warning: cxxbridge/crate/oiio-sys/include/ffi_imageio.h:11:11: note: ‘using ROI = struct OpenImageIO_v2_6_0::ROI’ has a previous declaration here
warning:    11 |     using ROI = OIIO::ROI;
warning:       |           ^~~

What am I doing wrong in this case? I thought that the ROI should be defined on the Rust side, and found on the C++ side. Or, is there a way to say "Hey, there's a struct in Rust that looks like this, and there's an exact copy of this on the C++ side. Use that one."?

silvanshade commented 10 months ago

When you are using "shared structs", the idea is that the cxx-bridge is the "source of truth" that provides the definition used in both the C++ and the Rust code.

The way this works in your example is that cxx generates a struct definition for ROI in the imageio.rs.cc file, and then the macro expansion on the Rust side also generates the Rust struct analogous to this.

The reason you are getting those errors is because now you indeed have two conflicting definitions of ROI: the one that cxx is generating, and the one you are including from <OpenImageIO/imageio.h> which you bind with using ROI = OIIO::ROI.

It may be the case that these definitions are structurally equivalent but -- as far as the C++ compiler is concerned -- they are two distinct type definitions (at different locations) using the same name, so it complains.

What you actually want to do in your example (since ROI is already defined by the C++ side) is refactor it in the following way:

imageio.rs

// ROI
#[repr(C)]
pub struct ROI {
    pub xbegin: i32,
    pub xend: i32,
    pub ybegin: i32,
    pub yend: i32,
    pub zbegin: i32,
    pub zend: i32,
    pub chbegin: i32,
    pub chend: i32,
}

// This is necessary (particularly the `Kind = cxx::kind::Trivial`) if you are going to treat `ROI` as a trivial
// type, which allows you to be able to return the type by-value (for `roi_default`) without wrapping
// in a pointer type. (NOTE: you shouldn't actually do this though if it's not trivial)
unsafe impl cxx::ExternType for ROI {
    type Id = cxx::type_id!("oiio::ROI");
    type Kind = cxx::kind::Trivial;
}

#[cxx::bridge(namespace = oiio)]
mod ffi {
    unsafe extern "C++" {
        include!("oiio-sys/include/ffi_imageio.h");

        // refer to the already defined Rust `ROI` type, which can also be placed in completely separate module
        type ROI = crate::imageio::ROI;

        // ROI
        pub fn roi_default() -> ROI;
    }
}
scott-wilson commented 10 months ago

That seems to do the trick. Thanks!