[libcxx-commits] [PATCH] D119112: [libc++] Implement P1165R1 (Make stateful allocator propagation more consistent)

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libcxx/include/string:4147
 {
-    basic_string<_CharT, _Traits, _Allocator> __r(__lhs.get_allocator());
-    typename basic_string<_CharT, _Traits, _Allocator>::size_type __lhs_sz = __lhs.size();
-    typename basic_string<_CharT, _Traits, _Allocator>::size_type __rhs_sz = __rhs.size();
+    using _String = basic_string<_CharT, _Traits, _Allocator>;
+    _String __r(_String::__alloc_traits::select_on_container_copy_construction(__lhs.get_allocator()));
----------------
philnik wrote:
> ldionne wrote:
> > Is there any reason why we don't implement those naively like in the Standard? I don't think there's any performance gain here, right? This applies to all the overloads AFAICT.
> IIUC this implementation makes only one allocation, while making a copy and appending makes two allocations.
Oh, right, because we already reserve enough space for the `append` to be allocation-free. Makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119112