[PATCH] Fix ptype/o bug with "<no data fields>"

[PATCH] Fix ptype/o bug with "<no data fields>"

[Tom Tromey]

I noticed that when using ptype/o, the "<no data fields>" text that
may be emitted is indented incorrectly.  This patch fixes the bug and
adds a new test case.

I also removed a stray backslash from ptype-offsets.exp that I noticed
while writing the test.  This seemed too trivial to warrant a separate
patch.

gdb/ChangeLog
2021-04-21  Tom Tromey  <tom@tromey.com>

	* c-typeprint.c (c_type_print_base_struct_union): Use
	print_spaces_filtered_with_print_options.

gdb/testsuite/ChangeLog
2021-04-21  Tom Tromey  <tom@tromey.com>

	* gdb.base/ptype-offsets.cc (struct empty_member): New.
	(main): Use empty_member.
	* gdb.base/ptype-offsets.exp: Add new test.
---
 gdb/ChangeLog                            |  5 +++++
 gdb/c-typeprint.c                        |  7 +++----
 gdb/testsuite/ChangeLog                  |  6 ++++++
 gdb/testsuite/gdb.base/ptype-offsets.cc  |  7 +++++++
 gdb/testsuite/gdb.base/ptype-offsets.exp | 17 ++++++++++++++++-
 5 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index f81f1c2c113..72616a59af0 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -1121,13 +1121,12 @@ c_type_print_base_struct_union (struct type *type, struct ui_file *stream,
       if (type->num_fields () == 0 && TYPE_NFN_FIELDS (type) == 0
 	  && TYPE_TYPEDEF_FIELD_COUNT (type) == 0)
 	{
+	  print_spaces_filtered_with_print_options (level + 4, stream, flags);
 	  if (type->is_stub ())
-	    fprintf_filtered (stream, _("%*s%p[<incomplete type>%p]\n"),
-			      level + 4, "",
+	    fprintf_filtered (stream, _("%p[<incomplete type>%p]\n"),
 			      metadata_style.style ().ptr (), nullptr);
 	  else
-	    fprintf_filtered (stream, _("%*s%p[<no data fields>%p]\n"),
-			      level + 4, "",
+	    fprintf_filtered (stream, _("%p[<no data fields>%p]\n"),
 			      metadata_style.style ().ptr (), nullptr);
 	}
 
diff --git a/gdb/testsuite/gdb.base/ptype-offsets.cc b/gdb/testsuite/gdb.base/ptype-offsets.cc
index 4582af48cef..dc21e34752f 100644
--- a/gdb/testsuite/gdb.base/ptype-offsets.cc
+++ b/gdb/testsuite/gdb.base/ptype-offsets.cc
@@ -185,6 +185,12 @@ struct static_member
   int abc;
 };
 
+struct empty_member
+{
+  struct { } empty;
+  int an_int;
+};
+
 int
 main (int argc, char *argv[])
 {
@@ -196,6 +202,7 @@ main (int argc, char *argv[])
   struct asd f;
   uint8_t i;
   static_member stmember;
+  empty_member emember;
 
   return 0;
 }
diff --git a/gdb/testsuite/gdb.base/ptype-offsets.exp b/gdb/testsuite/gdb.base/ptype-offsets.exp
index f466db9a7f2..0a0bbc87062 100644
--- a/gdb/testsuite/gdb.base/ptype-offsets.exp
+++ b/gdb/testsuite/gdb.base/ptype-offsets.exp
@@ -336,7 +336,22 @@ gdb_test "ptype/o static_member" \
     [string_to_regexp [multi_line \
 "/* offset    |  size */  type = struct static_member \{" \
 "                           static static_member Empty;" \
-"\/*    0      |     4 */    int abc;" \
+"/*    0      |     4 */    int abc;" \
 "" \
 "                           /* total size (bytes):    4 */" \
 "                         \}"]]
+
+# Test that the "no data fields" text is indented properly.
+gdb_test "ptype/o empty_member" \
+    [string_to_regexp [multi_line \
+"/* offset    |  size */  type = struct empty_member \{" \
+"/*    0      |     1 */    struct {" \
+"                               <no data fields>" \
+"" \
+"                               /* total size (bytes):    1 */" \
+"                           } empty;" \
+"/* XXX  3-byte hole  */" \
+"/*    4      |     4 */    int an_int;" \
+"" \
+"                           /* total size (bytes):    8 */" \
+"                         \}"]]
-- 
2.26.2

Re: [PATCH] Fix ptype/o bug with "<no data fields>"

[Andrew Burgess]

* Tom Tromey <tom@tromey.com> [2021-04-21 19:49:04 -0600]:

> I noticed that when using ptype/o, the "<no data fields>" text that
> may be emitted is indented incorrectly.  This patch fixes the bug and
> adds a new test case.
> 
> I also removed a stray backslash from ptype-offsets.exp that I noticed
> while writing the test.  This seemed too trivial to warrant a separate
> patch.
> 
> gdb/ChangeLog
> 2021-04-21  Tom Tromey  <tom@tromey.com>
> 
> 	* c-typeprint.c (c_type_print_base_struct_union): Use
> 	print_spaces_filtered_with_print_options.
> 
> gdb/testsuite/ChangeLog
> 2021-04-21  Tom Tromey  <tom@tromey.com>
> 
> 	* gdb.base/ptype-offsets.cc (struct empty_member): New.
> 	(main): Use empty_member.
> 	* gdb.base/ptype-offsets.exp: Add new test.

LGTM.

Thanks,
Andrew


> ---
>  gdb/ChangeLog                            |  5 +++++
>  gdb/c-typeprint.c                        |  7 +++----
>  gdb/testsuite/ChangeLog                  |  6 ++++++
>  gdb/testsuite/gdb.base/ptype-offsets.cc  |  7 +++++++
>  gdb/testsuite/gdb.base/ptype-offsets.exp | 17 ++++++++++++++++-
>  5 files changed, 37 insertions(+), 5 deletions(-)
> 
> diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
> index f81f1c2c113..72616a59af0 100644
> --- a/gdb/c-typeprint.c
> +++ b/gdb/c-typeprint.c
> @@ -1121,13 +1121,12 @@ c_type_print_base_struct_union (struct type *type, struct ui_file *stream,
>        if (type->num_fields () == 0 && TYPE_NFN_FIELDS (type) == 0
>  	  && TYPE_TYPEDEF_FIELD_COUNT (type) == 0)
>  	{
> +	  print_spaces_filtered_with_print_options (level + 4, stream, flags);
>  	  if (type->is_stub ())
> -	    fprintf_filtered (stream, _("%*s%p[<incomplete type>%p]\n"),
> -			      level + 4, "",
> +	    fprintf_filtered (stream, _("%p[<incomplete type>%p]\n"),
>  			      metadata_style.style ().ptr (), nullptr);
>  	  else
> -	    fprintf_filtered (stream, _("%*s%p[<no data fields>%p]\n"),
> -			      level + 4, "",
> +	    fprintf_filtered (stream, _("%p[<no data fields>%p]\n"),
>  			      metadata_style.style ().ptr (), nullptr);
>  	}
>  
> diff --git a/gdb/testsuite/gdb.base/ptype-offsets.cc b/gdb/testsuite/gdb.base/ptype-offsets.cc
> index 4582af48cef..dc21e34752f 100644
> --- a/gdb/testsuite/gdb.base/ptype-offsets.cc
> +++ b/gdb/testsuite/gdb.base/ptype-offsets.cc
> @@ -185,6 +185,12 @@ struct static_member
>    int abc;
>  };
>  
> +struct empty_member
> +{
> +  struct { } empty;
> +  int an_int;
> +};
> +
>  int
>  main (int argc, char *argv[])
>  {
> @@ -196,6 +202,7 @@ main (int argc, char *argv[])
>    struct asd f;
>    uint8_t i;
>    static_member stmember;
> +  empty_member emember;
>  
>    return 0;
>  }
> diff --git a/gdb/testsuite/gdb.base/ptype-offsets.exp b/gdb/testsuite/gdb.base/ptype-offsets.exp
> index f466db9a7f2..0a0bbc87062 100644
> --- a/gdb/testsuite/gdb.base/ptype-offsets.exp
> +++ b/gdb/testsuite/gdb.base/ptype-offsets.exp
> @@ -336,7 +336,22 @@ gdb_test "ptype/o static_member" \
>      [string_to_regexp [multi_line \
>  "/* offset    |  size */  type = struct static_member \{" \
>  "                           static static_member Empty;" \
> -"\/*    0      |     4 */    int abc;" \
> +"/*    0      |     4 */    int abc;" \
>  "" \
>  "                           /* total size (bytes):    4 */" \
>  "                         \}"]]
> +
> +# Test that the "no data fields" text is indented properly.
> +gdb_test "ptype/o empty_member" \
> +    [string_to_regexp [multi_line \
> +"/* offset    |  size */  type = struct empty_member \{" \
> +"/*    0      |     1 */    struct {" \
> +"                               <no data fields>" \
> +"" \
> +"                               /* total size (bytes):    1 */" \
> +"                           } empty;" \
> +"/* XXX  3-byte hole  */" \
> +"/*    4      |     4 */    int an_int;" \
> +"" \
> +"                           /* total size (bytes):    8 */" \
> +"                         \}"]]
> -- 
> 2.26.2
> 

Re: [PATCH] Fix ptype/o bug with "<no data fields>"

[Tom Tromey]

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

>> gdb/ChangeLog
>> 2021-04-21  Tom Tromey  <tom@tromey.com>
>> 
>> * c-typeprint.c (c_type_print_base_struct_union): Use
>> print_spaces_filtered_with_print_options.
>> 
>> gdb/testsuite/ChangeLog
>> 2021-04-21  Tom Tromey  <tom@tromey.com>
>> 
>> * gdb.base/ptype-offsets.cc (struct empty_member): New.
>> (main): Use empty_member.
>> * gdb.base/ptype-offsets.exp: Add new test.

Andrew> LGTM.

Thanks.  I think I'll hold this one until the other ptype/o patches
land.

Tom

Re: [PATCH] Fix ptype/o bug with "<no data fields>"

[Lancelot SIX]

On Thu, Apr 22, 2021 at 07:17:16AM -0600, Tom Tromey wrote:
> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> >> gdb/ChangeLog
> >> 2021-04-21  Tom Tromey  <tom@tromey.com>
> >> 
> >> * c-typeprint.c (c_type_print_base_struct_union): Use
> >> print_spaces_filtered_with_print_options.
> >> 
> >> gdb/testsuite/ChangeLog
> >> 2021-04-21  Tom Tromey  <tom@tromey.com>
> >> 
> >> * gdb.base/ptype-offsets.cc (struct empty_member): New.
> >> (main): Use empty_member.
> >> * gdb.base/ptype-offsets.exp: Add new test.
> 
> Andrew> LGTM.
> 
> Thanks.  I think I'll hold this one until the other ptype/o patches
> land.
> 
> Tom

Hi,

I think you are referring to my ptype patch
(https://sourceware.org/pipermail/gdb-patches/2021-April/177841.html)
which still needs to be approved for the documentation part.

I do not mind reworking it to take into account this patch if it lands
in master first.  It should only have conflicts on the tests, which will
be easy to fix.

Lancelot.

Re: [PATCH] Fix ptype/o bug with "<no data fields>"

[Tom Tromey]

Tom> Thanks.  I think I'll hold this one until the other ptype/o patches
Tom> land.

>> I think you are referring to my ptype patch
>> (https://sourceware.org/pipermail/gdb-patches/2021-April/177841.html)
>> which still needs to be approved for the documentation part.

Yep.

I didn't check if Eli approved the docs in v1.  If so then you're good.

>> I do not mind reworking it to take into account this patch if it lands
>> in master first.  It should only have conflicts on the tests, which will
>> be easy to fix.

Ok, thanks, I will go ahead.
Probably the test just needs a few spaces added.

Tom

Re: [PATCH] Fix ptype/o bug with "<no data fields>"

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Date: Thu, 22 Apr 2021 19:39:15 -0600
> Cc: Tom Tromey <tom@tromey.com>, gdb-patches@sourceware.org
> 
> Tom> Thanks.  I think I'll hold this one until the other ptype/o patches
> Tom> land.
> 
> >> I think you are referring to my ptype patch
> >> (https://sourceware.org/pipermail/gdb-patches/2021-April/177841.html)
> >> which still needs to be approved for the documentation part.
> 
> Yep.
> 
> I didn't check if Eli approved the docs in v1.  If so then you're good.

If I missed it, I apologize; please point me to the un-reviewed patch,
and I will review.

Re: [PATCH] Fix ptype/o bug with "<no data fields>"

[Lancelot SIX]

On Fri, Apr 23, 2021 at 09:12:49AM +0300, Eli Zaretskii wrote:
> > From: Tom Tromey <tom@tromey.com>
> > Date: Thu, 22 Apr 2021 19:39:15 -0600
> > Cc: Tom Tromey <tom@tromey.com>, gdb-patches@sourceware.org
> > 
> > Tom> Thanks.  I think I'll hold this one until the other ptype/o patches
> > Tom> land.
> > 
> > >> I think you are referring to my ptype patch
> > >> (https://sourceware.org/pipermail/gdb-patches/2021-April/177841.html)
> > >> which still needs to be approved for the documentation part.
> > 
> > Yep.
> > 
> > I didn't check if Eli approved the docs in v1.  If so then you're good.
> 
> If I missed it, I apologize; please point me to the un-reviewed patch,
> and I will review.

Hi,

The patch that needs to be reviewed for the documentation part it this
one: https://sourceware.org/pipermail/gdb-patches/2021-April/177843.html

Thanks,
Lancelot.

Re: [PATCH] Fix ptype/o bug with "<no data fields>"

[Eli Zaretskii]

> Date: Fri, 23 Apr 2021 11:41:53 +0100
> From: Lancelot SIX <lsix@lancelotsix.com>
> Cc: Tom Tromey <tom@tromey.com>, gdb-patches@sourceware.org
> 
> The patch that needs to be reviewed for the documentation part it this
> one: https://sourceware.org/pipermail/gdb-patches/2021-April/177843.html

Thanks, see below:

> +When @value{GDBN} prints sizes and offsets of struct members, decimal or
> +hexadecimal notations can be used.  Selecting one or the other can be
> +done either by passing the appropriate flag to @code{ptype}, or by using
> +@command{set print type hex}.

This uses the passive tense too much, and thus is awkward to read.
Suggest to rephrase:

  When @value{GDBN} prints sizes and offsets of struct members, it can
  use either the decimal or hexadecimal notation.  You can select one or
  the other either by passing the appropriate flag to @code{ptype}, or
  by using the @command{set print type hex} command.

> +@item show print type hex
> +This command show the current setting used to configure whether decimal or
> +hexadecimal notation should be used when printing sized and offsets of
> +struct members.

"shows", singular.  But I'd also suggest slight rewording to make the
text simpler:

  This command shows whether the sizes and offsets of struct members
  are printed in decimal or hexadecimal notation.

> +			   _("Show printing of struct members sizes and offsets \
> +			     using hex notation."), nullptr,

I think

 Show whether sizes and offsets of struct members are printed
 using hex notation.

is better.

Thanks.