[RFA,17/22] Remove make_cleanup_restore_current_uiout
Commit Message
This removes make_cleanup_restore_current_uiout in favor of an
RAII-based class.
2016-09-26 Tom Tromey <tom@tromey.com>
* ui-out.c (make_cleanup_restore_current_uiout)
(restore_current_uiout_cleanup): Remove.
* infrun.c (print_stop_event): Use scoped_restore_uiout.
* ui-out.h (scoped_restore_uiout): New class.
(make_cleanup_restore_current_uiout): Don't declare.
---
gdb/ChangeLog | 8 ++++++++
gdb/infrun.c | 15 +++++++--------
gdb/python/python.c | 3 +--
gdb/ui-out.c | 18 ------------------
gdb/ui-out.h | 25 +++++++++++++++++++++++--
5 files changed, 39 insertions(+), 30 deletions(-)
Comments
> +class scoped_restore_uiout
> +{
> + public:
> +
> + scoped_restore_uiout () : saved (current_uiout)
> + {
> + }
> +
> + ~scoped_restore_uiout ()
> + {
> + current_uiout = saved;
> + }
> +
> + private:
> +
> + // No need for these. They are intentionally not defined anywhere.
> + scoped_restore_uiout &operator= (const scoped_restore_uiout &);
> + scoped_restore_uiout (const scoped_restore_uiout &);
> +
> + // The saved ui out.
> + struct ui_out *saved;
isn't this just scoped_restore<ui_out *> ? why do you need a separate
class?
Trev
>>>>> "Trevor" == Trevor Saunders <tbsaunde@tbsaunde.org> writes:
Trevor> isn't this just scoped_restore<ui_out *> ? why do you need a separate
Trevor> class?
In an earlier thread there was a discussion of not having this cleanup
be over-general -- that is, it was intentionally specific to just
current_uiout.
Tom
On 2016-09-29 10:05, Tom Tromey wrote:
>>>>>> "Trevor" == Trevor Saunders <tbsaunde@tbsaunde.org> writes:
>
> Trevor> isn't this just scoped_restore<ui_out *> ? why do you need a
> separate
> Trevor> class?
>
> In an earlier thread there was a discussion of not having this cleanup
> be over-general -- that is, it was intentionally specific to just
> current_uiout.
In that case, may I suggest naming the class
scoped_restore_current_uiout?
IIRC, the original comment was that it was not necessary to have a
parametrized make_cleanup_restore_uiout, when the only uiout we restore
is the current_ui. And I don't think there was a cleanup to restore a
generic pointer, so we had to have a specialized cleanup anyway.
Here we have the option to have one less class by using your
scoped_restore:
scoped_restore<ui_out *> uiout_restore (¤t_uiout,
other_uiout);
vs having a specific one:
scoped_restore_current_uiout uiout_restore;
current_uiout = other_uiout;
I am ok with both methods, it's not a big deal.
Simon
Simon> Here we have the option to have one less class by using your
Simon> scoped_restore:
Simon> scoped_restore<ui_out *> uiout_restore (¤t_uiout,
Simon> other_uiout);
I don't mind if you don't mind. I will make this change.
Tom
On 09/29/2016 03:05 PM, Tom Tromey wrote:
>>>>>> "Trevor" == Trevor Saunders <tbsaunde@tbsaunde.org> writes:
>
> Trevor> isn't this just scoped_restore<ui_out *> ? why do you need a separate
> Trevor> class?
>
> In an earlier thread there was a discussion of not having this cleanup
> be over-general -- that is, it was intentionally specific to just
> current_uiout.
how about:
struct scoped_restore_current_uiout : public scoped_restore<ui_out *>
{
scoped_restore_current_uiout ()
: scoped_restore (¤t_uiout)
{}
};
?
Thanks,
Pedro Alves
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>> In an earlier thread there was a discussion of not having this cleanup
>> be over-general -- that is, it was intentionally specific to just
>> current_uiout.
Pedro> how about:
Pedro> struct scoped_restore_current_uiout : public scoped_restore<ui_out *>
With the make_scoped_restore change it was simpler to just change the
few (one or two? I forget) users to mention current_uiout explicitly.
Tom
On 09/30/2016 10:41 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
>>> In an earlier thread there was a discussion of not having this cleanup
>>> be over-general -- that is, it was intentionally specific to just
>>> current_uiout.
>
> Pedro> how about:
>
> Pedro> struct scoped_restore_current_uiout : public scoped_restore<ui_out *>
>
> With the make_scoped_restore change it was simpler to just change the
> few (one or two? I forget) users to mention current_uiout explicitly.
OK.
Thanks,
Pedro Alves
@@ -1,5 +1,13 @@
2016-09-26 Tom Tromey <tom@tromey.com>
+ * ui-out.c (make_cleanup_restore_current_uiout)
+ (restore_current_uiout_cleanup): Remove.
+ * infrun.c (print_stop_event): Use scoped_restore_uiout.
+ * ui-out.h (scoped_restore_uiout): New class.
+ (make_cleanup_restore_current_uiout): Don't declare.
+
+2016-09-26 Tom Tromey <tom@tromey.com>
+
* elfread.c (elf_read_minimal_symbols): Use std::vector.
2016-09-26 Tom Tromey <tom@tromey.com>
@@ -8084,22 +8084,21 @@ print_stop_location (struct target_waitstatus *ws)
void
print_stop_event (struct ui_out *uiout)
{
- struct cleanup *old_chain;
struct target_waitstatus last;
ptid_t last_ptid;
struct thread_info *tp;
get_last_target_status (&last_ptid, &last);
- old_chain = make_cleanup_restore_current_uiout ();
- current_uiout = uiout;
-
- print_stop_location (&last);
+ {
+ scoped_restore_uiout save_uiout;
+ current_uiout = uiout;
- /* Display the auto-display expressions. */
- do_displays ();
+ print_stop_location (&last);
- do_cleanups (old_chain);
+ /* Display the auto-display expressions. */
+ do_displays ();
+ }
tp = inferior_thread ();
if (tp->thread_fsm != NULL
@@ -647,8 +647,7 @@ execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
struct interp *interp;
scoped_restore<int> save_async (¤t_ui->async, 0);
-
- make_cleanup_restore_current_uiout ();
+ scoped_restore_uiout save_uiout;
/* Use the console interpreter uiout to have the same print format
for console or MI. */
@@ -953,24 +953,6 @@ ui_out_destroy (struct ui_out *uiout)
xfree (uiout);
}
-/* Cleanup that restores a previous current uiout. */
-
-static void
-restore_current_uiout_cleanup (void *arg)
-{
- struct ui_out *saved_uiout = (struct ui_out *) arg;
-
- current_uiout = saved_uiout;
-}
-
-/* See ui-out.h. */
-
-struct cleanup *
-make_cleanup_restore_current_uiout (void)
-{
- return make_cleanup (restore_current_uiout_cleanup, current_uiout);
-}
-
/* Standard gdb initialization hook. */
void
@@ -247,8 +247,29 @@ extern void ui_out_destroy (struct ui_out *uiout);
extern int ui_out_redirect (struct ui_out *uiout, struct ui_file *outstream);
-/* Make a cleanup that restores the previous current uiout. */
+/* An RAII-based class that saves and restores the current uiout. */
-extern struct cleanup *make_cleanup_restore_current_uiout (void);
+class scoped_restore_uiout
+{
+ public:
+
+ scoped_restore_uiout () : saved (current_uiout)
+ {
+ }
+
+ ~scoped_restore_uiout ()
+ {
+ current_uiout = saved;
+ }
+
+ private:
+
+ // No need for these. They are intentionally not defined anywhere.
+ scoped_restore_uiout &operator= (const scoped_restore_uiout &);
+ scoped_restore_uiout (const scoped_restore_uiout &);
+
+ // The saved ui out.
+ struct ui_out *saved;
+};
#endif /* UI_OUT_H */