[RFA 0/7] more ui-out cleanup removal

[RFA 0/7] more ui-out cleanup removal

[Tom Tromey]

This series removes more ui-out related cleanups.
make_cleanup_ui_out_table_begin_end and
make_cleanup_ui_out_redirect_pop are completely removed.

The remaining uses of ui-out cleanups seem tricky to remove, as the
code using them is very convoluted.

Regression tested by the buildbot.

Tom

[RFA 6/7] Remove make_cleanup_ui_out_redirect_pop

[Tom Tromey]

This patch introduces ui_out_redirect_pop.  All uses of
make_cleanup_ui_out_redirect_pop are replaced with this new class.

ChangeLog
2017-09-09  Tom Tromey  <tom@tromey.com>

	* mi/mi-interp.c (mi_user_selected_context_changed): Use
	ui_out_redirect_pop.
	* guile/scm-ports.c (ioscm_with_output_to_port_worker): Use
	ui_out_redirect_pop.
	* utils.c (do_ui_out_redirect_pop)
	(make_cleanup_ui_out_redirect_pop): Remove.
	* top.c (execute_command_to_string): Use ui_out_redirect_pop.
	* utils.h (make_cleanup_ui_out_redirect_pop): Remove.
	* ui-out.h (ui_out_redirect_pop): New class.
---
 gdb/ChangeLog         | 12 ++++++++++++
 gdb/guile/scm-ports.c | 22 +++++++++++++---------
 gdb/mi/mi-interp.c    |  5 ++---
 gdb/top.c             | 32 +++++++++++++++++---------------
 gdb/ui-out.h          | 23 +++++++++++++++++++++++
 gdb/utils.c           | 19 -------------------
 gdb/utils.h           |  4 ----
 7 files changed, 67 insertions(+), 50 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a8c1642..cc8e4e4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,17 @@
 2017-09-09  Tom Tromey  <tom@tromey.com>
 
+	* mi/mi-interp.c (mi_user_selected_context_changed): Use
+	ui_out_redirect_pop.
+	* guile/scm-ports.c (ioscm_with_output_to_port_worker): Use
+	ui_out_redirect_pop.
+	* utils.c (do_ui_out_redirect_pop)
+	(make_cleanup_ui_out_redirect_pop): Remove.
+	* top.c (execute_command_to_string): Use ui_out_redirect_pop.
+	* utils.h (make_cleanup_ui_out_redirect_pop): Remove.
+	* ui-out.h (ui_out_redirect_pop): New class.
+
+2017-09-09  Tom Tromey  <tom@tromey.com>
+
 	* mi/mi-main.c (output_cores): Use ui_out_emit_list.
 	(list_available_thread_groups, mi_cmd_list_thread_groups)
 	(mi_cmd_data_list_changed_registers, mi_cmd_data_read_memory)
diff --git a/gdb/guile/scm-ports.c b/gdb/guile/scm-ports.c
index 735abc2..78187c4 100644
--- a/gdb/guile/scm-ports.c
+++ b/gdb/guile/scm-ports.c
@@ -26,6 +26,7 @@
 #include "top.h"
 #include "target.h"
 #include "guile-internal.h"
+#include "common/gdb_optional.h"
 
 #ifdef HAVE_POLL
 #if defined (HAVE_POLL_H)
@@ -477,17 +478,20 @@ ioscm_with_output_to_port_worker (SCM port, SCM thunk, enum oport oport,
   scoped_restore save_file = make_scoped_restore (oport == GDB_STDERR
 						  ? &gdb_stderr : &gdb_stdout);
 
-  if (oport == GDB_STDERR)
-    gdb_stderr = port_file.get ();
-  else
-    {
-      current_uiout->redirect (port_file.get ());
-      make_cleanup_ui_out_redirect_pop (current_uiout);
+  {
+    gdb::optional<ui_out_redirect_pop> redirect_popper;
+    if (oport == GDB_STDERR)
+      gdb_stderr = port_file.get ();
+    else
+      {
+	current_uiout->redirect (port_file.get ());
+	redirect_popper.emplace (current_uiout);
 
-      gdb_stdout = port_file.get ();
-    }
+	gdb_stdout = port_file.get ();
+      }
 
-  result = gdbscm_safe_call_0 (thunk, NULL);
+    result = gdbscm_safe_call_0 (thunk, NULL);
+  }
 
   do_cleanups (cleanups);
 
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 6b6d8f0..1331860 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -1314,10 +1314,9 @@ mi_user_selected_context_changed (user_selected_what selection)
       mi_uiout = interp_ui_out (top_level_interpreter ());
 
       mi_uiout->redirect (mi->event_channel);
+      ui_out_redirect_pop redirect_popper (mi_uiout);
 
-      old_chain = make_cleanup_ui_out_redirect_pop (mi_uiout);
-
-      make_cleanup_restore_target_terminal ();
+      old_chain = make_cleanup_restore_target_terminal ();
       target_terminal_ours_for_output ();
 
       if (selection & USER_SELECTED_INFERIOR)
diff --git a/gdb/top.c b/gdb/top.c
index a4fd262..742c1e7 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -679,21 +679,23 @@ execute_command_to_string (char *p, int from_tty)
 
   string_file str_file;
 
-  current_uiout->redirect (&str_file);
-  make_cleanup_ui_out_redirect_pop (current_uiout);
-
-  scoped_restore save_stdout
-    = make_scoped_restore (&gdb_stdout, &str_file);
-  scoped_restore save_stderr
-    = make_scoped_restore (&gdb_stderr, &str_file);
-  scoped_restore save_stdlog
-    = make_scoped_restore (&gdb_stdlog, &str_file);
-  scoped_restore save_stdtarg
-    = make_scoped_restore (&gdb_stdtarg, &str_file);
-  scoped_restore save_stdtargerr
-    = make_scoped_restore (&gdb_stdtargerr, &str_file);
-
-  execute_command (p, from_tty);
+  {
+    current_uiout->redirect (&str_file);
+    ui_out_redirect_pop redirect_popper (current_uiout);
+
+    scoped_restore save_stdout
+      = make_scoped_restore (&gdb_stdout, &str_file);
+    scoped_restore save_stderr
+      = make_scoped_restore (&gdb_stderr, &str_file);
+    scoped_restore save_stdlog
+      = make_scoped_restore (&gdb_stdlog, &str_file);
+    scoped_restore save_stdtarg
+      = make_scoped_restore (&gdb_stdtarg, &str_file);
+    scoped_restore save_stdtargerr
+      = make_scoped_restore (&gdb_stdtargerr, &str_file);
+
+    execute_command (p, from_tty);
+  }
 
   do_cleanups (cleanup);
 
diff --git a/gdb/ui-out.h b/gdb/ui-out.h
index d983837..9ed2bd2 100644
--- a/gdb/ui-out.h
+++ b/gdb/ui-out.h
@@ -242,4 +242,27 @@ private:
   struct ui_out *m_uiout;
 };
 
+/* On destruction, pop the last redirection by calling the uiout's
+   redirect method with a NULL parameter.  */
+class ui_out_redirect_pop
+{
+public:
+
+  ui_out_redirect_pop (ui_out *uiout)
+    : m_uiout (uiout)
+  {
+  }
+
+  ~ui_out_redirect_pop ()
+  {
+    m_uiout->redirect (NULL);
+  }
+
+  ui_out_redirect_pop (const ui_out_redirect_pop &) = delete;
+  ui_out_redirect_pop &operator= (const ui_out_redirect_pop &) = delete;
+
+private:
+  struct ui_out *m_uiout;
+};
+
 #endif /* UI_OUT_H */
