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

Thu Jun 2 08:01:04 PDT 2022

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

Looks OK, but I'd like to understand the need for `_LIBCPP_DLLIMPORT` before we go forward.

================
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
----------------
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?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125924