[PATCH vkd3d] build: Make the default symbol visibility "hidden".

[Henri Verbeet]

On Sat, 10 Jul 2021 at 23:02, Zebediah Figura (she/her)
<zfigura at codeweavers.com> wrote:
> On 9/14/20 10:47 AM, Henri Verbeet wrote:
> > On Sat, 12 Sep 2020 at 21:26, Zebediah Figura <zfigura at codeweavers.com> wrote:
> >>
> >> We tag far fewer symbols this way.
> >>
> > Using -fvisibility=hidden seems fine, but I'm not sure that's a
> > compelling reason to switch. It's been a while, but IIRC we chose the
> > current scheme to be consistent with Wine, and wined3d in particular.
> >
>
> At the time I was willing to leave this alone, but every time since that
> I try to shuffle functions around or add internal helpers, I find myself
> missing this patch, and I'd like to argue for it a second time.
>
Personally, adding DECLSPEC_HIDDEN has never particularly bothered me,
but in this particular case, if it bothers you that's probably
sufficient reason. I would be curious about opinions from Matteo and
Conor in particular as well though.

> As far as I see it, the choice of default-hidden or default-visible
> makes the most sense as determined by comparing the number of visible
> symbols with the number of hidden symbols, and using whichever is lesser.
>
> In the case of Wine, quite often there are at least as many visible
> symbols as hidden ones, either because most visible functions are stubs
> (or have relatively simple implementations) or because visible functions
> are sufficient as internal helpers such that we don't need to write our
> own. ntdll, kernelbase, rpcrt4, msvcrt, and most stub-only DLLs are good
> examples of this.
>
> On the other hand, that's not true of all Wine modules, and wined3d is a
> good example of one that would benefit from default-visible—it has 290
> exported symbols and 470 internal ones. And in general internal symbols
> are likely to see a lot more churn than exported ones, even in the case
> of wined3d where the external interface isn't stable.
>
> And in the case of vkd3d, we're dealing with a library with well-defined
> and implemented entry points, and a lot of internal complexity. And as a
> result we currently have 188 internal symbols and 52 external ones (or,
> well, in the part of my worktree I measured we do), which is a far more
> significant disparity than anything currently in Wine. And because COM
> is integrated into vkd3d, we don't need wined3d's billion
> vkd3d_device_set_* entry points either.
>
Maybe. The way I see it, the main argument for default hidden
visibility is that it prevents accidentally exporting internal
symbols. For vkd3d we already use linker scripts though, so that point
is somewhat moot.

> I don't necessarily want to argue that we should change the default for
> wined3d—consistency with the rest of wine is nice after all—but I think
> that vkd3d is different enough from wine as a whole that it makes sense
> to treat it differently.
>
> (I could also see an argument for switching to default-visible in Wine.
> In particular it might be reasonable to fold the ubiquitous "WINAPI
> DECLSPEC_HOTPATCH" into a single annotation in Wine, and then adding the
> visibility attribute into that, so that we essentially get visibility
> annotations for free. Food for thought.)

I wouldn't be opposed to changing the default for Wine either. With
the proliferation of PE modules, there may actually be some (minor)
advantage to having proper dllimport/dllexport attributes in our
headers as well.