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.