[PATCH] D21968: [libcxx] Externally threaded libc++ variant - Take 2

[Asiri Rathnayake via cfe-commits]

rmaprath added inline comments.

================
Comment at: CMakeLists.txt:139
@@ -138,1 +138,3 @@
 option(LIBCXX_HAS_PTHREAD_API "Ignore auto-detection and force use of pthread API" OFF)
+option(LIBCXX_HAS_EXTERNAL_THREAD_API
+  "Build libc++ with an externalized threading API.
----------------
compnerd wrote:
> Can you use `cmake_dependent_option` here instead please?
I have two problems with this: 
* Currently we don't use the `CMakeDepedentOption` module anywhere else in the llvm project
* This option silently forces the cmake switch in question to the `FORCE` (last) argument  when the dependency in question is not satisfied. Our current approach on the other hand is to issue an error to the user indicating the incompatible options (see hunk below).

I'm slightly leaning towards keeping the current approach. @EricWF: WDYT?



================
Comment at: include/__config:830
@@ -829,1 +829,3 @@
+    !defined(_LIBCPP_HAS_THREAD_API_PTHREAD) && \
+    !defined(_LIBCPP_HAS_THREAD_API_EXTERNAL)
 # if defined(__FreeBSD__) || \
----------------
compnerd wrote:
> clang-format?
Running `clang-format -style=llvm` on this file shows that it's going to do a lot of changes to it. Do we want that? Perhaps it should be a separate patch?

Not sure if this is what you meant.


https://reviews.llvm.org/D21968