[libcxx-commits] [PATCH] D90021: [libcxx] [libcxxabi] Set flags for visibility when statically linking libcxxabi into libcxx for windows

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/CMakeLists.txt:874
+    # instead of dllimport.
+    add_definitions(-D_LIBCXXABI_BUILDING_LIBRARY)
+  elseif(LIBCXX_ENABLE_STATIC AND LIBCXX_STATICALLY_LINK_ABI_IN_STATIC_LIBRARY)
----------------
Why does `_LIBCXXABI_BUILDING_LIBRARY` have an impact when building libc++?


================
Comment at: libcxxabi/CMakeLists.txt:290
+    # Define to the empty string (if just defined as -DFOO, it defines it to
+    # the value 1) as we have a number of "#define _LIBCPP_BUILDING_LIBRARY"
+    # in certain source files.
----------------
IMO, the right thing to do here is to get rid of the places where we `#define _LIBCPP_BUILDING_LIBRARY` in `libcxxabi`, and define `_LIBCPP_BUILDING_LIBRARY` unconditionally when building libc++abi. Then, as a follow-up change, we should clarify the status of these source files that are shared/duplicated between libc++ and libc++abi (that's `stdlib_new_delete.cpp` and `stdlib_exception.cpp` AFAICT).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90021