Re: [PATCH v5 1/5] gdb: make gdbarch store a vector of frame unwinders

[Guinevere Larsen <guinevere@redhat.com>]

On 10/11/24 10:34 PM, Thiago Jung Bauermann wrote:
> Hello,
>
> Tom Tromey <tom@tromey.com> writes:
>
>>>>>>> "Guinevere" == Guinevere Larsen <guinevere@redhat.com> writes:
>> Guinevere> Seeing as a future patch of this series wants to refactor the
>> Guinevere> frame_unwind struct to use inheritance, and we'd like to not restrict
>> Guinevere> the future derived classes on what destructors are allowed. In
>> Guinevere> preparation for that change, this commit adds an std::vector to gdbarch
>> Guinevere> to store the unwinders in.
>>
>> Using a vector is fine, but I still think that this approach
>> makes gdbarch mildly less opaque.  That is, now code anywhere can modify
>> the unwinder list in any way; whereas currently this is constrained to
>> the interface provided by frame-unwind.c.
> Do you think it would be better if the vector was kept in a registry,
> like the current frame_unwind_table is? I.e, changing:
>
> static const registry<gdbarch>::key<struct frame_unwind_table>
>       frame_unwind_data;
>
> to:
>
> static const registry<gdbarch>::key<std::vector<const frame_unwind *>>
>       frame_unwind_data;
>
> ?
>
I am not a big fan of the registry subsystem, so I would prefer to keep 
it as a simple vector. However, if you are concerned about the 
opaqueness change, Tromey, I can try to make it more opaque. I just 
personally find that registry tends to make the code too opaque for 
programmers as well as the program itself :-)

So if you want more separation, I would personally prefer to look into 
only receiving a copy of the unwinder list, and gdbarch is responsible 
for adding and removing things. Just let me know and I can give it a shot

-- 
Cheers,
Guinevere Larsen
She/Her/Hers