[libcxx-commits] [PATCH] D73223: Partially inline basic_string copy constructor in UNSTABLE
Eric Fiselier via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Sun Feb 23 17:21:45 PST 2020
EricWF added a comment.
This patch should contain the changes to the unstable extern template list.
Also, the added test should pass both before and after your change, correct?
That is: does this change introduce a behavioral change, or is it simply an optimization?
Otherwise, this patch LGTM.
================
Comment at: libcxx/include/string:1553
+ // Slow path for the (inlined) copy constructor.
+ // Always externally instantiated and not inlined.
+ // Asserts that __s is zero terminated.
----------------
This isn't always externally instantiated.
And it doesn't assert that __s is null terminated (nor can it)
What's this comment really trying to say?
================
Comment at: libcxx/include/string:1892
+template <class _CharT, class _Traits, class _Allocator>
+void basic_string<_CharT, _Traits, _Allocator>::__init_copy_ctor_external(
+ const value_type* __s, size_type __sz) {
----------------
It's a bit unfortunate that this function essentially duplicates `__init(char*, size_type)`.
Can you write a short comment as to why this provides benefit over simply calling `__init`?
The formulation is essentially the same.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73223/new/
https://reviews.llvm.org/D73223
More information about the libcxx-commits
mailing list