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

[Tacet via Phabricator via libcxx-commits]

AdvenamTacet added a comment.

I hope I understood all comments. I'm in the middle of testing right now.



================
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*)) \
----------------
philnik wrote:
> AdvenamTacet wrote:
> > philnik wrote:
> > > This is unfortunately also not possible (at least without some policy changes). Fortunately it's also not necessary for this patch.
> > I don't understand.
> Adding a new symbol to the dylib requires having availability annotations to aid in back-porting to older systems. If we add a new symbol in string, essentially all users would be required to target the latest OS when they use `std::string` in their programs, which isn't really an option.
Ok, I removed all references to `__grow_by_without_replace`.


================
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)) \
----------------
philnik wrote:
> Let's remove this when we add `__grow_by_without_replace`.
Removed `__grow_by` from the file.


================
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);
----------------
philnik wrote:
> We should mark this as `_LIBCPP_DEPRECATED_("use __grow_by_without_replace")` to avoid calling it accidentally.
Cannot call deprecated function, so I had to copy-paste the code.


================
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,
----------------
philnik wrote:
> This should be something like `_LIBCPP_HIDE_FROM_ABI_V1` or similar.
I used `_LIBCPP_HIDE_FROM_ABI_AFTER_V1`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148693