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

[Ben Craig via cfe-commits]

bcraig added inline comments.

================
Comment at: CMakeLists.txt:131
@@ -130,2 +130,3 @@
 option(LIBCXX_HAS_MUSL_LIBC "Build libc++ with support for the Musl C library" OFF)
-option(LIBCXX_HAS_PTHREAD_API "Ignore auto-detection and force use of pthread API" OFF)
+option(LIBCXX_HAS_PTHREAD_THREAD_API "Ignore auto-detection and force use of pthread API" OFF)
+option(LIBCXX_HAS_EXTERNAL_THREAD_API
----------------
I would prefer that you not change this option name, as that will break my builds.  It's easy enough to fix on my side, but unless there's a really compelling reason, I'd rather not deal with the breakage.

================
Comment at: include/__external_threading:26
@@ +25,3 @@
+
+#if !defined(_LIBCPP_MUTEX_T) || \
+    !defined(_LIBCPP_MUTEX_INITIALIZER) || \
----------------
So users of external pthreading (or their compiler driver) would need to provide a pile of -D options on the command line?  Or is it expected that users will have their own __external_threading header that comes earlier in the include path?  Please document your expectation.

================
Comment at: include/__threading_support:33
@@ -32,2 +32,2 @@
 #define _LIBCPP_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER
 typedef pthread_mutex_t __libcpp_mutex_t;
----------------
More context would be useful here. "-U99999" perhaps (if using git).


http://reviews.llvm.org/D21968