diff --git a/gdb/utils.c b/gdb/utils.c
index f2da2df..1941e7c 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -137,25 +137,6 @@ show_pagination_enabled (struct ui_file *file, int from_tty,
    because while they use the "cleanup API" they are not part of the
    "cleanup API".  */
 
-/* Helper function for make_cleanup_ui_out_redirect_pop.  */
-
-static void
-do_ui_out_redirect_pop (void *arg)
-{
-  struct ui_out *uiout = (struct ui_out *) arg;
-
-  uiout->redirect (NULL);
-}
-
-/* Return a new cleanup that pops the last redirection by ui_out_redirect
-   with NULL parameter.  */
-
-struct cleanup *
-make_cleanup_ui_out_redirect_pop (struct ui_out *uiout)
-{
-  return make_cleanup (do_ui_out_redirect_pop, uiout);
-}
-
 static void
 do_free_section_addr_info (void *arg)
 {
diff --git a/gdb/utils.h b/gdb/utils.h
index 1c8b95c..6d33e8d 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -205,10 +205,6 @@ private:
 \f
 /* Cleanup utilities.  */
 
-struct ui_out;
-extern struct cleanup *
-  make_cleanup_ui_out_redirect_pop (struct ui_out *uiout);
-
 struct section_addr_info;
 extern struct cleanup *(make_cleanup_free_section_addr_info 
                         (struct section_addr_info *));
-- 
2.9.4

[RFA 4/7] Use ui_out_emit_tuple in disasm.c

[Tom Tromey]

This changes one spot in disasm.c to use ui_out_emit_tuple.  This
patch required a large reindentation, so I've separated it out.

ChangeLog
2017-09-09  Tom Tromey  <tom@tromey.com>

	* disasm.c (gdb_pretty_print_disassembler::pretty_print_insn): Use
	ui_out_emit_tuple.
---
 gdb/ChangeLog |   5 ++
 gdb/disasm.c  | 164 +++++++++++++++++++++++++++++-----------------------------
 2 files changed, 87 insertions(+), 82 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a52b1fb..bef61b4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2017-09-09  Tom Tromey  <tom@tromey.com>
 
+	* disasm.c (gdb_pretty_print_disassembler::pretty_print_insn): Use
+	ui_out_emit_tuple.
+
+2017-09-09  Tom Tromey  <tom@tromey.com>
+
 	* target.c (flash_erase_command): Use ui_out_emit_tuple.
 	* stack.c (print_frame): Use ui_out_emit_tuple.
 	* spu-tdep.c (info_spu_event_command): Use ui_out_emit_tuple.
diff --git a/gdb/disasm.c b/gdb/disasm.c
index c3528e0..ad596f9 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -198,95 +198,95 @@ gdb_pretty_print_disassembler::pretty_print_insn (struct ui_out *uiout,
   int offset;
   int line;
   int size;
-  struct cleanup *ui_out_chain;
   char *filename = NULL;
   char *name = NULL;
   CORE_ADDR pc;
   struct gdbarch *gdbarch = arch ();
 
-  ui_out_chain = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
-  pc = insn->addr;
-
-  if (insn->number != 0)
-    {
-      uiout->field_fmt ("insn-number", "%u", insn->number);
-      uiout->text ("\t");
-    }
-
-  if ((flags & DISASSEMBLY_SPECULATIVE) != 0)
-    {
-      if (insn->is_speculative)
-	{
-	  uiout->field_string ("is-speculative", "?");
-
-	  /* The speculative execution indication overwrites the first
-	     character of the PC prefix.
-	     We assume a PC prefix length of 3 characters.  */
-	  if ((flags & DISASSEMBLY_OMIT_PC) == 0)
-	    uiout->text (pc_prefix (pc) + 1);
-	  else
-	    uiout->text ("  ");
-	}
-      else if ((flags & DISASSEMBLY_OMIT_PC) == 0)
-	uiout->text (pc_prefix (pc));
-      else
-	uiout->text ("   ");
-    }
-  else if ((flags & DISASSEMBLY_OMIT_PC) == 0)
-    uiout->text (pc_prefix (pc));
-  uiout->field_core_addr ("address", gdbarch, pc);
-
-  if (!build_address_symbolic (gdbarch, pc, 0, &name, &offset, &filename,
-			       &line, &unmapped))
-    {
-      /* We don't care now about line, filename and unmapped.  But we might in
-	 the future.  */
-      uiout->text (" <");
-      if ((flags & DISASSEMBLY_OMIT_FNAME) == 0)
-	uiout->field_string ("func-name", name);
-      uiout->text ("+");
-      uiout->field_int ("offset", offset);
-      uiout->text (">:\t");
-    }
-  else
-    uiout->text (":\t");
-
-  if (filename != NULL)
-    xfree (filename);
-  if (name != NULL)
-    xfree (name);
-
-  m_insn_stb.clear ();
-
-  if (flags & DISASSEMBLY_RAW_INSN)
-    {
-      CORE_ADDR end_pc;
-      bfd_byte data;
-      int err;
-      const char *spacer = "";
-
-      /* Build the opcodes using a temporary stream so we can
-	 write them out in a single go for the MI.  */
-      m_opcode_stb.clear ();
-
+  {
+    ui_out_emit_tuple tuple_emitter (uiout, NULL);
+    pc = insn->addr;
+
+    if (insn->number != 0)
+      {
+	uiout->field_fmt ("insn-number", "%u", insn->number);
+	uiout->text ("\t");
+      }
+
+    if ((flags & DISASSEMBLY_SPECULATIVE) != 0)
+      {
+	if (insn->is_speculative)
+	  {
+	    uiout->field_string ("is-speculative", "?");
+
+	    /* The speculative execution indication overwrites the first
+	       character of the PC prefix.
+	       We assume a PC prefix length of 3 characters.  */
+	    if ((flags & DISASSEMBLY_OMIT_PC) == 0)
+	      uiout->text (pc_prefix (pc) + 1);
+	    else
+	      uiout->text ("  ");
+	  }
+	else if ((flags & DISASSEMBLY_OMIT_PC) == 0)
+	  uiout->text (pc_prefix (pc));
+	else
+	  uiout->text ("   ");
+      }
+    else if ((flags & DISASSEMBLY_OMIT_PC) == 0)
+      uiout->text (pc_prefix (pc));
+    uiout->field_core_addr ("address", gdbarch, pc);
+
+    if (!build_address_symbolic (gdbarch, pc, 0, &name, &offset, &filename,
+				 &line, &unmapped))
+      {
+	/* We don't care now about line, filename and unmapped.  But we might in
+	   the future.  */
+	uiout->text (" <");
+	if ((flags & DISASSEMBLY_OMIT_FNAME) == 0)
+	  uiout->field_string ("func-name", name);
+	uiout->text ("+");
+	uiout->field_int ("offset", offset);
+	uiout->text (">:\t");
+      }
+    else
+      uiout->text (":\t");
+
+    if (filename != NULL)
+      xfree (filename);
+    if (name != NULL)
+      xfree (name);
+
+    m_insn_stb.clear ();
+
+    if (flags & DISASSEMBLY_RAW_INSN)
+      {
+	CORE_ADDR end_pc;
+	bfd_byte data;
+	int err;
+	const char *spacer = "";
+
+	/* Build the opcodes using a temporary stream so we can
+	   write them out in a single go for the MI.  */
+	m_opcode_stb.clear ();
+
+	size = m_di.print_insn (pc);
+	end_pc = pc + size;
+
+	for (;pc < end_pc; ++pc)
+	  {
+	    read_code (pc, &data, 1);
+	    m_opcode_stb.printf ("%s%02x", spacer, (unsigned) data);
+	    spacer = " ";
+	  }
+
+	uiout->field_stream ("opcodes", m_opcode_stb);
+	uiout->text ("\t");
+      }
+    else
       size = m_di.print_insn (pc);
-      end_pc = pc + size;
 
-      for (;pc < end_pc; ++pc)
-	{
-	  read_code (pc, &data, 1);
-	  m_opcode_stb.printf ("%s%02x", spacer, (unsigned) data);
-	  spacer = " ";
-	}
-
-      uiout->field_stream ("opcodes", m_opcode_stb);
-      uiout->text ("\t");
-    }
-  else
-    size = m_di.print_insn (pc);
-
-  uiout->field_stream ("inst", m_insn_stb);
-  do_cleanups (ui_out_chain);
+    uiout->field_stream ("inst", m_insn_stb);
+  }
   uiout->text ("\n");
 
   return size;
-- 
2.9.4

[RFA 5/7] Use ui_out_emit_list in more places

[Tom Tromey]

This changes various spots to use ui_out_emit_list, removing some
cleanups.

ChangeLog
2017-09-09  Tom Tromey  <tom@tromey.com>

	* mi/mi-main.c (output_cores): Use ui_out_emit_list.
	(list_available_thread_groups, mi_cmd_list_thread_groups)
	(mi_cmd_data_list_changed_registers, mi_cmd_data_read_memory)
	(mi_cmd_data_read_memory_bytes, mi_cmd_trace_frame_collected):
	Likewise.
---
 gdb/ChangeLog    |  8 ++++++
 gdb/mi/mi-main.c | 82 ++++++++++++++++++++++++++------------------------------
 2 files changed, 46 insertions(+), 44 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index bef61b4..a8c1642 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2017-09-09  Tom Tromey  <tom@tromey.com>
 
+	* mi/mi-main.c (output_cores): Use ui_out_emit_list.
+	(list_available_thread_groups, mi_cmd_list_thread_groups)
+	(mi_cmd_data_list_changed_registers, mi_cmd_data_read_memory)
+	(mi_cmd_data_read_memory_bytes, mi_cmd_trace_frame_collected):
+	Likewise.
+
+2017-09-09  Tom Tromey  <tom@tromey.com>
+
 	* disasm.c (gdb_pretty_print_disassembler::pretty_print_insn): Use
 	ui_out_emit_tuple.
 
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index ca66a77..feea452 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -714,17 +714,12 @@ print_one_inferior (struct inferior *inferior, void *xdata)
 static void
 output_cores (struct ui_out *uiout, const char *field_name, const char *xcores)
 {
-  struct cleanup *back_to = make_cleanup_ui_out_list_begin_end (uiout,
-								field_name);
-  char *cores = xstrdup (xcores);
-  char *p = cores;
-
-  make_cleanup (xfree, cores);
+  ui_out_emit_list list_emitter (uiout, field_name);
+  gdb::unique_xmalloc_ptr<char> cores (xstrdup (xcores));
+  char *p = cores.get ();
 
   for (p = strtok (p, ","); p;  p = strtok (NULL, ","))
     uiout->field_string (NULL, p);
-
-  do_cleanups (back_to);
 }
 
 static void
@@ -818,7 +813,7 @@ list_available_thread_groups (VEC (int) *ids, int recurse)
 	}
     }
 
-  make_cleanup_ui_out_list_begin_end (uiout, "groups");
+  ui_out_emit_list list_emitter (uiout, "groups");
 
   for (ix_items = 0;
        VEC_iterate (osdata_item_s, data->items,
@@ -861,7 +856,7 @@ list_available_thread_groups (VEC (int) *ids, int recurse)
 	      struct osdata_item *child;
 	      int ix_child;
 
-	      make_cleanup_ui_out_list_begin_end (uiout, "threads");
+	      ui_out_emit_list thread_list_emitter (uiout, "threads");
 
 	      for (ix_child = 0;
 		   VEC_iterate (osdata_item_s, children, ix_child, child);
@@ -976,7 +971,7 @@ mi_cmd_list_thread_groups (const char *command, char **argv, int argc)
 	 print everything, or several explicit ids.  In both cases,
 	 we print more than one group, and have to use 'groups'
 	 as the top-level element.  */
-      make_cleanup_ui_out_list_begin_end (uiout, "groups");
+      ui_out_emit_list list_emitter (uiout, "groups");
       update_thread_list ();
       iterate_over_inferiors (print_one_inferior, &data);
     }
@@ -1063,7 +1058,7 @@ mi_cmd_data_list_changed_registers (const char *command, char **argv, int argc)
   gdbarch = get_regcache_arch (this_regs);
   numregs = gdbarch_num_regs (gdbarch) + gdbarch_num_pseudo_regs (gdbarch);
 
-  make_cleanup_ui_out_list_begin_end (uiout, "changed-registers");
+  ui_out_emit_list list_emitter (uiout, "changed-registers");
 
   if (argc == 0)
     {
@@ -1490,43 +1485,43 @@ mi_cmd_data_read_memory (const char *command, char **argv, int argc)
   {
     int row;
     int row_byte;
-    struct cleanup *cleanup_list;
 
     string_file stream;
 
-    cleanup_list = make_cleanup_ui_out_list_begin_end (uiout, "memory");
+    ui_out_emit_list list_emitter (uiout, "memory");
     for (row = 0, row_byte = 0;
 	 row < nr_rows;
 	 row++, row_byte += nr_cols * word_size)
       {
 	int col;
 	int col_byte;
-	struct cleanup *cleanup_list_data;
 	struct value_print_options opts;
 
 	ui_out_emit_tuple tuple_emitter (uiout, NULL);
 	uiout->field_core_addr ("addr", gdbarch, addr + row_byte);
 	/* ui_out_field_core_addr_symbolic (uiout, "saddr", addr +
 	   row_byte); */
-	cleanup_list_data = make_cleanup_ui_out_list_begin_end (uiout, "data");
-	get_formatted_print_options (&opts, word_format);
-	for (col = 0, col_byte = row_byte;
-	     col < nr_cols;
-	     col++, col_byte += word_size)
-	  {
-	    if (col_byte + word_size > nr_bytes)
-	      {
-		uiout->field_string (NULL, "N/A");
-	      }
-	    else
-	      {
-		stream.clear ();
-		print_scalar_formatted (&mbuf[col_byte], word_type, &opts,
-					word_asize, &stream);
-		uiout->field_stream (NULL, stream);
-	      }
-	  }
-	do_cleanups (cleanup_list_data);
+	{
+	  ui_out_emit_list list_data_emitter (uiout, "data");
+	  get_formatted_print_options (&opts, word_format);
+	  for (col = 0, col_byte = row_byte;
+	       col < nr_cols;
+	       col++, col_byte += word_size)
+	    {
+	      if (col_byte + word_size > nr_bytes)
+		{
+		  uiout->field_string (NULL, "N/A");
+		}
+	      else
+		{
+		  stream.clear ();
+		  print_scalar_formatted (&mbuf[col_byte], word_type, &opts,
+					  word_asize, &stream);
+		  uiout->field_stream (NULL, stream);
+		}
+	    }
+	}
+
 	if (aschar)
 	  {
 	    int byte;
@@ -1545,7 +1540,6 @@ mi_cmd_data_read_memory (const char *command, char **argv, int argc)
 	    uiout->field_stream ("ascii", stream);
 	  }
       }
-    do_cleanups (cleanup_list);
   }
 }
 
@@ -1603,7 +1597,7 @@ mi_cmd_data_read_memory_bytes (const char *command, char **argv, int argc)
   if (VEC_length (memory_read_result_s, result) == 0)
     error (_("Unable to read memory."));
 
-  make_cleanup_ui_out_list_begin_end (uiout, "memory");
+  ui_out_emit_list list_emitter (uiout, "memory");
   for (ix = 0;
        VEC_iterate (memory_read_result_s, result, ix, read_result);
        ++ix)
@@ -2820,15 +2814,15 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
 
   /* Trace state variables.  */
   {
-    struct cleanup *list_cleanup;
+    struct cleanup *cleanups;
     int tvar;
     char *tsvname;
     int i;
 
-    list_cleanup = make_cleanup_ui_out_list_begin_end (uiout, "tvars");
+    ui_out_emit_list list_emitter (uiout, "tvars");
 
     tsvname = NULL;
-    make_cleanup (free_current_contents, &tsvname);
+    cleanups = make_cleanup (free_current_contents, &tsvname);
 
     for (i = 0; VEC_iterate (int, tinfo->tvars, i, tvar); i++)
       {
@@ -2856,20 +2850,20 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
 	  }
       }
 
-    do_cleanups (list_cleanup);
+    do_cleanups (cleanups);
   }
 
   /* Memory.  */
   {
-    struct cleanup *list_cleanup;
+    struct cleanup *cleanups;
     VEC(mem_range_s) *available_memory = NULL;
     struct mem_range *r;
     int i;
 
     traceframe_available_memory (&available_memory, 0, ULONGEST_MAX);
-    make_cleanup (VEC_cleanup(mem_range_s), &available_memory);
+    cleanups = make_cleanup (VEC_cleanup(mem_range_s), &available_memory);
 
-    list_cleanup = make_cleanup_ui_out_list_begin_end (uiout, "memory");
+    ui_out_emit_list list_emitter (uiout, "memory");
 
     for (i = 0; VEC_iterate (mem_range_s, available_memory, i, r); i++)
       {
@@ -2901,7 +2895,7 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
 	  }
       }
 
-    do_cleanups (list_cleanup);
+    do_cleanups (cleanups);
   }
 }
 
-- 
2.9.4

[RFA 1/7] Use ui_out_emit_table and ui_out_emit_list in print_thread_info_1

[Tom Tromey]

This changes print_thread_info_1 to use ui_out_emit_table and
ui_out_emit_list.  Which one is used depends on whether the ui-out is
mi-like; so the emitters are wrapped in gdb::optional.

ChangeLog
2017-09-09  Tom Tromey  <tom@tromey.com>

	* thread.c (print_thread_info_1): Use ui_out_emit_table,
	ui_out_emit_list, gdb::optional.
---
 gdb/ChangeLog |  5 ++++
 gdb/thread.c  | 92 +++++++++++++++++++++++++++++------------------------------
 2 files changed, 51 insertions(+), 46 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 96369d5..b37089e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2017-09-09  Tom Tromey  <tom@tromey.com>
+
+	* thread.c (print_thread_info_1): Use ui_out_emit_table,
+	ui_out_emit_list, gdb::optional.
+
 2017-09-08  Keith Seitz  <keiths@redhat.com>
 
 	* dwarf2read.c (struct field_info) <fnfields>: Remove unused
diff --git a/gdb/thread.c b/gdb/thread.c
index 8a0ed0c..e0ab7f0 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -45,6 +45,7 @@
 #include "thread-fsm.h"
 #include "tid-parse.h"
 #include <algorithm>
+#include "common/gdb_optional.h"
 
 /* Definition of struct thread_info exported to gdbthread.h.  */
 
