Re: [PATCH 00/21] ui / interp cleansup

[Andrew Burgess <aburgess@redhat.com>]

Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

> When trying to untangle things in the ui and interp area (notably,
> trying to understand what happens when setting and restoring
> interpreters, in the context of interpreter-exec), I came up with a few
> cleanups.  Here is a first round.
>
> One goal is to make some interp operations be methods of struct ui.
> Interpreters are children of UIs, so I think it makes sense if the
> process of looking up / creating interpreters is implemented in methods
> of struct ui.  The concept of top level interpreter is also per UI, so I
> made getting and setting the top level interpreter methods of struct ui.
> I plan to do the same with the current and the command interpreter
> later, but it wasn't as obvious.
>
> Another goal is to try to reduce the number of references to current_ui
> in the code that manages interpreters.  Such references are often behind
> macros, which in my opinion makes it difficult to understand how things
> work.  To this end, there is one patch that adds a backlink from
> interpreters to their parent UI, so that interpreters know which UI they
> belong to (and they should never need to touch current_ui).

Anything we can do to make GDB's I/O & interpreter sub-system simpler --
or at least easier to understand -- is a good thing, I've always found
this part of GDB painful to work with.

I've gone through all the patches and left a few comments, but mostly I
think this looks great.

Reviewed-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew

>
> Simon Marchi (21):
>   gdb: use intrusive_list for struct ui linked list
>   gdb: make interp_lookup_existing a method of struct ui
>   gdb: make interp_add a method of struct ui
>   gdb: make interp_lookup a method of struct ui
>   gdb: remove ui:::add_interp and ui::lookup_existing_interp
>   gdb: uncover some current_ui uses in interp_set
>   gdb: add backlink to ui in interp
>   gdb: pass ui down to gdb_setup_readline and gdb_disable_readline
>   gdb/python: use m_ui instead of current_ui in dap_interp::init
>   gdb/mi: use m_ui instead of current_ui in mi_interp::init
>   gdb/cli: use m_ui instead of current_ui in cli_interp::resume
>   gdb/tui: use m_ui instead of current_ui in tui_interp::resume
>   gdb/mi: use m_ui instead of current_ui in mi_interp::resume
>   gdb/cli: use m_ui instead of current_ui in cli_interp::suspend
>   gdb/tui: use m_ui instead of current_ui in tui_interp::suspend
>   gdb/mi: use m_ui instead of current_ui in mi_interp::suspend
>   gdb: pass current_ui down to interp_set
>   gdb: make interp_set a method of struct ui
>   gdb: pass down current_ui to set_top_level_interpreter
>   gdb: make set_top_level_interpreter a method of struct ui
>   gdb: make top_level_interpreter a method of struct ui
>
>  gdb/cli/cli-interp.c |  25 +++----
>  gdb/cli/cli-interp.h |   2 +-
>  gdb/cli/cli-script.c |   2 +-
>  gdb/event-top.c      |  10 +--
>  gdb/event-top.h      |   4 +-
>  gdb/infrun.c         |   8 +--
>  gdb/interps.c        | 159 ++++---------------------------------------
>  gdb/interps.h        |  40 ++++++-----
>  gdb/linespec.c       |   3 +-
>  gdb/main.c           |   6 +-
>  gdb/mi/mi-interp.c   |  23 +++----
>  gdb/mi/mi-interp.h   |   4 +-
>  gdb/python/py-dap.c  |  12 ++--
>  gdb/python/python.c  |   2 +-
>  gdb/solib.c          |   4 +-
>  gdb/top.c            |   6 +-
>  gdb/tui/tui-interp.c |  19 +++---
>  gdb/tui/tui.c        |   2 +-
>  gdb/ui.c             | 124 +++++++++++++++++++++++++--------
>  gdb/ui.h             |  64 ++++++++++++-----
>  gdb/utils.c          |   5 +-
>  21 files changed, 243 insertions(+), 281 deletions(-)
>
>
> base-commit: 15db2284f2f8259e46635ca6df3efc772d951fac
> -- 
> 2.42.0