Re: [PATCH] Eliminate interp::quiet_p

[Simon Marchi <simon dot marchi at ericsson dot com>]

On 17-02-06 01:40 PM, Pedro Alves wrote:
> [Here's something I noticed back when working on new-ui.]
> 
> This commit removes interp::quiet_p / interp_quiet_p /
> interp_set_quiet, because AFAICS, it doesn't really do anything.
> 
> interp_quiet is only ever checked inside interp_set nowadays:
> 
>   if (!first_time && !interp_quiet_p (interp))
>     {
>       xsnprintf (buffer, sizeof (buffer),
> 		 "Switching to interpreter \"%.24s\".\n", interp->name);
>       current_uiout->text (buffer);
>     }
> 
> I did a bit of archaelogy, and found that back in 4a8f6654 (2003), it
> was also called in another place, to decide whether to print the CLI
> prompt.
> 
> AFAICS, that condition is always false today, making that if/then
> block always dead code.  If we remove that code, then there are no
> interp_quiet_p uses left in the tree, so we can remove it all.
> 
> There are two paths that lead to interp_set calls:
> 
> #1 - When installing the top level interpreter.  In this case,
> FIRST_TIME is true.
> 
> #2 - In interpreter_exec_cmd.  In this case, the interpreter is always
> set quiet before interp_set is called.
> 
> Grepping a gdb.log of an x86_64 GNU/Linux run for "Switching to
> interpreter" (before this patch) doesn't find any hits.
> 
> I suspect the intention of this message was to support something like
> a "set interpreter ..." command that would change the interpreter
> permanently.  But there's no such command.
> 
> Tested on x86_64 Fedora 23.
> 
> gdb/ChangeLog:
> 2017-02-03  Pedro Alves  <palves@redhat.com>
> 
> 	* interps.c (interp::interp): Remove reference to quiet_p.
> 	(interp_set): Make static.  Remove dead "Switching to" output
> 	code.
> 	(interp_quiet_p, interp_set_quiet): Delete.
> 	(interpreter_exec_cmd): Don't set the interpreter quiet.
> 	* interps.h (interp_quiet_p): Make static.
> 	(class interp) <quiet_p>: Remove field
> ---
>  gdb/interps.c | 44 +-------------------------------------------
>  gdb/interps.h |  4 ----
>  2 files changed, 1 insertion(+), 47 deletions(-)
> 
> diff --git a/gdb/interps.c b/gdb/interps.c
> index d31d53b..322b9f5 100644
> --- a/gdb/interps.c
> +++ b/gdb/interps.c
> @@ -82,7 +82,6 @@ static struct interp *interp_lookup_existing (struct ui *ui,
>  interp::interp (const char *name)
>  {
>    this->name = xstrdup (name);
> -  this->quiet_p = false;
>    this->inited = false;
>  }
>  
> @@ -156,12 +155,11 @@ interp_add (struct ui *ui, struct interp *interp)
>     events such as target stops and new thread creation, even if they
>     are caused by CLI commands.  */
>  
> -void
> +static void
>  interp_set (struct interp *interp, bool top_level)
>  {
>    struct ui_interp_info *ui_interp = get_current_interp_info ();
>    struct interp *old_interp = ui_interp->current_interpreter;
> -  int first_time = 0;
>    char buffer[64];
>  
>    /* If we already have an interpreter, then trying to
> @@ -174,10 +172,6 @@ interp_set (struct interp *interp, bool top_level)
>        current_uiout->flush ();
>        old_interp->suspend ();
>      }
> -  else
> -    {
> -      first_time = 1;
> -    }
>  
>    ui_interp->current_interpreter = interp;
>    if (top_level)
> @@ -207,13 +201,6 @@ interp_set (struct interp *interp, bool top_level)
>    clear_interpreter_hooks ();
>  
>    interp->resume ();
> -
> -  if (!first_time && !interp_quiet_p (interp))
> -    {
> -      xsnprintf (buffer, sizeof (buffer),
> -		 "Switching to interpreter \"%.24s\".\n", interp->name);
> -      current_uiout->text (buffer);
> -    }
>  }
>  
>  /* Look up the interpreter for NAME.  If no such interpreter exists,
> @@ -375,26 +362,6 @@ interp_supports_command_editing (struct interp *interp)
>    return interp->supports_command_editing ();
>  }
>  
> -int
> -interp_quiet_p (struct interp *interp)
> -{
> -  struct ui_interp_info *ui_interp = get_current_interp_info ();
> -
> -  if (interp != NULL)
> -    return interp->quiet_p;
> -  else
> -    return ui_interp->current_interpreter->quiet_p;
> -}
> -
> -static int
> -interp_set_quiet (struct interp *interp, int quiet)
> -{
> -  int old_val = interp->quiet_p;
> -
> -  interp->quiet_p = quiet;
> -  return old_val;
> -}
> -
>  /* interp_exec - This executes COMMAND_STR in the current 
>     interpreter.  */
>  
> @@ -445,7 +412,6 @@ interpreter_exec_cmd (char *args, int from_tty)
>    char **trule = NULL;
>    unsigned int nrules;
>    unsigned int i;
> -  int old_quiet, use_quiet;
>    struct cleanup *cleanup;
>  
>    if (args == NULL)
> @@ -467,10 +433,6 @@ interpreter_exec_cmd (char *args, int from_tty)
>    if (interp_to_use == NULL)
>      error (_("Could not find interpreter \"%s\"."), prules[0]);
>  
> -  /* Temporarily set interpreters quiet.  */
> -  old_quiet = interp_set_quiet (old_interp, 1);
> -  use_quiet = interp_set_quiet (interp_to_use, 1);
> -
>    interp_set (interp_to_use, false);
>  
>    for (i = 1; i < nrules; i++)
> @@ -480,15 +442,11 @@ interpreter_exec_cmd (char *args, int from_tty)
>        if (e.reason < 0)
>  	{
>  	  interp_set (old_interp, 0);
> -	  interp_set_quiet (interp_to_use, use_quiet);
> -	  interp_set_quiet (old_interp, old_quiet);
>  	  error (_("error in command: \"%s\"."), prules[i]);
>  	}
>      }
>  
>    interp_set (old_interp, 0);
> -  interp_set_quiet (interp_to_use, use_quiet);
> -  interp_set_quiet (old_interp, old_quiet);
>  
>    do_cleanups (cleanup);
>  }
> diff --git a/gdb/interps.h b/gdb/interps.h
> index e564980..1b9580c 100644
> --- a/gdb/interps.h
> +++ b/gdb/interps.h
> @@ -39,7 +39,6 @@ extern int interp_resume (struct interp *interp);
>  extern int interp_suspend (struct interp *interp);
>  extern struct gdb_exception interp_exec (struct interp *interp,
>  					 const char *command);
> -extern int interp_quiet_p (struct interp *interp);
>  
>  class interp
>  {
> @@ -86,12 +85,9 @@ public:
>  
>    /* Has the init method been run?  */
>    bool inited;
> -
> -  bool quiet_p;
>  };
>  
>  extern void interp_add (struct ui *ui, struct interp *interp);
> -extern void interp_set (struct interp *interp, bool top_level);
>  
>  /* Look up the interpreter for NAME, creating one if none exists yet.
>     If NAME is not a interpreter type previously registered with
> 

LGTM.