[RFA v2 01/24] Introduce and use ui_out_emit_table

[Simon Marchi]

Hi Tom,

Just some formatting comments, I think it's fine to push with those 
fixed.

> @@ -6851,48 +6850,43 @@ breakpoint_1 (char *args, int allflag,
>  	}
>      }
> 
> -  if (opts.addressprint)
> -    bkpttbl_chain
> -      = make_cleanup_ui_out_table_begin_end (uiout, 6,
> -					     nr_printable_breakpoints,
> -                                             "BreakpointTable");
> -  else
> -    bkpttbl_chain
> -      = make_cleanup_ui_out_table_begin_end (uiout, 5,
> -					     nr_printable_breakpoints,
> -                                             "BreakpointTable");
> -
> -  if (nr_printable_breakpoints > 0)
> -    annotate_breakpoints_headers ();
> -  if (nr_printable_breakpoints > 0)
> -    annotate_field (0);
> -  uiout->table_header (7, ui_left, "number", "Num"); /* 1 */
> -  if (nr_printable_breakpoints > 0)
> -    annotate_field (1);
> -  uiout->table_header (print_type_col_width, ui_left, "type", "Type"); 
> /* 2 */
> -  if (nr_printable_breakpoints > 0)
> -    annotate_field (2);
> -  uiout->table_header (4, ui_left, "disp", "Disp"); /* 3 */
> -  if (nr_printable_breakpoints > 0)
> -    annotate_field (3);
> -  uiout->table_header (3, ui_left, "enabled", "Enb"); /* 4 */
> -  if (opts.addressprint)
> -    {
> -      if (nr_printable_breakpoints > 0)
> -	annotate_field (4);
> -      if (print_address_bits <= 32)
> -	uiout->table_header (10, ui_left, "addr", "Address"); /* 5 */
> -      else
> -	uiout->table_header (18, ui_left, "addr", "Address"); /* 5 */
> -    }
> -  if (nr_printable_breakpoints > 0)
> -    annotate_field (5);
> -  uiout->table_header (40, ui_noalign, "what", "What"); /* 6 */
> -  uiout->table_body ();
> -  if (nr_printable_breakpoints > 0)
> -    annotate_breakpoints_table ();
> +  {
> +    ui_out_emit_table table_emitter (uiout,
> +				     opts.addressprint ? 6 : 5,
> +				     nr_printable_breakpoints,
> +				     "BreakpointTable");
> +
> +    if (nr_printable_breakpoints > 0)
> +      annotate_breakpoints_headers ();
> +    if (nr_printable_breakpoints > 0)
> +      annotate_field (0);
> +    uiout->table_header (7, ui_left, "number", "Num"); /* 1 */
> +    if (nr_printable_breakpoints > 0)
> +      annotate_field (1);
> +    uiout->table_header (print_type_col_width, ui_left, "type",
> "Type"); /* 2 */
> +    if (nr_printable_breakpoints > 0)
> +      annotate_field (2);
> +    uiout->table_header (4, ui_left, "disp", "Disp"); /* 3 */
> +    if (nr_printable_breakpoints > 0)
> +      annotate_field (3);
> +    uiout->table_header (3, ui_left, "enabled", "Enb"); /* 4 */
> +    if (opts.addressprint)
> +      {
> +	if (nr_printable_breakpoints > 0)
> +	  annotate_field (4);
> +	if (print_address_bits <= 32)
> +	  uiout->table_header (10, ui_left, "addr", "Address"); /* 5 */
> +	else
> +	  uiout->table_header (18, ui_left, "addr", "Address"); /* 5 */
> +      }
> +    if (nr_printable_breakpoints > 0)
> +      annotate_field (5);
> +    uiout->table_header (40, ui_noalign, "what", "What"); /* 6 */
> +    uiout->table_body ();
> +    if (nr_printable_breakpoints > 0)
> +      annotate_breakpoints_table ();
> 
> -  ALL_BREAKPOINTS (b)
> +    ALL_BREAKPOINTS (b)

Shouldn't the scope just under this be indented as well?

> @@ -1075,19 +1074,18 @@ info_sharedlibrary_command (char *pattern, int 
> from_tty)
>  	}
>      }
> 
> -  table_cleanup =
> -    make_cleanup_ui_out_table_begin_end (uiout, 4, nr_libs,
> -					 "SharedLibraryTable");
> +  {
> +    ui_out_emit_table table_emitter (uiout, 4, nr_libs, 
> "SharedLibraryTable");
> 
> -  /* The "- 1" is because ui_out adds one space between columns.  */
> -  uiout->table_header (addr_width - 1, ui_left, "from", "From");
> -  uiout->table_header (addr_width - 1, ui_left, "to", "To");
> -  uiout->table_header (12 - 1, ui_left, "syms-read", "Syms Read");
> -  uiout->table_header (0, ui_noalign, "name", "Shared Object 
> Library");
> +    /* The "- 1" is because ui_out adds one space between columns.  */
> +    uiout->table_header (addr_width - 1, ui_left, "from", "From");
> +    uiout->table_header (addr_width - 1, ui_left, "to", "To");
> +    uiout->table_header (12 - 1, ui_left, "syms-read", "Syms Read");
> +    uiout->table_header (0, ui_noalign, "name", "Shared Object 
> Library");
> 
> -  uiout->table_body ();
> +    uiout->table_body ();
> 
> -  ALL_SO_LIBS (so)
> +    ALL_SO_LIBS (so)

I think the scope below should be indented as well.

>      {
>        if (! so->so_name[0])
>  	continue;
> @@ -1121,8 +1119,7 @@ info_sharedlibrary_command (char *pattern, int 
> from_tty)
> 
>        uiout->text ("\n");
>      }
> -
> -  do_cleanups (table_cleanup);
> +  }
> 
>    if (nr_libs == 0)
>      {
> diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
> index 4f2bac5..6721e22 100644
> --- a/gdb/tracepoint.c
> +++ b/gdb/tracepoint.c
> @@ -482,7 +482,6 @@ tvariables_info_1 (void)
>    struct trace_state_variable *tsv;
>    int ix;
>    int count = 0;
> -  struct cleanup *back_to;
>    struct ui_out *uiout = current_uiout;
> 
>    if (VEC_length (tsv_s, tvariables) == 0 && !uiout->is_mi_like_p ())
> @@ -496,8 +495,7 @@ tvariables_info_1 (void)
>      tsv->value_known = target_get_trace_state_variable_value 
> (tsv->number,
>  							      &(tsv->value));
> 
> -  back_to = make_cleanup_ui_out_table_begin_end (uiout, 3,
> -                                                 count, 
> "trace-variables");
> +  ui_out_emit_table table_emitter (uiout, 3, count, 
> "trace-variables");
>    uiout->table_header (15, ui_left, "name", "Name");
>    uiout->table_header (11, ui_left, "initial", "Initial");
>    uiout->table_header (11, ui_left, "current", "Current");
> @@ -531,8 +529,6 @@ tvariables_info_1 (void)
>          uiout->field_string ("current", c);
>        uiout->text ("\n");
>      }
> -
> -  do_cleanups (back_to);
>  }
> 
>  /* List all the trace state variables.  */
> @@ -3952,9 +3948,8 @@ info_static_tracepoint_markers_command (char
> *arg, int from_tty)
>       don't work without in-process agent, so we don't bother users to 
> type
>       `set agent on' when to use static tracepoint.  */
> 
> -  old_chain
> -    = make_cleanup_ui_out_table_begin_end (uiout, 5, -1,
> -					   "StaticTracepointMarkersTable");
> +  ui_out_emit_table table_emitter (uiout, 5, -1,
> +				   "StaticTracepointMarkersTable");
> 
>    uiout->table_header (7, ui_left, "counter", "Cnt");
> 
> @@ -3970,7 +3965,7 @@ info_static_tracepoint_markers_command (char
> *arg, int from_tty)
>    uiout->table_body ();
> 
>    markers = target_static_tracepoint_markers_by_strid (NULL);
> -  make_cleanup (VEC_cleanup (static_tracepoint_marker_p), &markers);
> +  old_chain = make_cleanup (VEC_cleanup (static_tracepoint_marker_p),
> &markers);
> 
>    for (i = 0;
>         VEC_iterate (static_tracepoint_marker_p,
> diff --git a/gdb/ui-out.h b/gdb/ui-out.h
> index 9278cab..2293f26 100644
> --- a/gdb/ui-out.h
> +++ b/gdb/ui-out.h
> @@ -220,4 +220,31 @@ 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.  */
> +class ui_out_emit_table
> +{
> +public:
> +
> +  ui_out_emit_table (struct ui_out *uiout, int nr_cols, int nr_rows,
> +		     const char *tblid)
> +

Spurious empty line?

> +    : m_uiout (uiout)
> +  {
> +    m_uiout->table_begin (nr_cols, nr_rows, tblid);
> +  }
> +
> +  ~ui_out_emit_table ()
> +  {
> +    m_uiout->table_end ();
> +  }
> +
> +  ui_out_emit_table (const ui_out_emit_table &) = delete;
> +  ui_out_emit_table &operator= (const ui_out_emit_table &) = delete;
> +
> +private:
> +
> +  struct ui_out *m_uiout;
> +};
> +
>  #endif /* UI_OUT_H */

Thanks,

Simon