Fixes alignment errors with new Rust union type #909
Conversation
|
Looks reasonable, but could you add a test for this, please? (or, we could merge #892 first, I guess...) |
|
Also, thanks a lot! :) |
|
Yeah, I would liked to add a test. But for the test we currently would need to enable --unstable-rust and that probably not works on stable? Or does this option just enables the Union? |
|
☔ The latest upstream changes (presumably #892) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@emilio I rebased upstream and adapted the tests expectations :) This should be enough test cases? |
|
This looks great to me, thanks! I think we should consider making this opt-in (or allowing to opt-out), since the people that check-in their bindings into their repos will break all sorts of 32-bit stuff... But that can be a followup. I think if we don't have any test that fails without these changes we should definitely add one. I'm happy to check it in after your PR lands if you prefer. Also, could you squash the commits, please? Thanks again! |
This fix creates a new private field with the required aligned size. This new private field ensures that the union has the required size.
|
Yeah we had a test that failed without these changes, thanks for reminding me (https://github.com/rust-lang-nursery/rust-bindgen/pull/909/files#diff-08776cbee31094781c38902a13aad151) :D I forgot to remove the ifdef in the first place. |
|
@bors-servo r+ Neat, thank you! |
|
📌 Commit 7603eb1 has been approved by |
Fixes alignment errors with new Rust union type This fix creates a new private field with the required aligned size. This new private field ensures that the union has the required size. This fixes: #908
|
☀️ Test successful - status-travis |
This fix creates a new private field with the required aligned size. This new
private field ensures that the union has the required size.
This fixes: #908