long double becomes u128

[aDotInTheVoid]

Input C/C++ Header

typedef long double foo

Bindgen Invocation

$ bindgen input.h 

Actual Results

/* automatically generated by rust-bindgen */

pub type a = u128;

Expected Results

I'm not entirely sure. Here are the idea's I can think of

• use f64, but that leeds to it's own set of issues (Incorrect size for long double when using old rust_target #1529).
• use the f128 crate but the layout of the f128 is different to the layout of a c long double
• add a c_longdouble type to std::os::raw. The problem is that on many machines (including mine) long double is a 16 byte float, which rust doesn't suport
• Through an error. This will avoid confusion, but also make also will mean losing suport for many .h files
• use u128. The problem here is were treating a float as an int, and that will mean all sorts of problems

[emilio]

I find all the first ones are strictly worse than the current behavior. With the current behavior at least you don't have ABI issues, and you can pass it by value transparently to C / C++.

Unfortunately erroring out is a no-go since it's used in a lot of standard headers to implement max_align_t.

[aDotInTheVoid]

Maybe the best option would be to still use u128, but then give a warning that passing u128 values to c funcs that take long double won't behave normally.

It might also make sense to provide a way of converting f64/f32/fsize to u128 and vice verse

[emilio]

Well, it will behave normally, right? As in, a long double value that you got from C should be fine to be passed to C again. It's only constructing rust values what's impossible / hard.

[parasyte]

rustc also emits warnings about this:

warning: `extern` block uses type `u128` which is not FFI-safe: 128-bit integers don't currently have a known stable ABI

You're right, it should behave correctly, at least where long double is 16-bytes, barring any strange alignment issues. But expecting Rust to treat these types as a black box is unrealistic. :) Constructing isn't the only problem, but also reading and writing the values from Rust.

[Lokathor]

I also hit this just now, using SDL2. Throwing an error is obviously bad and not acceptable.

Could we, for the time, emit some sort of dummy struct value with a meaningful name that's the correct size and alignment, and then has the bits as an inner field. This would at least make it very obvious that you should be treating it as a black box if you get it from C, though perhaps an expert could throw a lib at it or something if they really needed f128 support.

If a type is added to std::os::raw be sure to also add it to libc as well so that no_std can use it.

[emilio]

Could we, for the time, emit some sort of dummy struct value with a meaningful name that's the correct size and alignment, and then has the bits as an inner field. This would at least make it very obvious that you should be treating it as a black box if you get it from C, though perhaps an expert could throw a lib at it or something if they really needed f128 support.

Probably not, since in MSVC struct vs. POD type matters ABI-wise, and I think it'd break.

[Lokathor]

you'd want to use repr(transparent)

#[repr(transparent)]
pub struct IAmF128PleaseDontTouch {
  raw: u128
}

EDIT: well that gives the right size, but I do not know about the right alignment

[emilio]

Yeah, #[repr(transparent)] should be fine, though bindgen supports not generating repr(transparent). But as long as we fallback to the current behavior if there's no #[repr(transparent)] support it should be ok, yeah.

[smarnach]

In gcc and clang on x86_64, long double has an alignment of 16, whereas u128 has an alignment of 8. This means structs containing a long double usually end up with broken layouts. So far I haven't found a good work-around for this problem.

[elichai]

What about something like this:

#[repr(C, align(16))]
struct f128 {
    a: [u8; 16]
}

[emilio]

#[repr(C)] is out of the game because structs are not abi-compatible with ints, I think. Though I guess it can be technically better than the current solution in practice, as the current thing is also not ABI-compatible.

Not sure if rust would like #[repr(transparent, align(16))] struct f128(u128) or such...

[elichai]

You can't do align+transparent

[ldm0]

Related things about Rustc removing f128:

1. https://github.com/rust-lang/meeting-minutes/blob/master/weekly-meetings/2014-06-24.md#removing-f128
2. Remove the quad_precision_float feature gate rust#15160
3. i128 / u128 are not compatible with C's definition. rust#54341

f128 isn't supported across all LLVM backends in sane ways. There are ABI issues with it.

It seems that using f128 with FFI is prone to causing error, and also it's usually brought to our view by math.h and people usually not using things related to it. So I think filtering out all functions & types related to long double is acceptable.

[ldm0]

cc @emilio
Do we have any solution on filtering out things related to a primitive type?

[benbrittain]

Would bindgen be amicable to a --no-u128 or similar flag? Maybe a more generic primitive type filtering function? The u128 not being a valid FFI type is a pretty concerning warning to spew in certain circumstances.

[whqee]

These functions are obsolete. It's no use in Rust code. Bindgen with
.blocklist_function("qgcvt")
.blocklist_function("qgcvt_r")
.blocklist_function("qfcvt")
.blocklist_function("qfcvt_r")
.blocklist_function("qecvt")
.blocklist_function("qecvt_r")
.blocklist_function("strtold")

[JMS55]

https://blog.rust-lang.org/2024/03/30/i128-layout-update.html