On Sun, 7 Jan 2024 at 18:50, François Dumont <frs.dum...@gmail.com> wrote:
>
>
> On 07/01/2024 17:34, Jonathan Wakely wrote:
> > On Sun, 7 Jan 2024 at 12:57, François Dumont <frs.dum...@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.
>
> 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.
