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

Macro defining a constant with unsigned long suffix generates unsigned integer

Open SuperHeron opened this issue 8 years ago • 10 comments

Input C/C++ Header

#define CK_INVALID_HANDLE	(0UL)

Bindgen Invocation

bindgen::Builder::default()
    .header("input.h")
    .generate()
    .expect("Unable to generate bindings");

Actual Results

pub const CK_INVALID_HANDLE: ::std::os::raw::c_uint = 0;

Expected Results

pub const CK_INVALID_HANDLE: ::std::os::raw::c_ulong = 0;

The U (unsigned) prefix is taken into account, but not the L (long) one. This causes types and casts problems.

Debugging logs: bindgen.txt

SuperHeron avatar Aug 20 '17 16:08 SuperHeron

Thanks for the report!

For macro definitions we use cexpr, and it only provides an EvalResult::Int, so we choose a type based on the value.

We have a way to customise it from bindgen though, which is using the ParseCallbacks::int_macro type.

Maybe that suits you, otherwise, would you consider filing an issue against https://github.com/jethrogb/rust-cexpr?

Thanks!

emilio avatar Aug 20 '17 22:08 emilio

would you consider filing an issue against https://github.com/jethrogb/rust-cexpr?

Looks like this never happened, but someone later filed a bug about this here: https://github.com/jethrogb/rust-cexpr/issues/16

davidben avatar Aug 08 '23 20:08 davidben

Though given this crate is also used to evaluate expressions, there's probably a deeper flaw here that bindgen does not correctly incorporate types when evaluating these.

(This then opens a whole different can of worms around how #if ... evaluates integer expressions slightly differently than the underlying language. But given that most constants are used in code, not preprocessor definitions, I think it's clear bindgen should be evaluating expressions as in the underlying language, taking types into account.)

davidben avatar Aug 08 '23 21:08 davidben

There's a more important issue here and it is the fact that rust-cexpr is virtually unmaintained. There's an ongoing effort into moving to cmacro which can handle suffixes correctly iirc: https://github.com/rust-lang/rust-bindgen/pull/2369

pvdrz avatar Aug 08 '23 21:08 pvdrz

Since ed5776206ccd3873e372d4960208fe6cec1235f7, bindgen generates Rust types, i.e. nowadays the "Actual Results" are:

pub const CK_INVALID_HANDLE: u32 = 0;

ojeda avatar Sep 28 '23 11:09 ojeda

All that did was switch the hardcoded types from c_uint to u32, right? I.e. bindgen is still failing to consume C APIs correctly. It's just doing it differently incorrectly.

davidben avatar Sep 28 '23 15:09 davidben

All that did was switch the hardcoded types from c_uint to u32, right? I.e. bindgen is still failing to consume C APIs correctly. It's just doing it differently incorrectly.

yes

pvdrz avatar Oct 02 '23 20:10 pvdrz

A workaround for this issue was to manually regex replace these values inside your build.rs:

let pattern = Regex::new(r" u32").expect("could not construct regex pattern (1)");
let content = pattern.replace_all(&content, " u64");

joelteply avatar Mar 19 '24 20:03 joelteply

This assumes that there are no other uses of u32 in there. A C header may quite reasonably have constants of different types.

Ultimately the problem here is that bindgen is not correctly consuming C headers. All the information is in there, in a perfectly machine-readable manner. bindgen just throws the information away.

davidben avatar Mar 19 '24 21:03 davidben