Re: [PATCHv2 11/16] gdb: remove reggroup_next and reggroup_prev

[Simon Marchi <simon.marchi@polymtl.ca>]

> @@ -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)

Should this return a reference to the existing vector, so it doesn't
make a copy of the vector each time?

> diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
> index b968947fa1c..acffc75238a 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 last
> +   register group will be returned.  */

last or next?  This is for tui_reg_next.

>  
>  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;

My only thought for the conversion to vector is that using an
intrusive_list would be a nice lightweight solution for those next/prev
cases.  You wouldn't have to iterate the vector to find the current
reggroup, to find the one after it.  However, it would probably not
handle the "if the architecture has changed since the register window
was last updated case", where you want to check if the current group is
part of gdbarch's reggroups.  When the current architecture changes
though, I think it should trigger an update of the register window to
show that new architecture's registers so... maybe we don't need this
check?