[PATCH] D52662: [libc++] Make sure we can build libc++ with -fvisibility=hidden

[Louis Dionne via Phabricator]

ldionne added a comment.

In https://reviews.llvm.org/D52662#1249706, @smeenai wrote:

> Fair point. I think it's worth distinguishing between symbols that we actually need to export (for check-cxx or applications or similar) and symbols that just happen to be exported because of the default visibility but don't actually need to be exported. If you're already doing that in this patch, or if there aren't actually any symbols in the first category anymore, then just ignore me :)


I'm not sure how we can ever check that a symbol is exported but not actually needed. Wouldn't this require analyzing the set of all programs linking to `libc++.dylib` in the wild?

> 
> 
>> 
>> 
>>> Internally, we've been building libc++ with hidden visibility for a long time now, and it's been working well. I'm linking some of the issues from when I was working on that; I've lost all context on these by now, but I figured it might save you some trouble if you ran into similar issues:
>> 
>> Why have you been building libc++ with hidden visibility? What is the advantage of doing that?
> 
> For libc++, there's not too much advantage, since (a) it already controls its ABI pretty thoroughly, and (b) it already builds with `-fvisibility-inlines-hidden`, which takes care of a lot of cases. We did find a few symbols that were over-exported, but not enough to make any significant reductions in dynamic symbol table size. Mostly it was a cleanliness thing, where we wanted all our libraries having controlled ABIs and building with hidden visibility. It also helped us with cross-platform compatibility, since Windows always has the equivalent of hidden visibility and you have to explicitly declare your exports (which is a much nicer default IMO).

Got it. I agree that Windows is much saner in that respect. In the future, we could actually apply hidden visibility to all of namespace `std` and then only cherry-pick entities that we want to export (and apply a default-visibility attribute to those). However, that requires the ability to explicitly instantiate vtables and RTTI, which we don't have right now (but we could add as an extension to Clang). This would allow us to drop almost all of the visibility annotations we currently have in libc++.


Repository:
  rCXX libc++

https://reviews.llvm.org/D52662