Re: [PATCH 00/12] remove some cleanups using a cleanup function

[Pedro Alves <palves@redhat.com>]

On 01/12/2019 11:50 AM, Andrew Burgess wrote:
> I've been thinking about a similar idea for a while too.  I wondered
> if there was a way we could make use of templates to generate some of
> the common boiler plate cleanups.
> 
> This mini-series changes the first 4 of your patches to you this idea
> so you can see how it might work.
> 
> First, the ideal, description, a new templated class
> `cleanup_function` that allows you to do something like this:
> 
>     void
>     delete_longjmp_breakpoint (int arg)
>     {
>       /* Blah, blah, blah...  */
>     }
> 
>     using longjmp_breakpoint_cleanup
>         = cleanup_function <delete_longjmp_breakpoint, int>;

It seems unnecessary to pass in the types of the arguments of
delete_longjmp_breakpoint.  Couldn't those be extracted from
delete_longjmp_breakpoint's type?  See below.

> 
> This creates a new cleanup class `longjmp_breakpoint_cleanup` than can
> then be used like this:
> 
>     longjmp_breakpoint_cleanup obj (thread);

I think this results in an inferior cleanup_function solution, because this
way you can't pass in a bespoke small lambda on the spot.  I.e. you're
forced to create a cleanup with a named function -- right?

If it's the lambda itself you don't like, I think it should be
possible to add make cleanup_function's ctor have a std::bind-like interface [1],
so that you'd pass cleanup_function's ctor the function and arguments:

  template <typename F, typename... Args>
  cleanup_function (F &&func, Args &&...args);

so you'd create the cleanup like:

  cleanup_function cleanup (delete_longjmp_breakpoint, thread);

Since Tromey's cleanup_function is not a template, to implement such
a ctor, it would have to rely on std::function (or something like it) for
type erasure, which might heap allocate if the resulting callable is large
enough.  The advantage would be that with that you can create a cleanup_function
without passing specifying the called function's type.  The disadvantage is
the cost of the std::function type erasure / potential heap allocation, of course.

But we could avoid the cost/heap if we make cleanup_function a template,
as  Andrew's version is, but I have to say that I don't really like that
version's way of declaring the cleanup_function typedef:

 +/* Cleanup class that calls delete_longjmp_breakpoint.  */
 +#ifdef __cpp_template_auto
 +using longjmp_breakpoint_cleanup
 +	= cleanup_function <delete_longjmp_breakpoint, int>;
 +#else
 +using longjmp_breakpoint_cleanup
 += cleanup_function <void (*) (int), delete_longjmp_breakpoint, int>;
 +#endif

I think it should be possible to code cleanup_function's template
such that you could instantiate it like:

  cleanup_function<void (int)>

similarly to gdb::function_view?

That doesn't tie cleanup_function to the actual function called, just
its type, but I wouldn't see that as a disadvantage, given this way this
works with all sorts of callables, including lambdas.

Now, ctors don't deduce template parameter types until C++17, so with
that template interface and C++11 we wouldn't be able to just write:

  cleanup_function cleanup (delete_longjmp_breakpoint, 0);

But, that is fixable with a helper make_cleanup_function, which would
have us write:

  auto cleanup = make_cleanup_function (delete_longjmp_breakpoint, 0);

For the optional cleanup case, we'd need to somehow spell out the
type, no way around it, but that's not too horrible with that interface,
IMO:
  cleanup_function<void (int)> cleanup;
or:
  cleanup_function<decltype (delete_longjmp_breakpoint)> cleanup;

(and of course a typedef could put the function's type away from view)

I'm not really sure we need a std::bind-like interface though.  I'd
be super fine with the implementation simplicity of having to pass a
lambda, like in scope_exit:

  auto cleanup
    = make_scope_exit ([] { delete_longjmp_breakpoint (0); });
  ... 
  cleanup.release ();

It's simpler to implement, because then all you need for the
template parameter type is a generic callable:

 template <typename Callable>
 class scope_exit
 ...

Note that with Alexandrescu's scope_exit (see
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4152.pdf), if you
don't need to cancel the cleanup, you can write:

 SCOPE_EXIT { delete_longjmp_breakpoint (0); };

which arguably looks cleaner.  Some people prefer avoiding macros
that "extend" the C++ language like that, though, I think.

[1] https://en.cppreference.com/w/cpp/utility/functional/bind

Thanks,
Pedro Alves