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: [RFA 03/22] Use scoped_restore for ui_file


>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon>   shared_ptr< scoped_restore<ui_file *> > save_file;

Why shared_ptr and not unique_ptr?

Simon> We can't use std::shared_ptr, since it's only in c++11, but I think
Simon> it's just a matter of time before we define our own version of it.

... Pedro's branch has unique_ptr :)

Simon> An alternative would be to have a default constructor for
Simon> scoped_restore, that creates an inactive scoped_restore, and then
Simon> assign it a variable to restore later with acquire(T* var)/acquire(T*
Simon> var, T value) methods or something.  I am not sure which one is
Simon> better.

FWIW Mozilla uses an Option class for this kind of thing.
It works like:

   Option< scoped_restore<ui_file *> > save_file;
   if (mumble) {
     save_file.emplace (make_scoped_restore (&var));
   }

The main advantage of this over *_ptr is that Option contains the
object, so no heap allocations are required.

Simon> To support some use cases where discard_cleanups is used, we might
Simon> need a way to "release" the scoped_restore, which would essentially
Simon> cancel it.  So if we have a "release" method, maybe having the
Simon> symmetrical "acquire" would make sense.

Simon> What do you think?

It seems sensible to me.

One idea to consider is whether it's better to have a separate
"discardable" variant of scoped_restore (or whatever else); the idea
being that then the type name serves as a signal to look more closely at
the logic.  I think most spots don't need discardable cleanups.

Tom


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