[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]

philnik marked 7 inline comments as done.
philnik added inline comments.


================
Comment at: libcxx/include/string:1486
+        if (__s > 127)
+            __libcpp_unreachable();
+        __r_.first().__s.__size_ = __s;
----------------
Mordante wrote:
> philnik wrote:
> > 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. 
> From a maintainability PoV I like `_LIBCPP_ASSERT` better. Would it be an option to adjust `_LIBCPP_ASSERT?` the "generate" this code when using GCC?
I agree that `_LIBCPP_ASSERT` would be nicer to read. The problem is that using this trick for `_LIBCPP_ASSERT` could lead to extra code generation in more complex scenarios. We would have to introduce something else like `_LIBCPP_ASSUME`, which should only be used for trivial cases. But for two instances I don't think this is justified.


================
Comment at: libcxx/utils/gdb/libcxx/printers.py:192
 
 class StdStringPrinter(object):
     """Print a std::string."""
----------------
Mordante wrote:
> philnik wrote:
> > Mordante wrote:
> > > Does this also break the LLDB pretty printer?
> > Probably. Would be nice to have a test runner for that.
> I already planned to look into that, D97044#3440904 ;-)
Do you know where I would have to look to know what the LLDB pretty printers do?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123580