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

Mon Jun 12 12:21:13 PDT 2023

ldionne added inline comments.

================
Comment at: libcxx/include/__string/extern_template_lists.h:57
   _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_and_replace(size_type, size_type, size_type, size_type, size_type, size_type, value_type const*)) \
----------------
We can't remove this function from the explicit instantiation list, since that would be an ABI break.

================
Comment at: libcxx/include/string:1845
     _LIBCPP_CONSTEXPR_SINCE_CXX20
+    void __grow_by_without_replace(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);
----------------
I would be tempted to make this `_LIBCPP_HIDE_FROM_ABI` and not include it in the explicit instantiation list. So far, our explicit instantiation lists have only made our lives really hard with `std::string` (e.g. getting in the way of fixing bugs).

================
Comment at: libcxx/include/string:2284-2285
 void
-_LIBCPP_CONSTEXPR_SINCE_CXX20
+_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI_AFTER_V1 _LIBCPP_DEPRECATED_("use __grow_by_without_replace")
 basic_string<_CharT, _Traits, _Allocator>::__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)
----------------
Please document the bug that this function has and why we're keeping it around.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148693