[libcxx-commits] [PATCH] D123058: [libc++] Add capacity constructor to string

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/string:4185-4188
+    _String __r(__capacity_tag(),
+                __lhs_sz + __rhs_sz,
+                _String::__alloc_traits::select_on_container_copy_construction(__lhs.get_allocator()));
+    __r.append(__lhs.data(), __lhs_sz);
----------------
ldionne wrote:
> IMO this makes a lot more sense. However, I'm going to push the reasoning further and ask this: Would this *also* be equivalent?
> 
> ```
> using _String = basic_string<_CharT, _Traits, _Allocator>;
> typename _String::size_type __lhs_sz = __lhs.size();
> typename _String::size_type __rhs_sz = __rhs.size();
> _String __r(_String::__alloc_traits::select_on_container_copy_construction(__lhs.get_allocator()));
> __r.reserve(__lhs_sz + __rhs_sz);
> __r.append(__lhs.data(), __lhs_sz);
> __r.append(__rhs.data(), __rhs_sz);
> return __r;
> ```
> 
> If so, then perhaps we don't really need the constructor after all?
It think this is equivalent for runtime. For constant evaluation the constructor saves an allocation, but I don't think we care enough about that to add a new constructor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123058