[binutils-gdb] gdb: remove reggroup_next and reggroup_prev

[binutils-gdb] gdb: remove reggroup_next and reggroup_prev

[Andrew Burgess]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=1bca9b1e6be9cb45684f38dba0d4d323447a653a

commit 1bca9b1e6be9cb45684f38dba0d4d323447a653a
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Thu Mar 31 11:43:13 2022 +0100

    gdb: remove reggroup_next and reggroup_prev
    
    Add a new function gdbarch_reggroups that returns a reference to a
    vector containing all the reggroups for an architecture.
    
    Make use of this function throughout GDB instead of the existing
    reggroup_next and reggroup_prev functions.
    
    Finally, delete the reggroup_next and reggroup_prev functions.
    
    Most of these changes are pretty straight forward, using range based
    for loops instead of the old style look using reggroup_next.  There
    are two places where the changes are less straight forward.
    
    In gdb/python/py-registers.c, the register group iterator needed to
    change slightly.  As the iterator is tightly coupled to the gdbarch, I
    just fetch the register group vector from the gdbarch when needed, and
    use an index counter to find the next item from the vector when
    needed.
    
    In gdb/tui/tui-regs.c the tui_reg_next and tui_reg_prev functions are
    just wrappers around reggroup_next and reggroup_prev respectively.
    I've just inlined the logic of the old functions into the tui
    functions.  As the tui function had its own special twist (wrap around
    behaviour) I think this is OK.
    
    There should be no user visible changes after this commit.

Diff:
---
 gdb/completer.c           |   6 +--
 gdb/infcmd.c              |  14 +++---
 gdb/python/py-registers.c |  17 +++----
 gdb/regcache-dump.c       |   6 +--
 gdb/reggroups.c           | 118 ++++++++++------------------------------------
 gdb/reggroups.h           |  12 ++---
 gdb/tui/tui-regs.c        |  65 ++++++++++++++-----------
 7 files changed, 84 insertions(+), 154 deletions(-)

