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

codegen: Track union layout more accurately. #1984

Merged
merged 1 commit into from
Feb 7, 2021

Conversation

emilio
Copy link
Contributor

@emilio emilio commented Feb 7, 2021

Instead of always generating the _bindgen_union_align method (which
shouldn't be needed at all for Rust structs, since the struct layout
tracker already deals with adding repr(align) as necessary) make sure to
visit all fields appropriately to generate the correct alignment.

@highfive
Copy link

highfive commented Feb 7, 2021

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

@frewsxcv
Copy link
Member

frewsxcv commented Feb 7, 2021

@emilio Can you merge master into this? And then remove the #[cfg_attr(target_arch = "aarch64", should_panic)] // This line should be removed after the bug linked above is fixed line. I just tried this locally on my MacBook Air M1 and that test now passes, which should resolve #1973 🎉

Instead of always generating the _bindgen_union_align method (which
shouldn't be needed at all for Rust structs, since the struct layout
tracker already deals with adding repr(align) as necessary) make sure to
visit all fields appropriately to generate the correct alignment.
@emilio emilio force-pushed the rust-union-layout-tracker branch from 68ea89e to eaadc3f Compare February 7, 2021 21:31
@emilio emilio merged commit 8ac787a into master Feb 7, 2021
@emilio emilio deleted the rust-union-layout-tracker branch February 7, 2021 22:10
@frewsxcv
Copy link
Member

frewsxcv commented Feb 7, 2021

@emilio No pressure and no rush, but any chance there could be a release for this? Let me know if I can help with that process

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants