[haiku-commits] Re: r37602 - in haiku/trunk: headers/os/locale src/kits/locale src/tools/locale
- From: Ingo Weinhold <ingo_weinhold@xxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Mon, 19 Jul 2010 18:59:49 +0200
On 2010-07-19 at 18:23:47 [+0200], Adrien Destugues
<pulkomandy@xxxxxxxxxxxxxxxxx> wrote:
> > Same visibility change needed here. The same holds true for sCatalog
> > and
> > sCatalogInitOnce, though I would simply pull them out of the class
> > scope and
> > make them compilation unit local (i.e. "static").
>
> Is it really needed ? The linking will fail anyway if Getcatalog() is
> not visible, so there should be no need to hide the others too. Also,
> the initOnce and the catalog itself are private to the class, so they
> aren't visible anyway.
You're confusing C++ visibility, which is only relevant for the compiler,
and symbol visibility (an ELF feature). Functions and variables not
declared static (meaning the compilation unit static, not the C++ static)
are exported by the shared object. Which is exactly the problem you have
encountered with the link order. Hence the visibility of all symbols in the
static library should be reduced to visible within the shared object at
most (i.e. "hidden" visibility). For the two variables it is even simpler,
since moving them out of the class into the source file and declaring them
static already reduces their visibility to the compilation unit. It has the
additional benefit that they won't even appear in the dynamic symbol table
and the compiler, knowing that they are compilation unit local, can
potentially do additional optimizations.
> Why is this class giving you problems ? Moving the members out of it
> would make the namespace more polluted (4 identifiers instead of only
> the class name), and I don't see what problem it could solve ...
I was only referring to the symbol visibility here. The existence of the
class is another matter. It just seems useless to me -- a visible
implementation detail, if you want. I don't see why the methods can't
simply live in BCatalog (or BLocaleRoster). That would definitely feel more
natural to the API user.
CU, ingo
Other related posts:
