diff mbox

[19/22] Class-ify ui_out_impl

Message ID 1480173587-7053-19-git-send-email-simon.marchi@polymtl.ca
State New, archived
Headers

Commit Message

Simon Marchi Nov. 26, 2016, 3:19 p.m. UTC 
  From: Simon Marchi <simon.marchi@polymtl.ca>

This patch makes the ui_out_impl implementations an actual class
hierarchy, instead of an array of function pointers.

gdb/ChangeLog:

	* ui-out.h (class ui_out_impl_base): New class, based on now removed
	struct ui_out_impl.
	(table_begin_ftype): Remove, replace with pure virtual method in
	class ui_out_impl_base.
	(table_body_ftype): Likewise.
	(table_end_ftype): Likewise.
	(table_header_ftype): Likewise.
	(ui_out_begin_ftype): Likewise.
	(ui_out_end_ftype): Likewise.
	(field_int_ftype): Likewise.
	(field_skip_ftype): Likewise.
	(field_string_ftype): Likewise.
	(field_fmt_ftype): Likewise.
	(spaces_ftype): Likewise.
	(text_ftype): Likewise.
	(message_ftype): Likewise.
	(wrap_hint_ftype): Likewise.
	(flush_ftype): Likewise.
	(redirect_ftype): Likewise.
	(data_destroy_ftype): Likewise.
	(struct ui_out_impl): Remove.
	(ui_out_data): Remove.
	(ui_out_new): Change type of parameter impl, remove parameter
	data, add default value to parameter flags.
	(ui_out_impl): New function.
	(uo_field_string): Move declaration to ui-out.c.
	* ui-out.c (struct ui_out) <impl>: Change type to unique_ptr to
	ui_out_impl_base.
	<data>: Remove.
	(uo_field_string): Move declaration from ui-out.h.
	(ui_out_is_mi_like_p): Update to call virtual method.
	(uo_table_begin): Remove null check, update call to virtual
	method.
	(uo_table_body): Likewise.
	(uo_table_end): Likewise.
	(uo_table_header): Likewise.
	(uo_begin): Likewise.
	(uo_end): Likewise.
	(uo_field_int): Likewise.
	(uo_field_skip): Likewise.
	(uo_field_string): Likewise.
	(uo_field_fmt): Likewise.
	(uo_spaces): Likewise.
	(uo_text): Likewise.
	(uo_message): Likewise.
	(uo_wrap_hint): Likewise.
	(uo_flush): Likewise.
	(uo_redirect): Likewise.
	(ui_out_data): Remove.
	(ui_out_new): Change type of parameter impl, remove parameter
	data, add default value to parameter flags.  Update assignment
	to unique_ptr
	(ui_out_impl): New function.
	* cli-out.h (cli_ui_out_data): Remove, replace with class
	cli_ui_out_impl.
	(class cli_ui_out_impl): New class.
	(cli_ui_out_impl): Remove global variable.
	(cli_out_data_ctor): Remove.
	* cli-out.c (cli_out_data): Remove.
	(cli_uiout_dtor): Remove.
	(cli_table_begin): Replace with ...
	(cli_ui_out_impl::table_begin): ... this new method.
	(cli_table_body): Replace with ...
	(cli_ui_out_impl::table_body): ... this new method.
	(cli_table_end): Replace with ...
	(cli_ui_out_impl::table_end): ... this new method.
	(cli_table_header): Replace with ...
	(cli_ui_out_impl::table_header): ... this new method.
	(cli_begin): Replace with ...
	(cli_ui_out_impl::begin): ... this new method.
	(cli_end): Replace with ...
	(cli_ui_out_impl::table_end): ... this new method.
	(cli_field_int): Replace with ...
	(cli_ui_out_impl::field_int): ... this new method.
	(cli_field_skip): Replace with ...
	(cli_ui_out_impl::field_skip): ... this new method.
	(cli_field_string): Replace with ...
	(cli_ui_out_impl::field_string): ... this new method.
	(cli_field_fmt): Replace with ...
	(cli_ui_out_impl::field_fmt): ... this new method.
	(cli_spaces): Replace with ...
	(cli_ui_out_impl::spaces): ... this new method.
	(cli_text): Replace with ...
	(cli_ui_out_impl::text): ... this new method.
	(cli_message): Replace with ...
	(cli_ui_out_impl::message): ... this new method.
	(cli_wrap_hint): Replace with ...
	(cli_ui_out_impl::wrap_hint): ... this new method.
	(cli_flush): Replace with ...
	(cli_ui_out_impl::flush): ... this new method.
	(cli_redirect): Replace with ...
	(cli_ui_out_impl::redirect): ... this new method.
	(out_field_fmt): Replace with ...
	(cli_ui_out_impl::out_field_fmt): ... this new method.
	(field_separator): Replace with ...
	(cli_ui_out_impl::field_separator): ... this new method.
	(cli_ui_out_impl): Remove.
	(cli_out_data_ctor): Remove.
	(cli_ui_out_impl::cli_ui_out_impl): New constructor.
	(cli_ui_out_impl::~cli_ui_out_impl): New destructor.
	(cli_out_new): Instantiate
	(cli_field_string): Likewise.
	(cli_redirect): Likewise.
	(cli_out_data_ctor): Likewise.
	* mi/mi-out.c (UNKNOWN): Likewise.
	(mi_field_skip): Likewise.
	(mi_field_string): Likewise.
	(mi_field_fmt): Likewise.
	(mi_out_put): Likewise.
	(mi_out_data_dtor): Likewise.
	(mi_ui_out_impl_from): New function.
	* mi/mi-out.h (UNKNOWN): Likewise.
	* tui/tui-out.c (UNKNOWN): Likewise.
---
 gdb/cli-out.c     | 306 +++++++++++++++++--------------------------------
 gdb/cli-out.h     |  58 ++++++++--
 gdb/mi/mi-out.c   | 333 +++++++++++++++++++++---------------------------------
 gdb/mi/mi-out.h   |  69 ++++++++++-
 gdb/tui/tui-out.c | 158 +++++++++++---------------
 gdb/ui-out.c      |  95 +++++-----------
 gdb/ui-out.h      | 125 +++++++-------------
 7 files changed, 479 insertions(+), 665 deletions(-)

Comments

Pedro Alves Nov. 30, 2016, 1:09 p.m. UTC | #1 
On 11/26/2016 03:19 PM, simon.marchi@polymtl.ca wrote:
> From: Simon Marchi <simon.marchi@polymtl.ca>
> 
> This patch makes the ui_out_impl implementations an actual class
> hierarchy, instead of an array of function pointers.

LGTM with nits below.

