[RFA,07/13] Remove unused declaration from py-prettyprint.c

Message ID 20180712205208.32646-8-tom@tromey.com
State New
Headers show
Series
  • Add -Wunused-variable
Related show

Commit Message

Tom Tromey July 12, 2018, 8:52 p.m.
This removes an unused declaration from py-prettyprint.c, but leaves
the call to value_contents_for_printing, as it may throw.

2018-07-12  Tom Tromey  <tom@tromey.com>

	* python/py-prettyprint.c (gdbpy_apply_val_pretty_printer): Call
	value_contents_for_printing for effect.
---
 gdb/ChangeLog               | 5 +++++
 gdb/python/py-prettyprint.c | 4 +++-
 2 files changed, 8 insertions(+), 1 deletion(-)

-- 
2.13.6

Comments

Simon Marchi July 14, 2018, 1:24 a.m. | #1
On 2018-07-12 04:52 PM, Tom Tromey wrote:
> This removes an unused declaration from py-prettyprint.c, but leaves

> the call to value_contents_for_printing, as it may throw.>

> 2018-07-12  Tom Tromey  <tom@tromey.com>

> 

> 	* python/py-prettyprint.c (gdbpy_apply_val_pretty_printer): Call

> 	value_contents_for_printing for effect.

> ---

>  gdb/ChangeLog               | 5 +++++

>  gdb/python/py-prettyprint.c | 4 +++-

>  2 files changed, 8 insertions(+), 1 deletion(-)

> 

> diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c

> index 21b1ce94879..bfcbc95178e 100644

> --- a/gdb/python/py-prettyprint.c

> +++ b/gdb/python/py-prettyprint.c

> @@ -662,7 +662,9 @@ gdbpy_apply_val_pretty_printer (const struct extension_language_defn *extlang,

>    struct gdbarch *gdbarch = get_type_arch (type);

>    struct value *value;

>    enum string_repr_result print_result;

> -  const gdb_byte *valaddr = value_contents_for_printing (val);

> +

> +  /* Call for side effects.  */

> +  value_contents_for_printing (val);

>  

>    /* No pretty-printer support for unavailable values.  */

>    if (!value_bytes_available (val, embedded_offset, TYPE_LENGTH (type)))

> 



Also LGTM.  However, since both extension languages call value_contents_for_printing
(from what I understand, to ensure the value has been fetched/is not lazy) and bail
if some bytes are unavailable, perhaps apply_ext_lang_val_pretty_printer should
do it instead.

Also, doing

  if (value->lazy)
    value_fetch_lazy (value);

would communicate better the intent than calling value_contents_for_printing for the
same side-effect.

Simon
Tom Tromey July 14, 2018, 12:39 p.m. | #2
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:


Simon> Also LGTM.  However, since both extension languages call value_contents_for_printing
Simon> (from what I understand, to ensure the value has been fetched/is not lazy) and bail
Simon> if some bytes are unavailable, perhaps apply_ext_lang_val_pretty_printer should
Simon> do it instead.

It seems possible that someday we'd want to expose unavailable bytes via
the Python Value API.

Simon> Also, doing

Simon>   if (value->lazy)
Simon>     value_fetch_lazy (value);

Simon> would communicate better the intent than calling value_contents_for_printing for the
Simon> same side-effect.

True, but then this is exposed to other possible changes in the value API.

Tom
Pedro Alves July 16, 2018, 2:11 p.m. | #3
On 07/14/2018 01:39 PM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

> 

> Simon> Also LGTM.  However, since both extension languages call value_contents_for_printing

> Simon> (from what I understand, to ensure the value has been fetched/is not lazy) and bail

> Simon> if some bytes are unavailable, perhaps apply_ext_lang_val_pretty_printer should

> Simon> do it instead.

> 

> It seems possible that someday we'd want to expose unavailable bytes via

> the Python Value API.


Right, but that's not an issue if we morph the call to a direct
value_fetch_lazy call instead, AFAICS.

> 

> Simon> Also, doing

> 

> Simon>   if (value->lazy)

> Simon>     value_fetch_lazy (value);

> 

> Simon> would communicate better the intent than calling value_contents_for_printing for the

> Simon> same side-effect.


Right, what I was saying too in the guile patch.

> 

> True, but then this is exposed to other possible changes in the value API.


I don't see how using value_contents_for_printing instead helps with that?
 
Thanks,
Pedro Alves

Patch

diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
index 21b1ce94879..bfcbc95178e 100644
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -662,7 +662,9 @@  gdbpy_apply_val_pretty_printer (const struct extension_language_defn *extlang,
   struct gdbarch *gdbarch = get_type_arch (type);
   struct value *value;
   enum string_repr_result print_result;
-  const gdb_byte *valaddr = value_contents_for_printing (val);
+
+  /* Call for side effects.  */
+  value_contents_for_printing (val);
 
   /* No pretty-printer support for unavailable values.  */
   if (!value_bytes_available (val, embedded_offset, TYPE_LENGTH (type)))