[PATCH] D37182: [libcxx] Special visibility macros for the experimental library

Hamza Sood via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 29 04:53:25 PDT 2017


hamzasood added a comment.

In https://reviews.llvm.org/D37182#855262, @smeenai wrote:

> The other question, of course, is why the experimental library needs to be static. If it were built shared, the annotations would just work on Windows in theory (though I'm sure there are other issues there).


Even if libc++experimental is no longer forced to be static, it'd probably be too restrictive to force libc++ and libc++experimental to be the same library type. E.g. someone might want a dynamic libc++ but a static libc++experimental. If that's possible (even if not by default), different visibility macros will be needed.



================
Comment at: include/experimental/__config:57
 
+#if defined(_LIBCPP_OBJECT_FORMAT_COFF)
+# define _LIBCPPX_TYPE_VIS
----------------
smeenai wrote:
> I think it would make more sense to make these macros empty on all platforms, not just Windows. It's true that they'll only cause link errors on Windows (since you'll attempt to dllimport functions from a static library), but on ELF and Mach-O, the visibility annotations would cause these functions to be exported from whatever library c++experimental gets linked into, which is probably not desirable either.
> 
> The exception (if you'll pardon the pun) is `_LIBCPPX_EXCEPTION_ABI`, which will still need to expand to at least `__type_visibility__((default))` for non-COFF in order for throwing and catching those types to work correctly.
I might be mistaken, but I think the regular libc++ library still uses visibility annotations when it's being built as a static library on non-Windows platforms. So I figured it'd be best to match that behaviour.


https://reviews.llvm.org/D37182





More information about the cfe-commits mailing list