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

[Tacet via Phabricator via libcxx-commits]

AdvenamTacet marked an inline comment as not done.
AdvenamTacet added inline comments.


================
Comment at: libcxx/include/string:2283-2285
+// __grow_by is deprecated because it does not set size. It not only may not update when size is changed,
+// but also not set size at all when string was short initially, leading to unpredictable size value.
+// It is not removed or changed to avoid breaking the ABI.
----------------
ldionne wrote:
> Just rewording, IMO this is easier to read. WDYT?
Looks good, thank you! I applied that change.


================
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);
----------------
AdvenamTacet wrote:
> 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!
Hey @philnik, does it look ok for you? If yes, please, accept the review. If you want me to work on something, please, let me know how I should improve it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148693