@@ -1247,55 +1248,55 @@ print_thread_info_1 (struct ui_out *uiout, char *requested_threads,
   update_thread_list ();
   current_ptid = inferior_ptid;
 
-  struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
-
-  /* For backward compatibility, we make a list for MI.  A table is
-     preferable for the CLI, though, because it shows table
-     headers.  */
-  if (uiout->is_mi_like_p ())
-    make_cleanup_ui_out_list_begin_end (uiout, "threads");
-  else
-    {
-      int n_threads = 0;
+  {
+    /* For backward compatibility, we make a list for MI.  A table is
+       preferable for the CLI, though, because it shows table
+       headers.  */
+    gdb::optional<ui_out_emit_list> list_emitter;
+    gdb::optional<ui_out_emit_table> table_emitter;
+
+    if (uiout->is_mi_like_p ())
+      list_emitter.emplace (uiout, "threads");
+    else
+      {
+	int n_threads = 0;
 
-      for (tp = thread_list; tp; tp = tp->next)
-	{
-	  if (!should_print_thread (requested_threads, default_inf_num,
-				    global_ids, pid, tp))
-	    continue;
+	for (tp = thread_list; tp; tp = tp->next)
+	  {
+	    if (!should_print_thread (requested_threads, default_inf_num,
+				      global_ids, pid, tp))
+	      continue;
 
-	  ++n_threads;
-	}
+	    ++n_threads;
+	  }
 
-      if (n_threads == 0)
-	{
-	  if (requested_threads == NULL || *requested_threads == '\0')
-	    uiout->message (_("No threads.\n"));
-	  else
-	    uiout->message (_("No threads match '%s'.\n"),
-			    requested_threads);
-	  do_cleanups (old_chain);
-	  return;
-	}
+	if (n_threads == 0)
+	  {
+	    if (requested_threads == NULL || *requested_threads == '\0')
+	      uiout->message (_("No threads.\n"));
+	    else
+	      uiout->message (_("No threads match '%s'.\n"),
+			      requested_threads);
+	    return;
+	  }
 
-      if (show_global_ids || uiout->is_mi_like_p ())
-	make_cleanup_ui_out_table_begin_end (uiout, 5, n_threads, "threads");
-      else
-	make_cleanup_ui_out_table_begin_end (uiout, 4, n_threads, "threads");
+	table_emitter.emplace (uiout,
+			       (show_global_ids || uiout->is_mi_like_p ())
+			       ? 5 : 4,
+			       n_threads, "threads");
 
-      uiout->table_header (1, ui_left, "current", "");
+	uiout->table_header (1, ui_left, "current", "");
 
-      if (!uiout->is_mi_like_p ())
-	uiout->table_header (4, ui_left, "id-in-tg", "Id");
-      if (show_global_ids || uiout->is_mi_like_p ())
-	uiout->table_header (4, ui_left, "id", "GId");
-      uiout->table_header (17, ui_left, "target-id", "Target Id");
-      uiout->table_header (1, ui_left, "frame", "Frame");
-      uiout->table_body ();
-    }
+	if (!uiout->is_mi_like_p ())
+	  uiout->table_header (4, ui_left, "id-in-tg", "Id");
+	if (show_global_ids || uiout->is_mi_like_p ())
+	  uiout->table_header (4, ui_left, "id", "GId");
+	uiout->table_header (17, ui_left, "target-id", "Target Id");
+	uiout->table_header (1, ui_left, "frame", "Frame");
+	uiout->table_body ();
+      }
 
-  /* We'll be switching threads temporarily.  */
-  {
+    /* We'll be switching threads temporarily.  */
     scoped_restore_current_thread restore_thread;
 
     ALL_THREADS_BY_INFERIOR (inf, tp)
@@ -1383,14 +1384,13 @@ print_thread_info_1 (struct ui_out *uiout, char *requested_threads,
 	core = target_core_of_thread (tp->ptid);
 	if (uiout->is_mi_like_p () && core != -1)
 	  uiout->field_int ("core", core);
-    }
+      }
 
     /* This end scope restores the current thread and the frame
-       selected before the "info threads" command.  */
+       selected before the "info threads" command, and it finishes the
+       ui-out list or table.  */
   }
 
-  do_cleanups (old_chain);
-
   if (pid == -1 && requested_threads == NULL)
     {
       if (uiout->is_mi_like_p ()
-- 
2.9.4

[RFA 3/7] Use ui_out_emit_tuple in more places

[Tom Tromey]

This changes more places to use ui_out_emit_tuple, removing cleanups.

ChangeLog
2017-09-09  Tom Tromey  <tom@tromey.com>

	* target.c (flash_erase_command): Use ui_out_emit_tuple.
	* stack.c (print_frame): Use ui_out_emit_tuple.
	* spu-tdep.c (info_spu_event_command): Use ui_out_emit_tuple.
	(info_spu_mailbox_command, info_spu_dma_command)
	(info_spu_proxydma_command): Likewise.
	* mi/mi-main.c (mi_cmd_trace_frame_collected): Use
	ui_out_emit_tuple, gdb::byte_vector, gdb;:unique_xmalloc_ptr.
	* mi/mi-cmd-file.c (mi_cmd_file_list_shared_libraries): Use
	ui_out_emit_tuple.
	* breakpoint.c (print_it_watchpoint): Use ui_out_emit_tuple.
---
 gdb/ChangeLog        |  13 ++++
 gdb/breakpoint.c     |  14 ++---
 gdb/mi/mi-cmd-file.c |   5 +-
 gdb/mi/mi-main.c     |  20 +++---
 gdb/spu-tdep.c       |  24 ++-----
 gdb/stack.c          | 174 +++++++++++++++++++++++++--------------------------
 gdb/target.c         |   5 +-
 7 files changed, 121 insertions(+), 134 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4c9c419..a52b1fb 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,18 @@
 2017-09-09  Tom Tromey  <tom@tromey.com>
 
+	* target.c (flash_erase_command): Use ui_out_emit_tuple.
+	* stack.c (print_frame): Use ui_out_emit_tuple.
+	* spu-tdep.c (info_spu_event_command): Use ui_out_emit_tuple.
+	(info_spu_mailbox_command, info_spu_dma_command)
+	(info_spu_proxydma_command): Likewise.
+	* mi/mi-main.c (mi_cmd_trace_frame_collected): Use
+	ui_out_emit_tuple, gdb::byte_vector, gdb;:unique_xmalloc_ptr.
+	* mi/mi-cmd-file.c (mi_cmd_file_list_shared_libraries): Use
+	ui_out_emit_tuple.
+	* breakpoint.c (print_it_watchpoint): Use ui_out_emit_tuple.
+
+2017-09-09  Tom Tromey  <tom@tromey.com>
+
 	* ui-out.h (make_cleanup_ui_out_table_begin_end): Remove.
 	(class ui_out_emit_table): Update comment.
 	* ui-out.c (do_cleanup_table_end)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 778a011..2916470 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -82,6 +82,7 @@
 #include <algorithm>
 #include "progspace-and-thread.h"
 #include "common/array-view.h"
+#include "common/gdb_optional.h"
 
 /* Enums for exception-handling support.  */
 enum exception_event_kind
@@ -10538,7 +10539,6 @@ works_in_software_mode_watchpoint (const struct breakpoint *b)
 static enum print_stop_action
 print_it_watchpoint (bpstat bs)
 {
-  struct cleanup *old_chain;
   struct breakpoint *b;
   enum print_stop_action result;
   struct watchpoint *w;
@@ -10549,13 +10549,12 @@ print_it_watchpoint (bpstat bs)
   b = bs->breakpoint_at;
   w = (struct watchpoint *) b;
 
-  old_chain = make_cleanup (null_cleanup, NULL);
-
   annotate_watchpoint (b->number);
   maybe_print_thread_hit_breakpoint (uiout);
 
   string_file stb;
 
+  gdb::optional<ui_out_emit_tuple> tuple_emitter;
   switch (b->type)
     {
     case bp_watchpoint:
@@ -10564,7 +10563,7 @@ print_it_watchpoint (bpstat bs)
 	uiout->field_string
 	  ("reason", async_reason_lookup (EXEC_ASYNC_WATCHPOINT_TRIGGER));
       mention (b);
-      make_cleanup_ui_out_tuple_begin_end (uiout, "value");
+      tuple_emitter.emplace (uiout, "value");
       uiout->text ("\nOld value = ");
       watchpoint_value_print (bs->old_val, &stb);
       uiout->field_stream ("old", stb);
@@ -10581,7 +10580,7 @@ print_it_watchpoint (bpstat bs)
 	uiout->field_string
 	  ("reason", async_reason_lookup (EXEC_ASYNC_READ_WATCHPOINT_TRIGGER));
       mention (b);
-      make_cleanup_ui_out_tuple_begin_end (uiout, "value");
+      tuple_emitter.emplace (uiout, "value");
       uiout->text ("\nValue = ");
       watchpoint_value_print (w->val, &stb);
       uiout->field_stream ("value", stb);
@@ -10597,7 +10596,7 @@ print_it_watchpoint (bpstat bs)
 	      ("reason",
 	       async_reason_lookup (EXEC_ASYNC_ACCESS_WATCHPOINT_TRIGGER));
 	  mention (b);
-	  make_cleanup_ui_out_tuple_begin_end (uiout, "value");
+	  tuple_emitter.emplace (uiout, "value");
 	  uiout->text ("\nOld value = ");
 	  watchpoint_value_print (bs->old_val, &stb);
 	  uiout->field_stream ("old", stb);
@@ -10610,7 +10609,7 @@ print_it_watchpoint (bpstat bs)
 	    uiout->field_string
 	      ("reason",
 	       async_reason_lookup (EXEC_ASYNC_ACCESS_WATCHPOINT_TRIGGER));
-	  make_cleanup_ui_out_tuple_begin_end (uiout, "value");
+	  tuple_emitter.emplace (uiout, "value");
 	  uiout->text ("\nValue = ");
 	}
       watchpoint_value_print (w->val, &stb);
@@ -10622,7 +10621,6 @@ print_it_watchpoint (bpstat bs)
       result = PRINT_UNKNOWN;
     }
 
-  do_cleanups (old_chain);
   return result;
 }
 
diff --git a/gdb/mi/mi-cmd-file.c b/gdb/mi/mi-cmd-file.c
index 2b3522b..a39ef3b 100644
--- a/gdb/mi/mi-cmd-file.c
+++ b/gdb/mi/mi-cmd-file.c
@@ -153,10 +153,7 @@ mi_cmd_file_list_shared_libraries (const char *command, char **argv, int argc)
       if (pattern != NULL && !re_exec (so->so_name))
 	continue;
 
-      struct cleanup *tuple_clean_up
-        = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
+      ui_out_emit_tuple tuple_emitter (uiout, NULL);
       mi_output_solib_attribs (uiout, so);
-
-      do_cleanups (tuple_clean_up);
     }
 }
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index c485544..ca66a77 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2873,36 +2873,32 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
 
     for (i = 0; VEC_iterate (mem_range_s, available_memory, i, r); i++)
       {
-	struct cleanup *cleanup_child;
-	gdb_byte *data;
 	struct gdbarch *gdbarch = target_gdbarch ();
 
-	cleanup_child = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
+	ui_out_emit_tuple tuple_emitter (uiout, NULL);
 
 	uiout->field_core_addr ("address", gdbarch, r->start);
 	uiout->field_int ("length", r->length);
 
-	data = (gdb_byte *) xmalloc (r->length);
-	make_cleanup (xfree, data);
+	gdb::byte_vector data (r->length);
 
 	if (memory_contents)
 	  {
-	    if (target_read_memory (r->start, data, r->length) == 0)
+	    if (target_read_memory (r->start, data.data (), r->length) == 0)
 	      {
 		int m;
-		char *data_str, *p;
+		char *p;
 
-		data_str = (char *) xmalloc (r->length * 2 + 1);
-		make_cleanup (xfree, data_str);
+		gdb::unique_xmalloc_ptr<char> data_str
+		  ((char *) xmalloc (r->length * 2 + 1));
 
-		for (m = 0, p = data_str; m < r->length; ++m, p += 2)
+		for (m = 0, p = data_str.get (); m < r->length; ++m, p += 2)
 		  sprintf (p, "%02x", data[m]);
-		uiout->field_string ("contents", data_str);
+		uiout->field_string ("contents", data_str.get ());
 	      }
 	    else
 	      uiout->field_skip ("contents");
 	  }
-	do_cleanups (cleanup_child);
       }
 
     do_cleanups (list_cleanup);
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index 221f6de..6231790 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -2064,7 +2064,6 @@ info_spu_event_command (char *args, int from_tty)
   struct frame_info *frame = get_selected_frame (NULL);
   ULONGEST event_status = 0;
   ULONGEST event_mask = 0;
-  struct cleanup *chain;
   gdb_byte buf[100];
   char annex[32];
   LONGEST len;
@@ -2091,7 +2090,7 @@ info_spu_event_command (char *args, int from_tty)
   buf[len] = '\0';
   event_mask = strtoulst ((char *) buf, NULL, 16);
  
-  chain = make_cleanup_ui_out_tuple_begin_end (current_uiout, "SPUInfoEvent");
+  ui_out_emit_tuple tuple_emitter (current_uiout, "SPUInfoEvent");
 
   if (current_uiout->is_mi_like_p ())
     {
@@ -2105,8 +2104,6 @@ info_spu_event_command (char *args, int from_tty)
       printf_filtered (_("Event Status 0x%s\n"), phex (event_status, 4));
       printf_filtered (_("Event Mask   0x%s\n"), phex (event_mask, 4));
     }
