EricWF added a comment. Added inline comments.
The main issue is that this patch needs to use the same `<__threading_support>` forward declarations as every other supported threading API; Only new definitions should be provided. ================ Comment at: include/__threading_support:33 #include <__external_threading> +#elif defined(_WIN32) && defined(_LIBCPP_HAS_THREAD_API_WIN32) +#define WIN32_LEAN_AND_MEAN ---------------- Isn't checking `_LIBCPP_HAS_THREAD_API_WIN32` enough? ================ Comment at: include/__threading_support:46 +inline _LIBCPP_INLINE_VISIBILITY +int __libcpp_recursive_mutex_init(__libcpp_mutex_t *__m); + ---------------- The forward declarations of the `__libcpp_` threading wrapper should be shared between all API's. Please don't add your own forward declarations for Windows. ================ Comment at: src/algorithm.cpp:51 #ifndef _LIBCPP_HAS_NO_THREADS -_LIBCPP_SAFE_STATIC static __libcpp_mutex_t __rs_mut = _LIBCPP_MUTEX_INITIALIZER; +#if !defined(_WIN32) +_LIBCPP_SAFE_STATIC ---------------- This makes me sad because not having constant initialization manifests itself as a static initialization order fiasco in libc++. If we do go this route please file a libc++ bug stating that Win32 mutex's do not provide constant initialization. I think this should check `_LIBCPP_HAS_THREAD_API_WIN32`? ================ Comment at: src/memory.cpp:158 _LIBCPP_SAFE_STATIC static const std::size_t __sp_mut_count = 16; -_LIBCPP_SAFE_STATIC static __libcpp_mutex_t mut_back[__sp_mut_count] = +#if !defined(_WIN32) +_LIBCPP_SAFE_STATIC ---------------- I think this should check _LIBCPP_HAS_THREAD_API_WIN32? ================ Comment at: src/mutex.cpp:198 #ifndef _LIBCPP_HAS_NO_THREADS -_LIBCPP_SAFE_STATIC static __libcpp_mutex_t mut = _LIBCPP_MUTEX_INITIALIZER; +#if !defined(_WIN32) +_LIBCPP_SAFE_STATIC ---------------- I think this should check _LIBCPP_HAS_THREAD_API_WIN32? Repository: rL LLVM https://reviews.llvm.org/D28220 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
