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

long double varargs won't compile #154

Open
TheDan64 opened this issue Sep 4, 2019 · 7 comments
Open

long double varargs won't compile #154

TheDan64 opened this issue Sep 4, 2019 · 7 comments
Labels
blocked Cannot be supported without external support first bug Something isn't working

Comments

@TheDan64
Copy link
Contributor

TheDan64 commented Sep 4, 2019

error[E0277]: the trait bound `f128::f128: core::ffi::sealed_trait::VaArgSafe` is not satisfied
   --> src/vfwprintf.rs:789:44
    |
789 |                                         ap.arg::<f128::f128>().to_f64().unwrap()
    |                                            ^^^ the trait `core::ffi::sealed_trait::VaArgSafe` is not implemented for `f128::f128`

error: aborting due to 2 previous errors

Blocked on improved rustc support for arbitrary structs/types, possibly rust-lang/rust#61126

@TheDan64 TheDan64 added bug Something isn't working blocked Cannot be supported without external support first labels Sep 4, 2019
@ahomescu
Copy link
Contributor

ahomescu commented Sep 12, 2019

f128 is implemented in its crate as

#[repr(C)]
#[derive(Clone, Copy)]
pub struct f128(pub(crate) [u8; 16]);

At first I thought the fix was as simple as enabling VaArgSafe for aggregates, but looking at the rustc backend I now realize that va_arg is only implemented for scalars, so the compiler would crash with an ICE for any aggregate. To fix f128, we'd need to implement full aggregate support for va_arg in rustc.

Edit: the problem is that the current variadics implementation in rustc lowers Rust va_arg down to LLVM va_arg, which doesn't support aggregates. clang gets around this by manually implementing va_arg itself using memory operations.

@kkysen
Copy link
Contributor

kkysen commented Nov 28, 2022

f128 is implemented in its crate as

#[repr(C)]
#[derive(Clone, Copy)]
pub struct f128(pub(crate) [u8; 16]);

Why wouldn't f128 be implemented using u128? Doesn't [u8; 16] have the wrong alignment? bindgen uses u128 for long double (if u128 is supported). If u128 is used as the underlying type, does that fix this issue, as u128 is a scalar?

Also, rust-lang/rust#61126 seems to be about unsized types, but f128 and [u8; 16] aren't unsized.

rust-lang/rfcs#2580 is also now merged, if that was a blocker for rust-lang/rust#61126.

@LegNeato, this may be a reason to not use f128 (#745).

@ahomescu
Copy link
Contributor

bindgen uses u128 for long double

From rust-lang/rust-bindgen#1549 it sounds like they're not entirely happy with that either.

Why wouldn't f128 be implemented using u128?

IMHO the real problem is that variadics are part of core, but there is no "official" f128 equivalent in Rust. The language and the standard library would first have to pick one, and then add variadics support for it. The f128 type that C2Rust uses (or used, not sure we do anymore) is from an external crate.

You could define f128 as a newtype over u128, but I'm not sure that would get accepted into core.

@kkysen
Copy link
Contributor

kkysen commented Nov 28, 2022

Wow, I didn't realize {u,i}128 was not FFI-safe and that LLVM misaligns it to 8 bytes, not 16 (so it would have the wrong alignment for f128, but then so do {u,i}128).

I think the best way to support this would be to get a c_long_double added in libc (hopefully that happens one day), as what we want to do is match the C long double anyways, and long double is not always f128 (__float128 would be different, and would always be f128). But I'm not sure what libc's definition for c_long_double would be.

I also saw that libm is missing the *l functions that operate on long doubles.

@LegNeato
Copy link
Contributor

LegNeato commented Oct 5, 2023

@LegNeato
Copy link
Contributor

LegNeato commented Nov 2, 2023

rust-lang/rust#116909

@kkysen
Copy link
Contributor

kkysen commented Nov 2, 2023

rust-lang/rust#116909

The RFC for f{16,128} got accepted? Awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Cannot be supported without external support first bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants