[libcxx-commits] [PATCH] D131668: [libc++] Implement P2438R2 (std::string::substr() &&)

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/string:888
+        __str.__default_init();
+        _Traits::move(data(), data() + __pos, __len);
+        __set_size(__len);
----------------
Mordante wrote:
> Did you measure the overhead for this move when `__pos == 0`?
I haven't measured, but AFAICT every memmove implementation is in that case basically a no-op. Our `char_traits::move` has `if (__s1 < __s2 ) {...} else if (__s2 < __s1) {...}`, the LLVM libc has `if (dst < src) ... else if (dst > src) ...` and musl has `if (d == s) return d;`. I haven't checked glibc, but I would be very surprised if if wasn't optimized there.


================
Comment at: libcxx/include/string:1297
+    basic_string substr(size_type __pos = 0, size_type __n = npos) const& {
+      return basic_string(*this, __pos, __n, __alloc());
+    }
----------------
Mordante wrote:
> This seems to be https://wg21.link/lwg3752, can you mention in the status page we implement this not yet accepted LWG-issue?
I wouldn't consider LWG3752 to be implemented. It doesn't propose a solution and IMO the right thing would be to use `select_on_container_copy_construction` on the `const&` overload and passing the allocator on the `&&` overload.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131668