[PATCH] Add "set print finish"

[PATCH] Add "set print finish"

[Tom Tromey]

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;
 };
 
 /* The global print options set by the user.  In general this should
-- 
2.20.1

Re: [PATCH] Add "set print finish"

[Eli Zaretskii]

> From: Tom Tromey <tromey@adacore.com>
> Cc: Tom Tromey <tromey@adacore.com>
> Date: Thu, 16 May 2019 10:32:44 -0600
> 
> 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.

Thanks, the documentation parts are OK.

Re: [PATCH] Add "set print finish"

[Andrew Burgess]

* 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
> 

Re: [PATCH] Add "set print finish"

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

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


>> +  /* Whether "finish" should print the value.  */
>> +  int finish_print;
>> };

Andrew> Should new flags not be bool ?

Ordinarily, but in this case the address of one of these is passed to
add_setshow_boolean_cmd, which still takes an "int *" -- see the code I
quoted above.

Tom

Re: [PATCH] Add "set print finish"

[Andrew Burgess]

* Tom Tromey <tromey@adacore.com> [2019-05-16 12:08:39 -0600]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> >> +  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);
> 
> 
> >> +  /* Whether "finish" should print the value.  */
> >> +  int finish_print;
> >> };
> 
> Andrew> Should new flags not be bool ?
> 
> Ordinarily, but in this case the address of one of these is passed to
> add_setshow_boolean_cmd, which still takes an "int *" -- see the code I
> quoted above.

Of course.  Sorry for the noise :)

Thanks,
Andrew


> 
> Tom

Re: [PATCH] Add "set print finish"

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:

Tom> A user wanted to be able to disable the display of the value when
Tom> using "finish" -- but still have the value entered into the value
Tom> history in case it was useful later on.  Part of the rationale here is
Tom> that sometimes the value might be quite large, or expensive to display
Tom> (in their case this was compounded by a rogue pretty-printer).

Tom> This patch implements this idea.

I'm checking this in now.

Tom