[PATCH] Implement P0966 std::string::reserve should not shrink
Thu Aug 6 18:48:36 GMT 2020
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
>>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
>>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
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...
Size: 18087 bytes
Desc: not available
More information about the Libstdc++