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

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added reviewers: mclow.lists, compnerd.
smeenai resigned from this revision.
smeenai added a comment.

It's really cool that you're getting the filesystem library to work on Windows :)

This looks reasonable to me; it's the only way I can think of to get the experimental library working on Windows if we're keeping it static. It's a large change, but most of it is mechanical. Given its size, however, I also want to let at least one of Eric and Marshall take a look.

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).



================
Comment at: include/experimental/__config:57
 
+#if defined(_LIBCPP_OBJECT_FORMAT_COFF)
+# define _LIBCPPX_TYPE_VIS
----------------
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.


https://reviews.llvm.org/D37182