[PATCH] Add __cow_string C string constructor

[François Dumont]

On 07/01/2024 21:53, Jonathan Wakely wrote:
> On Sun, 7 Jan 2024 at 18:50, François Dumont <frs.dumont@gmail.com> wrote:
>>
>> On 07/01/2024 17:34, Jonathan Wakely wrote:
>>> On Sun, 7 Jan 2024 at 12:57, François Dumont <frs.dumont@gmail.com> wrote:
>>>> Hi
>>>>
>>>> While working on the patch to use the cxx11 abi in gnu version namespace
>>>> mode I got a small problem with this missing constructor. I'm not sure
>>>> that the main patch will be integrated in gcc 14 so I think it is better
>>>> if I propose this patch independently.
>>>>
>>>>        libstdc++: Add __cow_string constructor from C string
>>>>
>>>>        The __cow_string is instantiated from a C string in
>>>> cow-stdexcept.cc. At the moment
>>>>        the constructor from std::string is being used with the drawback of
>>>> an intermediate
>>>>        potential allocation/deallocation and copy. With the C string
>>>> constructor we bypass
>>>>        all those operations.
>>> But in that file, the std::string is the COW string, which means that
>>> when we construct a std::string and copy it, it's cheap. It's just a
>>> reference count increment/decrement. There should be no additional
>>> allocation or deallocation.
>> Good remark but AFAI understand in this case std::string is the cxx11
>> one. I'll take a second look.
>>
>> Clearly in my gnu version namespace patch it is the cxx11 implementation.
> I hope not! The whole point of that type is to always be a COW string,
> which it does by storing a COW std::basic_string in the union, but
> wrapping it in a class with a different name, __cow_string.
>
> If your patch to use the SSO string in the versioned namespace doesn't
> change that file to guarantee that __cow_string is still a
> copy-on-write type then the patch is wrong and must be fixed.

Don't worry, __cow_string is indeed wrapping a COW string.

What I meant is that in this constructor in <stdexcept>:

__cow_string(const std::string&);

The std::string parameter is the SSO string.

However, as you said, in cow-stdexcept.cc the similar constructor is in 
fact taking a COW string so it has less importance. It's just a ODR issue.

In my gnu version namespace patch however this type is still the SSO 
string in cow-stdexcept.cc so I'll keep it in this context.


>> Even if so, why do we want to do those additional operations ? Adding
>> this C string constructor will make sure that no useless operations will
>> be done.
> Yes, we could avoid an atomic increment and decrement, but that type
> is only used when throwing an exception so the overhead of allocating
> memory and calling __cxa_throw etc. is far higher than an atomic
> inc/dec pair.
>
> I was going to say that the new constructor would need to be exported
> from the shared lib, but I think the new constructor is only ever used
> in these two places, both defined in that same file:
>
>    logic_error::logic_error(const char* __arg)
>    : exception(), _M_msg(__arg) { }
>
>    runtime_error::runtime_error(const char* __arg)
>    : exception(), _M_msg(__arg) { }
>
> So I think the change is safe, but I don't think it's urgent, and
> certainly not needed for the reasons claimed in the patch description.
>
The ODR violation has no side effect, it confirms your statement, looks 
like the __cow_string(const std::string&) could be removed from <stdexcept>.