Ensure all derive analyses check array limit on bitfields

[aeleos]

Fixes #982

r? @fitzgen

[fitzgen]

Looks like CI is complaining because of compilation errors:

error[E0425]: cannot find value `RUST_DERIVE_IN_ARRAY_LIMIT` in this scope
   --> /home/travis/build/rust-lang-nursery/rust-bindgen/src/ir/analysis/derive_copy.rs:269:53
    |
269 |                             if bfu.layout().align > RUST_DERIVE_IN_ARRAY_LIMIT {
    |                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^ not found in this scope
    |

[fitzgen]

These changes look like how I'd expect, but we need to add the test case from the original bug report as well.

See https://github.com/rust-lang-nursery/rust-bindgen/blob/master/CONTRIBUTING.md#authoring-new-tests for information on how to add new tests to the test suite.

[aeleos]

Ah alright, I'll add that hopefully tonight

[aeleos]

Some tests seem to be failing because of this error

---- bindgen_test_layout__bindgen_ty_1 stdout ----
	thread 'bindgen_test_layout__bindgen_ty_1' panicked at 'assertion failed: `(left == right)`
  left: `128`,
 right: `80`: Size of: _bindgen_ty_1', tests/bitfield_large_overflow.rs:14:4
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:60
             at /checkout/src/libstd/panicking.rs:380
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:390
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:611
   5: std::panicking::begin_panic_new
             at /checkout/src/libstd/panicking.rs:553
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:521
   7: bitfield_large_overflow::bindgen_test_layout__bindgen_ty_1
   8: <F as test::FnBox<T>>::call_box
             at /checkout/src/libtest/lib.rs:1477
             at /checkout/src/libcore/ops/function.rs:143
             at /checkout/src/libtest/lib.rs:138
   9: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98

I am not sure what could cause this. The error appears if I use the tests/test-one.sh script, but not if I use cargo test. Any ideas @fitzgen?

[fitzgen]

When we generate bindings, we also emit #[test]s that assert the size, alignment, and offset of each field for each generated struct. One of these tests is failing. You should be able to reproduce outside of the test-one.sh script like this:

$ cd tests/expectations
$ cargo test

Is this happening in the new test that you added? I wouldn't expect this PR to have a serious impact on the generated bindings for most of our existing test suite, and wouldn't expect any existing tests to start failing their layout tests.

Ok, yeah it looks like this is the nest test. I'm not sure exactly why we are ultimately generating an incorrect layout, and would need to investigate more to figure it out. I expect it involves a bug in how we compute bitfields' physical allocation units.

I think we can land this PR now, however, since it fixes an existing bug, and file a follow up issue for the incorrect layout. In order to land this now, we'll need to add this to the top of the test so that it doesn't generate layout tests for the time being:

// bindgen-flags: --no-layout-tests

[fitzgen]

@aeleos, BTW, I'm not sure if you're aware that the "impl period" has just begun, but the folks hacking on bindgen during the impl period are hanging out in https://gitter.im/rust-impl-period/WG-dev-tools-bindgen if you want to join us :)

[bors-servo]

☔ The latest upstream changes (presumably #1002) made this pull request unmergeable. Please resolve the merge conflicts.

[aeleos]

@fitzgen I heard about it but I didn't see the one for bindgen, I'll definitely check it out. Also, I rebased after the recent merge and all the checks passed so it looks like everything is working.

[fitzgen]

@bors-servo r+

Thanks @aeleos ! Are you interested in hacking on the follow up issue to this, which is the incorrect layout? I'll file it and include some details in a bit, then drop a link back here. If you want to pick it up, you can tell @highfive :)

[bors-servo]

📌 Commit 4111a46 has been approved by fitzgen

[bors-servo]

⌛ Testing commit 4111a46 with merge a8f4bc9...

[aeleos]

@fitzgen Yea I am interested in the incorrect layout issue, it loos like it could be interesting. I'll start looking through the code now and see if I can find anything, but let me know when you have filed the new issue and I'll add myself to it.

[bors-servo]

💔 Test failed - status-travis

[aeleos]

@fitzgen It seems like one of the tests broke and froze for some reason. Only one test failed, and it ran for 4 hours with no logs. Have you ever encountered tests failing like this?

[fitzgen]

@fitzgen It seems like one of the tests broke and froze for some reason. Only one test failed, and it ran for 4 hours with no logs. Have you ever encountered tests failing like this?

I restarted it -- I've seen jobs hang before in Travis CI, before it even gets to running the bindgen tests. shrug

[bors-servo]

☀️ Test successful - status-travis
Approved by: fitzgen
Pushing a8f4bc9 to master...