Re: [PATCH 21/21] gdb: make top_level_interpreter a method of struct ui

[Andrew Burgess <aburgess@redhat.com>]

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

> Rename the field to m_top_level_interpreter and make it private.  Adjust
> all callers to use `current_ui->top_level_interpreter ()`.
>
> Change-Id: I6b5461d47d566af5fffc835454c08dd9a1dc9570
> ---
>  gdb/event-top.c    | 2 +-
>  gdb/infrun.c       | 4 ++--
>  gdb/interps.c      | 8 +-------
>  gdb/interps.h      | 3 ---
>  gdb/linespec.c     | 3 ++-
>  gdb/main.c         | 4 ++--
>  gdb/mi/mi-interp.c | 4 ++--
>  gdb/solib.c        | 4 +++-
>  gdb/tui/tui.c      | 2 +-
>  gdb/ui.c           | 6 +++---
>  gdb/ui.h           | 8 +++++++-
>  gdb/utils.c        | 5 +++--
>  12 files changed, 27 insertions(+), 26 deletions(-)
>
> diff --git a/gdb/event-top.c b/gdb/event-top.c
> index 54f199d2e393..0dd85cb24d49 100644
> --- a/gdb/event-top.c
> +++ b/gdb/event-top.c
> @@ -299,7 +299,7 @@ change_line_handler (int editing)
>  
>    /* Don't try enabling editing if the interpreter doesn't support it
>       (e.g., MI).  */
> -  if (!top_level_interpreter ()->supports_command_editing ()
> +  if (!current_ui->top_level_interpreter ()->supports_command_editing ()
>        || !command_interp ()->supports_command_editing ())
>      return;
>  
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index aaa7b2de14cd..9129abba8524 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -2916,7 +2916,7 @@ clear_proceed_status_thread (struct thread_info *tp)
>  static void
>  notify_about_to_proceed ()
>  {
> -  top_level_interpreter ()->on_about_to_proceed ();
> +  current_ui->top_level_interpreter ()->on_about_to_proceed ();
>    gdb::observers::about_to_proceed.notify ();
>  }
>  
> @@ -4282,7 +4282,7 @@ check_curr_ui_sync_execution_done (void)
>        && !gdb_in_secondary_prompt_p (ui))
>      {
>        target_terminal::ours ();
> -      top_level_interpreter ()->on_sync_execution_done ();
> +      current_ui->top_level_interpreter ()->on_sync_execution_done ();
>        ui->register_file_handler ();
>      }
>  }
> diff --git a/gdb/interps.c b/gdb/interps.c
> index f30357405877..0c5afa970651 100644
> --- a/gdb/interps.c
> +++ b/gdb/interps.c
> @@ -222,12 +222,6 @@ interpreter_completer (struct cmd_list_element *ignore,
>      }
>  }
>  
> -struct interp *
> -top_level_interpreter (void)
> -{
> -  return current_ui->top_level_interpreter;
> -}
> -
>  /* See interps.h.  */
>  
>  struct interp *
> @@ -245,7 +239,7 @@ interps_notify (void (interp::*method) (Args...), Args... args)
>  {
>    SWITCH_THRU_ALL_UIS ()
>      {
> -      interp *tli = top_level_interpreter ();
> +      interp *tli = current_ui->top_level_interpreter ();
>        if (tli != nullptr)
>  	(tli->*method) (args...);
>      }
> diff --git a/gdb/interps.h b/gdb/interps.h
> index 8416d657fb9a..1073d0516cfe 100644
> --- a/gdb/interps.h
> +++ b/gdb/interps.h
> @@ -258,9 +258,6 @@ extern void current_interp_set_logging (ui_file_up logfile,
>  					bool logging_redirect,
>  					bool debug_redirect);
>  
> -/* Returns the top-level interpreter.  */
> -extern struct interp *top_level_interpreter (void);
> -
>  /* Return the current UI's current interpreter.  */
>  extern struct interp *current_interpreter (void);
>  
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index fa733d880e38..bfee951bef2a 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -48,6 +48,7 @@
>  #include "gdbsupport/def-vector.h"
>  #include <algorithm>
>  #include "inferior.h"
> +#include "ui.h"
>  
>  /* An enumeration of the various things a user might attempt to
>     complete for a linespec location.  */
> @@ -3168,7 +3169,7 @@ decode_line_full (struct location_spec *locspec, int flags,
>  
>    if (select_mode == NULL)
>      {
> -      if (top_level_interpreter ()->interp_ui_out ()->is_mi_like_p ())
> +      if (current_ui->top_level_interpreter ()->interp_ui_out ()->is_mi_like_p ())
>  	select_mode = multiple_symbols_all;
>        else
>  	select_mode = multiple_symbols_select_mode ();
> diff --git a/gdb/main.c b/gdb/main.c
> index 58a79c518999..c31e28a79a74 100644
> --- a/gdb/main.c
> +++ b/gdb/main.c
> @@ -430,7 +430,7 @@ start_event_loop ()
>  	     get around to resetting the prompt, which leaves readline
>  	     in a messed-up state.  Reset it here.  */
>  	  current_ui->prompt_state = PROMPT_NEEDED;
> -	  top_level_interpreter ()->on_command_error ();
> +	  current_ui->top_level_interpreter ()->on_command_error ();
>  	  /* This call looks bizarre, but it is required.  If the user
>  	     entered a command that caused an error,
>  	     after_char_processing_hook won't be called from
> @@ -470,7 +470,7 @@ captured_command_loop ()
>  
>    /* Give the interpreter a chance to print a prompt, if necessary  */
>    if (ui->prompt_state != PROMPT_BLOCKED)
> -    top_level_interpreter ()->pre_command_loop ();
> +    current_ui->top_level_interpreter ()->pre_command_loop ();
>  
>    /* Now it's time to start the event loop.  */
>    start_event_loop ();
> diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
> index 60a2ed0bcc4c..30ba7a347b48 100644
> --- a/gdb/mi/mi-interp.c
> +++ b/gdb/mi/mi-interp.c
> @@ -235,7 +235,7 @@ mi_interp::on_sync_execution_done ()
>  static void
>  mi_execute_command_input_handler (gdb::unique_xmalloc_ptr<char> &&cmd)
>  {
> -  struct mi_interp *mi = as_mi_interp (top_level_interpreter ());
> +  mi_interp *mi = as_mi_interp (current_ui->top_level_interpreter ());
>    struct ui *ui = current_ui;
>  
>    ui->prompt_state = PROMPT_NEEDED;
> @@ -630,7 +630,7 @@ mi_output_running (struct thread_info *thread)
>  {
>    SWITCH_THRU_ALL_UIS ()
>      {
> -      struct mi_interp *mi = as_mi_interp (top_level_interpreter ());
> +      mi_interp *mi = as_mi_interp (current_ui->top_level_interpreter ());
>  
>        if (mi == NULL)
>  	continue;
> diff --git a/gdb/solib.c b/gdb/solib.c
> index 4f980e9365c4..e434c7145e4b 100644
> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -54,6 +54,7 @@
>  #include "source.h"
>  #include "cli/cli-style.h"
>  #include "solib-target.h"
> +#include "ui.h"
>  
>  /* See solib.h.  */
>  
> @@ -1146,7 +1147,8 @@ info_sharedlibrary_command (const char *pattern, int from_tty)
>  	    uiout->field_skip ("to");
>  	  }
>  
> -	if (! top_level_interpreter ()->interp_ui_out ()->is_mi_like_p ()
> +	if (!(current_ui->top_level_interpreter ()->interp_ui_out ()
> +	      ->is_mi_like_p ())
>  	    && so->symbols_loaded
>  	    && !objfile_has_symbols (so->objfile))
>  	  {
> diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
> index 941c65c970f3..818f5f8a1382 100644
> --- a/gdb/tui/tui.c
> +++ b/gdb/tui/tui.c
> @@ -399,7 +399,7 @@ tui_enable (void)
>  
>        /* If the top level interpreter is not the console/tui (e.g.,
>  	 MI), enabling curses will certainly lose.  */
> -      interp = top_level_interpreter ()->name ();
> +      interp = current_ui->top_level_interpreter ()->name ();
>        if (strcmp (interp, INTERP_TUI) != 0)
>  	error (_("Cannot enable the TUI when the interpreter is '%s'"), interp);
>  
> diff --git a/gdb/ui.c b/gdb/ui.c
> index ec74cc91cd21..e3d4a4765b80 100644
> --- a/gdb/ui.c
> +++ b/gdb/ui.c
> @@ -180,7 +180,7 @@ ui::set_current_interpreter (interp *interp, bool top_level)
>    /* If we already have an interpreter, then trying to
>       set top level interpreter is kinda pointless.  */
>    gdb_assert (!top_level || !this->current_interpreter);
> -  gdb_assert (!top_level || !this->top_level_interpreter);
> +  gdb_assert (!top_level || this->m_top_level_interpreter == nullptr);
>  
>    if (old_interp != NULL)
>      {
> @@ -190,7 +190,7 @@ ui::set_current_interpreter (interp *interp, bool top_level)
>  
>    this->current_interpreter = interp;
>    if (top_level)
> -    this->top_level_interpreter = interp;
> +    this->m_top_level_interpreter = interp;
>  
>    if (interpreter_p != interp->name ())
>      interpreter_p = interp->name ();
> @@ -295,7 +295,7 @@ new_ui_command (const char *args, int from_tty)
>  
>      current_ui->set_top_level_interpreter (interpreter_name);
>  
> -    top_level_interpreter ()->pre_command_loop ();
> +    current_ui->top_level_interpreter ()->pre_command_loop ();
>  
>      /* Make sure the file is not closed.  */
>      stream.release ();
> diff --git a/gdb/ui.h b/gdb/ui.h
> index cebf112f971c..1ba45760c186 100644
> --- a/gdb/ui.h
> +++ b/gdb/ui.h
> @@ -86,7 +86,6 @@ struct ui : public intrusive_list_node<ui>
>    /* Each UI has its own independent set of interpreters.  */
>    intrusive_list<interp> interp_list;
>    interp *current_interpreter = nullptr;
> -  interp *top_level_interpreter = nullptr;
>  
>    /* The interpreter that is active while `interp_exec' is active, NULL
>       at all other times.  */
> @@ -185,6 +184,13 @@ struct ui : public intrusive_list_node<ui>
>       Throws an error if NAME is not a known interpreter or the interpreter fails
>       to initialize.  */
>    void set_top_level_interpreter (const char *name);
> +
> +  /* Return this UI's top level interpreter.  */
> +  interp *top_level_interpreter ()
> +  { return m_top_level_interpreter; }

I think this function should be marked const.

Thanks,
Andrew

> +
> +private:
> +  interp *m_top_level_interpreter = nullptr;
>  };
>  
>  /* The main UI.  This is the UI that is bound to stdin/stdout/stderr.
> diff --git a/gdb/utils.c b/gdb/utils.c
> index cacd6cbd23e6..ef46b6bc292d 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -1606,8 +1606,9 @@ pager_file::puts (const char *linebuffer)
>    /* Don't do any filtering or wrapping if both are disabled.  */
>    if (batch_flag
>        || (lines_per_page == UINT_MAX && chars_per_line == UINT_MAX)
> -      || top_level_interpreter () == NULL
> -      || top_level_interpreter ()->interp_ui_out ()->is_mi_like_p ())
> +      || current_ui->top_level_interpreter () == NULL
> +      || (current_ui->top_level_interpreter ()->interp_ui_out ()
> +	  ->is_mi_like_p ()))
>      {
>        flush_wrap_buffer ();
>        m_stream->puts (linebuffer);
> -- 
> 2.42.0