This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA 03/22] Use scoped_restore for ui_file
- From: Simon Marchi <simon dot marchi at polymtl dot ca>
- To: Tom Tromey <tom at tromey dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Sat, 01 Oct 2016 07:47:13 -0400
- Subject: Re: [RFA 03/22] Use scoped_restore for ui_file
- Authentication-results: sourceware.org; auth=none
- References: <1474949330-4307-1-git-send-email-tom@tromey.com> <1474949330-4307-4-git-send-email-tom@tromey.com> <7417a71f254c7f42026408c5e143b374@simark.ca> <87bmz4fxtt.fsf@tromey.com>
On 2016-10-01 01:22, Tom Tromey wrote:
"Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
Simon> shared_ptr< scoped_restore<ui_file *> > save_file;
Why shared_ptr and not unique_ptr?
Right, I am not used yet to think about their differences, and which one
applies to a particular situation. unique_ptr is clearler a better
choice.
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 :)
Oh, right!
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.
That sounds good, it ends up with the same behavior, but implemented
better.
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.
Yep, alsop sounds good.