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

Avoid assigning (wrong) types to constants using #define

Open JanBeh opened this issue 4 years ago • 4 comments

Input C/C++ Header

#define MYCONST 2

Bindgen Invocation

$ bindgen input.h

Actual Results

/* automatically generated by rust-bindgen 0.59.1 */

pub const MYCONST: u32 = 2;

Expected Results

It would be better to generate something like

macro_rules! MYCONST { () => { 2 }; }

and to expect users to use MYCONST!() instead of MYCONST.

Reasoning

Constants using #define in a C header file do not have a type. They are inserted by the preprocessor as a literal where needed. Translating them to a Rust const with a type (u32, i32, or others) may lead to unexpected results. For example, i32 can be interchaged with std::os::raw::c_int on most platforms. But code that relies on that will not be portable. See discussion on Rust Users Forum "Interfacing C code with bindgen: #define and types".

Migration

Maybe it can be made possible to enable this new described behavior with an option that is not enabled by default; or the generated macro_rules!s could be created in addition to the consts (unless being switched off by an option).

JanBeh avatar Nov 20 '21 01:11 JanBeh

Related issues:

  • https://github.com/rust-lang/rust-bindgen/issues/1594
  • https://github.com/rust-lang/rust-bindgen/issues/316

JanBeh avatar Nov 20 '21 02:11 JanBeh

Yeah this is sorta expected. You can set the type via ParseCallbacks::int_macro. The main problem with doing the above is that integer promotion rules in C and Rust are quite different, so it might not behave so intuitively as you might think...

emilio avatar Nov 26 '21 01:11 emilio

There exists an easy workaround when comparing (small) constants such as 1, 2, 3, or -1, -2 with a c_int (or any other C type returned by a function, which might be different from c_int). You can write:

if some_ffi_module::some_func() == some_ffi_module::SOME_CONST as _ {
    /* … */
}

This cast should be okay if some_func's return type is big enough to contain the expected constant return value (which seems to be a reasonable assumption in most cases). But using as _ is at least confusing to whoever reads the code, and you won't get a warning if the constant doesn't fit the type returned by the function. Writing .try_into().unwrap() seems safer, but it is more verbose and would could cause runtime errors (where it should be a compile-time error if the constant really doesn't fit).

When matching constants, things get even more ugly, because it's impossible to use as _ or try_into().unwrap() directly in a pattern (it requires a match guard). See this post for some (ugly) examples.

I would thus say that assigning particular types to constants using #define should be avoided, and Rust macros should be used instead (at least as an option that can be enabled).

JanBeh avatar Nov 30 '21 11:11 JanBeh

Constants using #define in a C header file do not have a type. They are inserted by the preprocessor as a literal where needed.

It is true they are just macros, but integer constants do have a type in C.

I'm asking because in C, I can pass MYCONST to both functions expecting an u8 or an int. It's not specified/defined by the #define which type the 1 actually has.

In C that works because the argument gets converted, not because the integer constant (i.e. after replacement) did not have a type, i.e. the 1 has type int.

Now, yes, in Rust there is type inference for integer literals without a suffix, which is what you want to use here to get some convenience back. But note that you are forced to pick a type and cast when using const instead. It seems like you would like to have https://github.com/rust-lang/rfcs/issues/1349.

ojeda avatar Sep 28 '23 12:09 ojeda