[v2,2/5] Use classes to represent MI Command instead of structures

Message ID 20190514112418.24091-3-jan.vrany@fit.cvut.cz
State New
Headers show
Series
  • Untitled series #13312
Related show

Commit Message

Jan Vrany May 14, 2019, 11:24 a.m.
From: Didier Nadeau <didier.nadeau@gmail.com>


This commit changes the infrastructure of mi-cmds.h and associated
files to use classes instead of structure to populate the hashmap
containing the commands.

The base class is virtual and there is one subclass MI commands
implemented with pure MI implementation and another for MI commands
implemented over a CLI command.

Logic for suppress_notification and parsing of ARGV/ARGC has been moved
to the classes implementation.

gdb/ChangeLog:

	* mi/mi-cmds.c (create_mi_cmd): Remove.
	(mi_command::mi_command): New function.
	(mi_command::do_suppress_notification): New function.
	(mi_command::invoke): New function.
	(mi_command_mi::mi_command_mi): New function.
	(mi_command_mi::do_invoke): New function.
	(mi_command_cli::mi_command_cli): New function.
	(mi_command_cli::do_invoke): New function.
	(mi_cmd_lookup): Change return type.
	* mi/mi-cmds.h (struct mi_cli): Remove.
	(struct mi_cmd): Remove.
	(class mi_command): New class.
	(class mi_command_mi): New class.
	(class mi_command_cli): New class.
	(mi_cmd_loopkup): Change return type.
	* mi/mi-main.c (mi_execute_cli_command): Remove declaration.
	(mi_execute_command): Remove suppress_notification handling.
	(mi_cmd_execute): Remove call to argv_func.
	(mi_cmd_execute): Remove call to mi_execute_cli_command.
	(mi_cmd_execute): New call to mi_command::invoke.
	* mi/mi-main.h (mi_execute_cli_command): New declaration.
	* mi/mi-parse.c (mi_parse): Remove call to mi_parse_argv.
	* mi/mi-parse.h (struct mi_parse): Remove field struct mi_cmd.
	(struct mi_parse): New field class mi_command.
	(mi_parse_argv): New declaration.
---
 gdb/ChangeLog        |  29 ++++++++++++
 gdb/mi/mi-cmd-info.c |   2 +-
 gdb/mi/mi-cmds.c     | 103 ++++++++++++++++++++++++++++++-------------
 gdb/mi/mi-cmds.h     |  75 ++++++++++++++++++++++---------
 gdb/mi/mi-main.c     |  18 +-------
 gdb/mi/mi-main.h     |   1 +
 gdb/mi/mi-parse.c    |  18 ++------
 gdb/mi/mi-parse.h    |   6 ++-
 8 files changed, 167 insertions(+), 85 deletions(-)

-- 
2.20.1

Comments

Simon Marchi May 17, 2019, 3:12 a.m. | #1
Hi Jan,

This looks ok to me, I have noted a few styling issues, I fixed them as I went
through the code, so see the patch at the end which you can apply to your patch.

> @@ -36,9 +37,9 @@ static bool

>  insert_mi_cmd_entry (mi_cmd_up command)