> +ui_file *
> +cli_out_set_stream (struct ui_out *uiout, ui_file *stream)
> +{
> +  cli_ui_out_impl *impl = dynamic_cast<cli_ui_out_impl *> (ui_out_impl (uiout));

"dynamic_cast" is a code smell.  In this case, I think it
stems from the fact that we have separate ui_out and ui_out_impl
hierarchies.  If we had a simple hierarchy, then this function would
take a  cli_ui_out pointer as argument, or even be a
method of cli_ui_out.

Anyway, just a comment for another day.

Given the current design, this is OK.

> +
> +  gdb_assert (impl != NULL);
> +
> +  return impl->set_stream (stream);
> +}
> +
>  /* CLI interface to display tab-completion matches.  */
>  
>  /* CLI version of displayer.crlf.  */
> diff --git a/gdb/cli-out.h b/gdb/cli-out.h
> index 296b8c0..aacb8c5 100644
> --- a/gdb/cli-out.h
> +++ b/gdb/cli-out.h
> @@ -23,22 +23,58 @@
>  #include "ui-out.h"
>  #include <vector>
>  
> -/* These are exported so that they can be extended by other `ui_out'
> -   implementations, like TUI's.  */
> +class cli_ui_out_impl : public ui_out_impl_base
> +{
> +public:
>  
> -struct cli_ui_out_data
> -  {
> -    std::vector<ui_file *> streams;
> -    int suppress_output;
> -  };
> +  cli_ui_out_impl (ui_file *stream);

explicit.

> +  virtual ~cli_ui_out_impl ();
>  
> -extern const struct ui_out_impl cli_ui_out_impl;
> +  virtual void table_begin (int nbrofcols, int nr_rows,
> +				  const char *tblid) override;
> +  virtual void table_body () override;
> +  virtual void table_end () override;
> +  virtual void table_header (int width, ui_align align,
> +			     const std::string &col_name,
> +			     const std::string &col_hdr) override;
> +  /* Note: level 0 is the top-level so LEVEL is always greater than
> +     zero.  */
> +  virtual void begin (ui_out_type type, const char *id) override;
> +  virtual void end (ui_out_type type) override;
> +  virtual void field_int (int fldno, int width, ui_align align,
> +			  const char *fldname, int value) override;
> +  virtual void field_skip (int fldno, int width, ui_align align,
> +			   const char *fldname) override;
> +  virtual void field_string (int fldno, int width, ui_align align,
> +			     const char *fldname, const char *string) override;
> +  virtual void ATTRIBUTE_PRINTF(6,0) field_fmt (int fldno, int width,
> +						ui_align align,
> +						const char *fldname,
> +						const char *format,
> +						va_list args) override;
> +  virtual void spaces (int numspaces) override;
> +  virtual void text (const char *string) override;
> +  virtual void ATTRIBUTE_PRINTF(2,0)
> +    message (const char *format, va_list args) override;
> +  virtual void wrap_hint (const char *identstring) override;
> +  virtual void flush () override;
> +  virtual int redirect (struct ui_file * outstream) override;
>  
> +  ui_file *set_stream (ui_file *stream);
>  
> -extern struct ui_out *cli_out_new (struct ui_file *stream);
> +protected:
> +
> +  std::vector<ui_file *> m_streams;
> +  int m_suppress_output;
> +
> +private:
> +  void field_separator (void);

"(void)".  Probably easier to grep the whole patch set to
find such cases and handle all of them.


> +mi_ui_out_impl::table_begin (int nr_cols, int nr_rows,
> +			     const char *tblid)
>  {
> -  mi_open (uiout, tblid, ui_out_type_tuple);
> -  mi_field_int (uiout, -1, -1, ui_left, "nr_rows", nr_rows);
> -  mi_field_int (uiout, -1, -1, ui_left, "nr_cols", nr_cols);
> -  mi_open (uiout, "hdr", ui_out_type_list);
> +  open (tblid, ui_out_type_tuple);

FYI, it's due to names like "open" potentially conflicting
with gnulib replacing global namespace C functions with
#define open rpl_open
that I'm looking at switching to use gnulib's C++ namespace
support.  Luckily, looks like gnulib doesn't replace 
"open" currently, so you don't need to wait for that:

 $ grep -rn open | grep rpl_
 stdio.in.h:191:#   define fdopen rpl_fdopen
 stdio.in.h:264:#   define fopen rpl_fopen
 stdio.in.h:388:#   define freopen rpl_freopen
 stdio.in.h:820:#   define popen rpl_popen
 dirent.in.h:79:#   define opendir rpl_opendir
 dirent.in.h:198:#   define fdopendir rpl_fdopendir

>  void
> -mi_table_end (struct ui_out *uiout)
> +mi_ui_out_impl::table_end ()
>  {
> -  mi_out_data *data = (mi_out_data *) ui_out_data (uiout);
> -
> -  data->suppress_output = 0;
> -  mi_close (uiout, ui_out_type_list); /* body */
> -  mi_close (uiout, ui_out_type_tuple);
> +  m_suppress_output = 0;
> +  close (ui_out_type_list); /* body */
> +  close (ui_out_type_tuple);

However:

 $ grep -rn close | grep rpl_
 stdio.in.h:172:#   define fclose rpl_fclose
 dirent.in.h:131:#   define closedir rpl_closedir
 unistd.in.h:303:#   define close rpl_close


:-/

I'd like to post the gnulib namespace patch this week,
but I'm not sure I'll be able to.

> +  virtual void field_string (int fldno, int width, ui_align align,
> +			     const char *fldname, const char *string) override;
> +  virtual void ATTRIBUTE_PRINTF(6,0)
> +    field_fmt (int fldno, int width, ui_align align, const char *fldname,
> +	       const char *format, va_list args) override;

It's more usual to put the ATTRIBUTE_PRINTFs at the end of the
declaration.  Also, missing space before parens.  Something like:

  virtual void field_fmt (int fldno, int width, ui_align align, 
                          const char *fldname,
                          const char *format, va_list args) override
    ATTRIBUTE_PRINTF (6,0);

?

> +  virtual void spaces (int numspaces) override;
> +  virtual void text (const char *string) override;
> +  virtual void ATTRIBUTE_PRINTF(2,0) message (const char *format, va_list args)
> +    override;

Ditto.  There are other instances in the patch.  I won't point them all.

> +private:
> +
> +  void field_separator ();
> +  void open (const char *name, ui_out_type type);
> +  void close (ui_out_type type);
> +
> +  int m_suppress_field_separator;
> +  int m_suppress_output;

bool ?

> +  int m_mi_version;
> +  std::vector<ui_file *> m_streams;
> +};



> -typedef struct tui_ui_out_data tui_out_data;
> +class tui_ui_out_impl : public cli_ui_out_impl
> +{
> +public:
> +
> +  tui_ui_out_impl (ui_file *stream);

explicit.

> +
> +  void field_int (int fldno, int width, ui_align align, const char *fldname,
> +		  int value) override;
> +  void field_string (int fldno, int width, ui_align align, const char *fldname,
> +		     const char *string) override;
> +  void ATTRIBUTE_PRINTF(6,0)
> +    field_fmt (int fldno, int width, ui_align align, const char *fldname,
> +	       const char *format, va_list args) override;

attribute placement, space before parens.  

> +  void text (const char *string) override;
>  


> +  virtual int redirect (struct ui_file * outstream) = 0;
> +
> +  /* Set as not MI-like by default.  It is overridden in subclasses if
> +     necessary.  */
> +
> +  virtual int is_mi_like_p ()
> +  { return 0; }

bool ?

> +};
> +

Thanks,
Pedro Alves
Simon Marchi Nov. 30, 2016, 10:38 p.m. UTC | #2 
On 2016-11-30 08:09, Pedro Alves wrote:
>> +ui_file *
>> +cli_out_set_stream (struct ui_out *uiout, ui_file *stream)
>> +{
>> +  cli_ui_out_impl *impl = dynamic_cast<cli_ui_out_impl *> 
>> (ui_out_impl (uiout));
> 
> "dynamic_cast" is a code smell.  In this case, I think it
> stems from the fact that we have separate ui_out and ui_out_impl
> hierarchies.  If we had a simple hierarchy, then this function would
> take a  cli_ui_out pointer as argument, or even be a
> method of cli_ui_out.

Indeed it's not great.  This patchset is mostly about keeping the 
structure we have, but express it as C++.  So I'd argue that it uncovers 
something that already existed.  I think it makes the situation a tiny 
bit better though, as we have some type checking with dynamic_cast.  
With the static cast we have currently, if you happened to pass the 
wrong kind of object, it would probably fail catastrophically.

I'll consider working on merging ui_out and ui_out_impl_* in a single 
class hierarchy.  My first question is: what is a good pattern for the 
overlapping methods.  For example, table_begin.  We'll want to execute 
the version of the base class, which will then call the specialization.  
So we can't use the same name for both.  So, keep table_begin for the 
base class, and table_begin_impl for the derived classes?

>> +class cli_ui_out_impl : public ui_out_impl_base
>> +{
>> +public:
>> 
>> -struct cli_ui_out_data
>> -  {
>> -    std::vector<ui_file *> streams;
>> -    int suppress_output;
>> -  };
>> +  cli_ui_out_impl (ui_file *stream);
> 
> explicit.

Done.

>> +private:
>> +  void field_separator (void);
> 
> "(void)".  Probably easier to grep the whole patch set to
> find such cases and handle all of them.

Yep, that's what I did the first time I saw that comment.

>> +mi_ui_out_impl::table_begin (int nr_cols, int nr_rows,
>> +			     const char *tblid)
>>  {
>> -  mi_open (uiout, tblid, ui_out_type_tuple);
>> -  mi_field_int (uiout, -1, -1, ui_left, "nr_rows", nr_rows);
>> -  mi_field_int (uiout, -1, -1, ui_left, "nr_cols", nr_cols);
>> -  mi_open (uiout, "hdr", ui_out_type_list);
>> +  open (tblid, ui_out_type_tuple);
> 
> FYI, it's due to names like "open" potentially conflicting
> with gnulib replacing global namespace C functions with
> #define open rpl_open
> that I'm looking at switching to use gnulib's C++ namespace
> support.  Luckily, looks like gnulib doesn't replace
> "open" currently, so you don't need to wait for that:
> 
>  $ grep -rn open | grep rpl_
>  stdio.in.h:191:#   define fdopen rpl_fdopen
>  stdio.in.h:264:#   define fopen rpl_fopen
>  stdio.in.h:388:#   define freopen rpl_freopen
>  stdio.in.h:820:#   define popen rpl_popen
>  dirent.in.h:79:#   define opendir rpl_opendir
>  dirent.in.h:198:#   define fdopendir rpl_fdopendir
> 
>>  void
>> -mi_table_end (struct ui_out *uiout)
>> +mi_ui_out_impl::table_end ()
>>  {
>> -  mi_out_data *data = (mi_out_data *) ui_out_data (uiout);
>> -
>> -  data->suppress_output = 0;
>> -  mi_close (uiout, ui_out_type_list); /* body */
>> -  mi_close (uiout, ui_out_type_tuple);
>> +  m_suppress_output = 0;
>> +  close (ui_out_type_list); /* body */
>> +  close (ui_out_type_tuple);
> 
> However:
> 
>  $ grep -rn close | grep rpl_
>  stdio.in.h:172:#   define fclose rpl_fclose
>  dirent.in.h:131:#   define closedir rpl_closedir
>  unistd.in.h:303:#   define close rpl_close
> 
> 
> :-/
> 
> I'd like to post the gnulib namespace patch this week,
> but I'm not sure I'll be able to.

And I guess it happens to work anyway for me because both the 
declaration, definition and usages get replaced?

Should I wait for your patch to get in (I'm not particularly in a 
hurry), or we can get it in despite "close" getting replaced?

>> +  virtual void field_string (int fldno, int width, ui_align align,
>> +			     const char *fldname, const char *string) override;
>> +  virtual void ATTRIBUTE_PRINTF(6,0)
>> +    field_fmt (int fldno, int width, ui_align align, const char 
>> *fldname,
>> +	       const char *format, va_list args) override;
> 
> It's more usual to put the ATTRIBUTE_PRINTFs at the end of the
> declaration.  Also, missing space before parens.  Something like:
> 
>   virtual void field_fmt (int fldno, int width, ui_align align,
>                           const char *fldname,
>                           const char *format, va_list args) override
>     ATTRIBUTE_PRINTF (6,0);
> 
> ?

Ah, I put them there because I though it was the only place where it 
worked when applied to virtual pure functions.  But apparently, this 
works:

  98   virtual void field_fmt (int fldno, int width, ui_align align,
  99                           const char *fldname, const char *format, 
va_list args)
100     ATTRIBUTE_PRINTF(6,0) = 0;

I thought I had tried it and it didn't work, but I was wrong.  I'll fix 
them so they look like that.

>> +  virtual void spaces (int numspaces) override;
>> +  virtual void text (const char *string) override;
>> +  virtual void ATTRIBUTE_PRINTF(2,0) message (const char *format, 
>> va_list args)
>> +    override;
> 
> Ditto.  There are other instances in the patch.  I won't point them 
> all.

I'll go over all the patches and fix that.

>> +private:
>> +
>> +  void field_separator ();
>> +  void open (const char *name, ui_out_type type);
>> +  void close (ui_out_type type);
>> +
>> +  int m_suppress_field_separator;
>> +  int m_suppress_output;
> 
> bool ?

Right.  I changed it, but from what I can see MI's m_suppress_output is 
never used (as in never set to true).  If that's indeed the case, I'll 
make a separate patch to remove it in the current master code.

CLI has a m_suppress_output that is used though, I changed it as well to 
bool.

>> +  int m_mi_version;
>> +  std::vector<ui_file *> m_streams;
>> +};
> 
> 
> 
>> -typedef struct tui_ui_out_data tui_out_data;
>> +class tui_ui_out_impl : public cli_ui_out_impl
>> +{
>> +public:
>> +
>> +  tui_ui_out_impl (ui_file *stream);
> 
> explicit.

Done.

>> +
>> +  void field_int (int fldno, int width, ui_align align, const char 
>> *fldname,
>> +		  int value) override;
>> +  void field_string (int fldno, int width, ui_align align, const char 
>> *fldname,
>> +		     const char *string) override;
>> +  void ATTRIBUTE_PRINTF(6,0)
>> +    field_fmt (int fldno, int width, ui_align align, const char 
>> *fldname,
>> +	       const char *format, va_list args) override;
> 
> attribute placement, space before parens.

Done.

>> +  void text (const char *string) override;
>> 
> 
> 
>> +  virtual int redirect (struct ui_file * outstream) = 0;
>> +
>> +  /* Set as not MI-like by default.  It is overridden in subclasses 
>> if
>> +     necessary.  */
>> +
>> +  virtual int is_mi_like_p ()
>> +  { return 0; }
> 
> bool ?

Done.

Thanks for the review.  I'll try to push the small patches that were 
OK'ed without comments, and re-post a complete v2 series.
Pedro Alves Nov. 30, 2016, 10:57 p.m. UTC | #3 
On 11/30/2016 10:38 PM, Simon Marchi wrote:

> I'll consider working on merging ui_out and ui_out_impl_* in a single
> class hierarchy.  My first question is: what is a good pattern for the
> overlapping methods.  For example, table_begin.  We'll want to execute
> the version of the base class, which will then call the specialization. 
> So we can't use the same name for both.  So, keep table_begin for the
> base class, and table_begin_impl for the derived classes?

Yes.  I think gold's convention is to use $method for public methods, and
do_$method for protected virtual methods that implementations
override/provide.  I've seen "do_" used for the same purpose in other
projects too.  (IIRC, gold is even stricter and requires that virtual
methods must be protected, thus mandating that design everywhere.
Grep for "this->do_".)

>> I'd like to post the gnulib namespace patch this week,
>> but I'm not sure I'll be able to.
> 
> And I guess it happens to work anyway for me because both the
> declaration, definition and usages get replaced?

Yeah, unistd.h is included in defs.h, so it most probably 
won't be an issue in practice.

> 
> Should I wait for your patch to get in (I'm not particularly in a
> hurry), or we can get it in despite "close" getting replaced?

Given the above, no need to wait.

Thanks,
Pedro Alves
Simon Marchi Dec. 1, 2016, 7:04 p.m. UTC | #4 
On 2016-11-30 17:57, Pedro Alves wrote:
> On 11/30/2016 10:38 PM, Simon Marchi wrote:
> 
>> I'll consider working on merging ui_out and ui_out_impl_* in a single
>> class hierarchy.  My first question is: what is a good pattern for the
>> overlapping methods.  For example, table_begin.  We'll want to execute
>> the version of the base class, which will then call the 
>> specialization.
>> So we can't use the same name for both.  So, keep table_begin for the
>> base class, and table_begin_impl for the derived classes?
> 
> Yes.  I think gold's convention is to use $method for public methods, 
> and
> do_$method for protected virtual methods that implementations
> override/provide.  I've seen "do_" used for the same purpose in other
> projects too.  (IIRC, gold is even stricter and requires that virtual
> methods must be protected, thus mandating that design everywhere.
> Grep for "this->do_".)

As I am doing this, I realize we'll need a cast anyway, it just changes 
where it is.

For example, mi_cmd_var_list_children calls "mi_version (uiout)".  The 
uiout variable is of type "ui_out *", since we don't know at compile 
time that the current_uiout will be an MI uiout.  We know it will be 
because of how the program works, but that can only be verified at 
runtime.  So in the end, instead of doing a dynamic_cast in mi_version, 
we'll have to do one in the function using the uiout.

I still think that merging ui_out and ui_out_impl is a good idea though, 
there's no good reason for them to be separate.
Pedro Alves Dec. 1, 2016, 7:30 p.m. UTC | #5 
On 12/01/2016 07:04 PM, Simon Marchi wrote:
> On 2016-11-30 17:57, Pedro Alves wrote:
>> On 11/30/2016 10:38 PM, Simon Marchi wrote:
>>
>>> I'll consider working on merging ui_out and ui_out_impl_* in a single
>>> class hierarchy.  My first question is: what is a good pattern for the
>>> overlapping methods.  For example, table_begin.  We'll want to execute
>>> the version of the base class, which will then call the specialization.
>>> So we can't use the same name for both.  So, keep table_begin for the
>>> base class, and table_begin_impl for the derived classes?
>>
>> Yes.  I think gold's convention is to use $method for public methods, and
>> do_$method for protected virtual methods that implementations
>> override/provide.  I've seen "do_" used for the same purpose in other
>> projects too.  (IIRC, gold is even stricter and requires that virtual
>> methods must be protected, thus mandating that design everywhere.
>> Grep for "this->do_".)
> 
> As I am doing this, I realize we'll need a cast anyway, it just changes
> where it is.
> 
> For example, mi_cmd_var_list_children calls "mi_version (uiout)".  The
> uiout variable is of type "ui_out *", since we don't know at compile
> time that the current_uiout will be an MI uiout.  We know it will be
> because of how the program works, but that can only be verified at
> runtime.  So in the end, instead of doing a dynamic_cast in mi_version,
> we'll have to do one in the function using the uiout.

In that case, I'd say that problem is that that code uses "current_uiout".
If we walk up the call stack, we should find the MI interpreter somewhere,
which has a handle to the MI uiout, should know its type.  If somehow one
of those was passed down, either by parameter (or by being able to access
them from some existing MI-specific global context structure, maybe the
existing "current_context"), then the problem wouldn't exist.  We'd still be
shifting the casts higher up a bit, as we still have things like
as_mi_interp, which are effectively dynamic_casts in the ui<->interp code.
That one would probably be sorted by cleaning up the "virtual" methods
of "struct interpreter" and "struct ui", e.g., by adding an
"interpreter->execute_command (....)" virtual method.  Certainly
nothing urgent to fix, but I think good to keep in mind.

So as long as we're shifting casts closer to the top level, it seems like
we're heading in the proper direction.

BTW, in code that knows what the dynamic type of a pointer should be, we
should feel free to use static_cast<derived *>.  But I'm fine with
using dynamic_cast<> as validation aid.  Just be aware that it's not
free performance-wise.

> I still think that merging ui_out and ui_out_impl is a good idea though,
> there's no good reason for them to be separate.

*nod*

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/cli-out.c b/gdb/cli-out.c
index e042926..93ca5ed 100644
--- a/gdb/cli-out.c
+++ b/gdb/cli-out.c
@@ -27,163 +27,107 @@ 
 #include "vec.h"
 #include "readline/readline.h"
 
-typedef struct cli_ui_out_data cli_out_data;
-
-/* Prototypes for local functions */
-
-static void cli_text (struct ui_out *uiout, const char *string);
-
-static void field_separator (void);
-
-static void out_field_fmt (struct ui_out *uiout, int fldno,
-			   const char *fldname,
-			   const char *format,...) ATTRIBUTE_PRINTF (4, 5);
-
-/* The destructor.  */
-
-static void
-cli_uiout_dtor (struct ui_out *ui_out)
-{
-  cli_out_data *data = (cli_out_data *) ui_out_data (ui_out);
-
-  delete data;
-}
-
 /* These are the CLI output functions */
 
 /* Mark beginning of a table */
 
-static void
-cli_table_begin (struct ui_out *uiout, int nbrofcols,
-		 int nr_rows,
-		 const char *tblid)
+void
+cli_ui_out_impl::table_begin (int nbrofcols, int nr_rows, const char *tblid)
 {
-  cli_out_data *data = (cli_out_data *) ui_out_data (uiout);
-
   if (nr_rows == 0)
-    data->suppress_output = 1;
+    m_suppress_output = 1;
   else
     /* Only the table suppresses the output and, fortunately, a table
        is not a recursive data structure.  */
-    gdb_assert (data->suppress_output == 0);
+    gdb_assert (m_suppress_output == 0);
 }
 
 /* Mark beginning of a table body */
 
-static void
-cli_table_body (struct ui_out *uiout)
+void
+cli_ui_out_impl::table_body ()
 {
-  cli_out_data *data = (cli_out_data *) ui_out_data (uiout);
-
-  if (data->suppress_output)
+  if (m_suppress_output)
     return;
+
   /* first, close the table header line */
-  cli_text (uiout, "\n");
+  text ("\n");
 }
 
 /* Mark end of a table */
 
-static void
-cli_table_end (struct ui_out *uiout)
+void
+cli_ui_out_impl::table_end ()
 {
-  cli_out_data *data = (cli_out_data *) ui_out_data (uiout);
-
-  data->suppress_output = 0;
+  m_suppress_output = 0;
 }
 
 /* Specify table header */
 
-static void
-cli_table_header (struct ui_out *uiout, int width, enum ui_align alignment,
-		  const std::string &col_name, const std::string &col_hdr)
+void
+cli_ui_out_impl::table_header (int width, ui_align alignment,
+			       const std::string &col_name,
+			       const std::string &col_hdr)
 {
-  cli_out_data *data = (cli_out_data *) ui_out_data (uiout);
-
-  if (data->suppress_output)
+  if (m_suppress_output)
     return;
 
-  /* Always go through the function pointer (virtual function call).
-     We may have been extended.  */
-  uo_field_string (uiout, 0, width, alignment, 0, col_hdr.c_str ());
+  field_string (0, width, alignment, 0, col_hdr.c_str ());
 }
 
 /* Mark beginning of a list */
 
-static void
-cli_begin (struct ui_out *uiout,
-	   enum ui_out_type type,
-	   const char *id)
+void
+cli_ui_out_impl::begin (ui_out_type type, const char *id)
 {
-  cli_out_data *data = (cli_out_data *) ui_out_data (uiout);
-
-  if (data->suppress_output)
-    return;
 }
 
 /* Mark end of a list */
 
-static void
-cli_end (struct ui_out *uiout,
-	 enum ui_out_type type)
+void
+cli_ui_out_impl::end (ui_out_type type)
 {
-  cli_out_data *data = (cli_out_data *) ui_out_data (uiout);
-
-  if (data->suppress_output)
-    return;
 }
 
 /* output an int field */
 
-static void
-cli_field_int (struct ui_out *uiout, int fldno, int width,
-	       enum ui_align alignment,
-	       const char *fldname, int value)
+void
+cli_ui_out_impl::field_int (int fldno, int width, ui_align alignment,
+			    const char *fldname, int value)
 {
   char buffer[20];	/* FIXME: how many chars long a %d can become? */
-  cli_out_data *data = (cli_out_data *) ui_out_data (uiout);
 
-  if (data->suppress_output)
+  if (m_suppress_output)
     return;
+
   xsnprintf (buffer, sizeof (buffer), "%d", value);
 
-  /* Always go through the function pointer (virtual function call).
-     We may have been extended.  */
-  uo_field_string (uiout, fldno, width, alignment, fldname, buffer);
+  field_string (fldno, width, alignment, fldname, buffer);
 }
 
-/* used to ommit a field */
+/* used to omit a field */
 
-static void
-cli_field_skip (struct ui_out *uiout, int fldno, int width,
-		enum ui_align alignment,
-		const char *fldname)
+void
+cli_ui_out_impl::field_skip (int fldno, int width, ui_align alignment,
+			     const char *fldname)
 {
-  cli_out_data *data = (cli_out_data *) ui_out_data (uiout);
-
-  if (data->suppress_output)
+  if (m_suppress_output)
     return;
 
-  /* Always go through the function pointer (virtual function call).
-     We may have been extended.  */
-  uo_field_string (uiout, fldno, width, alignment, fldname, "");
+  field_string (fldno, width, alignment, fldname, "");
 }
 
 /* other specific cli_field_* end up here so alignment and field
    separators are both handled by cli_field_string */
 
-static void
-cli_field_string (struct ui_out *uiout,
-		  int fldno,
-		  int width,
-		  enum ui_align align,
-		  const char *fldname,
-		  const char *string)
+void
+cli_ui_out_impl::field_string (int fldno, int width, ui_align align,
+			       const char *fldname, const char *string)
 {
   int before = 0;
   int after = 0;
-  cli_out_data *data = (cli_out_data *) ui_out_data (uiout);
 
-  if (data->suppress_output)
+  if (m_suppress_output)
     return;
 
   if ((align != ui_noalign) && string)
@@ -210,11 +154,13 @@  cli_field_string (struct ui_out *uiout,
     }
 
   if (before)
-    ui_out_spaces (uiout, before);
+    spaces (before);
+
   if (string)
-    out_field_fmt (uiout, fldno, fldname, "%s", string);
+    out_field_fmt (fldno, fldname, "%s", string);
+
   if (after)
-    ui_out_spaces (uiout, after);
+    spaces (after);
 
   if (align != ui_noalign)
     field_separator ();
@@ -222,96 +168,73 @@  cli_field_string (struct ui_out *uiout,
 
 /* This is the only field function that does not align.  */
 
-static void ATTRIBUTE_PRINTF (6, 0)
-cli_field_fmt (struct ui_out *uiout, int fldno,
-	       int width, enum ui_align align,
-	       const char *fldname,
-	       const char *format,
-	       va_list args)
+void
+cli_ui_out_impl::field_fmt (int fldno, int width, ui_align align,
+			    const char *fldname, const char *format,
+			    va_list args)
 {
-  cli_out_data *data = (cli_out_data *) ui_out_data (uiout);
-  struct ui_file *stream;
-
-  if (data->suppress_output)
+  if (m_suppress_output)
     return;
 
-  stream = data->streams.back ();
-  vfprintf_filtered (stream, format, args);
+  vfprintf_filtered (m_streams.back (), format, args);
 
   if (align != ui_noalign)
     field_separator ();
 }
 
-static void
-cli_spaces (struct ui_out *uiout, int numspaces)
+void
+cli_ui_out_impl::spaces (int numspaces)
 {
-  cli_out_data *data = (cli_out_data *) ui_out_data (uiout);
-  struct ui_file *stream;
-
-  if (data->suppress_output)
+  if (m_suppress_output)
     return;
 
-  stream = data->streams.back ();
-  print_spaces_filtered (numspaces, stream);
+  print_spaces_filtered (numspaces, m_streams.back ());
 }
 
-static void
-cli_text (struct ui_out *uiout, const char *string)
+void
+cli_ui_out_impl::text (const char *string)
 {
-  cli_out_data *data = (cli_out_data *) ui_out_data (uiout);
-  struct ui_file *stream;
-
-  if (data->suppress_output)
+  if (m_suppress_output)
     return;
 
-  stream = data->streams.back ();
-  fputs_filtered (string, stream);
+  fputs_filtered (string, m_streams.back ());
 }
 
-static void ATTRIBUTE_PRINTF (2, 0)
-cli_message (struct ui_out *uiout, const char *format, va_list args)
+void
+cli_ui_out_impl::message (const char *format, va_list args)
 {
-  cli_out_data *data = (cli_out_data *) ui_out_data (uiout);
-
-  if (data->suppress_output)
+  if (m_suppress_output)
     return;
 
-  struct ui_file *stream = data->streams.back ();
-  vfprintf_unfiltered (stream, format, args);
+  vfprintf_unfiltered (m_streams.back (), format, args);
 }
 
-static void
-cli_wrap_hint (struct ui_out *uiout, const char *identstring)
+void
+cli_ui_out_impl::wrap_hint (const char *identstring)
 {
-  cli_out_data *data = (cli_out_data *) ui_out_data (uiout);
-
-  if (data->suppress_output)
+  if (m_suppress_output)
     return;
+
   wrap_here (identstring);
 }
 
-static void
-cli_flush (struct ui_out *uiout)
+void
+cli_ui_out_impl::flush ()
 {
-  cli_out_data *data = (cli_out_data *) ui_out_data (uiout);
-  struct ui_file *stream = data->streams.back ();
-
-  gdb_flush (stream);
+  gdb_flush (m_streams.back ());
 }
 
 /* OUTSTREAM as non-NULL will push OUTSTREAM on the stack of output streams
    and make it therefore active.  OUTSTREAM as NULL will pop the last pushed
    output stream; it is an internal error if it does not exist.  */
 
-static int
-cli_redirect (struct ui_out *uiout, struct ui_file *outstream)
+int
+cli_ui_out_impl::redirect (ui_file *outstream)
 {
-  cli_out_data *data = (cli_out_data *) ui_out_data (uiout);
-
   if (outstream != NULL)
-    data->streams.push_back (outstream);
+    m_streams.push_back (outstream);
   else
-    data->streams.pop_back ();
+    m_streams.pop_back ();
 
   return 0;
 }
@@ -322,66 +245,38 @@  cli_redirect (struct ui_out *uiout, struct ui_file *outstream)
    and makes a va_list and does not insert a separator.  */
 
 /* VARARGS */
-static void
-out_field_fmt (struct ui_out *uiout, int fldno,
-	       const char *fldname,
-	       const char *format,...)
+void
+cli_ui_out_impl::out_field_fmt (int fldno, const char *fldname,
+				const char *format, ...)
 {
-  cli_out_data *data = (cli_out_data *) ui_out_data (uiout);
-  struct ui_file *stream = data->streams.back ();
   va_list args;
 
   va_start (args, format);
-  vfprintf_filtered (stream, format, args);
+  vfprintf_filtered (m_streams.back (), format, args);
 
   va_end (args);
 }
 
 /* Access to ui_out format private members.  */
 
-static void
-field_separator (void)
+void
+cli_ui_out_impl::field_separator (void)
 {
-  cli_out_data *data = (cli_out_data *) ui_out_data (current_uiout);
-  struct ui_file *stream = data->streams.back ();
-
-  fputc_filtered (' ', stream);
+  fputc_filtered (' ', m_streams.back ());
 }
 
-/* This is the CLI ui-out implementation functions vector */
+/* Constructor for cli_ui_out_impl.  */
 
-const struct ui_out_impl cli_ui_out_impl =
-{
-  cli_table_begin,
-  cli_table_body,
-  cli_table_end,
-  cli_table_header,
-  cli_begin,
-  cli_end,
-  cli_field_int,
-  cli_field_skip,
-  cli_field_string,
-  cli_field_fmt,
-  cli_spaces,
-  cli_text,
-  cli_message,
-  cli_wrap_hint,
-  cli_flush,
-  cli_redirect,
-  cli_uiout_dtor,
-  0, /* Does not need MI hacks (i.e. needs CLI hacks).  */
-};
-
-/* Constructor for a `cli_out_data' object.  */
-
-void
-cli_out_data_ctor (cli_out_data *self, struct ui_file *stream)
+cli_ui_out_impl::cli_ui_out_impl (ui_file *stream)
+: m_suppress_output (0)
 {
   gdb_assert (stream != NULL);
 
-  self->streams.push_back (stream);
+  m_streams.push_back (stream);
+}
 
-  self->suppress_output = 0;
+cli_ui_out_impl::~cli_ui_out_impl ()
+{
 }
 
 /* Initialize private members at startup.  */
@@ -389,26 +284,33 @@  cli_out_data_ctor (cli_out_data *self, struct ui_file *stream)
 struct ui_out *
 cli_out_new (struct ui_file *stream)
 {
-  int flags = ui_source_list;
-  cli_out_data *data = new cli_out_data ();
+  cli_ui_out_impl *impl = new cli_ui_out_impl (stream);
 
-  cli_out_data_ctor (data, stream);
-  return ui_out_new (&cli_ui_out_impl, data, flags);
+  return ui_out_new (impl, ui_source_list);
 }
 
-struct ui_file *
-cli_out_set_stream (struct ui_out *uiout, struct ui_file *stream)
+ui_file *
+cli_ui_out_impl::set_stream (struct ui_file *stream)
 {
-  cli_out_data *data = (cli_out_data *) ui_out_data (uiout);
-  struct ui_file *old;
+  ui_file *old;
 
-  old = data->streams.back ();
-  data->streams.pop_back ();
-  data->streams.push_back (stream);
+  old = m_streams.back ();
+  m_streams.pop_back ();
+  m_streams.push_back (stream);
 
   return old;
 }
 
+ui_file *
+cli_out_set_stream (struct ui_out *uiout, ui_file *stream)
+{
+  cli_ui_out_impl *impl = dynamic_cast<cli_ui_out_impl *> (ui_out_impl (uiout));
+
+  gdb_assert (impl != NULL);
+
+  return impl->set_stream (stream);
+}
+
 /* CLI interface to display tab-completion matches.  */
 
 /* CLI version of displayer.crlf.  */
diff --git a/gdb/cli-out.h b/gdb/cli-out.h
index 296b8c0..aacb8c5 100644
--- a/gdb/cli-out.h
+++ b/gdb/cli-out.h
@@ -23,22 +23,58 @@ 
 #include "ui-out.h"
 #include <vector>
 
-/* These are exported so that they can be extended by other `ui_out'
-   implementations, like TUI's.  */
+class cli_ui_out_impl : public ui_out_impl_base
+{
+public:
 
-struct cli_ui_out_data
-  {
-    std::vector<ui_file *> streams;
-    int suppress_output;
-  };
+  cli_ui_out_impl (ui_file *stream);
+  virtual ~cli_ui_out_impl ();
 
-extern const struct ui_out_impl cli_ui_out_impl;
+  virtual void table_begin (int nbrofcols, int nr_rows,
+				  const char *tblid) override;
+  virtual void table_body () override;
+  virtual void table_end () override;
+  virtual void table_header (int width, ui_align align,
+			     const std::string &col_name,
+			     const std::string &col_hdr) override;
+  /* Note: level 0 is the top-level so LEVEL is always greater than
+     zero.  */
+  virtual void begin (ui_out_type type, const char *id) override;
+  virtual void end (ui_out_type type) override;
+  virtual void field_int (int fldno, int width, ui_align align,
+			  const char *fldname, int value) override;
+  virtual void field_skip (int fldno, int width, ui_align align,
+			   const char *fldname) override;
+  virtual void field_string (int fldno, int width, ui_align align,
+			     const char *fldname, const char *string) override;
+  virtual void ATTRIBUTE_PRINTF(6,0) field_fmt (int fldno, int width,
+						ui_align align,
+						const char *fldname,
+						const char *format,
+						va_list args) override;
+  virtual void spaces (int numspaces) override;
+  virtual void text (const char *string) override;
+  virtual void ATTRIBUTE_PRINTF(2,0)
+    message (const char *format, va_list args) override;
+  virtual void wrap_hint (const char *identstring) override;
+  virtual void flush () override;
+  virtual int redirect (struct ui_file * outstream) override;
 
+  ui_file *set_stream (ui_file *stream);
 
-extern struct ui_out *cli_out_new (struct ui_file *stream);
+protected:
+
+  std::vector<ui_file *> m_streams;
+  int m_suppress_output;
+
+private:
+  void field_separator (void);
 
-extern void cli_out_data_ctor (struct cli_ui_out_data *data,
-			       struct ui_file *stream);
+  void  ATTRIBUTE_PRINTF(4, 5)
+    out_field_fmt (int fldno, const char *fldname, const char *format, ...);
+};
+
+extern struct ui_out *cli_out_new (struct ui_file *stream);
 
 extern struct ui_file *cli_out_set_stream (struct ui_out *uiout,
 					   struct ui_file *stream);
diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
index b7cd433..744b113 100644
--- a/gdb/mi/mi-out.c
+++ b/gdb/mi/mi-out.c
@@ -25,185 +25,100 @@ 
 #include "vec.h"
 #include <vector>
 
-struct mi_ui_out_data
-  {
-    int suppress_field_separator;
-    int suppress_output;
-    int mi_version;
-    std::vector<ui_file *> streams;
-  };
-typedef struct mi_ui_out_data mi_out_data;
-
-/* These are the MI output functions */
-
-static void mi_out_data_dtor (struct ui_out *ui_out);
-static void mi_table_begin (struct ui_out *uiout, int nbrofcols,
-			    int nr_rows, const char *tblid);
-static void mi_table_body (struct ui_out *uiout);
-static void mi_table_end (struct ui_out *uiout);
-static void mi_table_header (struct ui_out *uiout, int width,
-			     enum ui_align alignment,
-			     const std::string &col_name,
-			     const std::string &col_hdr);
-static void mi_begin (struct ui_out *uiout, enum ui_out_type type,
-		      const char *id);
-static void mi_end (struct ui_out *uiout, enum ui_out_type type);
-static void mi_field_int (struct ui_out *uiout, int fldno, int width,
-			  enum ui_align alig, const char *fldname, int value);
-static void mi_field_skip (struct ui_out *uiout, int fldno, int width,
-			   enum ui_align alig, const char *fldname);
-static void mi_field_string (struct ui_out *uiout, int fldno, int width,
-			     enum ui_align alig, const char *fldname,
-			     const char *string);
-static void mi_field_fmt (struct ui_out *uiout, int fldno,
-			  int width, enum ui_align align,
-			  const char *fldname, const char *format,
-			  va_list args) ATTRIBUTE_PRINTF (6, 0);
-static void mi_spaces (struct ui_out *uiout, int numspaces);
-static void mi_text (struct ui_out *uiout, const char *string);
-static void mi_message (struct ui_out *uiout, const char *format, va_list args)
-     ATTRIBUTE_PRINTF (2, 0);
-static void mi_wrap_hint (struct ui_out *uiout, const char *identstring);
-static void mi_flush (struct ui_out *uiout);
-static int mi_redirect (struct ui_out *uiout, struct ui_file *outstream);
-
-/* This is the MI ui-out implementation functions vector */
-
-static const struct ui_out_impl mi_ui_out_impl =
-{
-  mi_table_begin,
-  mi_table_body,
-  mi_table_end,
-  mi_table_header,
-  mi_begin,
-  mi_end,
-  mi_field_int,
-  mi_field_skip,
-  mi_field_string,
-  mi_field_fmt,
-  mi_spaces,
-  mi_text,
-  mi_message,
-  mi_wrap_hint,
-  mi_flush,
-  mi_redirect,
-  mi_out_data_dtor,
-  1, /* Needs MI hacks.  */
-};
-
-/* Prototypes for local functions */
-
-static void field_separator (struct ui_out *uiout);
-static void mi_open (struct ui_out *uiout, const char *name,
-		     enum ui_out_type type);
-static void mi_close (struct ui_out *uiout, enum ui_out_type type);
-
 /* Mark beginning of a table.  */
 
 void
-mi_table_begin (struct ui_out *uiout,
-		int nr_cols,
-		int nr_rows,
-		const char *tblid)
+mi_ui_out_impl::table_begin (int nr_cols, int nr_rows,
+			     const char *tblid)
 {
-  mi_open (uiout, tblid, ui_out_type_tuple);
-  mi_field_int (uiout, -1, -1, ui_left, "nr_rows", nr_rows);
-  mi_field_int (uiout, -1, -1, ui_left, "nr_cols", nr_cols);
-  mi_open (uiout, "hdr", ui_out_type_list);
+  open (tblid, ui_out_type_tuple);
+  field_int (-1, -1, ui_left, "nr_rows", nr_rows);
+  field_int (-1, -1, ui_left, "nr_cols", nr_cols);
+  open ("hdr", ui_out_type_list);
 }
 
 /* Mark beginning of a table body.  */
 
 void
-mi_table_body (struct ui_out *uiout)
+mi_ui_out_impl::table_body ()
 {
-  mi_out_data *data = (mi_out_data *) ui_out_data (uiout);
-
-  if (data->suppress_output)
+  if (m_suppress_output)
     return;
   /* close the table header line if there were any headers */
-  mi_close (uiout, ui_out_type_list);
-  mi_open (uiout, "body", ui_out_type_list);
+  close (ui_out_type_list);
+  open ("body", ui_out_type_list);
 }
 
 /* Mark end of a table.  */
 
 void
-mi_table_end (struct ui_out *uiout)
+mi_ui_out_impl::table_end ()
 {
-  mi_out_data *data = (mi_out_data *) ui_out_data (uiout);
-
-  data->suppress_output = 0;
-  mi_close (uiout, ui_out_type_list); /* body */
-  mi_close (uiout, ui_out_type_tuple);
+  m_suppress_output = 0;
+  close (ui_out_type_list); /* body */
+  close (ui_out_type_tuple);
 }
 
 /* Specify table header.  */
 
 void
-mi_table_header (struct ui_out *uiout, int width, enum ui_align alignment,
-		 const std::string &col_name, const std::string &col_hdr)
+mi_ui_out_impl::table_header (int width, ui_align alignment,
+			      const std::string &col_name,
+			      const std::string &col_hdr)
 {
-  mi_out_data *data = (mi_out_data *) ui_out_data (uiout);
-
-  if (data->suppress_output)
+  if (m_suppress_output)
     return;
 
-  mi_open (uiout, NULL, ui_out_type_tuple);
-  mi_field_int (uiout, 0, 0, ui_center, "width", width);
-  mi_field_int (uiout, 0, 0, ui_center, "alignment", alignment);
-  mi_field_string (uiout, 0, 0, ui_center, "col_name", col_name.c_str ());
-  mi_field_string (uiout, 0, width, alignment, "colhdr", col_hdr.c_str ());
-  mi_close (uiout, ui_out_type_tuple);
+  open (NULL, ui_out_type_tuple);
+  field_int (0, 0, ui_center, "width", width);
+  field_int (0, 0, ui_center, "alignment", alignment);
+  field_string (0, 0, ui_center, "col_name", col_name.c_str ());
+  field_string (0, width, alignment, "colhdr", col_hdr.c_str ());
+  close (ui_out_type_tuple);
 }
 
 /* Mark beginning of a list.  */
 
 void
-mi_begin (struct ui_out *uiout, enum ui_out_type type, const char *id)
+mi_ui_out_impl::begin (ui_out_type type, const char *id)
 {
-  mi_out_data *data = (mi_out_data *) ui_out_data (uiout);
-
-  if (data->suppress_output)
+  if (m_suppress_output)
     return;
 
-  mi_open (uiout, id, type);
+  open (id, type);
 }
 
 /* Mark end of a list.  */
 
 void
-mi_end (struct ui_out *uiout, enum ui_out_type type)
+mi_ui_out_impl::end (ui_out_type type)
 {
-  mi_out_data *data = (mi_out_data *) ui_out_data (uiout);
-
-  if (data->suppress_output)
+  if (m_suppress_output)
     return;
 
-  mi_close (uiout, type);
+  close (type);
 }
 
 /* Output an int field.  */
 
-static void
-mi_field_int (struct ui_out *uiout, int fldno, int width,
-              enum ui_align alignment, const char *fldname, int value)
+void
+mi_ui_out_impl::field_int (int fldno, int width, ui_align alignment,
+			   const char *fldname, int value)
 {
   char buffer[20];	/* FIXME: how many chars long a %d can become? */
-  mi_out_data *data = (mi_out_data *) ui_out_data (uiout);
 
-  if (data->suppress_output)
+  if (m_suppress_output)
     return;
 
   xsnprintf (buffer, sizeof (buffer), "%d", value);
-  mi_field_string (uiout, fldno, width, alignment, fldname, buffer);
+  field_string (fldno, width, alignment, fldname, buffer);
 }
 
 /* Used to omit a field.  */
 
 void
-mi_field_skip (struct ui_out *uiout, int fldno, int width,
-               enum ui_align alignment, const char *fldname)
+mi_ui_out_impl::field_skip (int fldno, int width, ui_align alignment,
+			    const char *fldname)
 {
 }
 
@@ -211,17 +126,15 @@  mi_field_skip (struct ui_out *uiout, int fldno, int width,
    separators are both handled by mi_field_string. */
 
 void
-mi_field_string (struct ui_out *uiout, int fldno, int width,
-		 enum ui_align align, const char *fldname, const char *string)
+mi_ui_out_impl::field_string (int fldno, int width, ui_align align,
+			      const char *fldname, const char *string)
 {
-  mi_out_data *data = (mi_out_data *) ui_out_data (uiout);
-  struct ui_file *stream;
-
-  if (data->suppress_output)
+  if (m_suppress_output)
     return;
 
-  stream = data->streams.back ();
-  field_separator (uiout);
+  ui_file *stream = m_streams.back ();
+  field_separator ();
+
   if (fldname)
     fprintf_unfiltered (stream, "%s=", fldname);
   fprintf_unfiltered (stream, "\"");
@@ -233,18 +146,16 @@  mi_field_string (struct ui_out *uiout, int fldno, int width,
 /* This is the only field function that does not align.  */
 
 void
-mi_field_fmt (struct ui_out *uiout, int fldno, int width,
-	      enum ui_align align, const char *fldname,
-	      const char *format, va_list args)
+mi_ui_out_impl::field_fmt (int fldno, int width, ui_align align,
+			   const char *fldname, const char *format,
+			   va_list args)
 {
-  mi_out_data *data = (mi_out_data *) ui_out_data (uiout);
-  struct ui_file *stream;
-
-  if (data->suppress_output)
+  if (m_suppress_output)
     return;
 
-  stream = data->streams.back ();
-  field_separator (uiout);
+  ui_file *stream = m_streams.back ();
+  field_separator ();
+
   if (fldname)
     fprintf_unfiltered (stream, "%s=\"", fldname);
   else
@@ -254,125 +165,115 @@  mi_field_fmt (struct ui_out *uiout, int fldno, int width,
 }
 
 void
-mi_spaces (struct ui_out *uiout, int numspaces)
+mi_ui_out_impl::spaces (int numspaces)
 {
 }
 
 void
-mi_text (struct ui_out *uiout, const char *string)
+mi_ui_out_impl::text (const char *string)
 {
 }
 
 void
-mi_message (struct ui_out *uiout, const char *format, va_list args)
+mi_ui_out_impl::message (const char *format, va_list args)
 {
 }
 
 void
-mi_wrap_hint (struct ui_out *uiout, const char *identstring)
+mi_ui_out_impl::wrap_hint (const char *identstring)
 {
   wrap_here (identstring);
 }
 
 void
-mi_flush (struct ui_out *uiout)
+mi_ui_out_impl::flush ()
 {
-  mi_out_data *data = (mi_out_data *) ui_out_data (uiout);
-  struct ui_file *stream = data->streams.back ();
 
-  gdb_flush (stream);
+  gdb_flush (m_streams.back ());
 }
 
 int
-mi_redirect (struct ui_out *uiout, struct ui_file *outstream)
+mi_ui_out_impl::redirect (ui_file *outstream)
 {
-  mi_out_data *data = (mi_out_data *) ui_out_data (uiout);
-
   if (outstream != NULL)
-    data->streams.push_back (outstream);
+    m_streams.push_back (outstream);
   else
-    data->streams.pop_back ();
+    m_streams.pop_back ();
 
   return 0;
 }
 
-/* local functions */
-
-/* access to ui_out format private members */
-
-static void
-field_separator (struct ui_out *uiout)
+void
+mi_ui_out_impl::field_separator ()
 {
-  mi_out_data *data = (mi_out_data *) ui_out_data (uiout);
-  ui_file *stream = data->streams.back ();
-
-  if (data->suppress_field_separator)
-    data->suppress_field_separator = 0;
+  if (m_suppress_field_separator)
+    m_suppress_field_separator = 0;
   else
-    fputc_unfiltered (',', stream);
+    fputc_unfiltered (',', m_streams.back ());
 }
 
-static void
-mi_open (struct ui_out *uiout, const char *name, enum ui_out_type type)
+void
+mi_ui_out_impl::open (const char *name, ui_out_type type)
 {
-  mi_out_data *data = (mi_out_data *) ui_out_data (uiout);
-  ui_file *stream = data->streams.back ();
+  ui_file *stream = m_streams.back ();
+
+  field_separator ();
+  m_suppress_field_separator = 1;
 
-  field_separator (uiout);
-  data->suppress_field_separator = 1;
   if (name)
     fprintf_unfiltered (stream, "%s=", name);
+
   switch (type)
     {
     case ui_out_type_tuple:
       fputc_unfiltered ('{', stream);
       break;
+
     case ui_out_type_list:
       fputc_unfiltered ('[', stream);
       break;
+
     default:
       internal_error (__FILE__, __LINE__, _("bad switch"));
     }
 }
 
-static void
-mi_close (struct ui_out *uiout, enum ui_out_type type)
+void
+mi_ui_out_impl::close (ui_out_type type)
 {
-  mi_out_data *data = (mi_out_data *) ui_out_data (uiout);
-  ui_file *stream = data->streams.back ();
+  ui_file *stream = m_streams.back ();
 
   switch (type)
     {
     case ui_out_type_tuple:
       fputc_unfiltered ('}', stream);
       break;
+
     case ui_out_type_list:
       fputc_unfiltered (']', stream);
       break;
+
     default:
       internal_error (__FILE__, __LINE__, _("bad switch"));
     }
-  data->suppress_field_separator = 0;
+
+  m_suppress_field_separator = 0;
 }
 
 /* Clear the buffer.  */
 
 void
-mi_out_rewind (struct ui_out *uiout)
+mi_ui_out_impl::rewind ()
 {
-  mi_out_data *data = (mi_out_data *) ui_out_data (uiout);
-  ui_file *stream = data->streams.back ();
-
-  ui_file_rewind (stream);
+  ui_file_rewind (m_streams.back ());
 }
 
 /* Dump the buffer onto the specified stream.  */
 
 void
-mi_out_put (struct ui_out *uiout, struct ui_file *stream)
+mi_ui_out_impl::put (ui_file *stream)
 {
-  mi_out_data *data = (mi_out_data *) ui_out_data (uiout);
-  ui_file *outstream = data->streams.back ();
+  ui_file *outstream = m_streams.back ();
 
   ui_file_put (outstream, ui_file_write_for_put, stream);
   ui_file_rewind (outstream);
@@ -381,35 +282,25 @@  mi_out_put (struct ui_out *uiout, struct ui_file *stream)
 /* Return the current MI version.  */
 
 int
-mi_version (struct ui_out *uiout)
+mi_ui_out_impl::version ()
 {
-  mi_out_data *data = (mi_out_data *) ui_out_data (uiout);
-
-  return data->mi_version;
+  return m_mi_version;
 }
 
 /* Constructor for an `mi_out_data' object.  */
 
-static void
-mi_out_data_ctor (mi_out_data *self, int mi_version, struct ui_file *stream)
+mi_ui_out_impl::mi_ui_out_impl (int mi_version, ui_file *stream)
+: m_suppress_field_separator (0),
+  m_suppress_output (0),
+  m_mi_version (mi_version)
 {
   gdb_assert (stream != NULL);
 
-  self->streams.push_back (stream);
-
-  self->suppress_field_separator = 0;
-  self->suppress_output = 0;
-  self->mi_version = mi_version;
+  m_streams.push_back (stream);
 }
 
-/* The destructor.  */
-
-static void
-mi_out_data_dtor (struct ui_out *ui_out)
+mi_ui_out_impl::~mi_ui_out_impl ()
 {
-  mi_out_data *data = (mi_out_data *) ui_out_data (ui_out);
-
-  delete data;
 }
 
 /* Initialize private members at startup.  */
@@ -417,10 +308,40 @@  mi_out_data_dtor (struct ui_out *ui_out)
 struct ui_out *
 mi_out_new (int mi_version)
 {
-  int flags = 0;
-  mi_out_data *data = new mi_out_data ();
-  struct ui_file *stream = mem_fileopen ();
+  ui_file *stream = mem_fileopen ();
+  mi_ui_out_impl *impl = new mi_ui_out_impl (mi_version, stream);
+
+  return ui_out_new (impl);
+}
+
+/* Helper to return the implementation behind UIOUT, casted to a
+   mi_ui_out_impl *.  It is an error to call this function with an ui_out that
+   is not an MI.  */
+
+static mi_ui_out_impl *
+mi_ui_out_impl_from (ui_out *uiout)
+{
+  mi_ui_out_impl *impl = dynamic_cast<mi_ui_out_impl *> (ui_out_impl (uiout));
+
+  gdb_assert (impl != NULL);
+
+  return impl;
+}
+
+int
+mi_version (ui_out *uiout)
+{
+  return mi_ui_out_impl_from (uiout)->version ();
+}
+
+void
+mi_out_put (ui_out *uiout, struct ui_file *stream)
+{
+  return mi_ui_out_impl_from (uiout)->put (stream);
+}
 
-  mi_out_data_ctor (data, mi_version, stream);
-  return ui_out_new (&mi_ui_out_impl, data, flags);
+void
+mi_out_rewind (ui_out *uiout)
+{
+  return mi_ui_out_impl_from (uiout)->rewind ();
 }
diff --git a/gdb/mi/mi-out.h b/gdb/mi/mi-out.h
index ba18950..a45d2e3 100644
--- a/gdb/mi/mi-out.h
+++ b/gdb/mi/mi-out.h
@@ -20,14 +20,73 @@ 
 #ifndef MI_OUT_H
 #define MI_OUT_H 1
 
+#include <vector>
+
 struct ui_out;
 struct ui_file;
 
-extern struct ui_out *mi_out_new (int mi_version);
-extern void mi_out_put (struct ui_out *uiout, struct ui_file *stream);
-extern void mi_out_rewind (struct ui_out *uiout);
 
-/* Return the version number of the current MI.  */
-extern int mi_version (struct ui_out *uiout);
+class mi_ui_out_impl : public ui_out_impl_base
+{
+public:
+
+  mi_ui_out_impl (int mi_version, ui_file *stream);
+  virtual ~mi_ui_out_impl ();
+
+  virtual void table_begin (int nbrofcols, int nr_rows, const char *tblid)
+    override;
+  virtual void table_body () override;
+  virtual void table_header (int width, ui_align align,
+			     const std::string &col_name,
+			     const std::string &col_hdr) override;
+  virtual void table_end () override;
+
+  /* Note: level 0 is the top-level so LEVEL is always greater than
+     zero.  */
+  virtual void begin (ui_out_type type, const char *id) override;
+  virtual void end (ui_out_type type) override;
+  virtual void field_int (int fldno, int width, ui_align align,
+			  const char *fldname, int value) override;
+  virtual void field_skip (int fldno, int width, ui_align align,
+			   const char *fldname) override;
+  virtual void field_string (int fldno, int width, ui_align align,
+			     const char *fldname, const char *string) override;
+  virtual void ATTRIBUTE_PRINTF(6,0)
+    field_fmt (int fldno, int width, ui_align align, const char *fldname,
+	       const char *format, va_list args) override;
+  virtual void spaces (int numspaces) override;
+  virtual void text (const char *string) override;
+  virtual void ATTRIBUTE_PRINTF(2,0) message (const char *format, va_list args)
+    override;
+  virtual void wrap_hint (const char *identstring) override;
+  virtual void flush () override;
+  virtual int redirect (struct ui_file * outstream) override;
+
+  virtual int is_mi_like_p () override
+  { return 1; }
+
+  /* MI-specific */
+  void rewind ();
+  void put (struct ui_file *stream);
+  /* Return the version number of the current MI.  */
+  int version ();
+
+private:
+
+  void field_separator ();
+  void open (const char *name, ui_out_type type);
+  void close (ui_out_type type);
+
+  int m_suppress_field_separator;
+  int m_suppress_output;
+  int m_mi_version;
+  std::vector<ui_file *> m_streams;
+};
+
+ui_out *mi_out_new (int mi_version);
+int mi_version (ui_out *uiout);
+void mi_out_put (ui_out *uiout, struct ui_file *stream);
+void mi_out_rewind (ui_out *uiout);
+
 
 #endif /* MI_OUT_H */
diff --git a/gdb/tui/tui-out.c b/gdb/tui/tui-out.c
index 4856562..70633f3 100644
--- a/gdb/tui/tui-out.c
+++ b/gdb/tui/tui-out.c
@@ -24,154 +24,122 @@ 
 #include "ui-out.h"
 #include "cli-out.h"
 #include "tui.h"
-struct tui_ui_out_data
-  {
-    struct cli_ui_out_data base;
 
-    int line;
-    int start_of_line;
-  };
-typedef struct tui_ui_out_data tui_out_data;
+class tui_ui_out_impl : public cli_ui_out_impl
+{
+public:
+
+  tui_ui_out_impl (ui_file *stream);
+
+  void field_int (int fldno, int width, ui_align align, const char *fldname,
+		  int value) override;
+  void field_string (int fldno, int width, ui_align align, const char *fldname,
+		     const char *string) override;
+  void ATTRIBUTE_PRINTF(6,0)
+    field_fmt (int fldno, int width, ui_align align, const char *fldname,
+	       const char *format, va_list args) override;
+  void text (const char *string) override;
 
-/* This is the TUI ui-out implementation functions vector.  It is
-   initialized below in _initialize_tui_out, inheriting the CLI
-   version, and overriding a few methods.  */
+private:
 
-static struct ui_out_impl tui_ui_out_impl;
+  int m_line;
+  int m_start_of_line;
+};
 
 /* Output an int field.  */
 
-static void
-tui_field_int (struct ui_out *uiout, 
-	       int fldno, int width,
-	       enum ui_align alignment,
-	       const char *fldname, 
-	       int value)
+void
+tui_ui_out_impl::field_int (int fldno, int width, ui_align alignment,
+			    const char *fldname, int value)
 {
-  tui_out_data *data = (tui_out_data *) ui_out_data (uiout);
-
-  if (data->base.suppress_output)
+  if (m_suppress_output)
     return;
 
   /* Don't print line number, keep it for later.  */
-  if (data->start_of_line == 0 && strcmp (fldname, "line") == 0)
+  if (m_start_of_line == 0 && strcmp (fldname, "line") == 0)
     {
-      data->start_of_line ++;
-      data->line = value;
+      m_start_of_line++;
+      m_line = value;
       return;
     }
-  data->start_of_line ++;
+  m_start_of_line++;
 
-  (*cli_ui_out_impl.field_int) (uiout, fldno,
-				width, alignment, fldname, value);
+  cli_ui_out_impl::field_int (fldno, width, alignment, fldname, value);
 }
 
 /* Other cli_field_* end up here so alignment and field separators are
    both handled by tui_field_string.  */
 
-static void
-tui_field_string (struct ui_out *uiout,
-		  int fldno, int width,
-		  enum ui_align align,
-		  const char *fldname,
-		  const char *string)
+void
+tui_ui_out_impl::field_string (int fldno, int width, ui_align align,
+			       const char *fldname, const char *string)
 {
-  tui_out_data *data = (tui_out_data *) ui_out_data (uiout);
-
-  if (data->base.suppress_output)
+  if (m_suppress_output)
     return;
 
-  if (fldname && data->line > 0 && strcmp (fldname, "fullname") == 0)
+  if (fldname && m_line > 0 && strcmp (fldname, "fullname") == 0)
     {
-      data->start_of_line ++;
-      if (data->line > 0)
+      m_start_of_line++;
+      if (m_line > 0)
         {
-          tui_show_source (string, data->line);
+          tui_show_source (string, m_line);
         }
       return;
     }
   
-  data->start_of_line++;
+  m_start_of_line++;
 
-  (*cli_ui_out_impl.field_string) (uiout, fldno,
-				   width, align,
-				   fldname, string);
+  cli_ui_out_impl::field_string (fldno, width, align, fldname, string);
 }
 
 /* This is the only field function that does not align.  */
 
-static void
-tui_field_fmt (struct ui_out *uiout, int fldno,
-	       int width, enum ui_align align,
-	       const char *fldname,
-	       const char *format,
-	       va_list args)
+void
+tui_ui_out_impl::field_fmt (int fldno, int width, ui_align align,
+			    const char *fldname, const char *format,
+			    va_list args)
 {
-  tui_out_data *data = (tui_out_data *) ui_out_data (uiout);
-
-  if (data->base.suppress_output)
+  if (m_suppress_output)
     return;
 
-  data->start_of_line++;
+  m_start_of_line++;
 
-  (*cli_ui_out_impl.field_fmt) (uiout, fldno,
-				width, align,
-				fldname, format, args);
+  cli_ui_out_impl::field_fmt (fldno, width, align, fldname, format, args);
 }
 
-static void
-tui_text (struct ui_out *uiout, const char *string)
+void
+tui_ui_out_impl::text (const char *string)
 {
-  tui_out_data *data = (tui_out_data *) ui_out_data (uiout);
-
-  if (data->base.suppress_output)
+  if (m_suppress_output)
     return;
-  data->start_of_line ++;
-  if (data->line > 0)
+
+  m_start_of_line++;
+  if (m_line > 0)
     {
       if (strchr (string, '\n') != 0)
         {
-          data->line = -1;
-          data->start_of_line = 0;
+          m_line = -1;
+          m_start_of_line = 0;
         }
       return;
     }
   if (strchr (string, '\n'))
-    data->start_of_line = 0;
+    m_start_of_line = 0;
 
-  (*cli_ui_out_impl.text) (uiout, string);
+  cli_ui_out_impl::text (string);
 }
 
-struct ui_out *
-tui_out_new (struct ui_file *stream)
+tui_ui_out_impl::tui_ui_out_impl (ui_file *stream)
+: cli_ui_out_impl (stream),
+  m_line (0),
+  m_start_of_line (-1)
 {
-  int flags = 0;
-
-  tui_out_data *data = new tui_out_data ();
-
-  /* Initialize base "class".  */
-  cli_out_data_ctor (&data->base, stream);
-
-  /* Initialize our fields.  */
-  data->line = -1;
-  data->start_of_line = 0;
-
-  return ui_out_new (&tui_ui_out_impl, data, flags);
 }
 
-/* Standard gdb initialization hook.  */
-
-extern void _initialize_tui_out (void);
-
-void
-_initialize_tui_out (void)
+struct ui_out *
+tui_out_new (struct ui_file *stream)
 {
-  /* Inherit the CLI version.  */
-  tui_ui_out_impl = cli_ui_out_impl;
-
-  /* Override a few methods.  */
-  tui_ui_out_impl.field_int = tui_field_int;
-  tui_ui_out_impl.field_string = tui_field_string;
-  tui_ui_out_impl.field_fmt = tui_field_fmt;
-  tui_ui_out_impl.text = tui_text;
+  tui_ui_out_impl *impl = new tui_ui_out_impl (stream);
+
+  return ui_out_new (impl, 0);
 }
diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index 1717491..78db2be 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -28,6 +28,7 @@ 
 #include <vector>
 #include <memory>
 #include <string>
+#include <memory>
 
 /* A header of a ui_out_table.  */
 
@@ -175,8 +176,7 @@  struct ui_out
   {
     int flags;
     /* Specific implementation of ui-out.  */
-    const struct ui_out_impl *impl;
-    void *data;
+    std::unique_ptr<ui_out_impl_base> impl;
 
     /* Vector to store and track the ui-out levels.  */
     std::vector<std::unique_ptr<ui_out_level>> levels;
@@ -251,6 +251,10 @@  static void uo_message (struct ui_out *uiout,
 static void uo_wrap_hint (struct ui_out *uiout, const char *identstring);
 static void uo_flush (struct ui_out *uiout);
 static int uo_redirect (struct ui_out *uiout, struct ui_file *outstream);
+static void uo_field_string (struct ui_out *uiout, int fldno, int width,
+			     enum ui_align align, const char *fldname,
+			     const char *string);
+
 
 /* Prototypes for local functions */
 
@@ -603,7 +607,7 @@  ui_out_test_flags (struct ui_out *uiout, int mask)
 int
 ui_out_is_mi_like_p (struct ui_out *uiout)
 {
-  return uiout->impl->is_mi_like_p;
+  return uiout->impl->is_mi_like_p ();
 }
 
 /* Interface to the implementation functions.  */
@@ -613,34 +617,26 @@  uo_table_begin (struct ui_out *uiout, int nbrofcols,
 		int nr_rows,
 		const char *tblid)
 {
-  if (!uiout->impl->table_begin)
-    return;
-  uiout->impl->table_begin (uiout, nbrofcols, nr_rows, tblid);
+  uiout->impl->table_begin (nbrofcols, nr_rows, tblid);
 }
 
 void
 uo_table_body (struct ui_out *uiout)
 {
-  if (!uiout->impl->table_body)
-    return;
-  uiout->impl->table_body (uiout);
+  uiout->impl->table_body ();
 }
 
 void
 uo_table_end (struct ui_out *uiout)
 {
-  if (!uiout->impl->table_end)
-    return;
-  uiout->impl->table_end (uiout);
+  uiout->impl->table_end ();
 }
 
 void
 uo_table_header (struct ui_out *uiout, int width, enum ui_align align,
 		 const std::string &col_name, const std::string &col_hdr)
 {
-  if (!uiout->impl->table_header)
-    return;
-  uiout->impl->table_header (uiout, width, align, col_name, col_hdr);
+  uiout->impl->table_header (width, align, col_name, col_hdr);
 }
 
 /* Clear the table associated with UIOUT.  */
@@ -657,18 +653,14 @@  uo_begin (struct ui_out *uiout,
 	  enum ui_out_type type,
 	  const char *id)
 {
-  if (uiout->impl->begin == NULL)
-    return;
-  uiout->impl->begin (uiout, type, id);
+  uiout->impl->begin (type, id);
 }
 
 void
 uo_end (struct ui_out *uiout,
 	enum ui_out_type type)
 {
-  if (uiout->impl->end == NULL)
-    return;
-  uiout->impl->end (uiout, type);
+  uiout->impl->end (type);
 }
 
 void
@@ -676,18 +668,14 @@  uo_field_int (struct ui_out *uiout, int fldno, int width, enum ui_align align,
 	      const char *fldname,
 	      int value)
 {
-  if (!uiout->impl->field_int)
-    return;
-  uiout->impl->field_int (uiout, fldno, width, align, fldname, value);
+  uiout->impl->field_int (fldno, width, align, fldname, value);
 }
 
 void
 uo_field_skip (struct ui_out *uiout, int fldno, int width, enum ui_align align,
 	       const char *fldname)
 {
-  if (!uiout->impl->field_skip)
-    return;
-  uiout->impl->field_skip (uiout, fldno, width, align, fldname);
+  uiout->impl->field_skip (fldno, width, align, fldname);
 }
 
 void
@@ -696,9 +684,7 @@  uo_field_string (struct ui_out *uiout, int fldno, int width,
 		 const char *fldname,
 		 const char *string)
 {
-  if (!uiout->impl->field_string)
-    return;
-  uiout->impl->field_string (uiout, fldno, width, align, fldname, string);
+  uiout->impl->field_string (fldno, width, align, fldname, string);
 }
 
 void
@@ -707,26 +693,20 @@  uo_field_fmt (struct ui_out *uiout, int fldno, int width, enum ui_align align,
 	      const char *format,
 	      va_list args)
 {
-  if (!uiout->impl->field_fmt)
-    return;
-  uiout->impl->field_fmt (uiout, fldno, width, align, fldname, format, args);
+  uiout->impl->field_fmt (fldno, width, align, fldname, format, args);
 }
 
 void
 uo_spaces (struct ui_out *uiout, int numspaces)
 {
-  if (!uiout->impl->spaces)
-    return;
-  uiout->impl->spaces (uiout, numspaces);
+  uiout->impl->spaces (numspaces);
 }
 
 void
 uo_text (struct ui_out *uiout,
 	 const char *string)
 {
-  if (!uiout->impl->text)
-    return;
-  uiout->impl->text (uiout, string);
+  uiout->impl->text (string);
 }
 
 void
@@ -734,33 +714,25 @@  uo_message (struct ui_out *uiout,
 	    const char *format,
 	    va_list args)
 {
-  if (!uiout->impl->message)
-    return;
-  uiout->impl->message (uiout, format, args);
+  uiout->impl->message (format, args);
 }
 
 void
 uo_wrap_hint (struct ui_out *uiout, const char *identstring)
 {
-  if (!uiout->impl->wrap_hint)
-    return;
-  uiout->impl->wrap_hint (uiout, identstring);
+  uiout->impl->wrap_hint (identstring);
 }
 
 void
 uo_flush (struct ui_out *uiout)
 {
-  if (!uiout->impl->flush)
-    return;
-  uiout->impl->flush (uiout);
+  uiout->impl->flush ();
 }
 
 int
 uo_redirect (struct ui_out *uiout, struct ui_file *outstream)
 {
-  if (!uiout->impl->redirect)
-    return -1;
-  return uiout->impl->redirect (uiout, outstream);
+  return uiout->impl->redirect (outstream);
 }
 
 /* local functions */
@@ -857,15 +829,6 @@  specified after table_body and inside a list."));
     }
 }
 
-
-/* Access to ui-out members data.  */
-
-void *
-ui_out_data (struct ui_out *uiout)
-{
-  return uiout->data;
-}
-
 /* Access table field parameters.  */
 int
 ui_out_query_field (struct ui_out *uiout, int colno,
@@ -896,13 +859,11 @@  ui_out_query_field (struct ui_out *uiout, int colno,
 /* Initialize private members at startup.  */
 
 struct ui_out *
-ui_out_new (const struct ui_out_impl *impl, void *data,
-	    int flags)
+ui_out_new (ui_out_impl_base *impl, int flags)
 {
   struct ui_out *uiout = new ui_out ();
 
-  uiout->data = data;
-  uiout->impl = impl;
+  uiout->impl.reset (impl);
   uiout->flags = flags;
   uiout->table.flag = 0;
   uiout->table.state = TABLE_STATE_HEADERS;
@@ -914,3 +875,9 @@  ui_out_new (const struct ui_out_impl *impl, void *data,
 
   return uiout;
 }
+
+ui_out_impl_base *
+ui_out_impl (ui_out *uiout)
+{
+  return uiout->impl.get ();
+}
diff --git a/gdb/ui-out.h b/gdb/ui-out.h
index 06c05e2..17c502e 100644
--- a/gdb/ui-out.h
+++ b/gdb/ui-out.h
@@ -140,91 +140,52 @@  extern int ui_out_query_field (struct ui_out *uiout, int colno,
 
 extern int ui_out_is_mi_like_p (struct ui_out *uiout);
 
-/* From here on we have things that are only needed by implementation
-   routines and main.c.   We should pehaps have a separate file for that,
-   like a  ui-out-impl.h  file.  */
-
-/* User Interface Output Implementation Function Table */
-
-/* Type definition of all implementation functions.  */
-
-typedef void (table_begin_ftype) (struct ui_out * uiout,
-				  int nbrofcols, int nr_rows,
-				  const char *tblid);
-typedef void (table_body_ftype) (struct ui_out * uiout);
-typedef void (table_end_ftype) (struct ui_out * uiout);
-typedef void (table_header_ftype) (struct ui_out * uiout, int width,
-				   enum ui_align align,
-				   const std::string &col_name,
-				   const std::string &col_hdr);
-
-typedef void (ui_out_begin_ftype) (struct ui_out *uiout,
-				   enum ui_out_type type,
-				   const char *id);
-typedef void (ui_out_end_ftype) (struct ui_out *uiout,
-				 enum ui_out_type type);
-typedef void (field_int_ftype) (struct ui_out * uiout, int fldno, int width,
-				enum ui_align align,
-				const char *fldname, int value);
-typedef void (field_skip_ftype) (struct ui_out * uiout, int fldno, int width,
-				 enum ui_align align,
-				 const char *fldname);
-typedef void (field_string_ftype) (struct ui_out * uiout, int fldno, int width,
-				   enum ui_align align,
-				   const char *fldname,
-				   const char *string);
-typedef void (field_fmt_ftype) (struct ui_out * uiout, int fldno, int width,
-				enum ui_align align,
-				const char *fldname,
-				const char *format,
-				va_list args) ATTRIBUTE_FPTR_PRINTF(6,0);
-typedef void (spaces_ftype) (struct ui_out * uiout, int numspaces);
-typedef void (text_ftype) (struct ui_out * uiout,
-			   const char *string);
-typedef void (message_ftype) (struct ui_out * uiout,
-			      const char *format, va_list args)
-     ATTRIBUTE_FPTR_PRINTF(2,0);
-typedef void (wrap_hint_ftype) (struct ui_out * uiout, const char *identstring);
-typedef void (flush_ftype) (struct ui_out * uiout);
-typedef int (redirect_ftype) (struct ui_out * uiout,
-			      struct ui_file * outstream);
-typedef void (data_destroy_ftype) (struct ui_out *uiout);
-
-/* ui-out-impl */
-
-struct ui_out_impl
-  {
-    table_begin_ftype *table_begin;
-    table_body_ftype *table_body;
-    table_end_ftype *table_end;
-    table_header_ftype *table_header;
-    ui_out_begin_ftype *begin;
-    ui_out_end_ftype *end;
-    field_int_ftype *field_int;
-    field_skip_ftype *field_skip;
-    field_string_ftype *field_string;
-    field_fmt_ftype *field_fmt;
-    spaces_ftype *spaces;
-    text_ftype *text;
-    message_ftype *message;
-    wrap_hint_ftype *wrap_hint;
-    flush_ftype *flush;
-    redirect_ftype *redirect;
-    data_destroy_ftype *data_destroy;
-    int is_mi_like_p;
-  };
-
-extern void *ui_out_data (struct ui_out *uiout);
-
-extern void uo_field_string (struct ui_out *uiout, int fldno, int width,
-			     enum ui_align align, const char *fldname,
-			     const char *string);
+/* Base of all ui-out implementations.  */
+
+class ui_out_impl_base
+{
+ public:
+
+  virtual ~ui_out_impl_base () {}
+
+  virtual void table_begin (int nbrofcols, int nr_rows, const char *tblid) = 0;
+  virtual void table_body () = 0;
+  virtual void table_end () = 0;
+  virtual void table_header (int width, ui_align align,
+			     const std::string &col_name,
+			     const std::string &col_hdr) = 0;
+
+  virtual void begin (ui_out_type type, const char *id) = 0;
+  virtual void end (ui_out_type type) = 0;
+  virtual void field_int (int fldno, int width, ui_align align,
+			  const char *fldname, int value) = 0;
+  virtual void field_skip (int fldno, int width, ui_align align,
+			   const char *fldname) = 0;
+  virtual void field_string (int fldno, int width, ui_align align,
+			     const char *fldname, const char *string) = 0;
+  virtual void ATTRIBUTE_PRINTF(6,0)
+    field_fmt (int fldno, int width, ui_align align, const char *fldname,
+	       const char *format, va_list args) = 0;
+  virtual void spaces (int numspaces) = 0;
+  virtual void text (const char *string) = 0;
+  virtual void ATTRIBUTE_PRINTF(2,0)
+    message (const char *format, va_list args) = 0;
+  virtual void wrap_hint (const char *identstring) = 0;
+  virtual void flush () = 0;
+  virtual int redirect (struct ui_file * outstream) = 0;
+
+  /* Set as not MI-like by default.  It is overridden in subclasses if
+     necessary.  */
+
+  virtual int is_mi_like_p ()
+  { return 0; }
+};
+
+ui_out_impl_base *ui_out_impl (ui_out *uiout);
 
 /* Create a ui_out object */
 
-extern struct ui_out *ui_out_new (const struct ui_out_impl *impl,
-				  void *data,
-				  int flags);
+extern ui_out *ui_out_new (ui_out_impl_base *impl, int flags = 0);
 
 /* Redirect the ouptut of a ui_out object temporarily.  */