[PATCH v2][PR gdb/17320] Fix array pretty formatter

[PATCH v2][PR gdb/17320] Fix array pretty formatter

[Hannes Domani]

Currently, printing with array pretty formatting makes the output actually
less readable than without:

(gdb) p -array on -- {{1,2,3},{4,5,6}}
$1 =   {    {1,
    2,
    3},
      {4,
    5,
    6}}
(gdb) p -array on -array-indexes on -- {{1,2,3},{4,5,6}}
$2 =   {[0] =     {[0] = 1,
    [1] = 2,
    [2] = 3},
  [1] =     {[0] = 4,
    [1] = 5,
    [2] = 6}}

These changes now also put the first element and the array end bracket on a new
line, similar to the structure pretty formatter:

(gdb) p -array on -- {{1,2,3},{4,5,6}}
$1 = {
  {
    1,
    2,
    3
  },
  {
    4,
    5,
    6
  }
}
(gdb) p -array on -array-indexes on -- {{1,2,3},{4,5,6}}
$2 = {
  [0] = {
    [0] = 1,
    [1] = 2,
    [2] = 3
  },
  [1] = {
    [0] = 4,
    [1] = 5,
    [2] = 6
  }
}

gdb/ChangeLog:

2020-04-27  Hannes Domani  <ssbssa@yahoo.de>

	PR gdb/17320
	* ada-valprint.c (val_print_packed_array_elements): Move array
	end bracket to new line.
	(ada_val_print_string): Remove extra spaces before first array
	element.
	* c-valprint.c (c_value_print_array): Likewise.
	* m2-valprint.c (m2_print_array_contents): Likewise.
	(m2_value_print_inner): Likewise.
	* p-valprint.c (pascal_value_print_inner): Likewise.
	* valprint.c (generic_val_print_array): Likewise.
	(value_print_array_elements): Move first array element and array
	end bracket to new line.

gdb/testsuite/ChangeLog:

2020-04-27  Hannes Domani  <ssbssa@yahoo.de>

	PR gdb/17320
	* gdb.base/pretty-array.c: New test.
	* gdb.base/pretty-array.exp: New file.
---
v2:
- Added test case.
---
 gdb/ada-valprint.c                      |  8 +--
 gdb/c-valprint.c                        |  4 --
 gdb/m2-valprint.c                       |  4 --
 gdb/p-valprint.c                        |  4 --
 gdb/testsuite/gdb.base/pretty-array.c   | 24 +++++++++
 gdb/testsuite/gdb.base/pretty-array.exp | 65 +++++++++++++++++++++++++
 gdb/valprint.c                          | 15 ++++--
 7 files changed, 104 insertions(+), 20 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/pretty-array.c
 create mode 100644 gdb/testsuite/gdb.base/pretty-array.exp

diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index 2768829cdb..9233df1bb1 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -195,6 +195,11 @@ val_print_packed_array_elements (struct type *type, const gdb_byte *valaddr,
 	      fprintf_filtered (stream, ", ");
 	    }
 	}
+      else if (options->prettyformat_arrays)
+	{
+	  fprintf_filtered (stream, "\n");
+	  print_spaces_filtered (2 + 2 * recurse, stream);
+	}
       wrap_here (n_spaces (2 + 2 * recurse));
       maybe_print_array_index (index_type, i + low, stream, options);
 
