[PATCH] D55404: [libcxx] Support building static library with hidden visibility

Fri Dec 7 17:30:36 PST 2018

phosek added a comment.

In D55404#1323828 <https://reviews.llvm.org/D55404#1323828>, @ldionne wrote:

> Ahhh, this part is what I had missed. I didn't see you were disabling the visibility annotation. In that case everything is indeed hidden. Instead, I think a cleaner approach would be:
>
> 1. Build with hidden visibility by default at namespace scope (basically https://reviews.llvm.org/D54810), and
> 2. generalize https://github.com/llvm-mirror/libcxx/blob/master/CMakeLists.txt#L742 so that we can remove visibility annotations using a CMake option.
>
>   This way, we'd avoid adding yet another configuration option and instead leverage something we already (apparently) need and use on Windows. What do you think?

In general that sounds fine me, problem with generalizing https://github.com/llvm-mirror/libcxx/blob/master/CMakeLists.txt#L742 is that we also want to make sure that `new` and `delete` operators are hidden as well. Those operators may be implicitly declared by Clang with default visibility which defeats `-fvisibility=hidden` or explicit annotations which is why we've introduced `-fvisibility-global-new-delete-hidden`which is being set in this change as well. I don't know if that's something that would affect Windows, and in general I don't know if it's something that should be enabled automatically for static libc++?

> Also, I'd be curious about the relationship between this and the `_LIBCPP_HIDE_FROM_ABI_PER_TU` setting, which gives internal linkage to everything. Right now, it's possible to have both default visibility and internal linkage, but I'm not sure what that even means. Maybe both options should actually be merged under something more general like `_LIBCPP_ALLOW_DISTRIBUTING_STATIC_LIBRARY`, which would hide all symbols _and_ give internal linkage to everything. Not sure.

Do we even need `_LIBCPP_HIDE_FROM_ABI_PER_TU` now that we have `__attribute__((__exclude_from_explicit_instantiation))`? The only case I can think of are older versions of Clang or other compilers that don't support this attribute.

Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55404/new/

https://reviews.llvm.org/D55404