[Lldb-commits] [PATCH] D43662: [Utility] Simplify and generalize the CleanUp helper, NFC

[Vedant Kumar via Phabricator via lldb-commits]

vsk added a comment.

> Couldn't we just add a creator helper so that you can say:
> 
> auto cleanup = makeCleanupRAII([&] {
> 
>     process_sp->Destroy(/*force_kill=*/false);
>     debugger.StopEventHandlerThread();
>   });

@zturner In this version of the patch, CleanUp accepts a std::function, which gets rid of the lifetime issue with llvm::function_ref. So with this you should bee able to write "auto cleanup = CleanUp([&] { ... })". Does that work for you?

> What do you think about a syntax like:
> 
> lldb_utility::CleanUp cleanup_our(::close, our_socket); // The CleanUp constructor creates the std::function internally

@labath I find the current version of the code more readable, since the behavior of the cleanup is apparent in less time. And I'd prefer to keep the CleanUp utility very small, without any scope for expansion. I don't think the ergonomics of the new syntax outweigh the advantages of a simpler approach.

> we should also move the CleanUp class out of the lldb_utility and into lldb_private namespace

Sounds good, I'll update the patch once we hash out the interface.


https://reviews.llvm.org/D43662