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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D122931: [CMake][com... Louis Dionne via Phabricator via cfe-commits

Reply via email to