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

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

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed

Commit Message

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

Comments

Andrew Burgess Sept. 12, 2023, 11:35 a.m. UTC | #1
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
  
Simon Marchi Sept. 12, 2023, 5:54 p.m. UTC | #2
>> @@ -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.
If we want things to be const-correct, then I guess a const version of
this method should return a const interp.  And that would be a bunch
more work (if even possible).  But I guess that marking the method const
and returning a non-const interp doesn't have downsides versus what I
have above, so let's add const.

Simon
  

Patch

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; }
+
+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);