Re: [PATCH 00/20] Remove cleanups

[Pedro Alves <palves@redhat.com>]

On 02/13/2019 09:48 PM, John Baldwin wrote:
> On 2/13/19 1:29 PM, Tom Tromey wrote:
>> This series removes the remaining uses of make_cleanup from the tree,
>> then removes make_cleanup and some related functions (but note that
>> final cleanups remain).
>>
>> Then, it removes the TRY/CATCH macros in favor of ordinary C++ code.
>>
>> Finally, it cleans up a few spots that currently use TRY/CATCH but
>> that are more succinctly written using SCOPE_EXIT or RAII.
>>
>> Regression tested by the buildbot.
> 
> Yay!
> 
> The only comment I have is that the structure names for exceptions look a
> bit awkward in code now with the mix of lower and upper case, e.g.:
> 
>    try
>      {
>        e_msg = ada_exception_message_1 ();
>      }
>    catch (struct gdb_exception_RETURN_MASK_ERROR &e)
>      {
>        e_msg.reset (nullptr);
>      }
> 
> We could drop the 'struct' perhaps, but not sure if we could simplify the
> classes a bit to something like:
> 
> struct gdb_exception_error : public gdb_exception
> {
> };
> 
> struct gdb_exception_quit : public gdb_exception
> {
> };
> 
> And use 'gdb_execption' instead of 'struct gdb_exception_RETURN_MASK_ALL',
> 'gdb_exception_error' instead of 'struct gdb_exception_RETURN_MASK_ERROR',
> and 'gdb_exception_quit' instead of 'struct gdb_exception_RETURN_MASK_QUIT'?

Yes please.  The only reason I originally named those structs that way
was to make the CATCH macro simpler to map in the C++ variant:

 #define CATCH(EXCEPTION, MASK)                                          \
           } while (0);                                                  \
         }                                                               \
     catch (struct gdb_exception ## _ ## MASK &EXCEPTION)

We should also catch by const reference, IMO.  Is there a good reason
for the non-const-ness?

Thanks,
Pedro Alves