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

[Shoaib Meenai via Phabricator]

smeenai added a comment.

In https://reviews.llvm.org/D52662#1249623, @ldionne wrote:

> In https://reviews.llvm.org/D52662#1249528, @smeenai wrote:
>
> > It's awesome that you're looking into hidden visibility!
>
>
> The gray hair I'm growing begs to disagree :-).


Hah, I'm sure I've lost a good amount of hair to visibility issues as well :)

>> I'm guessing you figured out the missing symbols by running `check-cxx` or trying to link some applications or similar?
> 
> I added `-fvisibility=hidden` to the build flags, built libc++.dylib and ran `check-cxx-abilist`. There were several missing symbols, and I added the annotations in this commit until there were no missing symbols anymore.

That makes sense. Back when I was looking into this (which is over a year ago now, and sadly I got pulled away into other things before I had a chance to upstream all of it), I found that only some of the missing exports when building with hidden visibility were actually required for `check-cxx` to pass. The rest could be added if you want to preserve ABI compatibility for v1, but hidden visibility very much seems like an ABI v2 thing, so at that point you could also fix the over-exporting.

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:

https://bugs.llvm.org/show_bug.cgi?id=32114
https://bugs.llvm.org/show_bug.cgi?id=34614
https://github.com/smeenai/bad-visibility-finder


Repository:
  rCXX libc++

https://reviews.llvm.org/D52662