[00/21] ui / interp cleansup

Message ID 20230908190227.96319-1-simon.marchi@efficios.com
Headers
Series ui / interp cleansup |

Message

Simon Marchi Sept. 8, 2023, 6:22 p.m. UTC
  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).

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
  

Comments

Andrew Burgess Sept. 12, 2023, 11:38 a.m. UTC | #1
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
  
Simon Marchi Sept. 12, 2023, 5:51 p.m. UTC | #2
> 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, I'll send a v2 though.  I'll note the changes so you don't need
to go through everything again.

Simon
  
Tom Tromey Sept. 12, 2023, 6:06 p.m. UTC | #3
>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

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

Yes, me too.  It's very confusing.

I have an unfinished series to try to clean up the I/O redirect code.
It's unfinished because this area is so hard to deal with.  And due to
MI stuff it seems like we're locked into a bad design :(

Tom