[libcxx-commits] [PATCH] D110598: [libc++] Implement P0980R1 (constexpr std::string)

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/string:1409
 #if _LIBCPP_STD_VER > 17
     constexpr _LIBCPP_INLINE_VISIBILITY
     bool starts_with(__self_view __sv) const noexcept
----------------
Mordante wrote:
> Nit: I would keep the order `LIBCPP_INLINE_VISIBILITY constexpr` for consistency with the other changes.
I'd keep it as-is for now. I don't want to make this patch unnecessarily larger.


================
Comment at: libcxx/test/std/strings/basic.string/string.capacity/max_size.pass.cpp:82
-  // static_assert(test());
-#endif
 
----------------
Mordante wrote:
> Why don't you test this in `constexpr`? The signature has changed in the paper.
This test makes extremely large allocations, like `numeric_limits<size_t>::max()` large. Because of this the constant evaluations fails.


================
Comment at: libcxx/test/std/strings/basic.string/string.modifiers/string_insert/size_pointer_size.pass.cpp:53
-  {
-    typedef std::string S;
-    test(S(""), 0, "", 0, S(""));
----------------
Mordante wrote:
> Why is this part removed?
This is duplicated for `std::string` and `std::basic_string<char, std::char_traits<char>, min_allocator<char>>`. I just made this a template and removed the duplicated code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110598