[libcxx-commits] [PATCH] D155486: Optimize basic_string's memory usage

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

In D155486#4544305 <https://reviews.llvm.org/D155486#4544305>, @Mordante wrote:

> In D155486#4532334 <https://reviews.llvm.org/D155486#4532334>, @mvels wrote:
>
>> BTW: I did not understand the Apple max_size.pass failure, I adjusted the test and they all pass (check-cxx) on my local git repo on x86.
>
> On the backdeployment targets the new code uses the old version of the dylib. My suspicion is there is an ABI incompatibility. @philnik do you see an issue in this patch?

This is an ABI break, but I don't think that's the problem. `max_size` isn't instantiated. This look to me like there is an off-by-one error somewhere.

Could we split this into two patches? This is really seems like it has high potential for having subtile bugs.



================
Comment at: libcxx/include/string:2338
+                    : __ms - 1;
+    } else {
+        // The current string is inlined (SSO). In this case we do not want to
----------------
Why do we want to change the growth specifically for the case when we go from short to long?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155486