[RFC] Change "set print frame-arguments"'s default to scalars

[RFC] Change "set print frame-arguments"'s default to scalars

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 2477 bytes --]

This is a slight change in behavior that we made at AdaCore which
everyone seemed to have enjoyed.  The proposal is to change the default
for "set print frame-arguments" to "scalars" in order to unclutter
frames a bit.  For instance, I found the following example in our
testsuite. The debugger hit a breakpoing inside a function where
some of the arguments are structs. It looks like this:

> Breakpoint 7, sum_array_print (seed=10, linked_list1={next_index = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}, values = {4, 6, 8, 10, 12, 14, 16, 18, 20, 22}, head = 0}, linked_list2={next_index = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}, values = {8, 10, 12, 14, 16, 18, 20, 22, 24, 26}, head = 0}, linked_list3={next_index = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}, values = {10, 12, 14, 16, 18, 20, 22, 24, 26, 28}, head = 0}, linked_list4={next_index = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}, values = {20, 22, 24, 26, 28, 30, 32, 34, 36, 38}, head = 0}) at /home/brobecke/act/gdb-public/gdb/testsuite/gdb.base/call-ar-st.c:1105^M

(you should see what the above looks like when it's inside a backtrace)

The idea is that, most of the time, people are mostly interested
in knowing the name of the function where they stopped, and don't
care (yet!) about the value for their parameters. So simplifying
the output above to the following should be more productive most
of the time:

> Breakpoint 7, sum_array_print (seed=10, linked_list1=..., linked_list2=..., linked_list3=..., linked_list4=...) at /home/brobecke/act/gdb-public/gdb/testsuite/gdb.base/call-ar-st.c:1105^M

If the user needs to know the value of one of the non-scalar parameters,
he needs to use "print". From my personal experience, even if the parameter
value was printed in the frame, the frame is so cluttered at this point
that I would manaully get the parameter value with a print anyway.

To put things back into perspective, I think it's relatively uncommon
to pass structs by argument unless they are small. When bigger, I suspect
that the typical approach is to pass its address. So I don't think
this change would impact C users much. I'm not sure about C++, in particular
I'm not sure about how classes/objects are passed. I guess references,
which probably means little impact as well. In Ada, however, passing
structs as arguments is a common idiom, and changing the default seems
to have improved the life of many developers. I suspect that the same
will be true of Pascal users, but I can't say for sure.

Yay? Nay?

Thanks,
-- 
Joel

[-- Attachment #2: frame-args-scalars.diff --]
[-- Type: text/x-diff, Size: 718 bytes --]

commit ab83cdc6d0d12c1fcfd718e8c81038be77f02491
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Tue Mar 24 14:10:08 2009 -0700

        Change the default value for "set print frame-arguments" to scalars.
    
        * stack.c (print_frame_arguments): Set initial value to "scalars".

diff --git a/gdb/stack.c b/gdb/stack.c
index f185841..814e271 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -58,7 +58,7 @@ void (*deprecated_selected_frame_level_changed_hook) (int);
 
 static const char *print_frame_arguments_choices[] =
   {"all", "scalars", "none", NULL};
-static const char *print_frame_arguments = "all";
+static const char *print_frame_arguments = "scalars";
 
 /* Prototypes for local functions. */
 

Re: [RFC] Change "set print frame-arguments"'s default to scalars

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> This is a slight change in behavior that we made at AdaCore
Joel> which everyone seemed to have enjoyed.  The proposal is to
Joel> change the default for "set print frame-arguments" to "scalars"
Joel> in order to unclutter frames a bit.

Joel> So I don't think this change would impact C users much. I'm not
Joel> sure about C++, in particular I'm not sure about how
Joel> classes/objects are passed.

I implemented something vaguely similar to this as part of the Python
pretty-printing work.  I did not know about the frame-arguments
parameter, so I added a new "summary" flag in value_print_options, and
I have the pretty-printers print just part of complex structure
arguments.

So, I think your patch is reasonable (though it needs a doc update
:-).  I'll rework the pretty-printing stuff a bit to use
frame-arguments.

Tom

Re: [RFC] Change "set print frame-arguments"'s default to scalars

[Thiago Jung Bauermann]

El mar, 24-03-2009 a las 14:27 -0700, Joel Brobecker escribió:
> Yay? Nay?

Yay!
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [RFC] Change "set print frame-arguments"'s default to scalars

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> * stack.c (print_frame_arguments): Set initial value to "scalars".

I applied this to the Python branch, along with a fix to make
pretty-printers respect this parameter.

I needed the appended to make the test suite clean.

Also, Jan had the excellent idea that we could limit the output
(truncate it after some number of characters and add "...") for a
given value, rather than have the selection be all-or-none.  This
could easily be done by making a new kind of output stream, and then
using it when printing a frame in stack.c.  Let me know what you think
of this; I can implement it if people think it is a good idea.

Tom

2009-03-27  Tom Tromey  <tromey@redhat.com>

	* gdb.base/funcargs.exp: Set print frame-arguments to "all".
	* gdb.base/call-ar-st.exp: Set print frame-arguments to "all".
	* gdb.ada/ref_param.exp: Set print frame-arguments to "all".
	* gdb.ada/lang_switch.exp: Set print frame-arguments to "all".

diff --git a/gdb/testsuite/gdb.ada/lang_switch.exp b/gdb/testsuite/gdb.ada/lang_switch.exp
index 45a4e53..3fc3584 100644
--- a/gdb/testsuite/gdb.ada/lang_switch.exp
+++ b/gdb/testsuite/gdb.ada/lang_switch.exp
@@ -41,6 +41,8 @@ gdb_load ${binfile}
 set bp_location [gdb_get_line_number "STOP" ${testdir}/foo.c]
 runto "foo.c:$bp_location"
 
+gdb_test "set print frame-arguments all" ""
+
 # Make sure that the language is switched to Ada for the second frame
 # by checking the string parameter.
 gdb_test "bt" \
diff --git a/gdb/testsuite/gdb.ada/ref_param.exp b/gdb/testsuite/gdb.ada/ref_param.exp
index 5325dd2..4d49296 100644
--- a/gdb/testsuite/gdb.ada/ref_param.exp
+++ b/gdb/testsuite/gdb.ada/ref_param.exp
@@ -39,6 +39,8 @@ if ![runto call_me] then {
   return
 } 
 
+gdb_test "set print frame-arguments all" ""
+
 set expected_d "\\(one => 1, two => 2, three => 3, four => 4, five => 5, six => 6\\)"
 gdb_test "frame" \
          "#0\[ \t\]*pck\\.call_me \\(d=${expected_d}\\).*"  \
diff --git a/gdb/testsuite/gdb.base/call-ar-st.exp b/gdb/testsuite/gdb.base/call-ar-st.exp
index b88ff02..a058f67 100644
--- a/gdb/testsuite/gdb.base/call-ar-st.exp
+++ b/gdb/testsuite/gdb.base/call-ar-st.exp
@@ -332,6 +332,7 @@ gdb_expect {
 gdb_test "break sum_array_print" \
 	".*Breakpoint ${decimal}: file .*call-ar-st.c, line.*" \
 	"set breakpoint in sum_array_print"
+gdb_test "set print frame-arguments all" ""
 gdb_test "continue" \
 	".*Breakpoint ${decimal}, sum_array_print \\(seed=10, linked_list1=.next_index = .1, 2, 3, 4, 5, 6, 7, 8, 9, 10., values = .4, 6, 8, 10, 12, 14, 16, 18, 20, 22., head = 0., linked_list2=.next_index = .1, 2, 3, 4, 5, 6, 7, 8, 9, 10., values = .8, 10, 12, 14, 16, 18, 20, 22, 24, 26., head = 0., linked_list3=.next_index = .1, 2, 3, 4, 5, 6, 7, 8, 9, 10., values = .10, 12, 14, 16, 18, 20, 22, 24, 26, 28., head = 0., linked_list4=.next_index = .1, 2, 3, 4, 5, 6, 7, 8, 9, 10., values = .20, 22, 24, 26, 28, 30, 32, 34, 36, 38., head = 0.\\) at .*call-ar-st.c:1105\[ \t\n\r\]+1105.*printf\\(.Sum of 4 arrays, by element \\(add in seed as well\\).*\\);.*" \
 	"check args of sum_array_print"
diff --git a/gdb/testsuite/gdb.base/funcargs.exp b/gdb/testsuite/gdb.base/funcargs.exp
index 1cf3e9d..037f0a0 100644
--- a/gdb/testsuite/gdb.base/funcargs.exp
+++ b/gdb/testsuite/gdb.base/funcargs.exp
@@ -1194,6 +1194,8 @@ gdb_expect {
 
 # Perform tests
 
+gdb_test "set print frame-arguments all" ""
+
 integral_args
 funcargs_reload
 unsigned_integral_args

Re: [RFC] Change "set print frame-arguments"'s default to scalars

[Doug Evans]

On Fri, Mar 27, 2009 at 10:19 AM, Tom Tromey <tromey@redhat.com> wrote:
>
> Also, Jan had the excellent idea that we could limit the output
> (truncate it after some number of characters and add "...") for a
> given value, rather than have the selection be all-or-none.  This
> could easily be done by making a new kind of output stream, and then
> using it when printing a frame in stack.c.  Let me know what you think
> of this; I can implement it if people think it is a good idea.

I've seen various Scheme interpreters do this.  I like it.

Re: [RFC] Change "set print frame-arguments"'s default to scalars

[Joel Brobecker]

>         Change the default value for "set print frame-arguments" to scalars.
>         * stack.c (print_frame_arguments): Set initial value to "scalars".

Given the positive reaction to this change, I checked it in, together
with Tom's testsuite updates (Thank You!)

I'll look at the documentation now, in case an update might be useful.

-- 
Joel