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

[Jonathan Wakely]

On 30/07/20 16:39 +0100, Jonathan Wakely wrote:
>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.

I've now pushed that combined patch to master.

Sorry it took so long to integrate your changes, but thanks very much
for the contribution to GCC!


-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.txt
Type: text/x-patch
Size: 18087 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20200806/01dba1c3/attachment-0001.bin>