rmaprath added inline comments. ================ Comment at: src/config.h:22 @@ +21,3 @@ + +#if defined(__GNUC__) || defined(__clang__) +#define _LIBCXXABI_PRAGMA(_parameter_) _Pragma(#_parameter_) ---------------- EricWF wrote: > What's the point of defining `_LIBCXXABI_WARNING`? It's unused and seems > unneeded. Good catch. I have copy-pasted this lot without checking.
================ Comment at: src/config.h:41 @@ +40,3 @@ + defined(__CloudABI__) || \ + defined(__sun__) +# define _LIBCXXABI_HAS_THREAD_API_PTHREAD ---------------- compnerd wrote: > Can you change this to `defined(__sun__) && defined(_POSIX_THREADS)` at the > very least? There is no reason to assume pthreads on Solaris. Solaris > Threads are a valid threading model. Not sure if I understand completely, `libcxxabi` currently either supports `pthread` or no-threads, either of which can be explicitly configured through a cmake option. So I don't see the point in checking for `_POSIX_THREADS` here as the only current threading option is `pthread`. When we add another threading system into the mix (like external threading), that too will be configurable with a cmake option, the defaulting to `pthread` is only when no particular threading system is selected. Or is it that `pthread.h` et. al can be provided by Solaris Threads as well? So, the pthread API has two implementations on Solaris? ================ Comment at: src/config.h:42 @@ +41,3 @@ + defined(__sun__) +# define _LIBCXXABI_HAS_THREAD_API_PTHREAD +# else ---------------- compnerd wrote: > Can we use `_LIBCXXABI_USE_THREAD_API_PTHREAD` instead? You can have more > than one threading API on a platform, and wish to use a specific one. Makes sense. Need to change the convention used in `libcxx` too, but that can be done later. ================ Comment at: src/config.h:46 @@ +45,3 @@ +# endif +#endif + ---------------- compnerd wrote: > I really think that we should use `_POSIX_THREADS` macro rather than this > enumeration approach. `_LIBCXXABI_USE_THREAD_API_XXXX` allows us to be consistent with naming of the different threading variants (e.g. I'm going to add `_LIBCXXABI_USE_THREAD_API_EXTENRAL` soon). Is there some functional benefit to using `_POSIX_THREADS` for the pthread variant? ================ Comment at: src/fallback_malloc.ipp:30 @@ -29,3 +29,3 @@ #ifndef _LIBCXXABI_HAS_NO_THREADS -static pthread_mutex_t heap_mutex = PTHREAD_MUTEX_INITIALIZER; +static __libcxxabi_mutex_t heap_mutex = _LIBCXXABI_MUTEX_INITIALIZER; #else ---------------- Thanks for the catch. https://reviews.llvm.org/D24864 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
