Using an Option<> with a function/nonnull type alias generates incorrect bindings

[burtonageo]

With the following code:

use libc::int32_t;
type DoFn = unsafe extern "C" fn (x: int32_t, y: int32_t) -> int32_t;
type NonNullAlias<T> = NonNull<T>;

#[repr(C)]
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
struct Holder {
    func: Option<DoFn>,
}

#[repr(C)]
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
struct Data {
    data: Option<NonNullAlias<int32_t>>,
}

bindgen will generate the following incorrect header:

#include <stdarg.h>
// ..
struct Option_DoFn;

struct Holder {
    Option_DoFn func;
};

struct Data {
    Option_NonNullAlias_int32_t data;
};

instead of the expected

#include <stdarg.h>
// ..
typedef int32_t(*DoFn)(int32_t x, int32_t y);

struct Holder {
    DoFn func;
};

struct Data {
    int32_t* data;
};

[emilio]

So there are two issues here:

• Type::simplified_type is kind of naive, and doesn't handle well generic aliases.
• Type::is_repr_ptr doesn't deal very well with aliases either.

This is due to the same reason, which is that those methods don't really have the type information of what the path refers to. This is all conceptually fixable, but I haven't dug into how easy / worth it is it.

[Shnatsel]

FWIW this is blocking miniz_oxide from exposing proper C bindings.

[ultrasaurus]

This seems pretty serious. It makes C bindgen pretty much useless for expressing callbacks, which is a very common part of C libraries. For other functions and types, C bindgen is amazing!

I looked into working around this by excluding certain types / functions from C bindgen analysis, but I couldn't figure out how to do that effectively. I also thought that perhaps I could just include some text that I produced manually, but I didn't see how to do that either.

If there is a workaround that doesn't involve manually editing my header, I can't figure it out. (I made a sample project to experiment -- see

• rust code with externs
• cbingen config
• generated header which requires manual editing (but it's close and better than doing everything manually!)

[emilio]

This probably needs a bit of work on cbindgen's side to fix, but it should not be too terrible, at the very least for easy cases...

What probably needs to be done is to make simplify_standard_types a way to resolve a path, and then make is_repr_ptr resolve that and call itself recursively.

That being said:

I looked into working around this by excluding certain types / functions from C bindgen analysis, but I couldn't figure out how to do that effectively. I also thought that perhaps I could just include some text that I produced manually, but I didn't see how to do that either.

Are the header and trailer options not useful for that?

If there is a workaround that doesn't involve manually editing my header, I can't figure it out. (I made a sample project to experiment -- see

The easiest one (though I agree is not amazing) is just defining the "optional callback" as a separate typedef, so: pub type StatusCallbackFunction = Option<extern "C" fn(code: i32, user_data: * mut c_void) -> ()>;

Of course that's not terribly amazing. But it seems better than dealing with manually-generated stuff.

[ultrasaurus]

I believe that a workaround using hand-coded text for some of the API would require #303 (header is included before include_guard and sys_includes

defining the "optional callback" as a separate typedef makes it so the function signature is not available in the headerfile, I tried defining an extra type like this:

pub struct ExtThingStatus {
  callback: status_callback_t,
  userdata: * mut c_void,
}

impl StatusCallback for ExtThingStatus {
  fn call(&self, status: ThingStatus) -> () {
    (self.callback)(status.code, self.userdata);
  }
}
pub type status_callback_t = extern "C" fn(code: i32, user_data: * mut c_void) -> ();

pub type StatusCallbackFunction = extern "C" fn(code: i32, user_data: * mut c_void) -> ();

#[no_mangle]
pub extern "C" fn create_thing(num: i32, callback_or_null: Option<StatusCallbackFunction>, userdata: * mut c_void) -> * mut Thing<ExtThingStatus> {

then renaming in cbindgen.toml config:

[export]
include = ["status_callback_t"]
exclude = ["StatusCallbackFunction"]
prefix = "my_"

[export.rename]
"Thing_ExtThingStatus" = "thing_t"
"Option_StatusCallbackFunction" = "status_callback_t"

but then I get the extra typedef that requires manual removal (probably the order of rename operations causes Option_StatusCallbackFunction it to be defined as status_callback_t (after the includeof status_callback_t which was my attempted workaround.

[ultrasaurus]

The workaround I found was to just not use Option in the function interface and check for null

#[no_mangle]
pub extern "C" fn create_thing(num: i32, callback_or_null: StatusCallbackFunction, userdata: * mut c_void) -> * mut Thing<ExtThingStatus> {
  let mut callback_info = None;
  if !(callback_or_null as *mut c_void).is_null() {
    let callback = callback_or_null;
    callback_info = Some(ExtThingStatus {
      callback,
      userdata
    });
  }

  Box::into_raw(Box::new(Thing::new(num, callback_info)))
}

here's the change to my example: ultrasaurus/rust-clib@f93e819

I don't love this solution, but I like it better than having a manual step in my build process :)

[emilio]

That is UB, and probably the null check will be optimized away in release builds.

[emilio]

defining the "optional callback" as a separate typedef makes it so the function signature is not available in the headerfile, I tried defining an extra type like this:

That's not quite what I meant. This seems to do the trick, or am I missing something? The Option<> needs to be in the typedef.

pub type StatusCallback = Option<extern "C" fn(i32) -> ()>;

#[no_mangle]
pub extern "C" fn root(ptr: StatusCallback) {}

[ultrasaurus]

That totally worked! awesome :)

Btw: not sure what you mean by "That is UB" -- undefined behavior? what part of my workaround were you referring to?

[emilio]

From the point of view of the compiler, !(callback_or_null as *mut c_void).is_null() will always be true. Passing NULL to that function from C is undefined behavior.