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

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd accepted this revision.
compnerd added inline comments.
This revision is now accepted and ready to land.


================
Comment at: include/experimental/__config:57
 
+#if defined(_LIBCPP_OBJECT_FORMAT_COFF)
+# define _LIBCPPX_TYPE_VIS
----------------
hamzasood wrote:
> 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.
It's not about matching that behaviour, libc++ is built shared, this is built static.  The static link marked default visibility would make the functions be re-exported.  However, if I'm not mistaken, this already happens today, so fixing that in a follow-up is fine as the status quo is maintained.


https://reviews.llvm.org/D37182