diff --git a/gdb/completer.c b/gdb/completer.c
index 63b4cd22d24..f4b5917095d 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -1819,11 +1819,7 @@ reg_or_group_completer_1 (completion_tracker &tracker,
 
   if ((targets & complete_reggroup_names) != 0)
     {
-      struct reggroup *group;
-
-      for (group = reggroup_next (gdbarch, NULL);
-	   group != NULL;
-	   group = reggroup_next (gdbarch, group))
+      for (const struct reggroup *group : gdbarch_reggroups (gdbarch))
 	{
 	  name = reggroup_name (group);
 	  if (strncmp (word, name, len) == 0)
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index b76345d8426..9abb91f2faa 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2295,17 +2295,17 @@ registers_info (const char *addr_exp, int fpregs)
 
       /* A register group?  */
       {
-	struct reggroup *group;
-
-	for (group = reggroup_next (gdbarch, NULL);
-	     group != NULL;
-	     group = reggroup_next (gdbarch, group))
+	const struct reggroup *group = nullptr;
+	for (const struct reggroup *g : gdbarch_reggroups (gdbarch))
 	  {
 	    /* Don't bother with a length check.  Should the user
 	       enter a short register group name, go with the first
 	       group that matches.  */
-	    if (strncmp (start, reggroup_name (group), end - start) == 0)
-	      break;
+	    if (strncmp (start, reggroup_name (g), end - start) == 0)
+	      {
+		group = g;
+		break;
+	      }
 	  }
 	if (group != NULL)
 	  {
diff --git a/gdb/python/py-registers.c b/gdb/python/py-registers.c
index 41f6feab3c8..6255ddac9e7 100644
--- a/gdb/python/py-registers.c
+++ b/gdb/python/py-registers.c
@@ -64,8 +64,8 @@ extern PyTypeObject register_descriptor_object_type
 struct reggroup_iterator_object {
   PyObject_HEAD
 
-  /* The last register group returned.  Initially this will be NULL.  */
-  const struct reggroup *reggroup;
+  /* The index into GROUPS for the next group to return.  */
+  std::vector<const reggroup *>::size_type index;
 
   /* Pointer back to the architecture we're finding registers for.  */
   struct gdbarch *gdbarch;
@@ -224,17 +224,18 @@ gdbpy_reggroup_iter_next (PyObject *self)
 {
   reggroup_iterator_object *iter_obj
     = (reggroup_iterator_object *) self;
-  struct gdbarch *gdbarch = iter_obj->gdbarch;
 
-  const reggroup *next_group = reggroup_next (gdbarch, iter_obj->reggroup);
-  if (next_group == NULL)
+  const std::vector<const reggroup *> &groups
+    = gdbarch_reggroups (iter_obj->gdbarch);
+  if (iter_obj->index >= groups.size ())
     {
       PyErr_SetString (PyExc_StopIteration, _("No more groups"));
       return NULL;
     }
 
-  iter_obj->reggroup = next_group;
-  return gdbpy_get_reggroup (iter_obj->reggroup).release ();
+  const reggroup *group = groups[iter_obj->index];
+  iter_obj->index++;
+  return gdbpy_get_reggroup (group).release ();
 }
 
 /* Return a new gdb.RegisterGroupsIterator over all the register groups in
@@ -251,7 +252,7 @@ gdbpy_new_reggroup_iterator (struct gdbarch *gdbarch)
 		    &reggroup_iterator_object_type);
   if (iter == NULL)
     return NULL;
-  iter->reggroup = NULL;
+  iter->index = 0;
   iter->gdbarch = gdbarch;
   return (PyObject *) iter;
 }
diff --git a/gdb/regcache-dump.c b/gdb/regcache-dump.c
index 2a2578558f7..409a868be76 100644
--- a/gdb/regcache-dump.c
+++ b/gdb/regcache-dump.c
@@ -192,11 +192,7 @@ protected:
     else
       {
 	const char *sep = "";
-	struct reggroup *group;
-
-	for (group = reggroup_next (m_gdbarch, NULL);
-	     group != NULL;
-	     group = reggroup_next (m_gdbarch, group))
+	for (const struct reggroup *group : gdbarch_reggroups (m_gdbarch))
 	  {
 	    if (gdbarch_register_reggroup_p (m_gdbarch, regnum, group))
 	      {
diff --git a/gdb/reggroups.c b/gdb/reggroups.c
index 262bf49cea8..7b8537e94f6 100644
--- a/gdb/reggroups.c
+++ b/gdb/reggroups.c
@@ -100,7 +100,7 @@ struct reggroups
 
   /* Return a reference to the list of all groups.  */
 
-  const std::vector<struct reggroup *> &
+  const std::vector<const struct reggroup *> &
   groups () const
   {
     return m_groups;
@@ -108,7 +108,7 @@ struct reggroups
 
 private:
   /* The register groups.  */
-  std::vector<struct reggroup *> m_groups;
+  std::vector<const struct reggroup *> m_groups;
 };
 
 static struct gdbarch_data *reggroups_data;
@@ -146,62 +146,15 @@ reggroups_init (struct obstack *obstack)
   return groups;
 }
 
-/* A register group iterator.  */
-
-struct reggroup *
-reggroup_next (struct gdbarch *gdbarch, const struct reggroup *last)
-{
-  /* Don't allow this function to be called during architecture
-     creation.  If there are no groups, use the default groups list.  */
-  struct reggroups *groups
-    = (struct reggroups *) gdbarch_data (gdbarch, reggroups_data);
-  gdb_assert (groups != nullptr);
-  gdb_assert (groups->size () > 0);
-
-  /* Return the first/next reggroup.  */
-  if (last == nullptr)
-    return groups->groups ().front ();
-  for (int i = 0; i < groups->size (); ++i)
-    {
-      if (groups->groups ()[i] == last)
-	{
-	  if (i + 1 < groups->size ())
-	    return groups->groups ()[i + 1];
-	  else
-	    return nullptr;
-	}
-    }
-
-  return nullptr;
-}
-
 /* See reggroups.h.  */
-
-struct reggroup *
-reggroup_prev (struct gdbarch *gdbarch, const struct reggroup *curr)
+const std::vector<const reggroup *> &
+gdbarch_reggroups (struct gdbarch *gdbarch)
 {
-  /* Don't allow this function to be called during architecture
-     creation.  If there are no groups, use the default groups list.  */
   struct reggroups *groups
     = (struct reggroups *) gdbarch_data (gdbarch, reggroups_data);
   gdb_assert (groups != nullptr);
   gdb_assert (groups->size () > 0);
-
-  /* Return the first/next reggroup.  */
-  if (curr == nullptr)
-    return groups->groups ().back ();
-  for (int i = groups->size () - 1; i >= 0; --i)
-    {
-      if (groups->groups ()[i] == curr)
-	{
-	  if (i - 1 >= 0)
-	    return groups->groups ()[i - 1];
-	  else
-	    return nullptr;
-	}
-    }
-
-  return nullptr;
+  return groups->groups ();
 }
 
 /* Is REGNUM a member of REGGROUP?  */
@@ -239,11 +192,7 @@ default_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
 const reggroup *
 reggroup_find (struct gdbarch *gdbarch, const char *name)
 {
-  struct reggroup *group;
-
-  for (group = reggroup_next (gdbarch, NULL);
-       group != NULL;
-       group = reggroup_next (gdbarch, group))
+  for (const struct reggroup *group : gdbarch_reggroups (gdbarch))
     {
       if (strcmp (name, reggroup_name (group)) == 0)
 	return group;
@@ -256,52 +205,35 @@ reggroup_find (struct gdbarch *gdbarch, const char *name)
 static void
 reggroups_dump (struct gdbarch *gdbarch, struct ui_file *file)
 {
-  struct reggroup *group = NULL;
+  static constexpr const char *fmt = " %-10s %-10s\n";
 
-  do
+  gdb_printf (file, fmt, "Group", "Type");
+
+  for (const struct reggroup *group : gdbarch_reggroups (gdbarch))
     {
       /* Group name.  */
-      {
-	const char *name;
-
-	if (group == NULL)
-	  name = "Group";
-	else
-	  name = reggroup_name (group);
-	gdb_printf (file, " %-10s", name);
-      }
+      const char *name = reggroup_name (group);
 
       /* Group type.  */
-      {
-	const char *type;
-
-	if (group == NULL)
-	  type = "Type";
-	else
-	  {
-	    switch (reggroup_type (group))
-	      {
-	      case USER_REGGROUP:
-		type = "user";
-		break;
-	      case INTERNAL_REGGROUP:
-		type = "internal";
-		break;
-	      default:
-		internal_error (__FILE__, __LINE__, _("bad switch"));
-	      }
-	  }
-	gdb_printf (file, " %-10s", type);
-      }
+      const char *type;
+
+      switch (reggroup_type (group))
+	{
+	case USER_REGGROUP:
+	  type = "user";
+	  break;
+	case INTERNAL_REGGROUP:
+	  type = "internal";
+	  break;
+	default:
+	  internal_error (__FILE__, __LINE__, _("bad switch"));
+	}
 
       /* Note: If you change this, be sure to also update the
 	 documentation.  */
-      
-      gdb_printf (file, "\n");
 
-      group = reggroup_next (gdbarch, group);
+      gdb_printf (file, fmt, name, type);
     }
-  while (group != NULL);
 }
 
 static void
diff --git a/gdb/reggroups.h b/gdb/reggroups.h
index bc87a7325f0..4f0453871fc 100644
--- a/gdb/reggroups.h
+++ b/gdb/reggroups.h
@@ -53,14 +53,10 @@ extern void reggroup_add (struct gdbarch *gdbarch, struct reggroup *group);
 extern const char *reggroup_name (const struct reggroup *reggroup);
 extern enum reggroup_type reggroup_type (const struct reggroup *reggroup);
 
-/* Iterators for the architecture's register groups.  Pass in NULL, returns
-   the first (for next), or last (for prev) group.  Pass in a group,
-   returns the next or previous group, or NULL when either the end or the
-   beginning of the group list is reached.  */
-extern struct reggroup *reggroup_next (struct gdbarch *gdbarch,
-				       const struct reggroup *last);
-extern struct reggroup *reggroup_prev (struct gdbarch *gdbarch,
-				       const struct reggroup *curr);
+/* Return the list of all register groups for GDBARCH.  */
+extern const std::vector<const reggroup *> &
+	gdbarch_reggroups (struct gdbarch *gdbarch);
+
 /* Find a reggroup by name.  */
 extern const reggroup *reggroup_find (struct gdbarch *gdbarch,
 				      const char *name);
diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index b968947fa1c..2e597d061dc 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -514,34 +514,48 @@ tui_data_item_window::rerender (WINDOW *handle, int field_width)
     (void) wstandend (handle);
 }
 
-/* Helper for "tui reg next", wraps a call to REGGROUP_NEXT, but adds wrap
-   around behaviour.  Will never return nullptr.  If CURRENT_GROUP is
-   nullptr (e.g. if the tui register window has only just been displayed
-   and has no current group selected), then the first register group will
-   be returned.  */
+/* Helper for "tui reg next", returns the next register group after
+   CURRENT_GROUP in the register group list for GDBARCH, with wrap around
+   behaviour.
+
+   If CURRENT_GROUP is nullptr (e.g. if the tui register window has only
+   just been displayed and has no current group selected) or the currently
+   selected register group can't be found (e.g. if the architecture has
+   changed since the register window was last updated), then the first
+   register group will be returned.  */
 
 static const reggroup *
 tui_reg_next (const reggroup *current_group, struct gdbarch *gdbarch)
 {
-  const reggroup *group = reggroup_next (gdbarch, current_group);
-  if (group == NULL)
-    group = reggroup_next (gdbarch, NULL);
-  return group;
+  const std::vector<const reggroup *> &groups = gdbarch_reggroups (gdbarch);
+  auto it = std::find (groups.begin (), groups.end (), current_group);
+  if (it != groups.end ())
+    it++;
+  if (it == groups.end ())
+    return groups.front ();
+  return *it;
 }
 
-/* Helper for "tui reg prev", wraps a call to REGGROUP_PREV, but adds wrap
-   around behaviour.  Will never return nullptr.  If CURRENT_GROUP is
-   nullptr (e.g. if the tui register window has only just been displayed
-   and has no current group selected), then the last register group will
-   be returned.  */
+/* Helper for "tui reg prev", returns the register group previous to
+   CURRENT_GROUP in the register group list for GDBARCH, with wrap around
+   behaviour.
+
+   If CURRENT_GROUP is nullptr (e.g. if the tui register window has only
+   just been displayed and has no current group selected) or the currently
+   selected register group can't be found (e.g. if the architecture has
+   changed since the register window was last updated), then the last
+   register group will be returned.  */
 
 static const reggroup *
 tui_reg_prev (const reggroup *current_group, struct gdbarch *gdbarch)
 {
-  const reggroup *group = reggroup_prev (gdbarch, current_group);
-  if (group == NULL)
-    group = reggroup_prev (gdbarch, NULL);
-  return group;
+  const std::vector<const reggroup *> &groups = gdbarch_reggroups (gdbarch);
+  auto it = std::find (groups.rbegin (), groups.rend (), current_group);
+  if (it != groups.rend ())
+    it++;
+  if (it == groups.rend ())
+    return groups.back ();
+  return *it;
 }
 
 /* Implement the 'tui reg' command.  Changes the register group displayed
@@ -555,7 +569,6 @@ tui_reg_command (const char *args, int from_tty)
 
   if (args != NULL)
     {
-      const reggroup *group, *match = NULL;
       size_t len = strlen (args);
 
       /* Make sure the curses mode is enabled.  */
@@ -569,6 +582,7 @@ tui_reg_command (const char *args, int from_tty)
       if (TUI_DATA_WIN == NULL || !TUI_DATA_WIN->is_visible ())
 	tui_regs_layout ();
 
+      const reggroup *match = nullptr;
       const reggroup *current_group = TUI_DATA_WIN->get_current_group ();
       if (strncmp (args, "next", len) == 0)
 	match = tui_reg_next (current_group, gdbarch);
@@ -579,9 +593,7 @@ tui_reg_command (const char *args, int from_tty)
 	  /* This loop matches on the initial part of a register group
 	     name.  If this initial part in ARGS matches only one register
 	     group then the switch is made.  */
-	  for (group = reggroup_next (gdbarch, NULL);
-	       group != NULL;
-	       group = reggroup_next (gdbarch, group))
+	  for (const struct reggroup *group : gdbarch_reggroups (gdbarch))
 	    {
 	      if (strncmp (reggroup_name (group), args, len) == 0)
 		{
@@ -599,19 +611,16 @@ tui_reg_command (const char *args, int from_tty)
     }
   else
     {
-      const reggroup *group;
-      int first;
-
       gdb_printf (_("\"tui reg\" must be followed by the name of "
 		    "either a register group,\nor one of 'next' "
 		    "or 'prev'.  Known register groups are:\n"));
 
-      for (first = 1, group = reggroup_next (gdbarch, NULL);
-	   group != NULL;
-	   first = 0, group = reggroup_next (gdbarch, group))
+      bool first = true;
+      for (const struct reggroup *group : gdbarch_reggroups (gdbarch))
 	{
 	  if (!first)
 	    gdb_printf (", ");
+	  first = false;
 	  gdb_printf ("%s", reggroup_name (group));
 	}