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

Saleem Abdulrasool via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 3 11:24:58 PST 2017


compnerd added a comment.

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).



================
Comment at: include/__threading_support:30-50
 #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
----------------
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


https://reviews.llvm.org/D28229





More information about the cfe-commits mailing list