Re: [PATCH 2/2] Involve gdbarch in taking DWARF register pieces

[Andreas Arnez <arnez at linux dot vnet dot ibm dot com>]

On Mon, Apr 18 2016, Pedro Alves wrote:

> On 04/18/2016 12:53 PM, Andreas Arnez wrote:
>> On Fri, Apr 15 2016, Pedro Alves wrote:
>> 
>>> Could we have some comment somewhere on what a "part" is?
>>>
>>> This comment isn't very enlightening, IMHO:
>>>
>>> +# Locate a part of size LEN within register *REGNUM, possibly overwriting
>>> +# *REGNUM.  Return the offset of the part within the (possibly adjusted)
>>> +# register.
>>> +m:int:register_part:int len, int *regnum:len, regnum::default_register_part::0
>>>
>>> Reading this in isolation I have no idea what it's for.  I skimmed the patch
>>> and didn't find any.  Sorry if it's there and I missed it.
>> 
>> Hm, I used the word "part" to imply a slightly more general meaning than
>> the DWARF term "piece".  But that may actually be counterproductive...
>> So how about renaming the method to "register_piece" and then adjusting
>> the comment like this:
>> 
>>   Determine the placement of a DWARF piece (DW_OP_piece) of size LEN
>>   within register *REGNUM, possibly overwriting *REGNUM.  Return the
>>   offset of the piece within the (possibly adjusted) register.  This
>>   method also applies when interpreting a register as a LEN-sized type.
>> 
>> Does this help?
>
> Yes, it does.  Thanks.

OK.  Now, reading this again, it seems to me that I should better add
the following disclaimer at the end:

  ... This method also applies when interpreting a register as a
  LEN-sized type, except when convert_register_p indicates that a
  special conversion is required instead.

Still OK?

> I'd suggest even calling it "dwarf_register_piece_placement" for
> caller clarity?

Sure, but I find a name like 'gdbarch_dwarf_register_piece_placement' a
bit too unwieldy when trying to stick to an 80 char line size limit.
Maybe 'gdbarch_register_piece_placement'?

--
Andreas