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

Bindings for headers using rust-style integer aliases are awkward #2256

Open
LunarLambda opened this issue Aug 17, 2022 · 6 comments
Open

Bindings for headers using rust-style integer aliases are awkward #2256

LunarLambda opened this issue Aug 17, 2022 · 6 comments

Comments

@LunarLambda
Copy link

LunarLambda commented Aug 17, 2022

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>
@emilio
Copy link
Contributor

emilio commented Aug 17, 2022

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...

@pvdrz
Copy link
Contributor

pvdrz commented Aug 19, 2022

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

We could fix this after #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:

tokens.append_all(quote! {
pub type #rust_name = #inner_rust_type ;
});
) 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.

@LunarLambda
Copy link
Author

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.

@pvdrz
Copy link
Contributor

pvdrz commented Aug 19, 2022

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

@workingjubilee
Copy link
Member

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.

@pvdrz
Copy link
Contributor

pvdrz commented Oct 3, 2022

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;
}

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.

4 participants