[PATCH] Implement P0966 std::string::reserve should not shrink

[Jonathan Wakely]

On 30/07/20 15:29 +0100, Jonathan Wakely wrote:
>On 12/05/19 21:22 +0000, Andrew Luo wrote:
>>It's been a while, but thought I'd check in again now that GCC 9 has been branched, and master is now on GCC 10.
>>
>>I've merged my changes with the latest code and attached a diff... Let me know if there's any thoughts on this.
>
>Well I failed to get around to this for GCC 10, so let's try again
>now.
>
>I've done another review of the patch, and I'm now wondering why the
>new _M_shrink function takes a requested capacity, when the caller
>always passes zero. We can simplify both implementations of _M_shrink
>if we remove the parameter and change the callers from _M_shrink(0) to
>_M_shrink().
>
>Was there a reason to make it take an argument? Did you anticipate
>future uses of _M_shrink(n) for n >= 0?
>
>If we simplify it then we need fewer branches in _M_shrink, because we
>don't need to do:
>
>      // Make sure we don't shrink below the current size.
>      if (__requested_capacity < length())
>	__requested_capacity = length();
>
>We only need to check whether length() < capacity() (and whether the
>string is shared, for the COW implementation).
>
>And if we do that, we can get rid of _M_shrink() because it's now
>identical to the zero-argument form of reserve(). So we can just put
>the body of _M_shrink() straight in reserve(). The reserve() function
>is deprecated, but when we need to remove it we can just make it
>private, so that shrink_to_fit() can still call it.
>
>The only downside of this I see is that when the deprecated reserve()
>eventually gets removed from the standard, our users will get a
>"reserve() is private" error rather than a "wrong number of arguments"
>error. But that might actually be better, since they can go to the
>location of the private member and see the comments and attribute
>describing its status in different standard versions.
>
>I've attached a relative diff showing my suggested changes to your
>most recent patch. This also fixes some regressions, because the
>_M_shrink function was not swallowing exceptions that result from a
>failure to reallocate, which shrink_to_fit() was doing previously.
>
>What do you think?

Here's the combined patch, based on your original with my proposed
simplifications applied.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.txt
Type: text/x-patch
Size: 17054 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20200730/382c4d91/attachment.bin>