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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
