Closed Bug 1391103 Opened 3 years ago Closed 2 years ago
Storage in Maybe as when the type in a struct
59 bytes, text/x-review-board-request
On some platforms, a type can have two alignment: one is when it is used as itself, and the other is when it is used as a struct field. And alignof and alignas reports the first one, which isn't really useful for Maybe, because we are using it inside a struct. For example, on 32bit Linux, uint64_t and double have 8byte alignment as their own, but when they are inside a struct, they are aligned to 4byte instead.  Make Maybe::mStroage use the second alignment helps better packing in those cases, and that can also workaround a Rust inconsistency with C++  which causes bindgen issue  on 32bit Linux.  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79024  https://github.com/rust-lang/rust/issues/43899  https://github.com/rust-lang-nursery/rust-bindgen/issues/917
Note that MOZ_ALIGN_OF is different. https://bugzilla.mozilla.org/show_bug.cgi?id=1288016
Perhaps it should be called MOZ_ALIGNAS_IN_STRUCT to make it more clear that it's intended to be different from alignas (rather than, say, a replacement for use for compilers that don't support it)?
I cannot really think of a use case where the original alignas is more useful than this macro...
Everything seems to be fine with this patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ad70130ca3164aadc325ef007cd026bfe938f28e
Comment on attachment 8898050 [details] Bug 1391103 - Align Maybe::mStorage like when the type is in a struct. https://reviewboard.mozilla.org/r/169356/#review175934 ::: mfbt/Alignment.h:55 (Diff revision 1) > + * > + * Known examples are 64bit types (uint64_t, double) on 32bit Linux, > + * where they have 8byte alignment on their own, and 4byte alignment > + * when in struct. > + */ > +#define MOZ_ALIGNAS(T) alignas(mozilla::detail::AlignasHelper<T>) I think dbaron's suggestion of `MOZ_ALIGNAS_IN_STRUCT` is a better name for this macro, for the reasons he cites.
Attachment #8898050 - Flags: review?(nfroyd) → review+
Comment on attachment 8898050 [details] Bug 1391103 - Align Maybe::mStorage like when the type is in a struct. https://reviewboard.mozilla.org/r/169356/#review175938 r=me with the below change.
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/06d2f579a7f3 Align Maybe::mStorage like when the type is in a struct. r=froydnj
You need to log in before you can comment on or make changes to this bug.