[PATCH] D15553: PR25717: fatal IO error writing large outputs to console on Windows

[Aaron Ballman via llvm-commits]

aaron.ballman added inline comments.

================
Comment at: lib/Support/raw_ostream.cpp:61
@@ +60,3 @@
+#ifdef LLVM_ON_WIN32
+#define NOMINMAX
+#include "Windows/WindowsSupport.h"
----------------
ygao wrote:
> aaron.ballman wrote:
> > I think this should be moved into WindowsSupport.h, truth be told. I don't think *anyone* wants min or max to be macros in this project.
> Will do.
> lib/Driver/MSVCToolChain.cpp lives in cfe so I'll look into cleaning that up in a separate patch.
Sounds great, thank you!

================
Comment at: lib/Support/raw_ostream.cpp:67
@@ +66,3 @@
+// Cygwin does not have the IsWindows8OrGreater() API.
+static bool IsWindows8OrGreater() {
+  OSVERSIONINFO osvi = {};
----------------
ygao wrote:
> aaron.ballman wrote:
> > Would it make sense to add the VersionHelpers.h include and IsWindows8OrGreater() to WindowsSupport.h as these seem like much more generally useful than just for raw_ostream?
> > 
> > I don't feel strongly about this, but am curious what others think.
> It sounds like a good idea.
> It may be a bit odd that for cygwin I only add a checker for Windows 8 but not for other versions, but we probably do not need checker function for every Windows version either. I suppose I can add only Windows 8 for now, and then other versions can be added later as needed?
I think that makes the most sense -- as we need them, we can add them in. Eventually cygwin will catch up with the Win32 APIs and we won't need the functionality anyway.


http://reviews.llvm.org/D15553