[PATCH] D138747: [Support] On Windows 11, fix an affinity mask issue on large core count machines

Alexandre Ganea via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Nov 27 09:07:21 PST 2022


aganea added inline comments.


================
Comment at: llvm/lib/Support/Windows/Process.inc:502
+  if (!info)
+    return llvm::VersionTuple(0, 0, 0, 0);
+
----------------
tschuett wrote:
> This looks like cute way to model `nullopt`. Would returning `std::option<lvm::VersionTuple>` be an improvement?
I made the changes so you can see how it looks from the caller perspective. I think `std::option` internally in `Process.inc` makes sense. However externally, returning `std::option` from `llvm::GetWindowsOSVersion()` leaks the fact that the Win32 APIs can fail, which clients don't really care about, and requires a bit more client-side code. Semantically clients only need to compare < or > against a version number, and 0 is fine. Please let me know your thoughts.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138747/new/

https://reviews.llvm.org/D138747



More information about the llvm-commits mailing list