This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2 15/22] Make exceptions use std::string and be self-managing


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]