Re: [PATCH 3/3] Make scoped_restore_current_thread's cdtors exception free (RFC)

[Simon Marchi <simark@simark.ca>]

>> I don't know if it would be worth it, but I'd like if we could assert (abort
>> GDB) if an exception does try to exit the destructor.  The `restore` method
>> is non-trivial and calls into other non-trivial functions, so it would be
>> possible for a change far far away to cause that to happen.
> 
> It will already abort.  Destructors are noexcept by default, so if an exception
> escapes a destructor, std::terminate() is called, and that calls abort by default.

Oh, didn't know that!  I thought it was just "undefined behavior".

>> What do you think of keeping the try/catch, but using `gdb_assert_not_reached`
>> in it?
> 
> Not sure.  If we do that, we do get a nicer error message.  However if the user
> says "n" to "Quit this debugging session" we still abort.
> 
> /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/thread.c:1441: internal-error: scoped_restore_current_thread::~scoped_restore_current_thread(): unexpected exception thrown from destructor: hello
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Quit this debugging session? (y or n) n
> 
> This is a bug, please report it.  For instructions, see:
> <https://www.gnu.org/software/gdb/bugs/>.
> 
> /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/thread.c:1441: internal-error: scoped_restore_current_thread::~scoped_restore_current_thread(): unexpected exception thrown from destructor: hello
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Create a core file of GDB? (y or n) n
> terminate called after throwing an instance of 'gdb_exception_quit'
> Aborted (core dumped)
> 
> Maybe it would be interesting to add a variant of internal_error that did
> not throw a quit, so the user could swallow the exception...  Maybe consider
> wrapping that as a generic facility to add to all non-trivial RAII destructors
> we have?  Like a function that takes a function_view as parameter, so
> we would write:
> 
> foo::~foo ()
> {
>   safe_dtor (__FILE__, __LINE__, [&] () 
>     {
>       restore ();
>     });
> }
> 
> Even better, add a SAFE_DTOR macro using similar magic SCOPE_EXIT
> macro uses to be able to write:
> 
> foo::~foo ()
> {
>   SAFE_DTOR { restore (); };
> }

That's fancier than what I hoped for :).  My goal was just to make sure
we catch it if we ever make a change that causes an exception to escape.
Although I wouldn't be against what you proposed.

> Here's the current version of the patch.

That looks fine to me.

Simon