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

Vedant Kumar via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Thu Feb 22 18:33:12 PST 2018


vsk added a comment.

In https://reviews.llvm.org/D43662#1016950, @labath wrote:

> In https://reviews.llvm.org/D43662#1016939, @vsk wrote:
>
> > > 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.
>
>
> I don't think this would complicate anything. It should literally be a matter of replacing your constructor with:
>
>   template<typename F, typename... Args>
>   CleanUp(F &&f, Args &&..args)
>       : PerformCleanup(true), Clean(std::bind(std::forward<F>(f), std::forward<Args>(args)...) {}
>   
>
> Although I can see how you may find the other usage syntax more understandable, so I am not adamant about this...


Thanks for sharing this (I haven't reached for parameter packs before :). I'll still maintain that having the lambda written-out explicitly requires less effort from prospective readers, but am happy to be outvoted on this one. Pavel, others, wdyt?


https://reviews.llvm.org/D43662





More information about the lldb-commits mailing list