This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


> This is the series to introduce a new cleanup_function class, that can
> be used in situations where a simple function call is needed to do
> some cleanup.
> 
> As I mentioned in the other thread, I was (and still am) uncertain as
> to whether this is a good idea.  My worry is just that this will be
> used in ways leading to unclear code.  (You can judge for yourself
> whether this series has already entered that territory...)
> 
> A couple of rules I used when deciding when to use the new class:
> 
> * There should not be too many uses of the existing cleanup (one or
>   two).  If there are more than several it should probably be a
>   bespoke class.
> 
> * Any lambdas should capture only a small number of things (one or
>   maybe two), and only capture by value should be used.
> 
> Hopefully I actually followed these.
> 
> I wonder if the new class should take a std::function rather than a
> gdb::function_view.  Some of the changes needs a bit of extra code to
> deal with the latter.
> 
> Also I wonder if it should be possible to default-construct a
> cleanup_function, and then set it later.  This would eliminate some
> uses of gdb::optional; and seemed maybe reasonable given that the
> class already has to maintain a (sort of-) validity flag.
> 
> This series includes a couple of cleanup-related comment fixes for
> things I happened to notice along the way.

I don't know C++ well enough to have an opinion on the general
questions asked here, but FWIW, the patch series in itself seems
like a nice improvement already (compared to before).

-- 
Joel


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]