On 07/01/2024 21:53, Jonathan Wakely wrote:
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.
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>.
