More C++-ification for struct display
authorTom Tromey <tom@tromey.com>
Fri, 8 May 2020 20:21:22 +0000 (14:21 -0600)
committerTom Tromey <tromey@adacore.com>
Fri, 8 May 2020 20:21:23 +0000 (14:21 -0600)
This changes displays to have a constructor, use bool and std::string,
and to be stored using std::vector.  The ALL_DISPLAYS and
ALL_DISPLAYS_SAFE macros are removed.  While internal iteration is
still done via map_display_numbers, this is updated to use a
function_view.  These changes simplify the code somewhat; for example,
free_display can now be removed in favor of ordinary destruction.

gdb/ChangeLog
2020-05-08  Tom Tromey  <tom@tromey.com>

* printcmd.c (struct display) <next>: Remove.
<display>: New constructor.
<exp_string>: Now a std::string.
<enabled_p>: Now a bool.
(display_number): Move definition earlier.
(displays): Rename from display_chain.  Now a std::vector.
(ALL_DISPLAYS, ALL_DISPLAYS_SAFE): Remove.
(display_command): Update.
(do_one_display, disable_display)
(enable_disable_display_command, do_enable_disable_display):
Update.
(free_display): Remove.
(clear_displays): Rewrite.
(delete_display): Update.
(map_display_numbers): Use function_view.  Remove "data"
parameter.  Update.
(do_delete_display): Remove.
(undisplay_command): Update.
(do_one_display, do_displays, disable_display)
(info_display_command): Update.
(do_enable_disable_display): Remove.
(enable_disable_display_command)
(clear_dangling_display_expressions): Update.

gdb/ChangeLog
gdb/printcmd.c

index 747f4c3cb64c1c6003be7a98aea90dbf882fa14a..59125fd6ded6a9d7e7227b72ee5850125379750a 100644 (file)
@@ -1,3 +1,29 @@
+2020-05-08  Tom Tromey  <tom@tromey.com>
+
+       * printcmd.c (struct display) <next>: Remove.
+       <display>: New constructor.
+       <exp_string>: Now a std::string.
+       <enabled_p>: Now a bool.
+       (display_number): Move definition earlier.
+       (displays): Rename from display_chain.  Now a std::vector.
+       (ALL_DISPLAYS, ALL_DISPLAYS_SAFE): Remove.
+       (display_command): Update.
+       (do_one_display, disable_display)
+       (enable_disable_display_command, do_enable_disable_display):
+       Update.
+       (free_display): Remove.
+       (clear_displays): Rewrite.
+       (delete_display): Update.
+       (map_display_numbers): Use function_view.  Remove "data"
+       parameter.  Update.
+       (do_delete_display): Remove.
+       (undisplay_command): Update.
+       (do_one_display, do_displays, disable_display)
+       (info_display_command): Update.
+       (do_enable_disable_display): Remove.
+       (enable_disable_display_command)
+       (clear_dangling_display_expressions): Update.
+
 2020-05-08  Tom Tromey  <tom@tromey.com>
 
        * symtab.c (set_symbol_cache_size)
index de6d3d43bb4983ebdf02ae580461bba865650f06..00320d20898561b0f5b1ed8f25f7c9ba27c8e9dd 100644 (file)
@@ -116,13 +116,27 @@ show_print_symbol_filename (struct ui_file *file, int from_tty,
 
 static int current_display_number;
 
+/* Last allocated display number.  */
+
+static int display_number;
+
 struct display
   {
-    /* Chain link to next auto-display item.  */
-    struct display *next;
+    display (const char *exp_string_, expression_up &&exp_,
+            const struct format_data &format_, struct program_space *pspace_,
+            const struct block *block_)
+      : exp_string (exp_string_),
+       exp (std::move (exp_)),
+       number (++display_number),
+       format (format_),
+       pspace (pspace_),
+       block (block_),
+       enabled_p (true)
+    {
+    }
 
     /* The expression as the user typed it.  */
-    char *exp_string;
+    std::string exp_string;
 
     /* Expression to be evaluated and displayed.  */
     expression_up exp;
@@ -140,27 +154,13 @@ struct display
     const struct block *block;
 
     /* Status of this display (enabled or disabled).  */
-    int enabled_p;
+    bool enabled_p;
   };
 
-/* Chain of expressions whose values should be displayed
-   automatically each time the program stops.  */
-
-static struct display *display_chain;
+/* Expressions whose values should be displayed automatically each
+   time the program stops.  */
 
-static int display_number;
-
-/* Walk the following statement or block through all displays.
-   ALL_DISPLAYS_SAFE does so even if the statement deletes the current
-   display.  */
-
-#define ALL_DISPLAYS(B)                                \
-  for (B = display_chain; B; B = B->next)
-
-#define ALL_DISPLAYS_SAFE(B,TMP)               \
-  for (B = display_chain;                      \
-       B ? (TMP = B->next, 1): 0;              \
-       B = TMP)
+static std::vector<std::unique_ptr<struct display>> all_displays;
 
 /* Prototypes for local functions.  */
 
@@ -1763,27 +1763,9 @@ display_command (const char *arg, int from_tty)
   innermost_block_tracker tracker;
   expression_up expr = parse_expression (exp, &tracker);
 
-  newobj = new display ();
-
-  newobj->exp_string = xstrdup (exp);
-  newobj->exp = std::move (expr);
-  newobj->block = tracker.block ();
-  newobj->pspace = current_program_space;
-  newobj->number = ++display_number;
-  newobj->format = fmt;
-  newobj->enabled_p = 1;
-  newobj->next = NULL;
-
-  if (display_chain == NULL)
-    display_chain = newobj;
-  else
-    {
-      struct display *last;
-
-      for (last = display_chain; last->next != NULL; last = last->next)
-       ;
-      last->next = newobj;
-    }
+  newobj = new display (exp, std::move (expr), fmt,
+                       current_program_space, tracker.block ());
+  all_displays.emplace_back (newobj);
 
   if (from_tty)
     do_one_display (newobj);
@@ -1791,26 +1773,13 @@ display_command (const char *arg, int from_tty)
   dont_repeat ();
 }
 
