Allow pretty-print of static members

Message ID 20230427143145.1800596-1-tom@tromey.com
State New
Headers
Series Allow pretty-print of static members |

Commit Message

Tom Tromey April 27, 2023, 2:31 p.m. UTC
  Python pretty-printers haven't applied to static members for quite
some time.  I tracked this down to the call to cp_print_value_fields
in cp_print_static_field -- it doesn't let pretty-printers have a
chance to print the value.  This patch fixes the problem.

The way that static members are handled is very weird to me.  I tend
to think this should be done more globally, like in value_print.
However, I haven't made any big change.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30057
---
 gdb/cp-valprint.c                           | 47 ++++++++++++++-------
 gdb/testsuite/gdb.python/py-prettyprint.c   | 11 +++++
 gdb/testsuite/gdb.python/py-prettyprint.exp |  5 +++
 3 files changed, 47 insertions(+), 16 deletions(-)
  

Comments

Keith Seitz May 5, 2023, 6:38 p.m. UTC | #1
On 4/27/23 07:31, Tom Tromey wrote:
> Python pretty-printers haven't applied to static members for quite
> some time.  I tracked this down to the call to cp_print_value_fields
> in cp_print_static_field -- it doesn't let pretty-printers have a
> chance to print the value.  This patch fixes the problem.

I never noticed, either, and it LGTM.

Reviewed-by:  Keith Seitz <keiths@redhat.com>
Tested-by:  Keith Seitz <keiths@redhat.com>

> The way that static members are handled is very weird to me.  I tend
> to think this should be done more globally, like in value_print.
> However, I haven't made any big change.
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Yet? ;-)

Keith

> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30057
> ---
>   gdb/cp-valprint.c                           | 47 ++++++++++++++-------
>   gdb/testsuite/gdb.python/py-prettyprint.c   | 11 +++++
>   gdb/testsuite/gdb.python/py-prettyprint.exp |  5 +++
>   3 files changed, 47 insertions(+), 16 deletions(-)
> 
> diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
> index 167cf0314af..71bff16e3e6 100644
> --- a/gdb/cp-valprint.c
> +++ b/gdb/cp-valprint.c
> @@ -381,6 +381,32 @@ cp_print_value_fields (struct value *val, struct ui_file *stream,
>     gdb_printf (stream, "}");
>   }
>   
> +/* A wrapper for cp_print_value_fields that tries to apply a
> +   pretty-printer first.  */
> +
> +static void
> +cp_print_value_fields_pp (struct value *val,
> +			  struct ui_file *stream,
> +			  int recurse,
> +			  const struct value_print_options *options,
> +			  struct type **dont_print_vb,
> +			  int dont_print_statmem)
> +{
> +  int result = 0;
> +
> +  /* Attempt to run an extension language pretty-printer if
> +     possible.  */
> +  if (!options->raw)
> +    result
> +      = apply_ext_lang_val_pretty_printer (val, stream,
> +					   recurse, options,
> +					   current_language);
> +
> +  if (!result)
> +    cp_print_value_fields (val, stream, recurse, options, dont_print_vb,
> +			   dont_print_statmem);
> +}
> +
>   /* Special val_print routine to avoid printing multiple copies of
>      virtual baseclasses.  */
>   
> @@ -493,27 +519,16 @@ cp_print_value (struct value *val, struct ui_file *stream,
>   	val_print_invalid_address (stream);
>         else
>   	{
> -	  int result = 0;
> -
>   	  if (!val_print_check_max_depth (stream, recurse, options,
>   					  current_language))
>   	    {
>   	      struct value *baseclass_val = val->primitive_field (0,
>   								  i, type);
>   
> -	      /* Attempt to run an extension language pretty-printer on the
> -		 baseclass if possible.  */
> -	      if (!options->raw)
> -		result
> -		  = apply_ext_lang_val_pretty_printer (baseclass_val, stream,
> -						       recurse, options,
> -						       current_language);
> -
> -	      if (!result)
> -		cp_print_value_fields (baseclass_val, stream, recurse, options,
> -				       ((struct type **)
> -					obstack_base (&dont_print_vb_obstack)),
> -				       0);
> +	      cp_print_value_fields_pp
> +		(baseclass_val, stream, recurse, options,
> +		 (struct type **) obstack_base (&dont_print_vb_obstack),
> +		 0);
>   	    }
>   	}
>         gdb_puts (", ", stream);
> @@ -581,7 +596,7 @@ cp_print_static_field (struct type *type,
>   
>         obstack_grow (&dont_print_statmem_obstack, (char *) &addr,
>   		    sizeof (CORE_ADDR));
> -      cp_print_value_fields (val, stream, recurse, options, NULL, 1);
> +      cp_print_value_fields_pp (val, stream, recurse, options, nullptr, 1);
>         return;
>       }
>   
> diff --git a/gdb/testsuite/gdb.python/py-prettyprint.c b/gdb/testsuite/gdb.python/py-prettyprint.c
> index 0832f4b545b..7697a5312a8 100644
> --- a/gdb/testsuite/gdb.python/py-prettyprint.c
> +++ b/gdb/testsuite/gdb.python/py-prettyprint.c
> @@ -110,6 +110,14 @@ class Fake
>     {
>     }
>   };
> +
> +struct has_static_member
> +{
> +  static s global;
> +};
> +
> +s has_static_member::global;
> +
>   #endif
>   
>   struct to_string_returns_value_inner
> @@ -356,6 +364,9 @@ main ()
>     Derived derived;
>     
>     Fake fake (42);
> +
> +  init_s (&has_static_member::global, 23);
> +  has_static_member has_member;
>   #endif
>   
>     add_item (&c, 23);		/* MI breakpoint here */
> diff --git a/gdb/testsuite/gdb.python/py-prettyprint.exp b/gdb/testsuite/gdb.python/py-prettyprint.exp
> index b7661ff14ed..05507cba9c9 100644
> --- a/gdb/testsuite/gdb.python/py-prettyprint.exp
> +++ b/gdb/testsuite/gdb.python/py-prettyprint.exp
> @@ -82,6 +82,11 @@ proc run_lang_tests {exefile lang} {
>   	gdb_test "print ns" "embedded\\\\000n\.\.\.." \
>   	    "print ns with element limit of 10"
>   	gdb_test_no_output "set print elements 200"
> +
> +	gdb_test "print has_member" \
> +	    "=  a=<23> b=<$hex <has_static_member::global>>.*"
> +	gdb_test "print has_static_member::global" \
> +	    "=  a=<23> b=<$hex <has_static_member::global>>"
>       }
>   
>       if { ![is_address_zero_readable] } {
  
Tom Tromey May 6, 2023, 4:34 p.m. UTC | #2
>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

>> Python pretty-printers haven't applied to static members for quite
>> some time.  I tracked this down to the call to cp_print_value_fields
>> in cp_print_static_field -- it doesn't let pretty-printers have a
>> chance to print the value.  This patch fixes the problem.

Keith> I never noticed, either, and it LGTM.

Keith> Reviewed-by:  Keith Seitz <keiths@redhat.com>
Keith> Tested-by:  Keith Seitz <keiths@redhat.com>

Thanks, I'm going to check it in.

>> The way that static members are handled is very weird to me.  I tend
>> to think this should be done more globally, like in value_print.
>> However, I haven't made any big change.
Keith>            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Keith> Yet? ;-)

Well, I gave it a bit of a try but ran into some difficulties and
decided it would be good to just get a fix in.

Tom
  

Patch

diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
index 167cf0314af..71bff16e3e6 100644
--- a/gdb/cp-valprint.c
+++ b/gdb/cp-valprint.c
@@ -381,6 +381,32 @@  cp_print_value_fields (struct value *val, struct ui_file *stream,
   gdb_printf (stream, "}");
 }
 
