Re: [Patch, libstdc++] Add specific error message into exceptions

[Tim Shen <timshen at google dot com>]

On Mon, Sep 7, 2015 at 4:06 AM, Jonathan Wakely <jwakely@redhat.com> wrote:
> On 28/08/15 11:23 -0700, Tim Shen wrote:
>>
>> On Fri, Aug 28, 2015 at 8:59 AM, Jonathan Wakely <jwakely@redhat.com>
>> wrote:
>>>
>>> There seems to be no need to construct a std::string here, just pass a
>>> const char* (see below).
>>
>>
>> To be honest, I wasn't considering performance for a bit, since
>> exceptions are already considered slow by me :P. But yes, we can do
>> less allocations.
>>
>>> I wonder if we want to make this more efficient by adding a private
>>> member to regex_error that would allow information to be appended to
>>> the string, rather then creating a new regex_error with a new string.
>
>
> In case it wasn't clear, I was suggesting to add a private member
> *function* not data member.
>
>> I can add a helper function to _Scanner to construct the exception
>> object for only once. For functions that can't access this helper, use
>> return value for error handling.
>>
>>> I suggest adding another overload that takes a const char* rather than
>>> std::string. The reason is that when using the new ABI this function
>>> will take a std::__cxx11::string, so calling it will allocate memory
>>> for the string data, then that string is passed to the regex_error
>>> constructor which has to convert it internally to an old std::string,
>>> which has to allocate a second time.
>>
>>
>> First, to make it clear: due to _M_get_location_string(), we need
>> dynamic allocation.
>>
>> So is it good to have an owned raw pointer stored in runtime_error,
>> pointing to a heap allocated char chunk, which will be deallocated in
>> regex_error's dtor?
>
>
> No, adding that pointer is an ABI change.
>
> If you can't do it without an ABI change then you will have to lose
> the _M_get_location_string() functionality. It seems non-essential
> anyway.

Ok then, let's not append dynamic location information, but use a
string literal pointer only.


-- 
Regards,
Tim Shen

Attachment: c.diff
Description: Text document