-static void
-free_display (struct display *d)
-{
-  xfree (d->exp_string);
-  delete d;
-}
-
 /* Clear out the display_chain.  Done when new symtabs are loaded,
    since this invalidates the types stored in many expressions.  */
 
 void
-clear_displays (void)
+clear_displays ()
 {
-  struct display *d;
-
-  while ((d = display_chain) != NULL)
-    {
-      display_chain = d->next;
-      free_display (d);
-    }
+  all_displays.clear ();
 }
 
 /* Delete the auto-display DISPLAY.  */
@@ -1818,21 +1787,16 @@ clear_displays (void)
 static void
 delete_display (struct display *display)
 {
-  struct display *d;
-
   gdb_assert (display != NULL);
 
-  if (display_chain == display)
-    display_chain = display->next;
-
-  ALL_DISPLAYS (d)
-    if (d->next == display)
-      {
-       d->next = display->next;
-       break;
-      }
-
-  free_display (display);
+  auto iter = std::find_if (all_displays.begin (),
+                           all_displays.end (),
+                           [=] (const std::unique_ptr<struct display> &item)
+                           {
+                             return item.get () == display;
+                           });
+  gdb_assert (iter != all_displays.end ());
+  all_displays.erase (iter);
 }
 
 /* Call FUNCTION on each of the displays whose numbers are given in
@@ -1840,9 +1804,7 @@ delete_display (struct display *display)
 
 static void
 map_display_numbers (const char *args,
-                    void (*function) (struct display *,
-                                      void *),
-                    void *data)
+                    gdb::function_view<void (struct display *)> function)
 {
   int num;
 
@@ -1860,27 +1822,20 @@ map_display_numbers (const char *args,
        warning (_("bad display number at or near '%s'"), p);
       else
        {
-         struct display *d, *tmp;
-
-         ALL_DISPLAYS_SAFE (d, tmp)
-           if (d->number == num)
-             break;
-         if (d == NULL)
+         auto iter = std::find_if (all_displays.begin (),
+                                   all_displays.end (),
+                                   [=] (const std::unique_ptr<display> &item)
+                                   {
+                                     return item->number == num;
+                                   });
+         if (iter == all_displays.end ())
            printf_unfiltered (_("No display number %d.\n"), num);
          else
-           function (d, data);
+           function (iter->get ());
        }
     }
 }
 
-/* Callback for map_display_numbers, that deletes a display.  */
-
-static void
-do_delete_display (struct display *d, void *data)
-{
-  delete_display (d);
-}
-
 /* "undisplay" command.  */
 
 static void
@@ -1894,7 +1849,7 @@ undisplay_command (const char *args, int from_tty)
       return;
     }
 
-  map_display_numbers (args, do_delete_display, NULL);
+  map_display_numbers (args, delete_display);
   dont_repeat ();
 }
 
@@ -1907,7 +1862,7 @@ do_one_display (struct display *d)
 {
   int within_current_scope;
 
-  if (d->enabled_p == 0)
+  if (!d->enabled_p)
     return;
 
   /* The expression carries the architecture that was used at parse time.
@@ -1929,15 +1884,15 @@ do_one_display (struct display *d)
       try
        {
          innermost_block_tracker tracker;
-         d->exp = parse_expression (d->exp_string, &tracker);
+         d->exp = parse_expression (d->exp_string.c_str (), &tracker);
          d->block = tracker.block ();
        }
       catch (const gdb_exception &ex)
        {
          /* Can't re-parse the expression.  Disable this display item.  */
-         d->enabled_p = 0;
+         d->enabled_p = false;
          warning (_("Unable to display \"%s\": %s"),
-                  d->exp_string, ex.what ());
+                  d->exp_string.c_str (), ex.what ());
          return;
        }
     }
