[libcxx-commits] [PATCH] D123580: [libc++] Use bit field for checking if string is in long or short mode
Nikolas Klauser via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Apr 12 09:10:39 PDT 2022
philnik marked 2 inline comments as done.
philnik added inline comments.
Comment at: libcxx/include/string:681
+ size_type __cap_ : sizeof(size_type) * CHAR_BIT - 1;
+ bool __is_long_ : 1;
> Mordante wrote:
> > Do we have a test that validates the size of this struct?
> > On some platforms (IIRC AIX) the members `__cap_` and `__is_long_` won't be stored in one `size_type` since they have different types.
> Did you verify this isn't an ABI break when libc++14 and libc++15 compiled code is mixed?
> We should make sure that doesn't break an big and little endian.
https://godbolt.org/z/axTMxqaj3 is the code I used to verify that everything is OK. I think this should be good enough to say that they are equivalent. Do you know if there is any documentation about the bit field layout on different platforms?
Comment at: libcxx/include/string:703
+ static const size_type __endian_factor = 1;
> Is this a copy paste?
No, depending on the string layout the factors have to be reversed. It's essentially: If you use the LSB for `is_long` you have to multiply by two, otherwise keep the same number.
Comment at: libcxx/include/string:1486
+ if (__s > 127)
+ __r_.first().__s.__size_ = __s;
> Mainly curious, but why not a `_LIBCPP_ASSERT`?
This ensures optimal code generation. GCC doesn't have `__builtin_assume`. This is a workaround.
Comment at: libcxx/utils/gdb/libcxx/printers.py:192
"""Print a std::string."""
> Does this also break the LLDB pretty printer?
Probably. Would be nice to have a test runner for that.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the libcxx-commits