Re: [RFA 03/22] Use scoped_restore for ui_file

Tom Tromey
2016-10-01T05:22:25Z

>>>>> "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