EricWF added inline comments.
================ Comment at: include/__threading_support:30 +#define WIN32_LEAN_AND_MEAN +#include <Windows.h> +#include <process.h> ---------------- EricWF wrote: > 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++. Although I think avoiding the `Windows.h` include all together would be better (if possible). However I think that can be fixed after committing this. https://reviews.llvm.org/D28220 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
