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

[Jonathan Wakely]

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?


-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.txt
Type: text/x-patch
Size: 8647 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20200730/1222035c/attachment.bin>