-
-  do_cleanups (chain);
 }
 
 static void
@@ -2121,7 +2118,6 @@ info_spu_signal_command (char *args, int from_tty)
   ULONGEST signal2 = 0;
   ULONGEST signal2_type = 0;
   int signal2_pending = 0;
-  struct cleanup *chain;
   char annex[32];
   gdb_byte buf[100];
   LONGEST len;
@@ -2168,7 +2164,7 @@ info_spu_signal_command (char *args, int from_tty)
   buf[len] = '\0';
   signal2_type = strtoulst ((char *) buf, NULL, 16);
 
-  chain = make_cleanup_ui_out_tuple_begin_end (current_uiout, "SPUInfoSignal");
+  ui_out_emit_tuple tuple_emitter (current_uiout, "SPUInfoSignal");
 
   if (current_uiout->is_mi_like_p ())
     {
@@ -2201,8 +2197,6 @@ info_spu_signal_command (char *args, int from_tty)
       else
 	printf_filtered (_("(Type Overwrite)\n"));
     }
-
-  do_cleanups (chain);
 }
 
 static void
@@ -2239,7 +2233,6 @@ info_spu_mailbox_command (char *args, int from_tty)
   struct frame_info *frame = get_selected_frame (NULL);
   struct gdbarch *gdbarch = get_frame_arch (frame);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  struct cleanup *chain;
   char annex[32];
   gdb_byte buf[1024];
   LONGEST len;
@@ -2250,7 +2243,7 @@ info_spu_mailbox_command (char *args, int from_tty)
 
   id = get_frame_register_unsigned (frame, SPU_ID_REGNUM);
 
-  chain = make_cleanup_ui_out_tuple_begin_end (current_uiout, "SPUInfoMailbox");
+  ui_out_emit_tuple tuple_emitter (current_uiout, "SPUInfoMailbox");
 
   xsnprintf (annex, sizeof annex, "%d/mbox_info", id);
   len = target_read (&current_target, TARGET_OBJECT_SPU, annex,
@@ -2278,8 +2271,6 @@ info_spu_mailbox_command (char *args, int from_tty)
 
   info_spu_mailbox_list (buf, len / 4, byte_order,
 			 "wbox", "SPU Inbound Mailbox");
-
-  do_cleanups (chain);
 }
 
 static ULONGEST
@@ -2475,7 +2466,6 @@ info_spu_dma_command (char *args, int from_tty)
   ULONGEST dma_info_status;
   ULONGEST dma_info_stall_and_notify;
   ULONGEST dma_info_atomic_command_status;
-  struct cleanup *chain;
   char annex[32];
   gdb_byte buf[1024];
   LONGEST len;
@@ -2503,7 +2493,7 @@ info_spu_dma_command (char *args, int from_tty)
   dma_info_atomic_command_status
     = extract_unsigned_integer (buf + 32, 8, byte_order);
   
-  chain = make_cleanup_ui_out_tuple_begin_end (current_uiout, "SPUInfoDMA");
+  ui_out_emit_tuple tuple_emitter (current_uiout, "SPUInfoDMA");
 
   if (current_uiout->is_mi_like_p ())
     {
@@ -2542,7 +2532,6 @@ info_spu_dma_command (char *args, int from_tty)
     }
 
   info_spu_dma_cmdlist (buf + 40, 16, byte_order);
-  do_cleanups (chain);
 }
 
 static void
@@ -2554,7 +2543,6 @@ info_spu_proxydma_command (char *args, int from_tty)
   ULONGEST dma_info_type;
   ULONGEST dma_info_mask;
   ULONGEST dma_info_status;
-  struct cleanup *chain;
   char annex[32];
   gdb_byte buf[1024];
   LONGEST len;
@@ -2575,8 +2563,7 @@ info_spu_proxydma_command (char *args, int from_tty)
   dma_info_mask = extract_unsigned_integer (buf + 8, 8, byte_order);
   dma_info_status = extract_unsigned_integer (buf + 16, 8, byte_order);
   
-  chain = make_cleanup_ui_out_tuple_begin_end (current_uiout,
-					       "SPUInfoProxyDMA");
+  ui_out_emit_tuple tuple_emitter (current_uiout, "SPUInfoProxyDMA");
 
   if (current_uiout->is_mi_like_p ())
     {
@@ -2607,7 +2594,6 @@ info_spu_proxydma_command (char *args, int from_tty)
     }
 
   info_spu_dma_cmdlist (buf + 24, 8, byte_order);
-  do_cleanups (chain);
 }
 
 static void
diff --git a/gdb/stack.c b/gdb/stack.c
index 8a066b2..c88ded6 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1139,7 +1139,7 @@ print_frame (struct frame_info *frame, int print_level,
   struct ui_out *uiout = current_uiout;
   char *funname = NULL;
   enum language funlang = language_unknown;
-  struct cleanup *old_chain, *list_chain;
+  struct cleanup *old_chain;
   struct value_print_options opts;
   struct symbol *func;
   CORE_ADDR pc = 0;
@@ -1154,107 +1154,107 @@ print_frame (struct frame_info *frame, int print_level,
   annotate_frame_begin (print_level ? frame_relative_level (frame) : 0,
 			gdbarch, pc);
 
-  list_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "frame");
+  {
+    ui_out_emit_tuple tuple_emitter (uiout, "frame");
 
-  if (print_level)
-    {
-      uiout->text ("#");
-      uiout->field_fmt_int (2, ui_left, "level",
-			    frame_relative_level (frame));
-    }
-  get_user_print_options (&opts);
-  if (opts.addressprint)
-    if (!sal.symtab
-	|| frame_show_address (frame, sal)
-	|| print_what == LOC_AND_ADDRESS)
+    if (print_level)
       {
-	annotate_frame_address ();
-	if (pc_p)
-	  uiout->field_core_addr ("addr", gdbarch, pc);
-	else
-	  uiout->field_string ("addr", "<unavailable>");
-	annotate_frame_address_end ();
-	uiout->text (" in ");
+	uiout->text ("#");
+	uiout->field_fmt_int (2, ui_left, "level",
+			      frame_relative_level (frame));
       }
-  annotate_frame_function_name ();
-
-  string_file stb;
-  fprintf_symbol_filtered (&stb, funname ? funname : "??",
-			   funlang, DMGL_ANSI);
-  uiout->field_stream ("func", stb);
-  uiout->wrap_hint ("   ");
-  annotate_frame_args ();
-      
-  uiout->text (" (");
-  if (print_args)
-    {
-      struct gdbarch *gdbarch = get_frame_arch (frame);
-      int numargs;
-
-      if (gdbarch_frame_num_args_p (gdbarch))
+    get_user_print_options (&opts);
+    if (opts.addressprint)
+      if (!sal.symtab
+	  || frame_show_address (frame, sal)
+	  || print_what == LOC_AND_ADDRESS)
 	{
-	  numargs = gdbarch_frame_num_args (gdbarch, frame);
-	  gdb_assert (numargs >= 0);
+	  annotate_frame_address ();
+	  if (pc_p)
+	    uiout->field_core_addr ("addr", gdbarch, pc);
+	  else
+	    uiout->field_string ("addr", "<unavailable>");
+	  annotate_frame_address_end ();
+	  uiout->text (" in ");
 	}
-      else
-	numargs = -1;
-    
+    annotate_frame_function_name ();
+
+    string_file stb;
+    fprintf_symbol_filtered (&stb, funname ? funname : "??",
+			     funlang, DMGL_ANSI);
+    uiout->field_stream ("func", stb);
+    uiout->wrap_hint ("   ");
+    annotate_frame_args ();
+      
+    uiout->text (" (");
+    if (print_args)
       {
-	ui_out_emit_list list_emitter (uiout, "args");
-	TRY
-	  {
-	    print_frame_args (func, frame, numargs, gdb_stdout);
-	  }
-	CATCH (e, RETURN_MASK_ERROR)
+	struct gdbarch *gdbarch = get_frame_arch (frame);
+	int numargs;
+
+	if (gdbarch_frame_num_args_p (gdbarch))
 	  {
+	    numargs = gdbarch_frame_num_args (gdbarch, frame);
+	    gdb_assert (numargs >= 0);
 	  }
-	END_CATCH
+	else
+	  numargs = -1;
+    
+	{
+	  ui_out_emit_list list_emitter (uiout, "args");
+	  TRY
+	    {
+	      print_frame_args (func, frame, numargs, gdb_stdout);
+	    }
+	  CATCH (e, RETURN_MASK_ERROR)
+	    {
+	    }
+	  END_CATCH
 
-	/* FIXME: ARGS must be a list.  If one argument is a string it
-	   will have " that will not be properly escaped.  */
+	    /* FIXME: ARGS must be a list.  If one argument is a string it
+	       will have " that will not be properly escaped.  */
+	    }
+	QUIT;
       }
-      QUIT;
-    }
-  uiout->text (")");
-  if (sal.symtab)
-    {
-      const char *filename_display;
+    uiout->text (")");
+    if (sal.symtab)
+      {
+	const char *filename_display;
       
-      filename_display = symtab_to_filename_for_display (sal.symtab);
-      annotate_frame_source_begin ();
-      uiout->wrap_hint ("   ");
-      uiout->text (" at ");
-      annotate_frame_source_file ();
-      uiout->field_string ("file", filename_display);
-      if (uiout->is_mi_like_p ())
-	{
-	  const char *fullname = symtab_to_fullname (sal.symtab);
+	filename_display = symtab_to_filename_for_display (sal.symtab);
+	annotate_frame_source_begin ();
+	uiout->wrap_hint ("   ");
+	uiout->text (" at ");
+	annotate_frame_source_file ();
+	uiout->field_string ("file", filename_display);
+	if (uiout->is_mi_like_p ())
+	  {
+	    const char *fullname = symtab_to_fullname (sal.symtab);
 
-	  uiout->field_string ("fullname", fullname);
-	}
-      annotate_frame_source_file_end ();
-      uiout->text (":");
-      annotate_frame_source_line ();
-      uiout->field_int ("line", sal.line);
-      annotate_frame_source_end ();
-    }
+	    uiout->field_string ("fullname", fullname);
+	  }
+	annotate_frame_source_file_end ();
+	uiout->text (":");
+	annotate_frame_source_line ();
+	uiout->field_int ("line", sal.line);
+	annotate_frame_source_end ();
+      }
 
-  if (pc_p && (funname == NULL || sal.symtab == NULL))
-    {
-      char *lib = solib_name_from_address (get_frame_program_space (frame),
-					   get_frame_pc (frame));
+    if (pc_p && (funname == NULL || sal.symtab == NULL))
+      {
+	char *lib = solib_name_from_address (get_frame_program_space (frame),
+					     get_frame_pc (frame));
 
-      if (lib)
-	{
-	  annotate_frame_where ();
-	  uiout->wrap_hint ("  ");
-	  uiout->text (" from ");
-	  uiout->field_string ("from", lib);
-	}
-    }
+	if (lib)
+	  {
+	    annotate_frame_where ();
+	    uiout->wrap_hint ("  ");
+	    uiout->text (" from ");
+	    uiout->field_string ("from", lib);
+	  }
+      }
+  }
 
-  /* do_cleanups will call ui_out_tuple_end() for us.  */
-  do_cleanups (list_chain);
   uiout->text ("\n");
   do_cleanups (old_chain);
 }
diff --git a/gdb/target.c b/gdb/target.c
index 3312b89..2f7f317 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3926,9 +3926,7 @@ flash_erase_command (char *cmd, int from_tty)
           found_flash_region = true;
           target_flash_erase (m->lo, m->hi - m->lo);
 
-	  struct cleanup *cleanup_tuple
-	      = make_cleanup_ui_out_tuple_begin_end (current_uiout,
-						     "erased-regions");
+	  ui_out_emit_tuple tuple_emitter (current_uiout, "erased-regions");
 
           current_uiout->message (_("Erasing flash memory region at address "));
           current_uiout->field_fmt ("address", "%s", paddress (gdbarch,
@@ -3936,7 +3934,6 @@ flash_erase_command (char *cmd, int from_tty)
           current_uiout->message (", size = ");
           current_uiout->field_fmt ("size", "%s", hex_string (m->hi - m->lo));
           current_uiout->message ("\n");
-          do_cleanups (cleanup_tuple);
         }
     }
 
-- 
2.9.4

[RFA 2/7] Remove make_cleanup_ui_out_table_begin_end

[Tom Tromey]

This changes the few remaining uses of
make_cleanup_ui_out_table_begin_end to use ui_out_emit_table instead,
and then removes the cleanup.

ChangeLog
2017-09-09  Tom Tromey  <tom@tromey.com>

	* ui-out.h (make_cleanup_ui_out_table_begin_end): Remove.
	(class ui_out_emit_table): Update comment.
	* ui-out.c (do_cleanup_table_end)
	(make_cleanup_ui_out_table_begin_end): Remove.
	* spu-tdep.c (info_spu_mailbox_list): Use ui_out_emit_table.
	(info_spu_dma_cmdlist): Likewise.
	* probe.c (info_probes_for_ops): Use ui_out_emit_table.
	* darwin-nat-info.c (darwin_debug_regions_recurse): Use
	ui_out_emit_table.
