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

[Yunzhong Gao via llvm-commits]

ygao added inline comments.

================
Comment at: lib/Support/raw_ostream.cpp:61
@@ +60,3 @@
+#ifdef LLVM_ON_WIN32
+#define NOMINMAX
+#include "Windows/WindowsSupport.h"
----------------
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.

================
Comment at: lib/Support/raw_ostream.cpp:67
@@ +66,3 @@
+// Cygwin does not have the IsWindows8OrGreater() API.
+static bool IsWindows8OrGreater() {
+  OSVERSIONINFO osvi = {};
----------------
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?


http://reviews.llvm.org/D15553