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

[Hamza Sood via Phabricator via cfe-commits]

hamzasood added a comment.

Thanks. Could someone commit this for me please?



================
Comment at: include/experimental/__config:57
 
+#if defined(_LIBCPP_OBJECT_FORMAT_COFF)
+# define _LIBCPPX_TYPE_VIS
----------------
compnerd wrote:
> 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.
But isn't it possible to build libc++ as static instead of shared? (LIBCXX_ENABLE_STATIC=YES and LIBCXX_ENABLE_SHARED=NO).
I'm pretty sure that in that case, the visibility annotations are left active.


https://reviews.llvm.org/D37182