Re: [PATCH 1/3] New function value_has_address

[Pedro Alves <palves at redhat dot com>]

On 11/22/2016 04:50 PM, Joel Brobecker wrote:
> Hey Yao,
> 
>> +/* Return true if VALUE has address, otherwise return false.  */
>> +
>> +static int
>> +value_has_address (const struct value *value)
>> +{
>> +  return (value->lval != lval_internalvar
>> +	  && value->lval != lval_internalvar_component
>> +	  && value->lval != lval_xcallable);
> 
> I'm wondering about the function's name. Does a value that
> lives in a register, for instance, really have an address?
> For me, if there was a function value_has_address, it would
> return nonzero only for lval_memory. I'm not too sure if
> lval_computed would qualify or not.
> 
> Perhaps, what you were looking for, is something like
> value_lives_in_inferior?

The intention of the function is to return true if the value
uses the value.location.address union field as location:

  /* Location of value (if lval).  */
  union
  {
    /* If lval == lval_memory, this is the address in the inferior.
       If lval == lval_register, this is the byte offset into the
       registers structure.  */
    CORE_ADDR address;
...
  } location;

I think that it's good that the names match.  If one is renamed,
so should the other, IMO.  Maybe call the function
value_has_address_location?  I think it'd be good if the
function's intro comment made this link more explicit.
Actually, I see now that patch #3 tweaks the comment.

Thanks,
Pedro Alves