[libcxx-commits] [PATCH] D120727: [libc++] Overhaul how we select the ABI library

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

In D120727#3372594 <https://reviews.llvm.org/D120727#3372594>, @mstorsjo wrote:

> FWIW I think D116689 <https://reviews.llvm.org/D116689> interacts with this somewhat. I think D116689 <https://reviews.llvm.org/D116689> is useful for the fixes I want to do wrt libcxxabi integration on Windows (but I haven't studied it, nor this one, closely yet though). @phosek Do you see anything in this one that isn't compatible with D116689 <https://reviews.llvm.org/D116689>?

It does interact a lot, in fact. If we land this patch, I suspect that the libc++abi parts of D116689 <https://reviews.llvm.org/D116689> may not be necessary anymore. And I also had a plan (not concrete) to use a similar approach for deciding how the unwind library is picked up, which may be worth exploring. As far as I can tell, the main thing that is nicer with D116689 <https://reviews.llvm.org/D116689>'s approach over this patch is that we can get rid of logic around `"-Wl,-force_load"` for merging the static library into libc++, but I think the approach proposed here would be compatible with that too.

The thing I like about this approach is that it simplifies the amount of logic we need by properly encoding dependencies in the `libcxx-abi-shared|static` targets.



================
Comment at: libcxx/src/CMakeLists.txt:233-239
     if (APPLE)
-      target_link_libraries(cxx_shared PRIVATE "-Wl,-force_load" "${LIBCXX_CXX_STATIC_ABI_LIBRARY}")
+      target_link_libraries(cxx_shared PRIVATE "-Wl,-force_load" "$<TARGET_LINKER_FILE:libcxx-abi-static>")
     else()
-      target_link_libraries(cxx_shared PRIVATE "-Wl,--whole-archive,-Bstatic" "${LIBCXX_CXX_STATIC_ABI_LIBRARY}" "-Wl,-Bdynamic,--no-whole-archive")
+      target_link_libraries(cxx_shared PRIVATE "-Wl,--whole-archive,-Bstatic" "$<TARGET_LINKER_FILE:libcxx-abi-static>" "-Wl,-Bdynamic,--no-whole-archive")
     endif()
   else()
+    target_link_libraries(cxx_shared PUBLIC libcxx-abi-shared)
----------------
This part would become nicer if we had an `OBJECT` library like in D116689. I think this approach does not preclude improving this in the future by using an object library.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120727