Generate sensible code for long double / stuff that has more than 64-bit alignment. #550
Comments
tov
changed the title
long double
|
Yeah, this is known. We don't have any way of generating larger floats in rust though, so I don't know what could we do here :(. The test seems to be doing its job.
The padding in #468 was there, though incorrectly (and the code that generated the padding caused a few other bugs). I cleaned that up in #512. We could generate |
|
I suppose the test is doing its job, yes :) But because Clang’s I think that |
|
Note that you should be able to just do |
|
Oh, that should work. Thanks! |
|
To clarify, @emilio is referring to |
emilio
changed the title
See rust-lang/rust-bindgen#550 for details.
See rust-lang/rust-bindgen#550 for details.
|
This currently prevents anything that does |
|
@lukaslueg see the workarounds above: #550 (comment) and #550 (comment) Rust has no way to set these alignments, so our hands are tied for the moment. |
Until rust-lang/rust-bindgen#550 is resolved, this is necessary for the generated tests to pass.
As part of doing this, we blacklist the max_align_t type. This was causing a generated test failure. See: rust-lang/rust-bindgen#550
This works around a bug rust-lang/rust-bindgen#550
Enable Cargo features for quickchecking crate Logic to enable/disable special casing (due to known issues #550, #684, and #1153) has been exposed as features in the `quickchecking` crate's Cargo.toml file and corresponding `cfg` attributes in the source. In addition to adding Cargo features, this PR represents the following: - Documentation in `bindgen`'s CONTRIBUTING.md that points to a new README.md located in the `quickchecking` crate's directory. - The Debug trait was implemented for the `HeaderC` type. This enables failing property tests to be reported as C source code rather than a Rust data structure. - The ArrayDimensionC type is now used in header generation for union, struct, and basic declarations. Thanks for taking a look and for any feedback! Closes #1169 r? @fitzgen
As suggested in rust-lang/rust-bindgen#550
|
|
A simple workaround for rust-lang/rust-bindgen#550 that turned on a test error.
* Update to the current bindgen
* workaround broken max_align_t codegen
See rust-lang/rust-bindgen#550
A simple workaround for rust-lang/rust-bindgen#550 that turned on a test error.
|
Just got this error myself, here's a case for you: #[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct max_align_t {
pub __clang_max_align_nonce1: ::std::os::raw::c_longlong,
pub __bindgen_padding_0: u64,
pub __clang_max_align_nonce2: f64,
}
#[test]
fn bindgen_test_layout_max_align_t() {
assert_eq!(
::std::mem::size_of::<max_align_t>(),
32usize,
concat!("Size of: ", stringify!(max_align_t))
);
assert_eq!(
unsafe {
&(*(::std::ptr::null::<max_align_t>())).__clang_max_align_nonce1 as *const _ as usize
},
0usize,
concat!(
"Offset of field: ",
stringify!(max_align_t),
"::",
stringify!(__clang_max_align_nonce1)
)
);
assert_eq!(
unsafe {
&(*(::std::ptr::null::<max_align_t>())).__clang_max_align_nonce2 as *const _ as usize
},
16usize,
concat!(
"Offset of field: ",
stringify!(max_align_t),
"::",
stringify!(__clang_max_align_nonce2)
)
);
}I've temporarily resolved blacklisting |
|
@emilio This issue still isn't fixed as of bindgen 0.42.2. Those patches only deal with 128 bit ints, but not 80 bit floats. In fact, output is completely unchanged from the original post. Specifically, the struct: typedef struct {
long double ld;
} max_align;Is converted to: #[repr(C)]
#[derive(Debug, Copy)]
pub struct max_align {
pub ld: f64,
}When it instead should be something like: #[repr(C, align(16))]
#[derive(Debug, Copy)]
pub struct f80([u8; 10]);
#[repr(C)]
#[derive(Debug, Copy)]
pub struct max_align {
pub ld: f80,
} |
|
In master: ( Note the |
|
(If for some reason that output can be improved or what not, let's continue that discussion in another issue) |
|
@emilio maybe rust-bindgen should start warning / erroring on long double if |
|
Yeah, I agree. FWIW tests should fail when that happens, so this already kind-of happens. But should be trivial to add a warning to the place where we generate the |
ghost
mentioned this issue
|
Same problem with |
Seems very special-casey (if it's just about Given it produces test failures and that default rust target is newer now iirc, I don't personally think it's worth it, but patch welcome of course :) |
See rust-lang/rust-bindgen#550 for details.
See rust-lang/rust-bindgen#550 for details.
See rust-lang/rust-bindgen#550 for details.
See rust-lang/rust-bindgen#550 for details.
Input C/C++ Header
Bindgen Invokation
This is with both bindgen 0.22 and HEAD, on both of these:
Actual Results
The generated test is that
sizeof(max_align) == 16. This may be true for the original C struct, but it's not true for the generated Rust struct.Expected Results
I’m not sure how the struct should be translated, since Rust doesn’t have
f80or whatever, but even without a sane translation, the generated test should pass, right?Does this have something to do with #468? That shows the same code (from
<stddef.h>) that led me to discover this problem. The translation that it shows includes padding that I'm not seeing.RUST_LOG=bindgenOutputThe text was updated successfully, but these errors were encountered: