[PATCH] D119567: [libcxx] String format class marked as packed

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: libcxx/include/__format/parser_std_format_spec.h:111
   _Type __type{_Type::__default};
-};
+} __attribute__((packed));
 
----------------
Mordante wrote:
> hubert.reinterpretcast wrote:
> > Use double-underscore form to avoid macro problems.
> > 
> > Actually, are we assuming GNU attribute support in general "everywhere"?
> Is there something like godbolt where I can check the layout on AIX online?
> I've been working on some changes regarding this class. In that new version unconditionally pack the struct on all platforms might lead to sub-optimal code. For the current code I don't see an issue with the packing.
> 
> > Actually, are we assuming GNU attribute support in general "everywhere"?
> 
> I theory yes. The pre-commit CI tests all supported platforms. So when the CI passes it works. (The ASAN errors are unrelated.) But I think it would be cleaner to make the packing conditional.
> 
> 
> Is there something like godbolt where I can check the layout on AIX online?

Yes: godbolt

https://godbolt.org/z/9j49baqsM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119567/new/

https://reviews.llvm.org/D119567