[PATCH v2] Refactor struct trad_frame_saved_regs

[Luis Machado]

On 12/31/20 7:04 PM, Simon Marchi wrote:
> On 2020-12-31 9:44 a.m., Luis Machado wrote:
>>> I wouldn't mind removing the reset function. It is not that useful once you have a proper constructor. Ideally we'd also remove the trad_frame_set_value_* functions and just use the setters/getters of the new struct directly in code using it.
>>>
>>> Right now those trad_frame_set_value_* functions are just an indirection
>>
>> Sorry, I meant trad_frame_set_* functions here. So all the variants, like trad_frame_set_value, trad_frame_set_realreg, trad_frame_set_addr, trad_frame_set_unknown and trad_frame_set_value_bytes.
>>
>> Also trad_frame_value_p, trad_frame_addr_p, trad_frame_realreg_p and trad_frame_value_bytes_p.
>>
>>> that pass the register number so we can index the array of trad_frame_reset_saved_regs.
>>>
>>> Shall we pursue a more thorough cleanup?
> 
> It's up to you, there are always things to clean up, the only limiting
> factor is the available time :).
> 
> I think the patch you have right now is a good checkpoint, so I would push
> it (with my last comment about the constructor addressed) and do any further
> work as separate patches.

Thanks. Pushed now. I'll get the other cleanups going.

> 
> I agree that trad_frame_set_* and trad_frame_*_p functions can be removed,
> that would be the next logical step, and somewhat trivial.
> 
> Removing trad_frame_reset_saved_regs in favor of a non-default constructor
> on trad_frame_saved_reg requires some changes in avr_skip_prologue, for
> example, that need a bit more care.
> 
> Simon
>