[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:
> 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;
Mordante wrote:
> 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)
+            __libcpp_unreachable();
+        __r_.first().__s.__size_ = __s;
Mordante wrote:
> 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
 class StdStringPrinter(object):
     """Print a std::string."""
Mordante wrote:
> Does this also break the LLDB pretty printer?
Probably. Would be nice to have a test runner for that.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list