@@ -707,9 +712,6 @@ ada_val_print_string (struct type *type, const gdb_byte *valaddr,
   eltlen = TYPE_LENGTH (elttype);
   len = TYPE_LENGTH (type) / eltlen;
 
-  if (options->prettyformat_arrays)
-    print_spaces_filtered (2 + 2 * recurse, stream);
-
   /* If requested, look for the first null char and only print
      elements up to it.  */
   if (options->stop_print_at_null)
diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
index bde9c6cc88..52ea5eda0c 100644
--- a/gdb/c-valprint.c
+++ b/gdb/c-valprint.c
@@ -252,10 +252,6 @@ c_value_print_array (struct value *val,
 
       eltlen = TYPE_LENGTH (elttype);
       len = high_bound - low_bound + 1;
-      if (options->prettyformat_arrays)
-	{
-	  print_spaces_filtered (2 + 2 * recurse, stream);
-	}
 
       /* Print arrays of textual chars with a string syntax, as
 	 long as the entire array is valid.  */
diff --git a/gdb/m2-valprint.c b/gdb/m2-valprint.c
index 844a63f3bd..e210b5ec2f 100644
--- a/gdb/m2-valprint.c
+++ b/gdb/m2-valprint.c
@@ -265,8 +265,6 @@ m2_print_array_contents (struct value *val,
 
   if (TYPE_LENGTH (type) > 0)
     {
-      if (options->prettyformat_arrays)
-	print_spaces_filtered (2 + 2 * recurse, stream);
       /* For an array of chars, print with string syntax.  */
       if (TYPE_LENGTH (type) == 1 &&
 	  ((TYPE_CODE (type) == TYPE_CODE_INT)
@@ -318,8 +316,6 @@ m2_value_print_inner (struct value *val, struct ui_file *stream, int recurse,
 	{
 	  elttype = check_typedef (TYPE_TARGET_TYPE (type));
 	  len = TYPE_LENGTH (type) / TYPE_LENGTH (elttype);
-	  if (options->prettyformat_arrays)
-	    print_spaces_filtered (2 + 2 * recurse, stream);
 	  /* For an array of chars, print with string syntax.  */
 	  if (TYPE_LENGTH (elttype) == 1 &&
 	      ((TYPE_CODE (elttype) == TYPE_CODE_INT)
diff --git a/gdb/p-valprint.c b/gdb/p-valprint.c
index cbd7fb75e2..fbf5c5cf14 100644
--- a/gdb/p-valprint.c
+++ b/gdb/p-valprint.c
@@ -93,10 +93,6 @@ pascal_value_print_inner (struct value *val, struct ui_file *stream,
 	    len = high_bound - low_bound + 1;
 	    elttype = check_typedef (TYPE_TARGET_TYPE (type));
 	    eltlen = TYPE_LENGTH (elttype);
-	    if (options->prettyformat_arrays)
-	      {
-		print_spaces_filtered (2 + 2 * recurse, stream);
-	      }
 	    /* If 's' format is used, try to print out as string.
 	       If no format is given, print as string if element type
 	       is of TYPE_CODE_CHAR and element size is 1,2 or 4.  */
diff --git a/gdb/testsuite/gdb.base/pretty-array.c b/gdb/testsuite/gdb.base/pretty-array.c
new file mode 100644
index 0000000000..2adebcca8c
--- /dev/null
+++ b/gdb/testsuite/gdb.base/pretty-array.c
@@ -0,0 +1,24 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2020 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int nums[2][3] = {{11, 12, 13}, {21, 22, 23}};
+
+int
+main ()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/pretty-array.exp b/gdb/testsuite/gdb.base/pretty-array.exp
new file mode 100644
index 0000000000..e17ce18857
--- /dev/null
+++ b/gdb/testsuite/gdb.base/pretty-array.exp
@@ -0,0 +1,65 @@
+# Copyright 2020 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test pretty printing of arrays.
+
+standard_testfile
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
+    untested $testfile.exp
+    return -1
+}
+
+if ![runto_main] {
+    untested $testfile.exp
+    return -1
+}
+
+gdb_test "print nums" \
+    "= \\{\\{11, 12, 13\\}, \\{21, 22, 23\\}\\}"
+
+gdb_test_no_output "set print array on"
+
+gdb_test "print nums" \
+    [multi_line \
+	 " = {" \
+	 "  {" \
+	 "    11," \
+	 "    12," \
+	 "    13" \
+	 "  }," \
+	 "  {" \
+	 "    21," \
+	 "    22," \
+	 "    23" \
+	 "  }" \
+	 "}" ]
+
+gdb_test_no_output "set print array-indexes on"
+
+gdb_test "print nums" \
+    [multi_line \
+	 " = {" \
+	 "  \\\[0\\\] = {" \
+	 "    \\\[0\\\] = 11," \
+	 "    \\\[1\\\] = 12," \
+	 "    \\\[2\\\] = 13" \
+	 "  }," \
+	 "  \\\[1\\\] = {" \
+	 "    \\\[0\\\] = 21," \
+	 "    \\\[1\\\] = 22," \
+	 "    \\\[2\\\] = 23" \
+	 "  }" \
+	 "}" ]
diff --git a/gdb/valprint.c b/gdb/valprint.c
index b91aaaeb7c..5f95553838 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -442,11 +442,6 @@ generic_val_print_array (struct value *val,
       if (!get_array_bounds (type, &low_bound, &high_bound))
 	error (_("Could not determine the array high bound"));
 
-      if (options->prettyformat_arrays)
-	{
-	  print_spaces_filtered (2 + 2 * recurse, stream);
-	}
-
       fputs_filtered (decorations->array_start, stream);
       value_print_array_elements (val, stream, recurse, options, 0);
       fputs_filtered (decorations->array_end, stream);
@@ -1946,6 +1941,11 @@ value_print_array_elements (struct value *val, struct ui_file *stream,
 	  else
 	    fprintf_filtered (stream, ", ");
 	}
+      else if (options->prettyformat_arrays)
+	{
+	  fprintf_filtered (stream, "\n");
+	  print_spaces_filtered (2 + 2 * recurse, stream);
+	}
       wrap_here (n_spaces (2 + 2 * recurse));
       maybe_print_array_index (index_type, i + low_bound,
                                stream, options);
@@ -1990,6 +1990,11 @@ value_print_array_elements (struct value *val, struct ui_file *stream,
   annotate_array_section_end ();
   if (i < len)
     fprintf_filtered (stream, "...");
+  if (options->prettyformat_arrays)
+    {
+      fprintf_filtered (stream, "\n");
+      print_spaces_filtered (2 * recurse, stream);
+    }
 }
 
 /* Read LEN bytes of target memory at address MEMADDR, placing the
-- 
2.26.2

Re: [PATCH v2][PR gdb/17320] Fix array pretty formatter

[Simon Marchi]

On 2020-04-27 8:59 a.m., Hannes Domani via Gdb-patches wrote:
> Currently, printing with array pretty formatting makes the output actually
> less readable than without:
> 
> (gdb) p -array on -- {{1,2,3},{4,5,6}}
> $1 =   {    {1,
>     2,
>     3},
>       {4,
>     5,
>     6}}
> (gdb) p -array on -array-indexes on -- {{1,2,3},{4,5,6}}
> $2 =   {[0] =     {[0] = 1,
>     [1] = 2,
>     [2] = 3},
>   [1] =     {[0] = 4,
>     [1] = 5,
>     [2] = 6}}
> 
> These changes now also put the first element and the array end bracket on a new
> line, similar to the structure pretty formatter:
> 
> (gdb) p -array on -- {{1,2,3},{4,5,6}}
> $1 = {
>   {
>     1,
>     2,
>     3
>   },
>   {
>     4,
>     5,
>     6
>   }
> }
> (gdb) p -array on -array-indexes on -- {{1,2,3},{4,5,6}}
> $2 = {
>   [0] = {
>     [0] = 1,
>     [1] = 2,
>     [2] = 3
>   },
>   [1] = {
>     [0] = 4,
>     [1] = 5,
>     [2] = 6
>   }
> }

Thanks for providing a test case, this is ok.

Simon

Re: [PATCH v2][PR gdb/17320] Fix array pretty formatter

[Hannes Domani]

 Am Dienstag, 28. April 2020, 17:17:29 MESZ hat Simon Marchi <simark@simark.ca> Folgendes geschrieben:

> On 2020-04-27 8:59 a.m., Hannes Domani via Gdb-patches wrote:
>
> > Currently, printing with array pretty formatting makes the output actually
> > less readable than without:
> >
> > (gdb) p -array on -- {{1,2,3},{4,5,6}}
> > $1 =  {    {1,
> >    2,
> >    3},
> >      {4,
> >    5,
> >    6}}
> > (gdb) p -array on -array-indexes on -- {{1,2,3},{4,5,6}}
> > $2 =  {[0] =    {[0] = 1,
> >    [1] = 2,
> >    [2] = 3},
> >  [1] =    {[0] = 4,
> >    [1] = 5,
> >    [2] = 6}}
> >
> > These changes now also put the first element and the array end bracket on a new
> > line, similar to the structure pretty formatter:
> >
> > (gdb) p -array on -- {{1,2,3},{4,5,6}}
> > $1 = {
> >  {
> >    1,
> >    2,
> >    3
> >  },
> >  {
> >    4,
> >    5,
> >    6
> >  }
> > }
> > (gdb) p -array on -array-indexes on -- {{1,2,3},{4,5,6}}
> > $2 = {
> >  [0] = {
> >    [0] = 1,
> >    [1] = 2,
> >    [2] = 3
> >  },
> >  [1] = {
> >    [0] = 4,
> >    [1] = 5,
> >    [2] = 6
>
> >  }
> > }
>
> Thanks for providing a test case, this is ok.

Pushed, thanks.


Hannes

Re: [PATCH v2][PR gdb/17320] Fix array pretty formatter

[Tom de Vries]

On 29-04-2020 12:59, Hannes Domani via Gdb-patches wrote:
>  Am Dienstag, 28. April 2020, 17:17:29 MESZ hat Simon Marchi <simark@simark.ca> Folgendes geschrieben:
> 
>> On 2020-04-27 8:59 a.m., Hannes Domani via Gdb-patches wrote:
>>
>>> Currently, printing with array pretty formatting makes the output actually
>>> less readable than without:
>>>
>>> (gdb) p -array on -- {{1,2,3},{4,5,6}}
>>> $1 =  {    {1,
>>>     2,
>>>     3},
>>>       {4,
>>>     5,
>>>     6}}
>>> (gdb) p -array on -array-indexes on -- {{1,2,3},{4,5,6}}
>>> $2 =  {[0] =    {[0] = 1,
>>>     [1] = 2,
>>>     [2] = 3},
>>>   [1] =    {[0] = 4,
>>>     [1] = 5,
>>>     [2] = 6}}
>>>
>>> These changes now also put the first element and the array end bracket on a new
>>> line, similar to the structure pretty formatter:
>>>
>>> (gdb) p -array on -- {{1,2,3},{4,5,6}}
>>> $1 = {
>>>   {
>>>     1,
>>>     2,
>>>     3
>>>   },
>>>   {
>>>     4,
>>>     5,
>>>     6
>>>   }
>>> }
>>> (gdb) p -array on -array-indexes on -- {{1,2,3},{4,5,6}}
>>> $2 = {
>>>   [0] = {
>>>     [0] = 1,
>>>     [1] = 2,
>>>     [2] = 3
>>>   },
>>>   [1] = {
>>>     [0] = 4,
>>>     [1] = 5,
>>>     [2] = 6
>>
>>>   }
>>> }
>>
>> Thanks for providing a test case, this is ok.
> 
> Pushed, thanks.
> 

Hi,

this caused:
...
Running
/data/gdb_versions/devel/src/gdb/testsuite/gdb.python/py-format-string.exp
...
FAIL: gdb.python/py-format-string.exp: format_string: lang_c: an_array
with option pretty_arrays: pretty_arrays=true
FAIL: gdb.python/py-format-string.exp: format_string: lang_c:
an_array_with_repetition with option pretty_arrays: pretty_arrays=true
FAIL: gdb.python/py-format-string.exp: format_string: lang_c: set print
array on: an_array: no opts
FAIL: gdb.python/py-format-string.exp: format_string: lang_c: set print
array on: an_array: str
FAIL: gdb.python/py-format-string.exp: format_string: lang_c: set print
array on: an_array with option pretty_arrays: pretty_arrays=true
FAIL: gdb.python/py-format-string.exp: format_string: lang_c: set print
array on: an_array_with_repetition: no opts
FAIL: gdb.python/py-format-string.exp: format_string: lang_c: set print
array on: an_array_with_repetition: str
FAIL: gdb.python/py-format-string.exp: format_string: lang_c: set print
array on: an_array_with_repetition with option pretty_arrays:
pretty_arrays=true
FAIL: gdb.python/py-format-string.exp: format_string: lang_c: an_array
with option array_indexes=True, pretty_arrays=True
FAIL: gdb.python/py-format-string.exp: format_string: lang_cpp: an_array
with option pretty_arrays: pretty_arrays=true
FAIL: gdb.python/py-format-string.exp: format_string: lang_cpp:
an_array_with_repetition with option pretty_arrays: pretty_arrays=true
FAIL: gdb.python/py-format-string.exp: format_string: lang_cpp: set
print array on: an_array: no opts
FAIL: gdb.python/py-format-string.exp: format_string: lang_cpp: set
print array on: an_array: str
FAIL: gdb.python/py-format-string.exp: format_string: lang_cpp: set
print array on: an_array with option pretty_arrays: pretty_arrays=true
FAIL: gdb.python/py-format-string.exp: format_string: lang_cpp: set
print array on: an_array_with_repetition: no opts
FAIL: gdb.python/py-format-string.exp: format_string: lang_cpp: set
print array on: an_array_with_repetition: str
FAIL: gdb.python/py-format-string.exp: format_string: lang_cpp: set
print array on: an_array_with_repetition with option pretty_arrays:
pretty_arrays=true
FAIL: gdb.python/py-format-string.exp: format_string: lang_cpp: an_array
with option array_indexes=True, pretty_arrays=True

                === gdb Summary ===

# of expected passes            960
# of unexpected failures        18
...

Thanks,
- Tom

Re: [PATCH v2][PR gdb/17320] Fix array pretty formatter

[Hannes Domani]

 Am Mittwoch, 29. April 2020, 23:09:08 MESZ hat Tom de Vries <tdevries@suse.de> Folgendes geschrieben:

> On 29-04-2020 12:59, Hannes Domani via Gdb-patches wrote:
> this caused:
> ...
> Running
> /data/gdb_versions/devel/src/gdb/testsuite/gdb.python/py-format-string.exp
> ...
> FAIL: gdb.python/py-format-string.exp: format_string: lang_c: an_array
> with option pretty_arrays: pretty_arrays=true
> FAIL: gdb.python/py-format-string.exp: format_string: lang_c:
> an_array_with_repetition with option pretty_arrays: pretty_arrays=true
> FAIL: gdb.python/py-format-string.exp: format_string: lang_c: set print
> array on: an_array: no opts
> FAIL: gdb.python/py-format-string.exp: format_string: lang_c: set print
> array on: an_array: str
> FAIL: gdb.python/py-format-string.exp: format_string: lang_c: set print
> array on: an_array with option pretty_arrays: pretty_arrays=true
> FAIL: gdb.python/py-format-string.exp: format_string: lang_c: set print
> array on: an_array_with_repetition: no opts
> FAIL: gdb.python/py-format-string.exp: format_string: lang_c: set print
> array on: an_array_with_repetition: str
> FAIL: gdb.python/py-format-string.exp: format_string: lang_c: set print
> array on: an_array_with_repetition with option pretty_arrays:
> pretty_arrays=true
> FAIL: gdb.python/py-format-string.exp: format_string: lang_c: an_array
> with option array_indexes=True, pretty_arrays=True
> FAIL: gdb.python/py-format-string.exp: format_string: lang_cpp: an_array
> with option pretty_arrays: pretty_arrays=true
> FAIL: gdb.python/py-format-string.exp: format_string: lang_cpp:
> an_array_with_repetition with option pretty_arrays: pretty_arrays=true
> FAIL: gdb.python/py-format-string.exp: format_string: lang_cpp: set
> print array on: an_array: no opts
> FAIL: gdb.python/py-format-string.exp: format_string: lang_cpp: set
> print array on: an_array: str
> FAIL: gdb.python/py-format-string.exp: format_string: lang_cpp: set
> print array on: an_array with option pretty_arrays: pretty_arrays=true
> FAIL: gdb.python/py-format-string.exp: format_string: lang_cpp: set
> print array on: an_array_with_repetition: no opts
> FAIL: gdb.python/py-format-string.exp: format_string: lang_cpp: set
> print array on: an_array_with_repetition: str
> FAIL: gdb.python/py-format-string.exp: format_string: lang_cpp: set
> print array on: an_array_with_repetition with option pretty_arrays:
> pretty_arrays=true
> FAIL: gdb.python/py-format-string.exp: format_string: lang_cpp: an_array
> with option array_indexes=True, pretty_arrays=True
>
>                 === gdb Summary ===
>
> # of expected passes            960
> # of unexpected failures        18

I'm sorry about that, I didn't know that python has interface for the various
pretty printers (and I only tested gdb without python so far).
I will send a patch to update the test for the new output format.

On a related note, I'm not sure if the 'set print zero-values' patch will be
accepted, but if yes, should this then also have a python equivalent?


Hannes

Re: [PATCH v2][PR gdb/17320] Fix array pretty formatter

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:

Hannes> On a related note, I'm not sure if the 'set print zero-values' patch will be
Hannes> accepted, but if yes, should this then also have a python equivalent?

I suppose it wouldn't hurt, provided varobj display still always showed
the zeroes.

Tom

Re: [PATCH v2][PR gdb/17320] Fix array pretty formatter

[Hannes Domani]

 Am Donnerstag, 30. April 2020, 20:06:32 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> >>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Hannes> On a related note, I'm not sure if the 'set print zero-values' patch will be
> Hannes> accepted, but if yes, should this then also have a python equivalent?
>
> I suppose it wouldn't hurt, provided varobj display still always showed
> the zeroes.

What's varobj display?


Hannes

Re: [PATCH v2][PR gdb/17320] Fix array pretty formatter

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:

>> I suppose it wouldn't hurt, provided varobj display still always showed
>> the zeroes.

Hannes> What's varobj display?

varobj is an MI feature.  Python pretty-printers are called both by CLI
commands like "print" and "bt", and by varobj.  Normally, though, user
options like "set print mumble" should not affect varobj, as that is
intended for use by front ends.

Tom