EricWF added inline comments.

================
Comment at: include/__threading_support:30
+#define WIN32_LEAN_AND_MEAN
+#include <Windows.h>
+#include <process.h>
----------------
smeenai wrote:
> EricWF wrote:
> > EricWF wrote:
> > > smeenai wrote:
> > > > EricWF wrote:
> > > > > > Can we do as Reid suggests and not expose users to windows.h?
> > > > > 
> > > > > I was about to ask the same question.  These includes are dragging in 
> > > > > the `__deallocate` macro and I would love to avoid that.
> > > > I feel like we would end up with a //lot// of duplication if we went 
> > > > down this route, since this is using a fair amount of Windows APIs. 
> > > > @rnk suggested having a test for prototype mismatches, but even with 
> > > > those checks there could be a high maintenance burden to the 
> > > > duplication.
> > > > 
> > > > Was the main objection to `WIN32_LEAN_AND_MEAN` that it would be 
> > > > problematic for modules? If we're including `windows.h`, it seems 
> > > > strictly preferable to include it with `WIN32_LEAN_AND_MEAN` than 
> > > > without, since we'll pull in a lot less that way. Including `windows.h` 
> > > > without `WIN32_LEAN_AND_MEAN` can also interact with other headers 
> > > > badly sometimes, e.g. 
> > > > [`winsock2.h`](https://msdn.microsoft.com/en-us/library/windows/desktop/ms737629%28v=vs.85%29.aspx).
> > > It seems that dragging in the `__deallocate` macro is inevitable :-( 
> > > 
> > > I submitted a patch to work around `__deallocate` here: 
> > > https://reviews.llvm.org/D28426
> > > Was the main objection to WIN32_LEAN_AND_MEAN that it would be 
> > > problematic for modules? If we're including windows.h, it seems strictly 
> > > preferable to include it with WIN32_LEAN_AND_MEAN than without, since 
> > > we'll pull in a lot less that way. Including windows.h without 
> > > WIN32_LEAN_AND_MEAN can also interact with other headers badly sometimes, 
> > > e.g. winsock2.h.
> > 
> > The objection is that it breaks user code. For example:
> > 
> > ```
> > #include <thread>
> > #include <Windows.h> // Windows.h already included as lean and mean.
> > 
> > typedef NonLeanAndMeanSymbol foo; // ERROR NonLeanAndMeanSymbol not defined
> > 
> > ```
> > 
> But without the `WIN32_LEAN_AND_MEAN`, we're gonna break
> 
> ```
> #include <thread>
> #include <winsock2.h>
> ```
> 
> (you could fix this by reordering the includes, which would also fix your 
> example, or by defining `WIN32_LEAN_AND_MEAN` yourself, but it doesn't seem 
> great either)
I would much rather break that code. The fact that `Windows.h` doesn't play 
nice with other Windows headers is a problem for Windows not libc++.


https://reviews.llvm.org/D28220



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to