Re: [PATCH] New function value_has_address

[Yao Qi <qiyaoltc at gmail dot com>]

On Thu, Oct 27, 2016 at 12:29 PM, Simon Marchi <simon.marchi@polymtl.ca> wrote:
> That help clarifies the code, I like it.
>
>> diff --git a/gdb/value.c b/gdb/value.c
>> index b825aec..4b5cbde 100644
>> --- a/gdb/value.c
>> +++ b/gdb/value.c
>> @@ -1538,12 +1538,20 @@ value_lval_const (const struct value *value)
>>    return value->lval;
>>  }
>>
>> +/* Return true if VALUE has address, otherwise return false.  */
>
>
> This comment is really obvious, it's really just stating the function name.
> Could you expand it a bit, perhaps saying what it means for a value to "have
> address"?  I suppose it means that it has a memory address on the target?

It is intended to avoid expanding the comments because I can't give a
clear meaning, like "has a memory address on the target", unless I fully
understand "value" in gdb.  That is why I only make value_has_address
static.

I find some inconsistencies across the code,

  /* The only lval kinds which do not live in target memory.  */
  if (VALUE_LVAL (val) != not_lval
      && VALUE_LVAL (val) != lval_internalvar
      && VALUE_LVAL (val) != lval_xcallable)
    return 0;

lval_internalvar_component is not checked,

  /* set_value_component_location resets the address, so we may
     need to set it again.  */
  if (VALUE_LVAL (value) != lval_internalvar
      && VALUE_LVAL (value) != lval_internalvar_component
      && VALUE_LVAL (value) != lval_computed)
    set_value_address (value, address + embedded_offset);

lval_xcallable is not checked, but lval_computed is checked.  Before
we clearly document the meaning of value_has_address, we need to
figure out these things above first.

-- 
Yao (齐尧)