[libcxx-commits] [PATCH] D88843: [libcxx] Don't treat Windows specially with visibility annotations

Mon Oct 5 14:49:52 PDT 2020

phosek added a comment.

In D88843#2312850 <https://reviews.llvm.org/D88843#2312850>, @mstorsjo wrote:

> In D88843#2312846 <https://reviews.llvm.org/D88843#2312846>, @ldionne wrote:
>
>> In D88843#2312799 <https://reviews.llvm.org/D88843#2312799>, @mstorsjo wrote:
>>
>>> And to be clear, this isn't about manually defining `_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS` while building libc++ itself; it needs to be defined whenever user code includes libc++ headers.
>>
>> Let's add a proper `LIBCXX_DISABLE_VISIBILITY_ANNOTATIONS` option and `config_define_if(LIBCXX_DISABLE_VISIBILITY_ANNOTATIONS _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS)` like we do for other `__config_site` macros. Then, you can configure your build with `-DLIBCXX_DISABLE_VISIBILITY_ANNOTATIONS` and remove the platform specific logic from libc++'s CMake.
>>
>> WDYT?
>
> That sounds good to me and would be easy to configure going forward.
>
> Not sure how well it works in practice with @phosek 's original usecase from http://lists.llvm.org/pipermail/libcxx-dev/2020-October/000978.html, wanting to use one single `__config_site` for libc++ for multiple targets at once though.

Possible solution would be to allow per-target `__config` which is something I was going to propose separately since we have other use cases for it. That is, we would share headers across targets, but each target would have its own `__config`. This approach is already used by libstdc++. I have a local prototype and the implementation is pretty straightforward.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88843