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

[Tacet via Phabricator via libcxx-commits]

AdvenamTacet added inline comments.


================
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);
----------------
philnik wrote:
> ldionne wrote:
> > 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).
> I don't really see the problem of adding this to the ABIv2 list. It's not like we will freeze ABIv2 any time soon, and we probably want to reassess what we export from the dylib and what we don't before doing that anyways.
If you decide that I should revert the revert, let me know, I don't have an opinion about it. But also I don't see any real advantage of adding this function to ABI.


================
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:
> AdvenamTacet wrote:
> > 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.
> You can use `_LIBCPP_SUPPRESS_DEPRECATED_PUSH` and `_LIBCPP_SUPPRESS_DEPRECATED_POP` to ignore the diagnostic. IMO that's also what we should do to avoid code bloat.
Thx! I tried it, but `__grow_by` doesn't work because of  `_LIBCPP_HIDE_FROM_ABI_AFTER_V1`:
```
undefined reference to `std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::__grow_by[abi:v170000](unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, unsigned long)'
clang++: error: linker command failed with exit code 1 (use -v to see invocation)
```

Is there one more work-around?

It works when I remove `_LIBCPP_HIDE_FROM_ABI_AFTER_V1`, but I believe it's worse solution than a few more copy-pasted lines.
Let me know what you think!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148693