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

[Guinevere Larsen <guinevere@redhat.com>]

On 10/17/24 7:53 PM, Tom Tromey wrote:
> Guinevere> I am not a big fan of the registry subsystem, so I would prefer to
> Guinevere> keep it as a simple vector. However, if you are concerned about the
> Guinevere> opaqueness change, Tromey, I can try to make it more opaque. I just
> Guinevere> personally find that registry tends to make the code too opaque for
> Guinevere> programmers as well as the program itself :-)
>
> I think gdbarch is exposed to a lot of tdep code, and this code in
> particular to do questionable things.  So, preserving some opacity makes
> sense to me.
Understandable. It didn't sound like you were that worried about it 
previously, apologies for seemingly ignoring this feedback
>
> Guinevere> So if you want more separation, I would personally prefer to look into
> Guinevere> only receiving a copy of the unwinder list, and gdbarch is responsible
> Guinevere> for adding and removing things. Just let me know and I can give it a
> Guinevere> shot
>
> I think it would be more worthwhile to understand your objections to the
> registry.  It seems to me that switching back to using it is just a
> couple of lines of code, so it makes me think there must be some pretty
> strong reason.

My main objection is in the form of "this feels like the type of magic 
you don't want on the project". It feels like some very complicated 
arcane code that causes things to seem much more complicated for new 
contributors. I understand it is important in some cases, but in here it 
seemed to me like making a registry that stores a standard library class 
was just obfuscating the code for little gain.

I would propose that instead, the getter for the vector returns a const 
reference (asserts if it is called before initialization). gdbarch would 
then provide a "initialize_unwinders" method and a "add_unwinder" 
method, but those should only be called by frame-unwind.c (and that 
would be explicitly called out in the comments of those functions). This 
follows a bit more the traditional object orientation route for handling 
opaqueness, I believe, which would make it slightly easier to have new 
contributors up to speed.

I know this isn't the strongets, so I understand if you want me to keep 
the registry stuff.

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

> Tom
>