Re: [PATCH 4/5] struct symtab split part 1: buildsym api cleanup

[Doug Evans <xdje42@gmail.com>]

On Sun, Nov 16, 2014 at 5:47 PM, Yao Qi <yao@codesourcery.com> wrote:
> Doug Evans <xdje42@gmail.com> writes:
>
>> I realize there are a lot of globals in buildsym.c, and I'm not trying
>> to get rid of them in this patch set, but if one thinks of
>> buildsym as an object and start_symtab as a constructor,
>> then there's no need to pass the compilation directory to
>> start_subfile, and there's no need to pass objfile to end_symtab*.
>> This patch applies these changes.
>
> Hi Doug,
> I am worried about adding new static variables in buildsym.c.  Why do
> you have to the change like this? because part 2 needs such updated api?

Whether something is "needed" can be debatable, but the intent here is
to lay the groundwork for part 2.
The static globals get moved into a struct that contains some of the
buildsym state in part 2.

> I can't estimate the date that buildsym is rewritten as an object in
> c++, so in foreseeable future, the structure of buildsym still remains
> nearly unchanged, I assume.  Adding static variables runes in the opposite
> direction, IMO.  Secondly, shouldn't be buildsym a stateless processor,
> which gets objfile as input and ouputs symbols?  In this way, isn't it
> nicer to have argument objfile for the api?  I don't know much on
> buildsym, so I may miss something.

I understand where you're coming from.
The way I look at it, buildsym is what it is.
It's not where I want it to be, but OTOH cleaning it up is a lower
priority than other things.

This patch actually heads in the right direction because the API of
buildsym becomes more what I want it to be (not entirely so, just more
so).
I don't mind a few internal (local to buildsym.c) steps "backwards" in
the process.
Plus as mentioned above these static globals disappear in part 2.

Note that buildsym has a *lot* of global state.  Some of it is hidden
because of the EXTERN hack that is used in buildsym.h.
Cleaning up buildsym is a significant cleanup project in itself, but
it's also entirely local to buildsym.c and the debug info readers,
whereas cleaning up the symtab data structures provides a lot more
benefit.  Ergo deferring cleaning up buildsym.[ch] until after struct
symtab and symbol lookup is improved.