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

  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(-)
  

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
  

>>>>> "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
  

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