[libcxx] Fix testing of the externally-threaded library build after r290850

[rmaprath]

Summary

Before r290850, building libcxx with -DLIBCXX_HAS_EXTERNAL_THREAD_API=ON had two uses:

• Allow platform vendors to plug-in an __external_threading header which should take care of the entire threading infrastructure of libcxx
• Allow testing of an externally-threaded library build; where the thread API is declared using pthread data structures, and the implementation of this API is provided as a separate library (test/support/external_threads.cpp) and linked-in when running the test suite.

r290850 breaks the second use case (pthread data structures are no longer available). This patch re-stores the ability to build+test an externally-threaded library variant on a pthread based system.

This wasn't caught on any of the official bots because we don't have any builds of the externally-threaded library build at the moment (apart from my downstream one).

[rmaprath]

I will commit this now to get the testing working again. Thought of putting up the patch for review in case if there is a better way to do this.

@compnerd: I wondered, for windows threading support, why not do something similar to __external_threading (and how it is handled in __threading_support) and offload the entire implementation to a separate windows-only header?

[compnerd]

I dont think that making Win32 threading an external one makes much sense unless we also do the same for pthreads. Its just as valid a threading model as pthreads, so why give preferential treatment to that? (Sure, there are more buildbots using it, but thats not a strong technical justification).

include/__threading_support

30–50

Can you break this down into this which IMO is far more readable:

#if defined(_LIBCPP_HAS_THREAD_API_EXTERNAL)
#if __libcpp_has_include(<external_threading>)
#include <__external_threading>
#else
// Why pthread?
#define _LIBCPP_HAS_THREAD_API_EXTERNAL_PTHREAD
#endif
#endif

#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD) ||
    defined(_LIBCPP_HAS_THREAD_API_EXTERNAL_PTHREAD)
#include <pthread.h>
#include <sched.h>
#endif

[rmaprath]

In D28229#634278, @compnerd wrote:

I dont think that making Win32 threading an external one makes much sense unless we also do the same for pthreads. Its just as valid a threading model as pthreads, so why give preferential treatment to that? (Sure, there are more buildbots using it, but thats not a strong technical justification).

If the Win32 threading is support is going to be upstreamed, yes, it makes sense to give it the same treatment as pthreads (keep it inside __threading_support). libcxx has been tied to pthreads for a while, would be good to see something else supported as well. This will also allow us to iron out pthread-isms from the sources into the threading API. I have a couple of patches in that area (which I discovered while implementing the libcxx threading API with a different thread implementation), will put them up for review soon.

include/__threading_support

30–50

The reason I kept it in the current form is, when __external_threading is present, everything else in this header needs to be silenced. This is enforced by the current form because everything else in this header lives inside the #else side of the __external_threading inclusion.

Sure, the header will be a bit more readable if we arrange it the way you suggested, but then it's easy to add something to the header in a way that does not get removed when __external_threading is present.

[EricWF]

Accepting and Closing. This was committed in r290878.

include/__threading_support

 #ifndef __libcpp_has_include
   #ifndef __has_include
     #define __libcpp_has_include(x) 0
   #else
     #define __libcpp_has_include(x) __has_include(x)
   #endif
 #endif
 
 #if defined(_LIBCPP_HAS_THREAD_API_EXTERNAL) && \
+    !__libcpp_has_include(<__external_threading>)
+// If the <__external_threading> header is absent, build libc++ against a
+// pthread-oriented thread api but leave out its implementation. This setup
+// allows building+testing of an externally-threaded library variant (on any
+// platform that supports pthreads). Here, an 'externally-threaded' library
+// variant is one where the implementation of the libc++ thread api is provided
+// as a separate library.
+#define _LIBCPP_HAS_THREAD_API_EXTERNAL_PTHREAD
+#endif
+
+#if defined(_LIBCPP_HAS_THREAD_API_EXTERNAL) && \
     __libcpp_has_include(<__external_threading>)
 #include <__external_threading>
 #else
 
-#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
+#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD) || \
+    defined(_LIBCPP_HAS_THREAD_API_EXTERNAL_PTHREAD)
 #include <pthread.h>
 #include <sched.h>
 #endif
 
 #if defined(_LIBCPP_HAS_THREAD_API_EXTERNAL)
 #define _LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_FUNC_VIS
 #else
 #define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
+#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD) || \
+    defined(_LIBCPP_HAS_THREAD_API_EXTERNAL_PTHREAD)
 // Mutex
 typedef pthread_mutex_t __libcpp_mutex_t;
 #define _LIBCPP_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER
 
 // Condition Variable
 typedef pthread_cond_t __libcpp_condvar_t;
 #define _LIBCPP_CONDVAR_INITIALIZER PTHREAD_COND_INITIALIZER
 

 int __libcpp_tls_create(__libcpp_tls_key *__key, void (*__at_exit)(void *));
 
 _LIBCPP_THREAD_ABI_VISIBILITY
 void *__libcpp_tls_get(__libcpp_tls_key __key);
 
 _LIBCPP_THREAD_ABI_VISIBILITY
 void __libcpp_tls_set(__libcpp_tls_key __key, void *__p);
 
-#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
+#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD) || \
+    defined(_LIBCPP_BUILDING_THREAD_API_EXTERNAL_PTHREAD)
 
 int __libcpp_recursive_mutex_init(__libcpp_mutex_t *__m)
 {
   pthread_mutexattr_t attr;
   int __ec = pthread_mutexattr_init(&attr);
   if (__ec)
     return __ec;
   __ec = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);

test/support/external_threads.cpp

 //===----------------------------------------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
-#define _LIBCPP_HAS_THREAD_API_PTHREAD
+#define _LIBCPP_BUILDING_THREAD_API_EXTERNAL_PTHREAD
 #include <__threading_support>