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

long double becomes u128

Open aDotInTheVoid opened this issue 6 years ago • 17 comments
trafficstars

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 (#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

aDotInTheVoid avatar Apr 10 '19 12:04 aDotInTheVoid

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.

emilio avatar Apr 10 '19 17:04 emilio

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

aDotInTheVoid avatar Apr 10 '19 22:04 aDotInTheVoid

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.

emilio avatar Apr 10 '19 23:04 emilio

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.

parasyte avatar Apr 11 '19 00:04 parasyte

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.

Lokathor avatar Apr 11 '19 03:04 Lokathor

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.

emilio avatar Apr 11 '19 10:04 emilio

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

Lokathor avatar Apr 11 '19 14:04 Lokathor

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.

emilio avatar Apr 12 '19 10:04 emilio

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.

smarnach avatar Oct 01 '19 19:10 smarnach

What about something like this:

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

elichai avatar Jan 26 '20 14:01 elichai

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

emilio avatar Jan 26 '20 17:01 emilio

You can't do align+transparent

elichai avatar Jan 26 '20 18:01 elichai

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. https://github.com/rust-lang/rust/pull/15160
  3. https://github.com/rust-lang/rust/issues/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 avatar Jun 10 '20 22:06 ldm0

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

ldm0 avatar Jun 10 '20 22:06 ldm0

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.

benbrittain avatar Jan 11 '22 17:01 benbrittain

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")

whqee avatar Jan 07 '24 13:01 whqee

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

JMS55 avatar Mar 30 '24 20:03 JMS55