rnk added inline comments.
================
Comment at: include/__threading_support:44
+#define WIN32_LEAN_AND_MEAN
+#define VC_EXTRA_LEAN
+#include <Windows.h>
----------------
kimgr wrote:
> EricWF wrote:
> > compnerd wrote:
> > > EricWF wrote:
> > > > Do these definitions have any affect when `<Windows.h>` has already
> > > > been included?
> > > > Also are these definitions required before including the header, or
> > > > merely beneficial? If they are required this will make the
> > > > `<Windows.h>` header a pain to use with modules.
> > > >
> > > >
> > > No, they dont effect it once it has been included. They are beneficial
> > > since they reduce the amount of stuff that gets included (including
> > > things which, at least when I last checked, can cause clang to choke).
> > And can users re-include `<Windows.h>` afterwards in the same TU and get
> > all of the symbols?
> I don't think so.
>
> We've recently switched to defining these two symbols in our build system,
> and I think that's basically the only way to make this work in a project
> composed of headers from various authors. I think you're right that libc++
> should not define them.
I don't think libc++ should not include windows.h in a public header. I'd
rather write our own __threading_support_win.h that re-prototypes everything we
need from windows.h, and then we can have some test in libc++ that validates
that there are no mismatches when including windows.h before libc++ <mutex>. If
we do this, we should sink as much win32 API usage as possible out of headers
to reduce our duplication.
It's worth pointing out VS 2015's thread implementation also hides its win32
API usage.
================
Comment at: include/__threading_support:527
+static inline _LIBCPP_ALWAYS_INLINE unsigned int WINAPI
+__libcpp_thread_trampoline(void *__data)
+{
----------------
halyavin wrote:
> Trampolines will never be inlined. Should we put them in support *.cpp
> instead? The downside is new public symbols which can't be changed without
> breaking backward compatibility. The upside is that we will have only one
> copy of each trampoline. What do you think?
Considering that libc++ already has a __thread_proxy trampoline, let's just
give it the right CC and get rid of this trampoline.
================
Comment at: include/__threading_support:593
+{
+ // TODO(compnerd) provide a wrapper for CC adjustment
+ *__key = FlsAlloc(reinterpret_cast<void(WINAPI*)(void*)>(__at_exit));
----------------
Yeah, we need to fix that. We should find a way to make
`__thread_specific_ptr::__at_thread_exit(void*)` have the right convention out
of the box, rather than thunking. Something like:
#define __LIBCPP_TLS_CALLBACK_CC WINAPI
.. // else
#define __LIBCPP_TLS_CALLBACK_CC
... // <thread>
static void __LIBCPP_TLS_CALLBACK_CC __at_thread_exit(void*);
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