---
 gdb/ChangeLog         |  12 ++++
 gdb/darwin-nat-info.c |  49 +++++++--------
 gdb/probe.c           | 170 +++++++++++++++++++++++++-------------------------
 gdb/spu-tdep.c        |  90 ++++++++++++--------------
 gdb/ui-out.c          |  16 -----
 gdb/ui-out.h          |   8 +--
 6 files changed, 164 insertions(+), 181 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b37089e..4c9c419 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,17 @@
 2017-09-09  Tom Tromey  <tom@tromey.com>
 
+	* ui-out.h (make_cleanup_ui_out_table_begin_end): Remove.
+	(class ui_out_emit_table): Update comment.
+	* ui-out.c (do_cleanup_table_end)
+	(make_cleanup_ui_out_table_begin_end): Remove.
+	* spu-tdep.c (info_spu_mailbox_list): Use ui_out_emit_table.
+	(info_spu_dma_cmdlist): Likewise.
+	* probe.c (info_probes_for_ops): Use ui_out_emit_table.
+	* darwin-nat-info.c (darwin_debug_regions_recurse): Use
+	ui_out_emit_table.
+
+2017-09-09  Tom Tromey  <tom@tromey.com>
+
 	* thread.c (print_thread_info_1): Use ui_out_emit_table,
 	ui_out_emit_list, gdb::optional.
 
diff --git a/gdb/darwin-nat-info.c b/gdb/darwin-nat-info.c
index 91bc5b3..8951789 100644
--- a/gdb/darwin-nat-info.c
+++ b/gdb/darwin-nat-info.c
@@ -617,10 +617,9 @@ darwin_debug_regions_recurse (task_t task)
   vm_region_submap_short_info_data_64_t r_info;
   kern_return_t kret;
   int ret;
-  struct cleanup *table_chain;
   struct ui_out *uiout = current_uiout;
 
-  table_chain = make_cleanup_ui_out_table_begin_end (uiout, 9, -1, "regions");
+  ui_out_emit_table table_emitter (uiout, 9, -1, "regions");
 
   if (gdbarch_addr_bit (target_gdbarch ()) <= 32)
     {
@@ -647,7 +646,6 @@ darwin_debug_regions_recurse (task_t task)
   while (1)
     {
       const char *tag;
-      struct cleanup *row_chain;
 
       r_info_size = VM_REGION_SUBMAP_SHORT_INFO_COUNT_64;
       r_size = -1;
@@ -656,28 +654,29 @@ darwin_debug_regions_recurse (task_t task)
 				     &r_info_size);
       if (kret != KERN_SUCCESS)
 	break;
-      row_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "regions-row");
-
-      uiout->field_core_addr ("start", target_gdbarch (), r_start);
-      uiout->field_core_addr ("end", target_gdbarch (), r_start + r_size);
-      uiout->field_string ("min-prot",
-			   unparse_protection (r_info.protection));
-      uiout->field_string ("max-prot",
-			   unparse_protection (r_info.max_protection));
-      uiout->field_string ("inheritence",
-			   unparse_inheritance (r_info.inheritance));
-      uiout->field_string ("share-mode",
-			   unparse_share_mode (r_info.share_mode));
-      uiout->field_int ("depth", r_depth);
-      uiout->field_string ("submap",
-			   r_info.is_submap ? _("sm ") : _("obj"));
-      tag = unparse_user_tag (r_info.user_tag);
-      if (tag)
-	uiout->field_string ("tag", tag);
-      else
-	uiout->field_int ("tag", r_info.user_tag);
 
-      do_cleanups (row_chain);
+      {
+	ui_out_emit_tuple tuple_emitter (uiout, "regions-row");
+
+	uiout->field_core_addr ("start", target_gdbarch (), r_start);
+	uiout->field_core_addr ("end", target_gdbarch (), r_start + r_size);
+	uiout->field_string ("min-prot",
+			     unparse_protection (r_info.protection));
+	uiout->field_string ("max-prot",
+			     unparse_protection (r_info.max_protection));
+	uiout->field_string ("inheritence",
+			     unparse_inheritance (r_info.inheritance));
+	uiout->field_string ("share-mode",
+			     unparse_share_mode (r_info.share_mode));
+	uiout->field_int ("depth", r_depth);
+	uiout->field_string ("submap",
+			     r_info.is_submap ? _("sm ") : _("obj"));
+	tag = unparse_user_tag (r_info.user_tag);
+	if (tag)
+	  uiout->field_string ("tag", tag);
+	else
+	  uiout->field_int ("tag", r_info.user_tag);
+      }
 
       if (!uiout->is_mi_like_p ())
 	uiout->text ("\n");
@@ -687,8 +686,6 @@ darwin_debug_regions_recurse (task_t task)
       else
 	r_start += r_size;
     }
-  do_cleanups (table_chain);
-
 }
 
 
diff --git a/gdb/probe.c b/gdb/probe.c
index ce28361..a2e204a 100644
--- a/gdb/probe.c
+++ b/gdb/probe.c
@@ -616,90 +616,92 @@ info_probes_for_ops (const char *arg, int from_tty,
   else
     ui_out_extra_fields = get_number_extra_fields (pops);
 
-  make_cleanup_ui_out_table_begin_end (current_uiout,
-				       5 + ui_out_extra_fields,
-				       VEC_length (bound_probe_s, probes),
-				       "StaticProbes");
-
-  if (!VEC_empty (bound_probe_s, probes))
-    qsort (VEC_address (bound_probe_s, probes),
-	   VEC_length (bound_probe_s, probes),
-	   sizeof (bound_probe_s), compare_probes);
-
-  /* What's the size of an address in our architecture?  */
-  size_addr = gdbarch_addr_bit (gdbarch) == 64 ? 18 : 10;
-
-  /* Determining the maximum size of each field (`type', `provider',
-     `name' and `objname').  */
-  for (i = 0; VEC_iterate (bound_probe_s, probes, i, probe); ++i)
-    {
-      const char *probe_type = probe->probe->pops->type_name (probe->probe);
-
-      size_type = std::max (strlen (probe_type), size_type);
-      size_name = std::max (strlen (probe->probe->name), size_name);
-      size_provider = std::max (strlen (probe->probe->provider), size_provider);
-      size_objname = std::max (strlen (objfile_name (probe->objfile)),
-			       size_objname);
-    }
-
-  current_uiout->table_header (size_type, ui_left, "type", _("Type"));
-  current_uiout->table_header (size_provider, ui_left, "provider",
-			       _("Provider"));
-  current_uiout->table_header (size_name, ui_left, "name", _("Name"));
-  current_uiout->table_header (size_addr, ui_left, "addr", _("Where"));
-
-  if (pops == NULL)
-    {
-      const struct probe_ops *po;
-      int ix;
-
-      /* We have to generate the table header for each new probe type
-	 that we will print.  Note that this excludes probe types not
-	 having any defined probe with the search criteria.  */
-      for (ix = 0; VEC_iterate (probe_ops_cp, all_probe_ops, ix, po); ++ix)
-	if (exists_probe_with_pops (probes, po))
-	  gen_ui_out_table_header_info (probes, po);
-    }
-  else
-    gen_ui_out_table_header_info (probes, pops);
-
-  current_uiout->table_header (size_objname, ui_left, "object", _("Object"));
-  current_uiout->table_body ();
-
-  for (i = 0; VEC_iterate (bound_probe_s, probes, i, probe); ++i)
-    {
-      const char *probe_type = probe->probe->pops->type_name (probe->probe);
-
-      ui_out_emit_tuple tuple_emitter (current_uiout, "probe");
-
-      current_uiout->field_string ("type",probe_type);
-      current_uiout->field_string ("provider", probe->probe->provider);
-      current_uiout->field_string ("name", probe->probe->name);
-      current_uiout->field_core_addr (
-	"addr", probe->probe->arch,
-	get_probe_address (probe->probe, probe->objfile));
-
-      if (pops == NULL)
-	{
-	  const struct probe_ops *po;
-	  int ix;
-
-	  for (ix = 0; VEC_iterate (probe_ops_cp, all_probe_ops, ix, po);
-	       ++ix)
-	    if (probe->probe->pops == po)
-	      print_ui_out_info (probe->probe);
-	    else if (exists_probe_with_pops (probes, po))
-	      print_ui_out_not_applicables (po);
-	}
-      else
-	print_ui_out_info (probe->probe);
-
-      current_uiout->field_string ("object",
-			   objfile_name (probe->objfile));
-      current_uiout->text ("\n");
-    }
-
-  any_found = !VEC_empty (bound_probe_s, probes);
+  {
+    ui_out_emit_table table_emitter (current_uiout,
+				     5 + ui_out_extra_fields,
+				     VEC_length (bound_probe_s, probes),
+				     "StaticProbes");
+
+    if (!VEC_empty (bound_probe_s, probes))
+      qsort (VEC_address (bound_probe_s, probes),
+	     VEC_length (bound_probe_s, probes),
+	     sizeof (bound_probe_s), compare_probes);
+
+    /* What's the size of an address in our architecture?  */
+    size_addr = gdbarch_addr_bit (gdbarch) == 64 ? 18 : 10;
+
+    /* Determining the maximum size of each field (`type', `provider',
+       `name' and `objname').  */
+    for (i = 0; VEC_iterate (bound_probe_s, probes, i, probe); ++i)
+      {
+	const char *probe_type = probe->probe->pops->type_name (probe->probe);
+
+	size_type = std::max (strlen (probe_type), size_type);
+	size_name = std::max (strlen (probe->probe->name), size_name);
+	size_provider = std::max (strlen (probe->probe->provider), size_provider);
+	size_objname = std::max (strlen (objfile_name (probe->objfile)),
+				 size_objname);
+      }
+
+    current_uiout->table_header (size_type, ui_left, "type", _("Type"));
+    current_uiout->table_header (size_provider, ui_left, "provider",
+				 _("Provider"));
+    current_uiout->table_header (size_name, ui_left, "name", _("Name"));
+    current_uiout->table_header (size_addr, ui_left, "addr", _("Where"));
+
+    if (pops == NULL)
+      {
+	const struct probe_ops *po;
+	int ix;
+
+	/* We have to generate the table header for each new probe type
+	   that we will print.  Note that this excludes probe types not
+	   having any defined probe with the search criteria.  */
+	for (ix = 0; VEC_iterate (probe_ops_cp, all_probe_ops, ix, po); ++ix)
+	  if (exists_probe_with_pops (probes, po))
+	    gen_ui_out_table_header_info (probes, po);
+      }
+    else
+      gen_ui_out_table_header_info (probes, pops);
+
+    current_uiout->table_header (size_objname, ui_left, "object", _("Object"));
+    current_uiout->table_body ();
+
+    for (i = 0; VEC_iterate (bound_probe_s, probes, i, probe); ++i)
+      {
+	const char *probe_type = probe->probe->pops->type_name (probe->probe);
+
+	ui_out_emit_tuple tuple_emitter (current_uiout, "probe");
+
+	current_uiout->field_string ("type",probe_type);
+	current_uiout->field_string ("provider", probe->probe->provider);
+	current_uiout->field_string ("name", probe->probe->name);
+	current_uiout->field_core_addr (
+					"addr", probe->probe->arch,
+					get_probe_address (probe->probe, probe->objfile));
+
+	if (pops == NULL)
+	  {
+	    const struct probe_ops *po;
+	    int ix;
+
+	    for (ix = 0; VEC_iterate (probe_ops_cp, all_probe_ops, ix, po);
+		 ++ix)
+	      if (probe->probe->pops == po)
+		print_ui_out_info (probe->probe);
+	      else if (exists_probe_with_pops (probes, po))
+		print_ui_out_not_applicables (po);
+	  }
+	else
+	  print_ui_out_info (probe->probe);
+
+	current_uiout->field_string ("object",
+				     objfile_name (probe->objfile));
+	current_uiout->text ("\n");
+      }
+
+    any_found = !VEC_empty (bound_probe_s, probes);
+  }
   do_cleanups (cleanup);
 
   if (!any_found)
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index 59c51b2..221f6de 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -2209,31 +2209,28 @@ static void
 info_spu_mailbox_list (gdb_byte *buf, int nr, enum bfd_endian byte_order,
 		       const char *field, const char *msg)
 {
-  struct cleanup *chain;
   int i;
 
   if (nr <= 0)
     return;
 
-  chain = make_cleanup_ui_out_table_begin_end (current_uiout, 1, nr, "mbox");
+  ui_out_emit_table table_emitter (current_uiout, 1, nr, "mbox");
 
   current_uiout->table_header (32, ui_left, field, msg);
   current_uiout->table_body ();
 
   for (i = 0; i < nr; i++)
     {
-      struct cleanup *val_chain;
-      ULONGEST val;
-      val_chain = make_cleanup_ui_out_tuple_begin_end (current_uiout, "mbox");
-      val = extract_unsigned_integer (buf + 4*i, 4, byte_order);
-      current_uiout->field_fmt (field, "0x%s", phex (val, 4));
-      do_cleanups (val_chain);
+      {
+	ULONGEST val;
+	ui_out_emit_tuple tuple_emitter (current_uiout, "mbox");
+	val = extract_unsigned_integer (buf + 4*i, 4, byte_order);
+	current_uiout->field_fmt (field, "0x%s", phex (val, 4));
+      }
 
       if (!current_uiout->is_mi_like_p ())
 	printf_filtered ("\n");
     }
-
-  do_cleanups (chain);
 }
 
 static void
@@ -2333,7 +2330,6 @@ info_spu_dma_cmdlist (gdb_byte *buf, int nr, enum bfd_endian byte_order)
 
   int *seq = XALLOCAVEC (int, nr);
   int done = 0;
-  struct cleanup *chain;
   int i, j;
 
 
@@ -2371,8 +2367,7 @@ info_spu_dma_cmdlist (gdb_byte *buf, int nr, enum bfd_endian byte_order)
   nr = i;
 
 
-  chain = make_cleanup_ui_out_table_begin_end (current_uiout, 10, nr,
-					       "dma_cmd");
+  ui_out_emit_table table_emitter (current_uiout, 10, nr, "dma_cmd");
 
   current_uiout->table_header (7, ui_left, "opcode", "Opcode");
   current_uiout->table_header (3, ui_left, "tag", "Tag");
@@ -2389,7 +2384,6 @@ info_spu_dma_cmdlist (gdb_byte *buf, int nr, enum bfd_endian byte_order)
 
   for (i = 0; i < nr; i++)
     {
-      struct cleanup *cmd_chain;
       ULONGEST mfc_cq_dw0;
       ULONGEST mfc_cq_dw1;
       ULONGEST mfc_cq_dw2;
@@ -2425,51 +2419,49 @@ info_spu_dma_cmdlist (gdb_byte *buf, int nr, enum bfd_endian byte_order)
       ea_valid_p = spu_mfc_get_bitfield (mfc_cq_dw2, 39, 39);
       cmd_error_p = spu_mfc_get_bitfield (mfc_cq_dw2, 40, 40);
 
-      cmd_chain = make_cleanup_ui_out_tuple_begin_end (current_uiout, "cmd");
-
-      if (spu_mfc_opcode[mfc_cmd_opcode])
-	current_uiout->field_string ("opcode", spu_mfc_opcode[mfc_cmd_opcode]);
-      else
-	current_uiout->field_int ("opcode", mfc_cmd_opcode);
+      {
+	ui_out_emit_tuple tuple_emitter (current_uiout, "cmd");
 
-      current_uiout->field_int ("tag", mfc_cmd_tag);
-      current_uiout->field_int ("tid", tclass_id);
-      current_uiout->field_int ("rid", rclass_id);
+	if (spu_mfc_opcode[mfc_cmd_opcode])
+	  current_uiout->field_string ("opcode", spu_mfc_opcode[mfc_cmd_opcode]);
+	else
+	  current_uiout->field_int ("opcode", mfc_cmd_opcode);
 
-      if (ea_valid_p)
-	current_uiout->field_fmt ("ea", "0x%s", phex (mfc_ea, 8));
-      else
-	current_uiout->field_skip ("ea");
+	current_uiout->field_int ("tag", mfc_cmd_tag);
+	current_uiout->field_int ("tid", tclass_id);
+	current_uiout->field_int ("rid", rclass_id);
 
-      current_uiout->field_fmt ("lsa", "0x%05x", mfc_lsa << 4);
-      if (qw_valid_p)
-	current_uiout->field_fmt ("size", "0x%05x", mfc_size << 4);
-      else
-	current_uiout->field_fmt ("size", "0x%05x", mfc_size);
+	if (ea_valid_p)
+	  current_uiout->field_fmt ("ea", "0x%s", phex (mfc_ea, 8));
+	else
+	  current_uiout->field_skip ("ea");
 
-      if (list_valid_p)
-	{
-	  current_uiout->field_fmt ("lstaddr", "0x%05x", list_lsa << 3);
-	  current_uiout->field_fmt ("lstsize", "0x%05x", list_size << 3);
-	}
-      else
-	{
-	  current_uiout->field_skip ("lstaddr");
-	  current_uiout->field_skip ("lstsize");
-	}
+	current_uiout->field_fmt ("lsa", "0x%05x", mfc_lsa << 4);
+	if (qw_valid_p)
+	  current_uiout->field_fmt ("size", "0x%05x", mfc_size << 4);
+	else
+	  current_uiout->field_fmt ("size", "0x%05x", mfc_size);
 
-      if (cmd_error_p)
-	current_uiout->field_string ("error_p", "*");
-      else
-	current_uiout->field_skip ("error_p");
+	if (list_valid_p)
+	  {
+	    current_uiout->field_fmt ("lstaddr", "0x%05x", list_lsa << 3);
+	    current_uiout->field_fmt ("lstsize", "0x%05x", list_size << 3);
+	  }
+	else
+	  {
+	    current_uiout->field_skip ("lstaddr");
+	    current_uiout->field_skip ("lstsize");
+	  }
 
-      do_cleanups (cmd_chain);
+	if (cmd_error_p)
+	  current_uiout->field_string ("error_p", "*");
+	else
+	  current_uiout->field_skip ("error_p");
+      }
 
       if (!current_uiout->is_mi_like_p ())
 	printf_filtered ("\n");
     }
-
-  do_cleanups (chain);
 }
 
 static void
diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index 9c27742..97207f5 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -400,22 +400,6 @@ ui_out::table_end ()
   m_table_up = nullptr;
 }
 
-static void
-do_cleanup_table_end (void *data)
-{
-  ui_out *uiout = (ui_out *) data;
-
-  uiout->table_end ();
-}
-
-struct cleanup *
-make_cleanup_ui_out_table_begin_end (ui_out *uiout, int nr_cols, int nr_rows,
-				     const char *tblid)
-{
-  uiout->table_begin (nr_cols, nr_rows, tblid);
-  return make_cleanup (do_cleanup_table_end, uiout);
-}
-
 void
 ui_out::begin (ui_out_type type, const char *id)
 {
diff --git a/gdb/ui-out.h b/gdb/ui-out.h
index 857e252..d983837 100644
--- a/gdb/ui-out.h
+++ b/gdb/ui-out.h
@@ -66,10 +66,6 @@ enum ui_out_type
     ui_out_type_list
   };
 
-extern struct cleanup *make_cleanup_ui_out_table_begin_end (struct ui_out *ui_out,
-                                                            int nr_cols,
-							    int nr_rows,
-							    const char *tblid);
 /* Compatibility wrappers.  */
 
 extern struct cleanup *make_cleanup_ui_out_list_begin_end (struct ui_out *uiout,
@@ -220,8 +216,8 @@ private:
 typedef ui_out_emit_type<ui_out_type_tuple> ui_out_emit_tuple;
 typedef ui_out_emit_type<ui_out_type_list> ui_out_emit_list;
 
-/* This is similar to make_cleanup_ui_out_table_begin_end, but written
-   as an RAII class.  */
+/* Start a new table on construction, and end the table on
+   destruction.  */
 class ui_out_emit_table
 {
 public:
-- 
2.9.4

[RFA 7/7] Use ui_out_emit_list and ui_out_emit_tuple with gdb::optional

[Tom Tromey]

This changes a few spots to use ui_out_emit_list and/or
ui_out_emit_tuple with gdb::optional, to preserve existing behavior.
This allows for the removal of a few more cleanups.

ChangeLog
2017-09-09  Tom Tromey  <tom@tromey.com>

	* mi/mi-cmd-var.c (mi_cmd_var_list_children): Use gdb::optional,
	ui_out_emit_list, ui_out_emit_tuple.
	(mi_cmd_var_update): Likewise.
---
 gdb/ChangeLog       |  6 ++++++
 gdb/mi/mi-cmd-var.c | 24 +++++++++++++-----------
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index cc8e4e4..d1ed50f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2017-09-09  Tom Tromey  <tom@tromey.com>
 
+	* mi/mi-cmd-var.c (mi_cmd_var_list_children): Use gdb::optional,
+	ui_out_emit_list, ui_out_emit_tuple.
+	(mi_cmd_var_update): Likewise.
+
+2017-09-09  Tom Tromey  <tom@tromey.com>
+
 	* mi/mi-interp.c (mi_user_selected_context_changed): Use
 	ui_out_redirect_pop.
 	* guile/scm-ports.c (ioscm_with_output_to_port_worker): Use
diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
index 191e770..8b22b2f 100644
--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -410,14 +410,15 @@ mi_cmd_var_list_children (const char *command, char **argv, int argc)
 
   if (from < to)
     {
-      struct cleanup *cleanup_children;
+      /* For historical reasons this might emit a list or a tuple, so
+	 we construct one or the other.  */
+      gdb::optional<ui_out_emit_tuple> tuple_emitter;
+      gdb::optional<ui_out_emit_list> list_emitter;
 
       if (mi_version (uiout) == 1)
-	cleanup_children
-	  = make_cleanup_ui_out_tuple_begin_end (uiout, "children");
+	tuple_emitter.emplace (uiout, "children");
       else
-	cleanup_children
-	  = make_cleanup_ui_out_list_begin_end (uiout, "children");
+	list_emitter.emplace (uiout, "children");
       for (ix = from;
 	   ix < to && VEC_iterate (varobj_p, children, ix, child);
 	   ++ix)
@@ -426,7 +427,6 @@ mi_cmd_var_list_children (const char *command, char **argv, int argc)
 
 	  print_varobj (child, print_values, 1 /* print expression */);
 	}
-      do_cleanups (cleanup_children);
     }
 
   uiout->field_int ("has_more", varobj_has_more (var, to));
@@ -648,7 +648,6 @@ void
 mi_cmd_var_update (const char *command, char **argv, int argc)
 {
   struct ui_out *uiout = current_uiout;
-  struct cleanup *cleanup;
   char *name;
   enum print_values print_values;
 
@@ -665,10 +664,15 @@ mi_cmd_var_update (const char *command, char **argv, int argc)
   else
     print_values = PRINT_NO_VALUES;
 
+  /* For historical reasons this might emit a list or a tuple, so we
+     construct one or the other.  */
+  gdb::optional<ui_out_emit_tuple> tuple_emitter;
+  gdb::optional<ui_out_emit_list> list_emitter;
+
   if (mi_version (uiout) <= 1)
-    cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, "changelist");
+    tuple_emitter.emplace (uiout, "changelist");
   else
-    cleanup = make_cleanup_ui_out_list_begin_end (uiout, "changelist");
+    list_emitter.emplace (uiout, "changelist");
 
   /* Check if the parameter is a "*", which means that we want to
      update all variables.  */
@@ -693,8 +697,6 @@ mi_cmd_var_update (const char *command, char **argv, int argc)
 
       varobj_update_one (var, print_values, 1 /* explicit */);
     }
-
-  do_cleanups (cleanup);
 }
 
 /* Helper for mi_cmd_var_update().  */
-- 
2.9.4

Re: [RFA 2/7] Remove make_cleanup_ui_out_table_begin_end

[Tom Tromey]

I forgot to mention something I had meant to mention when submitting
this series:

Tom>	* spu-tdep.c (info_spu_mailbox_list): Use ui_out_emit_table.
Tom>	(info_spu_dma_cmdlist): Likewise.
[..]
Tom>	* darwin-nat-info.c (darwin_debug_regions_recurse): Use
Tom>	ui_out_emit_table.

I don't have any way to test these, or even compile the darwin patch.
So this requires extra careful review at least, or someone to volunteer
to try them out.

It would be very nice to have a darwin builder hooked up to the
buildbot.  I'm not sure how practical this is ... there are so many Macs
around Moz that maybe I could acquire one somehow, but I don't know
that I could host it anywhere.

Tom

Re: [RFA 1/7] Use ui_out_emit_table and ui_out_emit_list in print_thread_info_1

[Simon Marchi]

On 2017-09-09 17:35, Tom Tromey wrote:
> This changes print_thread_info_1 to use ui_out_emit_table and
> ui_out_emit_list.  Which one is used depends on whether the ui-out is
> mi-like; so the emitters are wrapped in gdb::optional.

LGTM.

I think overall this function is a bad example of how to share code 
between CLI and MI.  There are so many if (is_mi_like_p) that it's 
essentially two functions in one.  Apart from iterating on threads, the 
MI and CLI outputs don't share much...

