[libcxx-commits] [PATCH] D125924: [libcxx] Omit dllimport in public headers in MinGW mode

Thu Jun 2 09:28:26 PDT 2022

mstorsjo requested review of this revision.
mstorsjo added inline comments.

================
Comment at: libcxx/include/__config:538
+// requested by defining _LIBCPP_DLLIMPORT.
+#if defined(_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS) || (defined(__MINGW32__) && !defined(_LIBCPP_BUILDING_LIBRARY) && !defined(_LIBCPP_DLLIMPORT))
 #  define _LIBCPP_DLL_VIS
----------------
ldionne wrote:
> Aw, I don't like how that gets messy and this condition overlaps with `#elif defined(_LIBCPP_BUILDING_LIBRARY)` below, but I tried making it better and I don't see how. I suggest we go with this and then I can refactor the whole family of visibility macros to be per-platform, which would make everything much easier to understand IMO.
> 
> Also, why do we need to introduce `_LIBCPP_DLLIMPORT`? Can't we use the new behavior on MinGW unconditionally?
Yep, I don’t like it either, but the alternative would probably require even more reshuffling.

We don’t strictly need `_LIBCPP_DLLIMPORT` - I wanted to keep it as a convenient way to opt back in to the old dllimport style, which mostly would be useful for “in the field debugging”. But now when this configuration would be tested by CI, there should be less risk/need for such debugging - so if you don’t want that extra backdoor, I can leave it out.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125924/new/

https://reviews.llvm.org/D125924