This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2 15/22] Make exceptions use std::string and be self-managing
- From: Pedro Alves <palves at redhat dot com>
- To: Tom Tromey <tom at tromey dot com>, gdb-patches at sourceware dot org
- Date: Wed, 3 Apr 2019 17:20:36 +0100
- Subject: Re: [PATCH v2 15/22] Make exceptions use std::string and be self-managing
- References: <20190227201849.32210-1-tom@tromey.com> <20190227201849.32210-16-tom@tromey.com>
On 02/27/2019 08:18 PM, Tom Tromey wrote:
> This changes the exception's "message" member to be a std::string.
> This allows removing the stack of exception messages, because now
> exceptions will self-destruct when needed.
>
> Note that this does increase the cost of copying an exception. This
> will be somewhat addressed in a subsequent patch.
I have a concern about this patch, something that I alluded to
at the Cauldron.
I think it'd be better if we followed what the C++ standard library
does, and make copy constructor/assignment op nothrow:
"Each standard library class T that derives from class exception shall have
a publicly accessible copy constructor and a publicly accessible copy
assignment operator that do not exit with an exception." (N4659/21.8.2)
See:
https://stackoverflow.com/questions/21644735/should-i-declare-the-copy-constructor-of-my-exceptions-noexcept
I believe that rule exists because throwing an exception copies
the exception object, and if that copy operation throws, then the process
is terminated with std::terminate(). Copying a std::string may require
a heap allocation, which likely doesn't throw on most OSs with
over-commit enabled, but it pedantically can.
I think fixing this isn't hard, so I think we should do it from the
start -- instead of gdb_exception holding a std::string, hold a
reference counted immutable heap-allocated C string.
> throw_error (except.error,
> _("unable to read value of %s (%s)"),
> - xvz_name, except.message);
> + xvz_name, except.message.c_str ());
Since you're touching most (all?) lines that refer to "message",
did you consider adding a "what()" method, to model what
std::exception has?
const char *what() const noexcept;
Maybe that helps with newcomers' familiarity?
Thanks,
Pedro Alves