> @@ -1247,55 +1248,55 @@ print_thread_info_1 (struct ui_out *uiout,
> char *requested_threads,
>    update_thread_list ();
>    current_ptid = inferior_ptid;
> 
> -  struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
> -
> -  /* For backward compatibility, we make a list for MI.  A table is
> -     preferable for the CLI, though, because it shows table
> -     headers.  */
> -  if (uiout->is_mi_like_p ())
> -    make_cleanup_ui_out_list_begin_end (uiout, "threads");
> -  else
> -    {
> -      int n_threads = 0;
> +  {
> +    /* For backward compatibility, we make a list for MI.  A table is
> +       preferable for the CLI, though, because it shows table
> +       headers.  */
> +    gdb::optional<ui_out_emit_list> list_emitter;
> +    gdb::optional<ui_out_emit_table> table_emitter;
> +
> +    if (uiout->is_mi_like_p ())
> +      list_emitter.emplace (uiout, "threads");
> +    else
> +      {
> +	int n_threads = 0;
> 
> -      for (tp = thread_list; tp; tp = tp->next)
> -	{
> -	  if (!should_print_thread (requested_threads, default_inf_num,
> -				    global_ids, pid, tp))
> -	    continue;
> +	for (tp = thread_list; tp; tp = tp->next)
> +	  {
> +	    if (!should_print_thread (requested_threads, default_inf_num,
> +				      global_ids, pid, tp))
> +	      continue;
> 
> -	  ++n_threads;
> -	}
> +	    ++n_threads;
> +	  }
> 
> -      if (n_threads == 0)
> -	{
> -	  if (requested_threads == NULL || *requested_threads == '\0')
> -	    uiout->message (_("No threads.\n"));
> -	  else
> -	    uiout->message (_("No threads match '%s'.\n"),
> -			    requested_threads);
> -	  do_cleanups (old_chain);
> -	  return;
> -	}
> +	if (n_threads == 0)
> +	  {
> +	    if (requested_threads == NULL || *requested_threads == '\0')
> +	      uiout->message (_("No threads.\n"));
> +	    else
> +	      uiout->message (_("No threads match '%s'.\n"),
> +			      requested_threads);
> +	    return;
> +	  }
> 
> -      if (show_global_ids || uiout->is_mi_like_p ())
> -	make_cleanup_ui_out_table_begin_end (uiout, 5, n_threads, "threads");
> -      else
> -	make_cleanup_ui_out_table_begin_end (uiout, 4, n_threads, "threads");
> +	table_emitter.emplace (uiout,
> +			       (show_global_ids || uiout->is_mi_like_p ())
> +			       ? 5 : 4,
> +			       n_threads, "threads");
> 
> -      uiout->table_header (1, ui_left, "current", "");
> +	uiout->table_header (1, ui_left, "current", "");
> 
> -      if (!uiout->is_mi_like_p ())
> -	uiout->table_header (4, ui_left, "id-in-tg", "Id");
> -      if (show_global_ids || uiout->is_mi_like_p ())
> -	uiout->table_header (4, ui_left, "id", "GId");
> -      uiout->table_header (17, ui_left, "target-id", "Target Id");
> -      uiout->table_header (1, ui_left, "frame", "Frame");
> -      uiout->table_body ();
> -    }
> +	if (!uiout->is_mi_like_p ())
> +	  uiout->table_header (4, ui_left, "id-in-tg", "Id");
> +	if (show_global_ids || uiout->is_mi_like_p ())
> +	  uiout->table_header (4, ui_left, "id", "GId");
> +	uiout->table_header (17, ui_left, "target-id", "Target Id");
> +	uiout->table_header (1, ui_left, "frame", "Frame");
> +	uiout->table_body ();
> +      }

Actually, we could remove a lot of if (is_mi_like_p) from here, since 
they are already in an else branch of the same check.  I'll wait until 
this patch has been merged to clean that up, so you don't have a 
conflict.

Thanks,

Simon

Re: [RFA 2/7] Remove make_cleanup_ui_out_table_begin_end

[Simon Marchi]

On 2017-09-09 18:02, Tom Tromey wrote:
> I forgot to mention something I had meant to mention when submitting
> this series:
> 
> Tom>	* spu-tdep.c (info_spu_mailbox_list): Use ui_out_emit_table.
> Tom>	(info_spu_dma_cmdlist): Likewise.
> [..]
> Tom>	* darwin-nat-info.c (darwin_debug_regions_recurse): Use
> Tom>	ui_out_emit_table.
> 
> I don't have any way to test these, or even compile the darwin patch.
> So this requires extra careful review at least, or someone to volunteer
> to try them out.
> 
> It would be very nice to have a darwin builder hooked up to the
> buildbot.  I'm not sure how practical this is ... there are so many 
> Macs
> around Moz that maybe I could acquire one somehow, but I don't know
> that I could host it anywhere.

The patch LGTM.

I built-tested your series on a Mac, it builds fine (if we ignore all 
the current hurdles to get it to build).  Now when I try actually 
running, I get this:

(gdb) r
Starting program: /Users/simark/build/binutils-gdb/gdb/test_clang
[New Thread 0x1503 of process 79554]
[New Thread 0x1803 of process 79554]
Warning:
Cannot insert breakpoint -1.
Cannot access memory at address 0xeb19

Command aborted.

But it also does that before your patch.

Simon

Re: [RFA 3/7] Use ui_out_emit_tuple in more places

[Simon Marchi]

On 2017-09-09 17:35, Tom Tromey wrote:
> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> index c485544..ca66a77 100644
> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c
> @@ -2873,36 +2873,32 @@ mi_cmd_trace_frame_collected (const char
> *command, char **argv, int argc)
> 
>      for (i = 0; VEC_iterate (mem_range_s, available_memory, i, r); 
> i++)
>        {
> -	struct cleanup *cleanup_child;
> -	gdb_byte *data;
>  	struct gdbarch *gdbarch = target_gdbarch ();
> 
> -	cleanup_child = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
> +	ui_out_emit_tuple tuple_emitter (uiout, NULL);
> 
>  	uiout->field_core_addr ("address", gdbarch, r->start);
>  	uiout->field_int ("length", r->length);
> 
> -	data = (gdb_byte *) xmalloc (r->length);
> -	make_cleanup (xfree, data);
> +	gdb::byte_vector data (r->length);
> 
>  	if (memory_contents)
>  	  {
> -	    if (target_read_memory (r->start, data, r->length) == 0)
> +	    if (target_read_memory (r->start, data.data (), r->length) == 0)
>  	      {
>  		int m;
> -		char *data_str, *p;
> +		char *p;
> 
> -		data_str = (char *) xmalloc (r->length * 2 + 1);
> -		make_cleanup (xfree, data_str);
> +		gdb::unique_xmalloc_ptr<char> data_str
> +		  ((char *) xmalloc (r->length * 2 + 1));
> 
> -		for (m = 0, p = data_str; m < r->length; ++m, p += 2)
> +		for (m = 0, p = data_str.get (); m < r->length; ++m, p += 2)
>  		  sprintf (p, "%02x", data[m]);
> -		uiout->field_string ("contents", data_str);
> +		uiout->field_string ("contents", data_str.get ());

Can this conversion to hex be replaced with a call to bin2hex (the 
version that returns an std::string) ?

Otherwise, LGTM.

Simon

Re: [RFA 4/7] Use ui_out_emit_tuple in disasm.c

[Simon Marchi]

On 2017-09-09 17:35, Tom Tromey wrote:
> This changes one spot in disasm.c to use ui_out_emit_tuple.  This
> patch required a large reindentation, so I've separated it out.

"git show -w" does wonders!  The hard part is figuring out if there's 
something else in this big function using cleanups (and therefore if the 
cleanup variable should be kept).

Anyway, this LGTM.

Simon

Re: [RFA 1/7] Use ui_out_emit_table and ui_out_emit_list in print_thread_info_1

[Tom Tromey]

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

Simon> I think overall this function is a bad example of how to share code
Simon> between CLI and MI.  There are so many if (is_mi_like_p) that it's
Simon> essentially two functions in one.  Apart from iterating on threads,
Simon> the MI and CLI outputs don't share much...

I think a long term goal should be to remove all those is_mi_like_p
checks.  In some situations this might mean introducing "MI 4" and
fixing up the historical baggage; but that would also be a good thing,
as there are a few places where gdb has remained buggy for the sake of
not breaking existing MI readers.

What I'd really like is for MI to be defined as JSON (it is pretty close
already), but that's maybe a bit much to ask.

A related thought is that if more spots used ui-out (I'm thinking mainly
*-valprint, but maybe there are others), then colorization could be done
at the ui-out layer.

Tom

Re: [RFA 5/7] Use ui_out_emit_list in more places

[Simon Marchi]

On 2017-09-09 17:35, Tom Tromey wrote:
> This changes various spots to use ui_out_emit_list, removing some
> cleanups.

LGTM.

Re: [RFA 6/7] Remove make_cleanup_ui_out_redirect_pop

[Simon Marchi]

On 2017-09-09 17:35, Tom Tromey wrote:
> This patch introduces ui_out_redirect_pop.  All uses of
> make_cleanup_ui_out_redirect_pop are replaced with this new class.

LGTM.

Re: [RFA 7/7] Use ui_out_emit_list and ui_out_emit_tuple with gdb::optional

[Simon Marchi]

On 2017-09-09 17:35, Tom Tromey wrote:
> This changes a few spots to use ui_out_emit_list and/or
> ui_out_emit_tuple with gdb::optional, to preserve existing behavior.
> This allows for the removal of a few more cleanups.

LGTM.

Re: [RFA 1/7] Use ui_out_emit_table and ui_out_emit_list in print_thread_info_1

[Matt Rice]

On Sat, Sep 9, 2017 at 11:36 AM, Tom Tromey <tom@tromey.com> wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
>
> Simon> I think overall this function is a bad example of how to share code
> Simon> between CLI and MI.  There are so many if (is_mi_like_p) that it's
> Simon> essentially two functions in one.  Apart from iterating on threads,
> Simon> the MI and CLI outputs don't share much...
>
> I think a long term goal should be to remove all those is_mi_like_p
> checks.  In some situations this might mean introducing "MI 4" and
> fixing up the historical baggage; but that would also be a good thing,
> as there are a few places where gdb has remained buggy for the sake of
> not breaking existing MI readers.
>
> What I'd really like is for MI to be defined as JSON (it is pretty close
> already), but that's maybe a bit much to ask.

At some point I had it encoding into python literals, with MI output
being converted into something parsable by the python literal_eval function[1]

JSON is really not very different, the only thing which kind of sucked
about it was dealing with
the places a range of values, or a slice is sent, these needed the
range to then be marshalled and unmarshalled
into tuples of numbers then back into ranges, this is more of an
annoyance than anything catastrophic,
but it means you have to evaluate the literals, then evaluate the
objects returned by that first pass...
using some little snippet of code specific to the data being sent.

https://docs.python.org/2/library/ast.html#ast.literal_eval

I don't know JSON well, but i don't see its format having a notion of
slices either
neither do I remember where specifically this came up in mi,
but if both having and eating cake were on the table JSON + primitive
syntax for range/slices would
be nice.

> A related thought is that if more spots used ui-out (I'm thinking mainly
> *-valprint, but maybe there are others), then colorization could be done
> at the ui-out layer.
>
> Tom

Re: [RFA 3/7] Use ui_out_emit_tuple in more places

[Tom Tromey]

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

Simon> Can this conversion to hex be replaced with a call to bin2hex (the
Simon> version that returns an std::string) ?

Yes, thanks.  I've made this change.

Tom

Re: [RFA 0/7] more ui-out cleanup removal

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> This series removes more ui-out related cleanups.
Tom> make_cleanup_ui_out_table_begin_end and
Tom> make_cleanup_ui_out_redirect_pop are completely removed.

Tom> The remaining uses of ui-out cleanups seem tricky to remove, as the
Tom> code using them is very convoluted.

Tom> Regression tested by the buildbot.

I'm checking this in now.

Tom

Re: [RFA 4/7] Use ui_out_emit_tuple in disasm.c

[Simon Marchi]

On 2017-09-09 02:35 PM, Simon Marchi wrote:
> On 2017-09-09 17:35, Tom Tromey wrote:
>> This changes one spot in disasm.c to use ui_out_emit_tuple.  This
>> patch required a large reindentation, so I've separated it out.
> 
> "git show -w" does wonders!  The hard part is figuring out if there's something else in this big function using cleanups (and therefore if the cleanup variable should be kept).
> 
> Anyway, this LGTM.
> 
> Simon
> 

I found a regression, bisect pointed to this patch (which makes sense).

1. Run "disassemble /s" on a small enough terminal so that pagination kicks in.
2. Type q <return> to quit pagination.

I get:

---Type <return> to continue, or q <return> to quit---q
/home/emaisin/src/binutils-gdb/gdb/ui-out.c:344: internal-error: void ui_out::pop_level(ui_out_type): Assertion `current_level ()->type () == type' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n)

Simon

Re: [RFA 4/7] Use ui_out_emit_tuple in disasm.c

[Simon Marchi]

On 2017-10-12 11:36 AM, Simon Marchi wrote:
> On 2017-09-09 02:35 PM, Simon Marchi wrote:
>> On 2017-09-09 17:35, Tom Tromey wrote:
>>> This changes one spot in disasm.c to use ui_out_emit_tuple.  This
>>> patch required a large reindentation, so I've separated it out.
>>
>> "git show -w" does wonders!  The hard part is figuring out if there's something else in this big function using cleanups (and therefore if the cleanup variable should be kept).
>>
>> Anyway, this LGTM.
>>
>> Simon
>>
> 
> I found a regression, bisect pointed to this patch (which makes sense).
> 
> 1. Run "disassemble /s" on a small enough terminal so that pagination kicks in.
> 2. Type q <return> to quit pagination.
> 
> I get:
> 
> ---Type <return> to continue, or q <return> to quit---q
> /home/emaisin/src/binutils-gdb/gdb/ui-out.c:344: internal-error: void ui_out::pop_level(ui_out_type): Assertion `current_level ()->type () == type' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Quit this debugging session? (y or n)
> 
> Simon
> 

Mixing RAII and cleanups probably changed the order in which the uiout elements
are closed.

In an outer scope (likely do_mixed_source_and_assembly), some uiout elements are
opened and cleanups are installed.  In pretty_print_insn, some other uiout elements
are opened and RAII closers are used.  When typing 'q', the cleanups are ran, just
before the quit exception would be thrown.  The RAII closers have not ran yet, because
they would be when the exception is actually thrown.  The solution would be to finish
converting the uiout cleanups to RAII (I won't have time to do that in the near future,
feel free to pick it up).

Simon

Re: [RFA 4/7] Use ui_out_emit_tuple in disasm.c

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> The solution would be to finish
Simon> converting the uiout cleanups to RAII (I won't have time to do that in the near future,
Simon> feel free to pick it up).

I think I'll just back out this particular change.
The way disasm code uses cleanups is very hard to untangle.

Tom

Re: [RFA 4/7] Use ui_out_emit_tuple in disasm.c

[Tom Tromey]

Tom> I think I'll just back out this particular change.
Tom> The way disasm code uses cleanups is very hard to untangle.

It turns out that with gdb::optional, you don't have to untangle it.
I'm testing a patch to convert the rest of disasm.c away from cleanups.

Tom

Re: [RFA 4/7] Use ui_out_emit_tuple in disasm.c

[Tom Tromey]

Tom> It turns out that with gdb::optional, you don't have to untangle it.
Tom> I'm testing a patch to convert the rest of disasm.c away from cleanups.

How about this?

Tom

commit 1b83351de30ed32808aa8b09d720556d423e57c6
Author: Tom Tromey <tom@tromey.com>
Date:   Thu Oct 12 15:07:35 2017 -0600

    Remove cleanups from disasm.c
    
    This changes the remaining spots in disasm.c to use the RAII ui-out
    emitters, removing a few cleanups.  This also fixes a regression that
    Simon pointed out.
    
    2017-10-12  Tom Tromey  <tom@tromey.com>
    
            * disasm.c (do_mixed_source_and_assembly_deprecated): Use
            gdb::optional, ui_out_emit_list, ui_out_emit_tuple.
            (do_mixed_source_and_assembly): Likewise.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b868a1d..504124c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2017-10-12  Tom Tromey  <tom@tromey.com>
+
+	* disasm.c (do_mixed_source_and_assembly_deprecated): Use
+	gdb::optional, ui_out_emit_list, ui_out_emit_tuple.
+	(do_mixed_source_and_assembly): Likewise.
+
 2017-10-13  Tom Tromey  <tom@tromey.com>
 
 	* compile/compile-object-run.c (do_module_cleanup): Use delete.
diff --git a/gdb/disasm.c b/gdb/disasm.c
index cdab181..ab23384 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -29,6 +29,7 @@
 #include "source.h"
 #include "safe-ctype.h"
 #include <algorithm>
+#include "common/gdb_optional.h"
 
 /* Disassemble functions.
    FIXME: We should get rid of all the duplicate code in gdb that does
@@ -350,9 +351,6 @@ do_mixed_source_and_assembly_deprecated
   int next_line = 0;
   int num_displayed = 0;
   print_source_lines_flags psl_flags = 0;
-  struct cleanup *ui_out_chain;
-  struct cleanup *ui_out_tuple_chain = make_cleanup (null_cleanup, 0);
-  struct cleanup *ui_out_list_chain = make_cleanup (null_cleanup, 0);
 
   gdb_assert (symtab != NULL && SYMTAB_LINETABLE (symtab) != NULL);
 
@@ -414,7 +412,10 @@ do_mixed_source_and_assembly_deprecated
      they have been emitted before), followed by the assembly code
      for that line.  */
 
-  ui_out_chain = make_cleanup_ui_out_list_begin_end (uiout, "asm_insns");
+  ui_out_emit_list asm_insns_list (uiout, "asm_insns");
+
+  gdb::optional<ui_out_emit_tuple> outer_tuple_emitter;
+  gdb::optional<ui_out_emit_list> inner_list_emitter;
 
   for (i = 0; i < newlines; i++)
     {
@@ -426,9 +427,7 @@ do_mixed_source_and_assembly_deprecated
 	      /* Just one line to print.  */
 	      if (next_line == mle[i].line)
 		{
-		  ui_out_tuple_chain
-		    = make_cleanup_ui_out_tuple_begin_end (uiout,
-							   "src_and_asm_line");
+		  outer_tuple_emitter.emplace (uiout, "src_and_asm_line");
 		  print_source_lines (symtab, next_line, mle[i].line + 1, psl_flags);
 		}
 	      else
@@ -436,36 +435,27 @@ do_mixed_source_and_assembly_deprecated
 		  /* Several source lines w/o asm instructions associated.  */
 		  for (; next_line < mle[i].line; next_line++)
 		    {
-		      struct cleanup *ui_out_list_chain_line;
-		      
 		      ui_out_emit_tuple tuple_emitter (uiout,
 						       "src_and_asm_line");
 		      print_source_lines (symtab, next_line, next_line + 1,
 					  psl_flags);
-		      ui_out_list_chain_line
-			= make_cleanup_ui_out_list_begin_end (uiout,
-							      "line_asm_insn");
-		      do_cleanups (ui_out_list_chain_line);
+		      ui_out_emit_list inner_list_emitter (uiout,
+							   "line_asm_insn");
 		    }
 		  /* Print the last line and leave list open for
 		     asm instructions to be added.  */
-		  ui_out_tuple_chain
-		    = make_cleanup_ui_out_tuple_begin_end (uiout,
-							   "src_and_asm_line");
+		  outer_tuple_emitter.emplace (uiout, "src_and_asm_line");
 		  print_source_lines (symtab, next_line, mle[i].line + 1, psl_flags);
 		}
 	    }
 	  else
 	    {
-	      ui_out_tuple_chain
-		= make_cleanup_ui_out_tuple_begin_end (uiout,
-						       "src_and_asm_line");
+	      outer_tuple_emitter.emplace (uiout, "src_and_asm_line");
 	      print_source_lines (symtab, mle[i].line, mle[i].line + 1, psl_flags);
 	    }
 
 	  next_line = mle[i].line + 1;
-	  ui_out_list_chain
-	    = make_cleanup_ui_out_list_begin_end (uiout, "line_asm_insn");
+	  inner_list_emitter.emplace (uiout, "line_asm_insn");
 	}
 
       num_displayed += dump_insns (gdbarch, uiout,
@@ -476,16 +466,13 @@ do_mixed_source_and_assembly_deprecated
          assembly range for this source line, close out the list/tuple.  */
       if (i == (newlines - 1) || mle[i + 1].line > mle[i].line)
 	{
-	  do_cleanups (ui_out_list_chain);
-	  do_cleanups (ui_out_tuple_chain);
-	  ui_out_tuple_chain = make_cleanup (null_cleanup, 0);
-	  ui_out_list_chain = make_cleanup (null_cleanup, 0);
+	  inner_list_emitter.reset ();
+	  outer_tuple_emitter.reset ();
 	  uiout->text ("\n");
 	}
       if (how_many >= 0 && num_displayed >= how_many)
 	break;
     }
-  do_cleanups (ui_out_chain);
 }
 
 /* The idea here is to present a source-O-centric view of a
@@ -504,9 +491,6 @@ do_mixed_source_and_assembly (struct gdbarch *gdbarch,
   int i, nlines;
   int num_displayed = 0;
   print_source_lines_flags psl_flags = 0;
-  struct cleanup *ui_out_chain;
-  struct cleanup *ui_out_tuple_chain;
-  struct cleanup *ui_out_list_chain;
   CORE_ADDR pc;
   struct symtab *last_symtab;
   int last_line;
@@ -567,21 +551,21 @@ do_mixed_source_and_assembly (struct gdbarch *gdbarch,
      CLI output works on top of this because MI ignores ui_out_text output,
      which is where we put file name and source line contents output.
 
-     Cleanup usage:
-     ui_out_chain
+     Emitter usage:
+     asm_insns_emitter
        Handles the outer "asm_insns" list.
-     ui_out_tuple_chain
+     tuple_emitter
        The tuples for each group of consecutive disassemblies.
-     ui_out_list_chain
+     list_emitter
        List of consecutive source lines or disassembled insns.  */
 
   if (flags & DISASSEMBLY_FILENAME)
     psl_flags |= PRINT_SOURCE_LINES_FILENAME;
 
