[libcxx-commits] [PATCH] D148693: [libc++] Set correct size at the end of growing std::string

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__string/extern_template_lists.h:58
   _Func(_LIBCPP_FUNC_VIS void basic_string<_CharType>::__grow_by(size_type, size_type, size_type, size_type, size_type, size_type)) \
+  _Func(_LIBCPP_FUNC_VIS void basic_string<_CharType>::__grow_by_without_replace(size_type, size_type, size_type, size_type, size_type, size_type)) \
   _Func(_LIBCPP_FUNC_VIS void basic_string<_CharType>::__grow_by_and_replace(size_type, size_type, size_type, size_type, size_type, size_type, value_type const*)) \
----------------
This is unfortunately also not possible (at least without some policy changes). Fortunately it's also not necessary for this patch.


================
Comment at: libcxx/include/__string/extern_template_lists.h:108
   _Func(_LIBCPP_FUNC_VIS basic_string<_CharType>::size_type basic_string<_CharType>::find_last_of(value_type const*, size_type, size_type) const) \
   _Func(_LIBCPP_FUNC_VIS void basic_string<_CharType>::__grow_by(size_type, size_type, size_type, size_type, size_type, size_type)) \
+  _Func(_LIBCPP_FUNC_VIS void basic_string<_CharType>::__grow_by_without_replace(size_type, size_type, size_type, size_type, size_type, size_type)) \
----------------
Let's remove this when we add `__grow_by_without_replace`.


================
Comment at: libcxx/include/string:1831-1833
     _LIBCPP_CONSTEXPR_SINCE_CXX20
     void __grow_by(size_type __old_cap, size_type __delta_cap, size_type __old_sz,
                    size_type __n_copy,  size_type __n_del,     size_type __n_add = 0);
----------------
We should mark this as `_LIBCPP_DEPRECATED_("use __grow_by_without_replace")` to avoid calling it accidentally.


================
Comment at: libcxx/include/string:1834
                    size_type __n_copy,  size_type __n_del,     size_type __n_add = 0);
     _LIBCPP_CONSTEXPR_SINCE_CXX20
+    void __grow_by_without_replace(size_type __old_cap, size_type __delta_cap, size_type __old_sz,
----------------
This should be something like `_LIBCPP_HIDE_FROM_ABI_V1` or similar.


================
Comment at: libcxx/include/string:2294
 
+template <class _CharT, class _Traits, class _Allocator>
+void
----------------
Please clang-format!


================
Comment at: libcxx/include/string:2300
+{
+    __grow_by<_CharT, _Traits, _Allocator>(__old_cap, __delta_cap, __old_sz, __n_copy, __n_del, __n_add);
+    __set_long_size(__old_sz - __n_del + __n_add);
----------------
The template arguments shouldn't be necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148693