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

[Stefan Pintilie via Phabricator via llvm-commits]

stefanp added a comment.

In D119567#3321211 <https://reviews.llvm.org/D119567#3321211>, @ldionne wrote:

> You have a typo in your commit message (`becuase`). Generally LGTM with a small style comment. I'd also like to better understand the motivation before shipping this: can someone explain why/how the tests currently depend on the class being packed?

Thank you for catching that. I'll fix the typo before I commit.

I can explain why the tests depend on the class being packed. All of the tests check the size of the class `Parser` with an assert like this one:

  static_assert(sizeof(Parser<char>) == 2 * sizeof(uint32_t));

The class `_Flags` is part of the `Parser` class  and so if `_Flags` is not packed then `Parser` will be larger and the assert will fail.

The tests and asserts were added when the original Formatter implementations were done. (See `7fb9f99f3bb645337b4f4e6a2a3515219be82011` from patch https://reviews.llvm.org/D103670) . I didn't add the original implementation but I think the reason for these asserts was that the author wanted to make sure that the data structures where small.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119567