+/* A wrapper for cp_print_value_fields that tries to apply a
+   pretty-printer first.  */
+
+static void
+cp_print_value_fields_pp (struct value *val,
+			  struct ui_file *stream,
+			  int recurse,
+			  const struct value_print_options *options,
+			  struct type **dont_print_vb,
+			  int dont_print_statmem)
+{
+  int result = 0;
+
+  /* Attempt to run an extension language pretty-printer if
+     possible.  */
+  if (!options->raw)
+    result
+      = apply_ext_lang_val_pretty_printer (val, stream,
+					   recurse, options,
+					   current_language);
+
+  if (!result)
+    cp_print_value_fields (val, stream, recurse, options, dont_print_vb,
+			   dont_print_statmem);
+}
+
 /* Special val_print routine to avoid printing multiple copies of
    virtual baseclasses.  */
 
@@ -493,27 +519,16 @@  cp_print_value (struct value *val, struct ui_file *stream,
 	val_print_invalid_address (stream);
       else
 	{
-	  int result = 0;
-
 	  if (!val_print_check_max_depth (stream, recurse, options,
 					  current_language))
 	    {
 	      struct value *baseclass_val = val->primitive_field (0,
 								  i, type);
 
-	      /* Attempt to run an extension language pretty-printer on the
-		 baseclass if possible.  */
-	      if (!options->raw)
-		result
-		  = apply_ext_lang_val_pretty_printer (baseclass_val, stream,
-						       recurse, options,
-						       current_language);
-
-	      if (!result)
-		cp_print_value_fields (baseclass_val, stream, recurse, options,
-				       ((struct type **)
-					obstack_base (&dont_print_vb_obstack)),
-				       0);
+	      cp_print_value_fields_pp
+		(baseclass_val, stream, recurse, options,
+		 (struct type **) obstack_base (&dont_print_vb_obstack),
+		 0);
 	    }
 	}
       gdb_puts (", ", stream);
@@ -581,7 +596,7 @@  cp_print_static_field (struct type *type,
 
       obstack_grow (&dont_print_statmem_obstack, (char *) &addr,
 		    sizeof (CORE_ADDR));
-      cp_print_value_fields (val, stream, recurse, options, NULL, 1);
+      cp_print_value_fields_pp (val, stream, recurse, options, nullptr, 1);
       return;
     }
 
diff --git a/gdb/testsuite/gdb.python/py-prettyprint.c b/gdb/testsuite/gdb.python/py-prettyprint.c
index 0832f4b545b..7697a5312a8 100644
--- a/gdb/testsuite/gdb.python/py-prettyprint.c
+++ b/gdb/testsuite/gdb.python/py-prettyprint.c
@@ -110,6 +110,14 @@  class Fake
   {
   }
 };
+
+struct has_static_member
+{
+  static s global;
+};
+
+s has_static_member::global;
+
 #endif
 
 struct to_string_returns_value_inner
@@ -356,6 +364,9 @@  main ()
   Derived derived;
   
   Fake fake (42);
+
+  init_s (&has_static_member::global, 23);
+  has_static_member has_member;
 #endif
 
   add_item (&c, 23);		/* MI breakpoint here */
diff --git a/gdb/testsuite/gdb.python/py-prettyprint.exp b/gdb/testsuite/gdb.python/py-prettyprint.exp
index b7661ff14ed..05507cba9c9 100644
--- a/gdb/testsuite/gdb.python/py-prettyprint.exp
+++ b/gdb/testsuite/gdb.python/py-prettyprint.exp
@@ -82,6 +82,11 @@  proc run_lang_tests {exefile lang} {
 	gdb_test "print ns" "embedded\\\\000n\.\.\.." \
 	    "print ns with element limit of 10"
 	gdb_test_no_output "set print elements 200"
+
+	gdb_test "print has_member" \
+	    "=  a=<23> b=<$hex <has_static_member::global>>.*"
+	gdb_test "print has_static_member::global" \
+	    "=  a=<23> b=<$hex <has_static_member::global>>"
     }
 
     if { ![is_address_zero_readable] } {