[PATCH] D122931: [CMake][compiler-rt] Support for using in-tree libc++

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

I like this, however IMO it would be better to instead have something like

  COMPILER_RT_CXX_LIBRARY="libc++|system-libc++|libstdc++|..."

That way, this can be extended to support other C++ libraries without having to add new boolean options, and without forcing folks to use ad-hoc `CMAKE_CXX_FLAGS` in their builds. WDYT?



================
Comment at: compiler-rt/CMakeLists.txt:513-515
+    # Don't use libcxx if LLVM_ENABLE_PROJECTS does not enable it.
+    set(COMPILER_RT_LIBCXX_PATH ${LLVM_EXTERNAL_LIBCXX_SOURCE_DIR})
+    set(COMPILER_RT_LIBCXXABI_PATH ${LLVM_EXTERNAL_LIBCXXABI_SOURCE_DIR})
----------------
I'm not sure I follow here -- we don't support using `LLVM_ENABLE_PROJECTS=libcxx` anymore.


================
Comment at: compiler-rt/CMakeLists.txt:517-534
+    foreach(path IN ITEMS ${LLVM_MAIN_SRC_DIR}/projects/libcxx
+                          ${LLVM_MAIN_SRC_DIR}/runtimes/libcxx
+                          ${LLVM_MAIN_SRC_DIR}/../libcxx
+                          ${LLVM_EXTERNAL_LIBCXX_SOURCE_DIR})
+      if(IS_DIRECTORY ${path})
+        set(COMPILER_RT_LIBCXX_PATH ${path})
+        break()
----------------
Is there any reason why this can't be hardcoded to `${LLVM_MAIN_SRC_DIR}/.../libcxx`? Why do we need to handle the other layouts?

Edit: Oh, I see, this was only moved around. I assume you prefer making these other changes in separate patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122931