[patch/rfa] Tiny but delicate change to string::_M_mutate

[Paolo Carlini]

Nathan Myers wrote:

>On Fri, Oct 15, 2004 at 08:26:18PM +0200, Paolo Carlini wrote:
>  
>
>>Hi everyone, hi Nathan,
>>    
>>
>Apologies for the delay in replying...
>  
>
No problem, 4.0.0 is still a few months away :) ...

>This looks perfectly safe.  It seems probable that people often do
>append no characters, e.g. because the string they are appending
>is empty.  Probably the logic should still check for _S_empty_rep
>to avoid touching its reference count:
>
>     if ((((_M_rep() == &_S_empty_rep() || _M_rep()->_M_is_shared()) 
>            && (__len2 | __len1) != 0) 
>	  || __new_size > capacity())
>  
>
Why this complication? The conditional becomes *awful* (sorry ;) and we have
to keep the macro... What's wrong with reconfirming (in the *very few* 
cases the
codepath at issue typically triggers) the refcount to zero? (*) If we 
really want to
avoid that, I propose simply changing the end of _M_mutate to:

#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
    if (_M_rep() != &_S_empty_rep()
#endif
        {
          _M_rep()->_M_set_sharable();
          _M_rep()->_M_length = __new_size;
          _M_data()[__new_size] = _Rep::_S_terminal; // grrr. (per 21.3.4)
        }

But I would really prefer not adding this... 

>We might instead check that the proposed mutation actually changes 
>the string before calling _M_mutate; that would subsume the case of 
>appending the empty string, and make the present logic correct   
>(...if I understand correctly).
>  
>
This is an additional improvement worth looking into, thanks! I suspect
we gain a lot of performance for simple operations (i.e., single char
appends) if we avoid involving the complete _M_mutate...

Thanks Nathan,
Paolo.

(*) Ok, it's obvious, sorry, but _M_set_sharable, is just an inlined 
assignment
of an integer.