@@ -1977,7 +1932,7 @@ do_one_display (struct display *d)
 
       annotate_display_expression ();
 
-      puts_filtered (d->exp_string);
+      puts_filtered (d->exp_string.c_str ());
       annotate_display_expression_end ();
 
       if (d->format.count != 1 || d->format.format == 'i')
@@ -2016,7 +1971,7 @@ do_one_display (struct display *d)
 
       annotate_display_expression ();
 
-      puts_filtered (d->exp_string);
+      puts_filtered (d->exp_string.c_str ());
       annotate_display_expression_end ();
 
       printf_filtered (" = ");
@@ -2053,10 +2008,8 @@ do_one_display (struct display *d)
 void
 do_displays (void)
 {
-  struct display *d;
-
-  for (d = display_chain; d; d = d->next)
-    do_one_display (d);
+  for (auto &d : all_displays)
+    do_one_display (d.get ());
 }
 
 /* Delete the auto-display which we were in the process of displaying.
@@ -2065,12 +2018,10 @@ do_displays (void)
 void
 disable_display (int num)
 {
-  struct display *d;
-
-  for (d = display_chain; d; d = d->next)
+  for (auto &d : all_displays)
     if (d->number == num)
       {
-       d->enabled_p = 0;
+       d->enabled_p = false;
        return;
       }
   printf_unfiltered (_("No display number %d.\n"), num);
@@ -2093,15 +2044,13 @@ disable_current_display (void)
 static void
 info_display_command (const char *ignore, int from_tty)
 {
-  struct display *d;
-
-  if (!display_chain)
+  if (all_displays.empty ())
     printf_unfiltered (_("There are no auto-display expressions now.\n"));
   else
     printf_filtered (_("Auto-display expressions now in effect:\n\
 Num Enb Expression\n"));
 
-  for (d = display_chain; d; d = d->next)
+  for (auto &d : all_displays)
     {
       printf_filtered ("%d:   %c  ", d->number, "ny"[(int) d->enabled_p]);
       if (d->format.size)
@@ -2109,38 +2058,31 @@ Num Enb Expression\n"));
                         d->format.format);
       else if (d->format.format)
        printf_filtered ("/%c ", d->format.format);
-      puts_filtered (d->exp_string);
+      puts_filtered (d->exp_string.c_str ());
       if (d->block && !contained_in (get_selected_block (0), d->block, true))
        printf_filtered (_(" (cannot be evaluated in the current context)"));
       printf_filtered ("\n");
     }
 }
 
-/* Callback fo map_display_numbers, that enables or disables the
-   passed in display D.  */
-
-static void
-do_enable_disable_display (struct display *d, void *data)
-{
-  d->enabled_p = *(int *) data;
-}
-
 /* Implementation of both the "disable display" and "enable display"
    commands.  ENABLE decides what to do.  */
 
 static void
-enable_disable_display_command (const char *args, int from_tty, int enable)
+enable_disable_display_command (const char *args, int from_tty, bool enable)
 {
   if (args == NULL)
     {
-      struct display *d;
-
-      ALL_DISPLAYS (d)
+      for (auto &d : all_displays)
        d->enabled_p = enable;
       return;
     }
 
-  map_display_numbers (args, do_enable_disable_display, &enable);
+  map_display_numbers (args,
+                      [=] (struct display *d)
+                      {
+                        d->enabled_p = enable;
+                      });
 }
 
 /* The "enable display" command.  */
@@ -2148,7 +2090,7 @@ enable_disable_display_command (const char *args, int from_tty, int enable)
 static void
 enable_display_command (const char *args, int from_tty)
 {
-  enable_disable_display_command (args, from_tty, 1);
+  enable_disable_display_command (args, from_tty, true);
 }
 
 /* The "disable display" command.  */
@@ -2156,7 +2098,7 @@ enable_display_command (const char *args, int from_tty)
 static void
 disable_display_command (const char *args, int from_tty)
 {
-  enable_disable_display_command (args, from_tty, 0);
+  enable_disable_display_command (args, from_tty, false);
 }
 
 /* display_chain items point to blocks and expressions.  Some expressions in
@@ -2170,7 +2112,6 @@ disable_display_command (const char *args, int from_tty)
 static void
 clear_dangling_display_expressions (struct objfile *objfile)
 {
-  struct display *d;
   struct program_space *pspace;
 
   /* With no symbol file we cannot have a block or expression from it.  */
@@ -2183,7 +2124,7 @@ clear_dangling_display_expressions (struct objfile *objfile)
       gdb_assert (objfile->pspace == pspace);
     }
 
-  for (d = display_chain; d != NULL; d = d->next)
+  for (auto &d : all_displays)
     {
       if (d->pspace != pspace)
        continue;