-  ui_out_chain = make_cleanup_ui_out_list_begin_end (uiout, "asm_insns");
+  ui_out_emit_list asm_insns_emitter (uiout, "asm_insns");
 
-  ui_out_tuple_chain = NULL;
-  ui_out_list_chain = NULL;
+  gdb::optional<ui_out_emit_tuple> tuple_emitter;
+  gdb::optional<ui_out_emit_list> list_emitter;
 
   last_symtab = NULL;
   last_line = 0;
@@ -650,11 +634,11 @@ do_mixed_source_and_assembly (struct gdbarch *gdbarch,
 	  /* Skip the newline if this is the first instruction.  */
 	  if (pc > low)
 	    uiout->text ("\n");
-	  if (ui_out_tuple_chain != NULL)
+	  if (tuple_emitter.has_value ())
 	    {
-	      gdb_assert (ui_out_list_chain != NULL);
-	      do_cleanups (ui_out_list_chain);
-	      do_cleanups (ui_out_tuple_chain);
+	      gdb_assert (list_emitter.has_value ());
+	      list_emitter.reset ();
+	      tuple_emitter.reset ();
 	    }
 	  if (sal.symtab != last_symtab
 	      && !(flags & DISASSEMBLY_FILENAME))
@@ -676,7 +660,6 @@ do_mixed_source_and_assembly (struct gdbarch *gdbarch,
 		 We need to preserve the structure of the output, so output
 		 a bunch of line tuples with no asm entries.  */
 	      int l;
-	      struct cleanup *ui_out_list_chain_line;
 
 	      gdb_assert (sal.symtab != NULL);
 	      for (l = start_preceding_line_to_display;
@@ -685,28 +668,23 @@ do_mixed_source_and_assembly (struct gdbarch *gdbarch,
 		{
 		  ui_out_emit_tuple tuple_emitter (uiout, "src_and_asm_line");
 		  print_source_lines (sal.symtab, l, l + 1, psl_flags);
-		  ui_out_list_chain_line
-		    = make_cleanup_ui_out_list_begin_end (uiout,
-							  "line_asm_insn");
-		  do_cleanups (ui_out_list_chain_line);
+		  ui_out_emit_list chain_line_emitter (uiout, "line_asm_insn");
 		}
 	    }
-	  ui_out_tuple_chain
-	    = make_cleanup_ui_out_tuple_begin_end (uiout, "src_and_asm_line");
+	  tuple_emitter.emplace (uiout, "src_and_asm_line");
 	  if (sal.symtab != NULL)
 	    print_source_lines (sal.symtab, sal.line, sal.line + 1, psl_flags);
 	  else
 	    uiout->text (_("--- no source info for this pc ---\n"));
-	  ui_out_list_chain
-	    = make_cleanup_ui_out_list_begin_end (uiout, "line_asm_insn");
+	  list_emitter.emplace (uiout, "line_asm_insn");
 	}
       else
 	{
 	  /* Here we're appending instructions to an existing line.
 	     By construction the very first insn will have a symtab
 	     and follow the new_source_line path above.  */
-	  gdb_assert (ui_out_tuple_chain != NULL);
-	  gdb_assert (ui_out_list_chain != NULL);
+	  gdb_assert (tuple_emitter.has_value ());
+	  gdb_assert (list_emitter.has_value ());
 	}
 
       if (sal.end != 0)
@@ -723,8 +701,6 @@ do_mixed_source_and_assembly (struct gdbarch *gdbarch,
       last_symtab = sal.symtab;
       last_line = sal.line;
     }
-
-  do_cleanups (ui_out_chain);
 }
 
 static void

Re: [RFA 4/7] Use ui_out_emit_tuple in disasm.c

[Simon Marchi]

On 2017-10-13 12:13 PM, Tom Tromey wrote:
> Tom> It turns out that with gdb::optional, you don't have to untangle it.
> Tom> I'm testing a patch to convert the rest of disasm.c away from cleanups.
> 
> How about this?
> 
> Tom
> 
> commit 1b83351de30ed32808aa8b09d720556d423e57c6
> Author: Tom Tromey <tom@tromey.com>
> Date:   Thu Oct 12 15:07:35 2017 -0600
> 
>     Remove cleanups from disasm.c
>     
>     This changes the remaining spots in disasm.c to use the RAII ui-out
>     emitters, removing a few cleanups.  This also fixes a regression that
>     Simon pointed out.
>     
>     2017-10-12  Tom Tromey  <tom@tromey.com>
>     
>             * disasm.c (do_mixed_source_and_assembly_deprecated): Use
>             gdb::optional, ui_out_emit_list, ui_out_emit_tuple.
>             (do_mixed_source_and_assembly): Likewise.

That looks good to me (though I'm a bit overwhelmed by that code).

The regression I pointed out is fixed, mi-disassemble.exp passes and I manually
tested with the disassembly view of Eclipse, which exercises that code.  So I
think it's safe enough.

Thanks!

Simon

Re: [RFA 4/7] Use ui_out_emit_tuple in disasm.c

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> That looks good to me (though I'm a bit overwhelmed by that code).

Me too -- it's why I didn't write this change much earlier.

Tom