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

[Tom Tromey <tom at tromey dot com>]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> See patch below.  Since cleanup_function turned into scope_exit,
Pedro> this new class became forward_scope_exit, because it's a "forwarding"
Pedro> version of scope_exit?  I'm really not that sure about that name...
Pedro> wrap_scope_exit came to mind too.  Or maybe call it cleanup_function?

The name seems reasonable enough to me.

Pedro> Ideally we'd find a way to implement SCOPE_FAILURE/SCOPE_SUCCESS
Pedro> in C++11 instead...

Maybe we should start a C++17 meta-bug to collect future changes.

Pedro> I think this is the first time I had found a use for a function-try-block,
Pedro> see scope_exit ctor...

TIL!

Pedro> Here is patch with everything together.  WDYT?

I think it all looks good.  One nit and one question...

Pedro> +/* A forward_scope_exit is like scope_exit, but instead of giving it a
Pedro> +   callable, you instead specialize it for a given cleanup function,
Pedro> +   and the generated class automatically has a constructor with the

I think it would make sense to explain the origin of the "forward" name
somewhere in this comment.

Pedro> +#define SCOPE_EXIT \
Pedro> +  auto CONCAT(scope_exit_, __LINE__) = ::detail::scope_exit_lhs () + [&] ()
[...]
Pedro> +  auto invalidator
Pedro> +    = make_scope_exit ([&] { this->invalidate (regnum); });
 
In the current spots it doesn't matter, but I tend to think it's better
to capture by value rather than by reference.  The local being captured
might well be reused in the function.

On the other hand, this would be a difference from the spec.

Tom