rust-bindgen icon indicating copy to clipboard operation
rust-bindgen copied to clipboard

Bindings for headers using rust-style integer aliases are awkward

Open LunarLambda opened this issue 2 years ago • 4 comments

Input C/C++ Header

#include <stdint.h>

typedef u16 uint16_t;

extern void meow(u16 a);

Bindgen Invocation

bindgen::builder()
        .header("bindgen.h")
        .generate()
        .unwrap()

Actual Results

/* snip */

pub type u16_ = u16;

extern "C" {
    pub fn meow(a: u16_);
}

Expected Results

There doesn't appear to be an easy way for this to simply become:

extern "C" {
    pub fn meow(a: u16);
}

As renaming via ParseCallbacks doesn't appear to work.

It'd be highly unfortunate to have to hack around it like this:

// Pretend we already included the header with the type aliases
#define _LIB_TYPES_H
#include <stdint.h>

// Do the alias in the preprocessor instead
#define u16 uint16_t

// Include the actual header
#include <lib.h>

LunarLambda avatar Aug 17 '22 18:08 LunarLambda

There's no guarantee that generally u16 is a typedef to uint16_t tho, right? I guess we could add some sort of special-casing but...

emilio avatar Aug 17 '22 19:08 emilio

If someone is interested in tackling this (I might :p):

We could fix this after https://github.com/rust-lang/rust-bindgen/pull/2254 is merged by adding a pass over the AST that looks for all the items like type alias = name; where name is a native rust type, deletes them and s/alias/name/ everywhere else.

Alternatively, this could be done during code generation by searching all the quote! invocations that introduce type aliases (like this one: https://github.com/rust-lang/rust-bindgen/blob/ebb1ce98c6e92ce2384a5867bdcfeb933ed12433/src/codegen/mod.rs#L807-L809) and add an additional check that doesn't add the item if the rhs is a native rust type. How to replace the alias by the actual type in the rest of the code is not clear for me yet as I'm not entirely familiar with this part of the code.

pvdrz avatar Aug 19 '22 15:08 pvdrz

adding a pass over the AST that looks for all the items like type alias = name; where name is a native rust type, deletes them and s/alias/name/ everywhere else.

sounds good to me, although that should probably be opt-in, I suppose many libraries have more meaningful type aliases than just making the names shorter

Although maybe those aliases would best be generated as newtypes? No idea.

LunarLambda avatar Aug 19 '22 15:08 LunarLambda

Yeah I'm not sure if this should be a hard coded --no-alias-for-native-types (name is a work in progress :sweat_smile:) or an --omit-alias-types=list kind of option.

pvdrz avatar Aug 19 '22 15:08 pvdrz

Multiple C projects, especially the really old, big ones which are going to be the kind that Rust wants to bind against, have customized their own <stddef.h>-style headers, sometimes because they are e.g. still expecting to be built by compilers that only implement C89. Or they have some other name, like s32, that they otherwise use exactly like i32.

I would like to be able to rewrite these to simply be i32, etc.: https://github.com/postgres/postgres/blob/908e17151b4834bd4bbfb703e206b68f5db341f9/src/include/c.h#L427-L431

While retaining these as semantic, obviously: https://github.com/postgres/postgres/blob/908e17151b4834bd4bbfb703e206b68f5db341f9/src/include/c.h#L445-L451

So I would like a more general mechanism under user control that allows me to assert that a given C typedef is in fact "pure syntax" and semantically is always some other type.

workingjubilee avatar Oct 03 '22 23:10 workingjubilee

maybe something like this would fit your case of use? The main idea is that instead of using c_int, c_long and what not, you ask bindgen to use the sized integer types directly and independently you can remove "obvious" aliases. So you'd move from:

type int32 = core::ffi::c_int;

extern "C" {
    fn foo() -> int32;
}

to

type int32 = i32;

extern "C" {
    fn foo() -> int32;
}

and, if int32 is tagged as an "obvious" alias then to:

extern "C" {
    fn foo() -> i32;
}

pvdrz avatar Oct 03 '22 23:10 pvdrz