[PATCH] D54182: [Windows] Sink function bodies to .cpp file

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 6 15:04:30 PST 2018


rnk added inline comments.


================
Comment at: llvm/lib/Support/Windows/Process.inc:469
+bool llvm::RunningWindows8OrGreater() {
+  // Windows 8 is version 6.2, service pack 0.
+  OSVERSIONINFOEXW osvi = {};
----------------
aganea wrote:
> Can't you just replace the contents of this function by:
> ```
> return GetWindowsOSVersion() >= llvm::VersionTuple(6, 2, 0, 0);
> ```
Sounds good to me.


================
Comment at: llvm/lib/Support/Windows/Process.inc:503
+
+  OSVERSIONINFOEX info;
+  ZeroMemory(&info, sizeof(OSVERSIONINFOEX));
----------------
aganea wrote:
> I'm wondering after all if we shouldn't just drop the part below? It's deprecated anyway, and the call to `RtlGetVersion` would probably always succeed (if you can't aquire a handle to `ntdll.dll` most likely you won't be able to run anything on your machine). What do you think?
I'd be OK with that. I suppose we'd return `VersionTuple(0,0,0,0)` in that case, and we'd assume that flush file buffers was necessary, which is probably fine.


https://reviews.llvm.org/D54182





More information about the llvm-commits mailing list