[PATCH] D20874: [libcxx] Two more threading dependencies

[Ben Craig via cfe-commits]

bcraig added inline comments.

================
Comment at: include/__threading_support:187
@@ +186,3 @@
+{
+    std::terminate();
+}
----------------
rmaprath wrote:
> bcraig wrote:
> > Why does this need to go in threading_support?  Seems like this is mostly orthogonal to threading.  libcxxabi seems like the better place to hold changes to terminate anyway.
> I can clearly see the argument against it, was bit unsure of it myself.
> 
> Now, the reason I want this the most is because of the externally threaded API, where I need to do some cleanup of the storage allocated in `__libcpp_thread_create()`. See the change I had to do in D20328 (thread.cpp) where I introduced a `__libcpp_thread_finalize()` function just for the cleanup. Thought it would be much cleaner to bundle up the two together so that I can avoid an explicit `#ifdef` in the sources.
> 
> Is that enough of a justification? Or should I stick to the explicit `#ifdef` in the externally-threaded variant? I don't have a strong opinion here, either way is fine, this version is slightly more preferred.
I guess this is fine.  I just need to tell myself (and maybe it should be commented in the code?) that this isn't trying to replace std::terminate, it's trying to replace a chunk of the std::thread dtor.  Having a custom hook for the std::thread dtor is reasonable, and this is a reasonable default implementation.

Note that you only get a chance to do things here when a user does naughty things and lets a joinable thread reach the std::thread dtor.


http://reviews.llvm.org/D20874