[PATCH] D27575: [libcxxabi] Introduce an externally threaded libc++abi variant (take-2)

[Asiri Rathnayake via Phabricator via cfe-commits]

rmaprath added inline comments.


================
Comment at: src/fallback_malloc.cpp:37
 class mutexor {
 public:
----------------
EricWF wrote:
> rmaprath wrote:
> > EricWF wrote:
> > > Can't we replace this class with `std::mutex` directly?
> > Again, I should've included more context to the patch :(
> > 
> > The `mutexor` here is supposed to degrade to a nop when threading is disabled. I think we cannot use `std::mutex` without threading support.
> > 
> > Will update the patch with more context.
> We cannot use `std::mutex` without threading support but I would still rather use it.
> 
> I would `typedef std::lock_guard<std::mutex> mutexor` when threading is enabled and otherwise I would just `#ifdef` all usages away.
> 
> Also please add `_LIBCPP_SAFE_STATIC` to the heap_mutex declaration.
Hmmm, using `std::mutex` here breaks the shared library builds of `libcxxabi`. The problem is that methods like `std::mutex::lock()` and `~std::mutex()` are only declared in the `<mutex>` header, with the implementation going into the dylib.

I was just about to update the patch because it worked for all my other configurations, but those configurations are either static builds or ones that suppress the `-Wl, -z defs` linker option passed to the shared library builds by default (this is an inherited option).

Perhaps it's time to get rid of `-Wl, -z defs` for all the `libcxxabi` configurations? That makes sense if `libcxxabi` is inherently dependent on `libcxx`.


https://reviews.llvm.org/D27575