Re: [PATCH] Add "set print finish"

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Tom Tromey <tromey@adacore.com> [2019-05-16 10:32:44 -0600]:

> A user wanted to be able to disable the display of the value when
> using "finish" -- but still have the value entered into the value
> history in case it was useful later on.  Part of the rationale here is
> that sometimes the value might be quite large, or expensive to display
> (in their case this was compounded by a rogue pretty-printer).
> 
> This patch implements this idea.
> 
> gdb/ChangeLog
> 2019-05-16  Tom Tromey  <tromey@adacore.com>
> 
> 	* NEWS: Add entry.
> 	* infcmd.c (print_return_value_1): Handle finish_print
> 	option.
> 	(show_print_finish): New function.
> 	(_initialize_infcmd): Add "set/show print finish" commands.
> 	* valprint.c (user_print_options): Initialize new member.
> 	* valprint.h (struct value_print_options) <finish_print>: New
> 	member.
> 
> gdb/doc/ChangeLog
> 2019-05-16  Tom Tromey  <tromey@adacore.com>
> 
> 	* gdb.texinfo (Continuing and Stepping): Document new
> 	commands.
> 
> gdb/testsuite/ChangeLog
> 2019-05-16  Tom Tromey  <tromey@adacore.com>
> 
> 	* gdb.base/finish.exp (finish_no_print): New proc.
> 	(finish_tests): Call it.
> ---
>  gdb/ChangeLog                     | 11 +++++++++++
>  gdb/NEWS                          |  7 +++++++
>  gdb/doc/ChangeLog                 |  5 +++++
>  gdb/doc/gdb.texinfo               |  9 +++++++++
>  gdb/infcmd.c                      | 33 +++++++++++++++++++++++++++----
>  gdb/testsuite/ChangeLog           |  5 +++++
>  gdb/testsuite/gdb.base/finish.exp | 16 +++++++++++++++
>  gdb/valprint.c                    |  3 ++-
>  gdb/valprint.h                    |  3 +++
>  9 files changed, 87 insertions(+), 5 deletions(-)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 288615b8cd3..451480bf92b 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -46,6 +46,13 @@ show print max-depth
>    The default max-depth is 20, but this can be set to unlimited to get
>    the old behavior back.
>  
> +set print finish [on|off]
> +show print finish
> +  This controls whether the `finish' command will display the value
> +  that is returned by the current function.  When `off', the value is
> +  still entered into the value history, but it is not printed.  The
> +  default is `on'.
> +
>  *** Changes in GDB 8.3
>  
>  * GDB and GDBserver now support access to additional registers on
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 328d510dd85..7742616fdcc 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -5596,6 +5596,15 @@ abbreviated as @code{fin}.
>  Contrast this with the @code{return} command (@pxref{Returning,
>  ,Returning from a Function}).
>  
> +@kindex set print finish
> +@kindex show print finish
> +@item set print finish @r{[}on|off@r{]}
> +@itemx show print finish
> +By default the @code{finish} command will show the value that is
> +returned by the function.  This can be disabled using @code{set print
> +finish off}.  When disabled, the value is still entered into the value
> +history (@pxref{Value History}), but not displayed.
> +
>  @kindex until
>  @kindex u @r{(@code{until})}
>  @cindex run until specified location
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index 178f89e9520..1dfbe2329b7 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -1621,10 +1621,14 @@ print_return_value_1 (struct ui_out *uiout, struct return_value_info *rv)
>        uiout->text (" = ");
>        get_user_print_options (&opts);
>  
> -      string_file stb;
> -
> -      value_print (rv->value, &stb, &opts);
> -      uiout->field_stream ("return-value", stb);
> +      if (opts.finish_print)
> +	{
> +	  string_file stb;
> +	  value_print (rv->value, &stb, &opts);
> +	  uiout->field_stream ("return-value", stb);
> +	}
> +      else
> +	uiout->field_string ("return-value", _("<not displayed>"));
>        uiout->text ("\n");
>      }
>    else
> @@ -3096,6 +3100,19 @@ info_proc_cmd_all (const char *args, int from_tty)
>    info_proc_cmd_1 (args, IP_ALL, from_tty);
>  }
>  
> +/* Implement `show print finish'.  */
> +
> +static void
> +show_print_finish (struct ui_file *file, int from_tty,
> +		   struct cmd_list_element *c,
> +		   const char *value)
> +{
> +  fprintf_filtered (file, _("\
> +Printing of return value after `finish' is %s.\n"),
> +		    value);
> +}
> +
> +
>  /* This help string is used for the run, start, and starti commands.
>     It is defined as a macro to prevent duplication.  */
>  
> @@ -3420,4 +3437,12 @@ List files opened by the specified process."),
>    add_cmd ("all", class_info, info_proc_cmd_all, _("\
>  List all available info about the specified process."),
>  	   &info_proc_cmdlist);
> +
> +  add_setshow_boolean_cmd ("finish", class_support,
> +			   &user_print_options.finish_print, _("\
> +Set whether `finish' prints the return value."), _("\
> +Show whether `finish' prints the return value."), NULL,
> +			   NULL,
> +			   show_print_finish,
> +			   &setprintlist, &showprintlist);
>  }
> diff --git a/gdb/testsuite/gdb.base/finish.exp b/gdb/testsuite/gdb.base/finish.exp
> index 3bdc5d4726e..56f3c10e3f0 100644
> --- a/gdb/testsuite/gdb.base/finish.exp
> +++ b/gdb/testsuite/gdb.base/finish.exp
> @@ -86,6 +86,21 @@ proc finish_abbreviation { abbrev } {
>               "Testing the \"$abbrev\" abbreviation for \"finish\""
>  }
>  
> +# Test "set print finish off".
> +proc finish_no_print {} {
> +    global decimal
> +
> +    if {![runto "int_func"]} {
> +	untested "couldn't run to main"
> +	return
> +    }
> +    gdb_test_no_output "set print finish off"
> +    gdb_test "finish" \
> +	"Value returned is \\\$$decimal = <not displayed>"
> +    gdb_test "print \$" " = 1" \
> +	"Ensure return value was properly saved"
> +}
> +
>  proc finish_tests { } {
>      global gdb_prompt skip_float_test
>  
> @@ -105,6 +120,7 @@ proc finish_tests { } {
>  	finish_1 "double"
>      }
>      finish_abbreviation "fin"
> +    finish_no_print
>  }
>  
>  set prev_timeout $timeout
> diff --git a/gdb/valprint.c b/gdb/valprint.c
> index b9d8878c8e4..4c3d67a9ff8 100644
> --- a/gdb/valprint.c
> +++ b/gdb/valprint.c
> @@ -111,7 +111,8 @@ struct value_print_options user_print_options =
>    0,				/* raw */
>    0,				/* summary */
>    1,				/* symbol_print */
> -  PRINT_MAX_DEPTH_DEFAULT	/* max_depth */
> +  PRINT_MAX_DEPTH_DEFAULT,	/* max_depth */
> +  1				/* finish_print */
>  };
>  
>  /* Initialize *OPTS to be a copy of the user print options.  */
> diff --git a/gdb/valprint.h b/gdb/valprint.h
> index e5cc9477987..0bd3f1966c4 100644
> --- a/gdb/valprint.h
> +++ b/gdb/valprint.h
> @@ -95,6 +95,9 @@ struct value_print_options
>  
>    /* Maximum print depth when printing nested aggregates.  */
>    int max_depth;
> +
> +  /* Whether "finish" should print the value.  */
> +  int finish_print;
>  };

Should new flags not be bool ?

Thanks,
Andrew


>  
>  /* The global print options set by the user.  In general this should
> -- 
> 2.20.1
>