>  {

>    gdb_assert (command != NULL);

> -  gdb_assert (command->name != NULL);

> +  gdb_assert (! command->name ().empty ());


Remove space after !.

>  /* Create and register a new MI command with a pure MI implementation.  */

>  

>  static void

>  add_mi_cmd_mi (const char *name, mi_cmd_argv_ftype function,

>  	       int *suppress_notification = NULL)

>  {

> -  mi_cmd_up cmd_up = create_mi_cmd (name);

> -

> -  cmd_up->cli.cmd = NULL;

> -  cmd_up->cli.args_p = 0;

> -  cmd_up->argv_func = function;

> -  cmd_up->suppress_notification = suppress_notification;

> +  mi_command *micommand = new mi_command_mi (name, function,

> +					     suppress_notification);

>  

> -  bool success = insert_mi_cmd_entry (std::move (cmd_up));

> +  bool success = insert_mi_cmd_entry (std::move (mi_cmd_up (micommand)));


It would make more sense to make the local variable an mi_cmd_up.

>    gdb_assert (success);

>  }

>  

> @@ -83,17 +68,74 @@ static void

>  add_mi_cmd_cli (const char *name, const char *cli_name, int args_p,

>  		int *suppress_notification = NULL)

>  {

> -  mi_cmd_up cmd_up = create_mi_cmd (name);

> +  mi_command *micommand = new mi_command_cli (name, cli_name, args_p,

> +					      suppress_notification);


Likewise.

>  

> -  cmd_up->cli.cmd = cli_name;

> -  cmd_up->cli.args_p = args_p;

> -  cmd_up->argv_func = NULL;

> -  cmd_up->suppress_notification = suppress_notification;

> -

> -  bool success = insert_mi_cmd_entry (std::move (cmd_up));

> +  bool success = insert_mi_cmd_entry (std::move (mi_cmd_up (micommand)));

>    gdb_assert (success);

>  }

>  

> +/* See mi-cmds.h  */

> +mi_command::mi_command (const char *name, int *suppress_notification)

> +  : m_name (name),

> +    m_suppress_notification (suppress_notification)

> +{}

> +

> +

> +void

> +mi_command::invoke (struct mi_parse *parse)

> +{

> +  gdb::optional<scoped_restore_tmpl<int>> restore

> +    = do_suppress_notification ();

> +  this->do_invoke(parse);


Missing space.

> +}

> +

> +gdb::optional<scoped_restore_tmpl<int>>

> +mi_command::do_suppress_notification ()

> +{

> +  if (m_suppress_notification != NULL)

> +    return scoped_restore_tmpl<int> (m_suppress_notification, 1);

> +  else

> +    {

> +      return gdb::optional<scoped_restore_tmpl<int>> ();

> +    }


Remove the curly braces.  Also, this can be shortened to "return {};".


> @@ -126,38 +126,69 @@ extern mi_cmd_argv_ftype mi_cmd_enable_pretty_printing;

>  extern mi_cmd_argv_ftype mi_cmd_enable_frame_filters;

>  extern mi_cmd_argv_ftype mi_cmd_var_set_update_range;

>  

> -/* Description of a single command.  */

> +/* mi_command base virtual class.  */

>  

> -struct mi_cli

> +class mi_command

>  {

> -  /* Corresponding CLI command.  If ARGS_P is non-zero, the MI

> -     command's argument list is appended to the CLI command.  */

> -  const char *cmd;

> -  int args_p;

> +  public:

> +    mi_command (const char *name, int *suppress_notification);

> +    virtual ~mi_command () {};


Use the default destructor:

  virtual ~mi_command () = default;

> +

> +    const std::string &name ()

> +    { return m_name; }

> +

> +    /* Execute the MI command.  */

> +    void invoke (struct mi_parse *parse);

> +

> +  protected:

> +    gdb::optional<scoped_restore_tmpl<int>> do_suppress_notification ();

> +    virtual void do_invoke(struct mi_parse *parse) = 0;


It would be useful to have some short doc for these.

Also, I think do_invoke could be private.

> +

> +  private:

> +

> +    /* The name of the command.  */

> +    std::string m_name;

> +

> +    /* Pointer to integer to set during command's invocation.  */

> +    int *m_suppress_notification;

>  };


Unindent the content of the class: the public/private should be at the
same column as the "class" keyword.

> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c

> index 17ca807471..481f09f799 100644

> --- a/gdb/mi/mi-main.c

> +++ b/gdb/mi/mi-main.c

> @@ -90,9 +90,6 @@ int running_result_record_printed = 1;

>  int mi_proceeded;

>  

>  static void mi_cmd_execute (struct mi_parse *parse);

> -

> -static void mi_execute_cli_command (const char *cmd, int args_p,

> -				    const char *args);

>  static void mi_execute_async_cli_command (const char *cli_command,

>  					  char **argv, int argc);

>  static bool register_changed_p (int regnum, readonly_detached_regcache *,

> @@ -1954,9 +1951,6 @@ mi_execute_command (const char *cmd, int from_tty)

>  

>        gdb::optional<scoped_restore_tmpl<int>> restore_suppress;


This ends up ununsed and can be removed.

>  

> -      if (command->cmd != NULL && command->cmd->suppress_notification != NULL)

> -	restore_suppress.emplace (command->cmd->suppress_notification, 1);

> -

>        command->token = token;

>  

>        if (do_timings)

> @@ -2095,17 +2089,9 @@ mi_cmd_execute (struct mi_parse *parse)

>  

>    current_context = parse;

>  

> -  if (parse->cmd->argv_func != NULL)

> -    {

> -      parse->cmd->argv_func (parse->command, parse->argv, parse->argc);

> -    }

> -  else if (parse->cmd->cli.cmd != 0)

> +  if (parse->cmd != NULL)

>      {

> -      /* FIXME: DELETE THIS. */

> -      /* The operation is still implemented by a cli command.  */

> -      /* Must be a synchronous one.  */

> -      mi_execute_cli_command (parse->cmd->cli.cmd, parse->cmd->cli.args_p,

> -			      parse->args);

> +      parse->cmd->invoke (parse);


Remove resulting extra curly braces.

Here's the diff that should fix pretty much all these comments.

From 6e7b648b887e59eaadb01919ca0d665dae2b3e19 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>

Date: Thu, 16 May 2019 22:00:00 -0400
Subject: [PATCH] fixup patch 2

---
 gdb/mi/mi-cmds.c | 21 ++++++++---------
 gdb/mi/mi-cmds.h | 60 +++++++++++++++++++++++++-----------------------
 gdb/mi/mi-main.c |  6 +----
 3 files changed, 41 insertions(+), 46 deletions(-)

diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 44f3813fdc9a..7702011e39df 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -37,7 +37,7 @@ static bool
 insert_mi_cmd_entry (mi_cmd_up command)
 {
   gdb_assert (command != NULL);
-  gdb_assert (! command->name ().empty ());
+  gdb_assert (!command->name ().empty ());

   const std::string &name = command->name ();

@@ -55,10 +55,9 @@ static void
 add_mi_cmd_mi (const char *name, mi_cmd_argv_ftype function,
 	       int *suppress_notification = NULL)
 {
-  mi_command *micommand = new mi_command_mi (name, function,
-					     suppress_notification);
+  mi_cmd_up command (new mi_command_mi (name, function, suppress_notification));

-  bool success = insert_mi_cmd_entry (std::move (mi_cmd_up (micommand)));
+  bool success = insert_mi_cmd_entry (std::move (command));
   gdb_assert (success);
 }

@@ -68,14 +67,15 @@ static void
 add_mi_cmd_cli (const char *name, const char *cli_name, int args_p,
 		int *suppress_notification = NULL)
 {
-  mi_command *micommand = new mi_command_cli (name, cli_name, args_p,
-					      suppress_notification);
+  mi_cmd_up command (new mi_command_cli (name, cli_name, args_p,
+					 suppress_notification));

-  bool success = insert_mi_cmd_entry (std::move (mi_cmd_up (micommand)));
+  bool success = insert_mi_cmd_entry (std::move (command));
   gdb_assert (success);
 }

 /* See mi-cmds.h  */
+
 mi_command::mi_command (const char *name, int *suppress_notification)
   : m_name (name),
     m_suppress_notification (suppress_notification)
@@ -87,7 +87,7 @@ mi_command::invoke (struct mi_parse *parse)
 {
   gdb::optional<scoped_restore_tmpl<int>> restore
     = do_suppress_notification ();
-  this->do_invoke(parse);
+  this->do_invoke (parse);
 }

 gdb::optional<scoped_restore_tmpl<int>>
@@ -96,9 +96,7 @@ mi_command::do_suppress_notification ()
   if (m_suppress_notification != NULL)
     return scoped_restore_tmpl<int> (m_suppress_notification, 1);
   else
-    {
-      return gdb::optional<scoped_restore_tmpl<int>> ();
-    }
+    return {};
 }

 mi_command_mi::mi_command_mi (const char *name, mi_cmd_argv_ftype func,
@@ -303,4 +301,3 @@ _initialize_mi_cmds (void)
 {
   build_table ();
 }
-
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index fc432a16b9cc..12b9696c5f41 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -130,58 +130,60 @@ extern mi_cmd_argv_ftype mi_cmd_var_set_update_range;

 class mi_command
 {
-  public:
-    mi_command (const char *name, int *suppress_notification);
-    virtual ~mi_command () {};
+public:
+  mi_command (const char *name, int *suppress_notification);
+  virtual ~mi_command () = default;

-    const std::string &name ()
-    { return m_name; }
+  const std::string &name ()
+  { return m_name; }

-    /* Execute the MI command.  */
-    void invoke (struct mi_parse *parse);
+  /* Execute the MI command.  */
+  void invoke (struct mi_parse *parse);

-  protected:
-    gdb::optional<scoped_restore_tmpl<int>> do_suppress_notification ();
-    virtual void do_invoke(struct mi_parse *parse) = 0;
+protected:
+  /* Set the notification suppression flag for the command.  Return a
+     scoped_restore that undoes it.  */
+  gdb::optional<scoped_restore_tmpl<int>> do_suppress_notification ();

-  private:
+private:
+  /* Implementation of the behavior of the MI command, to be implemented by
+     child classes.  */
+  virtual void do_invoke (struct mi_parse *parse) = 0;

-    /* The name of the command.  */
-    std::string m_name;
+  /* The name of the command.  */
+  std::string m_name;

-    /* Pointer to integer to set during command's invocation.  */
-    int *m_suppress_notification;
+  /* Pointer to integer to set during command's invocation.  */
+  int *m_suppress_notification;
 };

 /* MI command with a pure MI implementation.  */

 class mi_command_mi : public mi_command
 {
-  public:
-    mi_command_mi (const char *name, mi_cmd_argv_ftype func,
-		   int *suppress_notification);
+public:
+  mi_command_mi (const char *name, mi_cmd_argv_ftype func,
+		 int *suppress_notification);

-  protected:
-    void do_invoke(struct mi_parse *parse) override;
+private:
+  virtual void do_invoke (struct mi_parse *parse) override;

-  private:
-    mi_cmd_argv_ftype *m_argv_function;
+  mi_cmd_argv_ftype *m_argv_function;
 };

 /* MI command implemented on top of a CLI command.  */

 class mi_command_cli : public mi_command
 {
-  public:
-    mi_command_cli (const char *name, const char *cli_name, int args_p,
+public:
+  mi_command_cli (const char *name, const char *cli_name, int args_p,
 		  int *suppress_notification);

-  protected:
-    void do_invoke(struct mi_parse *parse) override;
+private:
+  virtual void do_invoke (struct mi_parse *parse) override;

-  private:
-    std::string m_cli_name;
-    int m_args_p;
+  std::string m_cli_name;
+  int m_args_p;
 };

 typedef std::unique_ptr<mi_command> mi_cmd_up;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 754091656248..ccb7bf45bdf5 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1949,8 +1949,6 @@ mi_execute_command (const char *cmd, int from_tty)
     {
       ptid_t previous_ptid = inferior_ptid;

-      gdb::optional<scoped_restore_tmpl<int>> restore_suppress;
-
       command->token = token;

       if (do_timings)
@@ -2090,9 +2088,7 @@ mi_cmd_execute (struct mi_parse *parse)
   current_context = parse;

   if (parse->cmd != NULL)
-    {
-      parse->cmd->invoke (parse);
-    }
+    parse->cmd->invoke (parse);
   else
     {
       /* FIXME: DELETE THIS.  */
-- 
2.21.0

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d48d93e827..03eb42555e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,32 @@ 
+2019-05-02  Didier Nadeau  <didier.nadeau@gmail.com>
+	    Jan Vrany <jan.vrany@fit.cvut.cz>
+
+	* mi/mi-cmds.c (create_mi_cmd): Remove.
+	(mi_command::mi_command): New function.
+	(mi_command::do_suppress_notification): New function.
+	(mi_command::invoke): New function.
+	(mi_command_mi::mi_command_mi): New function.
+	(mi_command_mi::do_invoke): New function.
+	(mi_command_cli::mi_command_cli): New function.
+	(mi_command_cli::do_invoke): New function.
+	(mi_cmd_lookup): Change return type.
+	* mi/mi-cmds.h (struct mi_cli): Remove.
+	(struct mi_cmd): Remove.
+	(class mi_command): New class.
+	(class mi_command_mi): New class.
+	(class mi_command_cli): New class.
+	(mi_cmd_loopkup): Change return type.
+	* mi/mi-main.c (mi_execute_cli_command): Remove declaration.
+	(mi_execute_command): Remove suppress_notification handling.
+	(mi_cmd_execute): Remove call to argv_func.
+	(mi_cmd_execute): Remove call to mi_execute_cli_command.
+	(mi_cmd_execute): New call to mi_command::invoke.
+	* mi/mi-main.h (mi_execute_cli_command): New declaration.
+	* mi/mi-parse.c (mi_parse): Remove call to mi_parse_argv.
+	* mi/mi-parse.h (struct mi_parse): Remove field struct mi_cmd.
+	(struct mi_parse): New field class mi_command.
+	(mi_parse_argv): New declaration.
+
 2019-05-02  Didier Nadeau  <didier.nadeau@gmail.com>
             Jan Vrany <jan.vrany@fit.cvut.cz>
 
diff --git a/gdb/mi/mi-cmd-info.c b/gdb/mi/mi-cmd-info.c
index 8645fac294..3d8bd68264 100644
--- a/gdb/mi/mi-cmd-info.c
+++ b/gdb/mi/mi-cmd-info.c
@@ -67,7 +67,7 @@  void
 mi_cmd_info_gdb_mi_command (const char *command, char **argv, int argc)
 {
   const char *cmd_name;
-  struct mi_cmd *cmd;
+  mi_command *cmd;
   struct ui_out *uiout = current_uiout;
 
   /* This command takes exactly one argument.  */
diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index ce7ca35a6a..44f3813fdc 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -22,6 +22,7 @@ 
 #include "top.h"
 #include "mi-cmds.h"
 #include "mi-main.h"
+#include "mi-parse.h"
 #include <map>
 #include <string>
 
@@ -36,9 +37,9 @@  static bool
 insert_mi_cmd_entry (mi_cmd_up command)
 {
   gdb_assert (command != NULL);
-  gdb_assert (command->name != NULL);
+  gdb_assert (! command->name ().empty ());
 
-  std::string name (command->name);
+  const std::string &name = command->name ();
 
   if (mi_cmd_table.find (name) != mi_cmd_table.end ())
     return false;
@@ -48,32 +49,16 @@  insert_mi_cmd_entry (mi_cmd_up command)
   return true;
 }
 
-/* Create an mi_cmd structure with name NAME.  */
-
-static mi_cmd_up
-create_mi_cmd (const char *name)
-{
-  mi_cmd_up cmd (new mi_cmd ());
-
-  cmd->name = name;
-
-  return cmd;
-}
-
 /* Create and register a new MI command with a pure MI implementation.  */
 
 static void
 add_mi_cmd_mi (const char *name, mi_cmd_argv_ftype function,
 	       int *suppress_notification = NULL)
 {
-  mi_cmd_up cmd_up = create_mi_cmd (name);
-
-  cmd_up->cli.cmd = NULL;
-  cmd_up->cli.args_p = 0;
-  cmd_up->argv_func = function;
-  cmd_up->suppress_notification = suppress_notification;
+  mi_command *micommand = new mi_command_mi (name, function,
+					     suppress_notification);
 
-  bool success = insert_mi_cmd_entry (std::move (cmd_up));
+  bool success = insert_mi_cmd_entry (std::move (mi_cmd_up (micommand)));
   gdb_assert (success);
 }
 
@@ -83,17 +68,74 @@  static void
 add_mi_cmd_cli (const char *name, const char *cli_name, int args_p,
 		int *suppress_notification = NULL)
 {
-  mi_cmd_up cmd_up = create_mi_cmd (name);
+  mi_command *micommand = new mi_command_cli (name, cli_name, args_p,
+					      suppress_notification);
 
-  cmd_up->cli.cmd = cli_name;
-  cmd_up->cli.args_p = args_p;
-  cmd_up->argv_func = NULL;
-  cmd_up->suppress_notification = suppress_notification;
-
-  bool success = insert_mi_cmd_entry (std::move (cmd_up));
+  bool success = insert_mi_cmd_entry (std::move (mi_cmd_up (micommand)));
   gdb_assert (success);
 }
 
+/* See mi-cmds.h  */
+mi_command::mi_command (const char *name, int *suppress_notification)
+  : m_name (name),
+    m_suppress_notification (suppress_notification)
+{}
+
+
+void
+mi_command::invoke (struct mi_parse *parse)
+{
+  gdb::optional<scoped_restore_tmpl<int>> restore
+    = do_suppress_notification ();
+  this->do_invoke(parse);
+}
+
+gdb::optional<scoped_restore_tmpl<int>>
+mi_command::do_suppress_notification ()
+{
+  if (m_suppress_notification != NULL)
+    return scoped_restore_tmpl<int> (m_suppress_notification, 1);
+  else
+    {
+      return gdb::optional<scoped_restore_tmpl<int>> ();
+    }
+}
+
+mi_command_mi::mi_command_mi (const char *name, mi_cmd_argv_ftype func,
+			      int *suppress_notification)
+  : mi_command (name, suppress_notification),
+    m_argv_function (func)
+{
+  gdb_assert (func != NULL);
+}
+
+void
+mi_command_mi::do_invoke (struct mi_parse *parse)
+{
+  mi_parse_argv (parse->args, parse);
+
+  if (parse->argv == NULL)
+    error (_("Problem parsing arguments: %s %s"), parse->command, parse->args);
+
+  this->m_argv_function (parse->command, parse->argv, parse->argc);
+}
+
+mi_command_cli::mi_command_cli (const char *name, const char *cli_name,
+				int args_p, int *suppress_notification)
+  : mi_command (name, suppress_notification),
+    m_cli_name (cli_name),
+    m_args_p (args_p)
+{}
+
+void
+mi_command_cli::do_invoke (struct mi_parse *parse)
+{
+  mi_execute_cli_command (this->m_cli_name.c_str (), this->m_args_p,
+			  parse->args);
+}
+
+/* Initialize the available MI commands.  */
+
 static void
 build_table ()
 {
@@ -126,7 +168,7 @@  build_table ()
   add_mi_cmd_mi ("catch-exception", mi_cmd_catch_exception,
 		 &mi_suppress_notification.breakpoint);
   add_mi_cmd_mi ("catch-handlers", mi_cmd_catch_handlers,
-		 &mi_suppress_notification.breakpoint);		 
+		 &mi_suppress_notification.breakpoint);
   add_mi_cmd_mi ("catch-load", mi_cmd_catch_load,
 		 &mi_suppress_notification.breakpoint);
   add_mi_cmd_mi ("catch-unload", mi_cmd_catch_unload,
@@ -243,7 +285,7 @@  build_table ()
 
 /* See mi-cmds.h.  */
 
-struct mi_cmd *
+mi_command *
 mi_cmd_lookup (const char *command)
 {
   gdb_assert (command != NULL);
@@ -261,3 +303,4 @@  _initialize_mi_cmds (void)
 {
   build_table ();
 }
+
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index 97e1be819f..fc432a16b9 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -126,38 +126,69 @@  extern mi_cmd_argv_ftype mi_cmd_enable_pretty_printing;
 extern mi_cmd_argv_ftype mi_cmd_enable_frame_filters;
 extern mi_cmd_argv_ftype mi_cmd_var_set_update_range;
 
-/* Description of a single command.  */
+/* mi_command base virtual class.  */
 
-struct mi_cli
+class mi_command
 {
-  /* Corresponding CLI command.  If ARGS_P is non-zero, the MI
-     command's argument list is appended to the CLI command.  */
-  const char *cmd;
-  int args_p;
+  public:
+    mi_command (const char *name, int *suppress_notification);
+    virtual ~mi_command () {};
+
+    const std::string &name ()
+    { return m_name; }
+
+    /* Execute the MI command.  */
+    void invoke (struct mi_parse *parse);
+
+  protected:
+    gdb::optional<scoped_restore_tmpl<int>> do_suppress_notification ();
+    virtual void do_invoke(struct mi_parse *parse) = 0;
+
+  private:
+
+    /* The name of the command.  */
+    std::string m_name;
+
+    /* Pointer to integer to set during command's invocation.  */
+    int *m_suppress_notification;
 };
 
-struct mi_cmd
+/* MI command with a pure MI implementation.  */
+
+class mi_command_mi : public mi_command
 {
-  /* Official name of the command.  */
-  const char *name;
-  /* The corresponding CLI command that can be used to implement this
-     MI command (if cli.lhs is non NULL).  */
-  struct mi_cli cli;
-  /* If non-null, the function implementing the MI command.  */
-  mi_cmd_argv_ftype *argv_func;
-  /* If non-null, the pointer to a field in
-     'struct mi_suppress_notification', which will be set to true by MI
-     command processor (mi-main.c:mi_cmd_execute) when this command is
-     being executed.  It will be set back to false when command has been
-     executed.  */
-  int *suppress_notification;
+  public:
+    mi_command_mi (const char *name, mi_cmd_argv_ftype func,
+		   int *suppress_notification);
+
+  protected:
+    void do_invoke(struct mi_parse *parse) override;
+
+  private:
+    mi_cmd_argv_ftype *m_argv_function;
+};
+
+/* MI command implemented on top of a CLI command.  */
+
+class mi_command_cli : public mi_command
+{
+  public:
+    mi_command_cli (const char *name, const char *cli_name, int args_p,
+		  int *suppress_notification);
+
+  protected:
+    void do_invoke(struct mi_parse *parse) override;
+
+  private:
+    std::string m_cli_name;
+    int m_args_p;
 };
 
-typedef std::unique_ptr<struct mi_cmd> mi_cmd_up;
+typedef std::unique_ptr<mi_command> mi_cmd_up;
 
 /* Lookup a command in the MI command table.  */
 
-extern struct mi_cmd *mi_cmd_lookup (const char *command);
+extern mi_command *mi_cmd_lookup (const char *command);
 
 /* Debug flag */
 extern int mi_debug_p;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 17ca807471..481f09f799 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -90,9 +90,6 @@  int running_result_record_printed = 1;
 int mi_proceeded;
 
 static void mi_cmd_execute (struct mi_parse *parse);
-
-static void mi_execute_cli_command (const char *cmd, int args_p,
-				    const char *args);
 static void mi_execute_async_cli_command (const char *cli_command,
 					  char **argv, int argc);
 static bool register_changed_p (int regnum, readonly_detached_regcache *,
@@ -1954,9 +1951,6 @@  mi_execute_command (const char *cmd, int from_tty)
 
       gdb::optional<scoped_restore_tmpl<int>> restore_suppress;
 
-      if (command->cmd != NULL && command->cmd->suppress_notification != NULL)
-	restore_suppress.emplace (command->cmd->suppress_notification, 1);
-
       command->token = token;
 
       if (do_timings)
@@ -2095,17 +2089,9 @@  mi_cmd_execute (struct mi_parse *parse)
 
   current_context = parse;
 
-  if (parse->cmd->argv_func != NULL)
-    {
-      parse->cmd->argv_func (parse->command, parse->argv, parse->argc);
-    }
-  else if (parse->cmd->cli.cmd != 0)
+  if (parse->cmd != NULL)
     {
-      /* FIXME: DELETE THIS. */
-      /* The operation is still implemented by a cli command.  */
-      /* Must be a synchronous one.  */
-      mi_execute_cli_command (parse->cmd->cli.cmd, parse->cmd->cli.args_p,
-			      parse->args);
+      parse->cmd->invoke (parse);
     }
   else
     {
diff --git a/gdb/mi/mi-main.h b/gdb/mi/mi-main.h
index 72c4e59428..3733bc37ac 100644
--- a/gdb/mi/mi-main.h
+++ b/gdb/mi/mi-main.h
@@ -54,6 +54,7 @@  struct mi_suppress_notification
 };
 extern struct mi_suppress_notification mi_suppress_notification;
 
+void mi_execute_cli_command (const char *cmd, int args_p, const char *args);
 /* Implementation of -fix-multi-location-breakpoint-output.  */
 
 extern void mi_cmd_fix_multi_location_breakpoint_output (const char *command,
diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index 6c8d922d66..7ed2b2bbc6 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -106,7 +106,7 @@  mi_parse_escape (const char **string_ptr)
   return c;
 }
 
-static void
+void
 mi_parse_argv (const char *args, struct mi_parse *parse)
 {
   const char *chp = args;
@@ -363,20 +363,8 @@  mi_parse (const char *cmd, char **token)
       chp = skip_spaces (chp);
     }
 
-  /* For new argv commands, attempt to return the parsed argument
-     list.  */
-  if (parse->cmd->argv_func != NULL)
-    {
-      mi_parse_argv (chp, parse.get ());
-      if (parse->argv == NULL)
-	error (_("Problem parsing arguments: %s %s"), parse->command, chp);
-    }
-
-  /* FIXME: DELETE THIS */
-  /* For CLI commands, also return the remainder of the
-     command line as a single string. */
-  if (parse->cmd->cli.cmd != NULL)
-    parse->args = xstrdup (chp);
+  /* Save the rest of the arguments for the command.  */
+  parse->args = xstrdup (chp);
 
   /* Fully parsed, flag as an MI command.  */
   parse->op = MI_COMMAND;
diff --git a/gdb/mi/mi-parse.h b/gdb/mi/mi-parse.h
index 2262ff56f9..94bd48219c 100644
--- a/gdb/mi/mi-parse.h
+++ b/gdb/mi/mi-parse.h
@@ -49,7 +49,7 @@  struct mi_parse
     enum mi_command_type op;
     char *command;
     char *token;
-    const struct mi_cmd *cmd;
+    mi_command *cmd;
     struct mi_timestamp *cmd_start;
     char *args;
     char **argv;
@@ -79,4 +79,8 @@  extern std::unique_ptr<struct mi_parse> mi_parse (const char *cmd,
 
 enum print_values mi_parse_print_values (const char *name);
 
+/* Split ARGS into argc/argv and store the result in PARSE.  */
+
+void mi_parse_argv (const char *args, struct mi_parse *parse);
+
 #endif /* MI_MI_PARSE_H */