[PATCH] D74599: llvm: Set WINVER, _WIN32_WINNT to 0x601 (Windows 7) for MinGW

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

I was going to say that we have llvm/lib/Support/Windows/WindowsSupport.h to wrap windows.h inclusions, and it manages these macros. However, I see there are many includes of windows.h across the codebase:

  $ git grep 'include.*windows.h' ../llvm/lib
  ../llvm/lib/ExecutionEngine/IntelJITEvents/ittnotify_config.h:#include <windows.h>
  ../llvm/lib/ExecutionEngine/IntelJITEvents/jitprofiling.c:#include <windows.h>
  ../llvm/lib/Support/Atomic.cpp:// We must include windows.h after intrin.h.
  ../llvm/lib/Support/Atomic.cpp:#include <windows.h>
  ../llvm/lib/Support/CodeGenCoverage.cpp:#include <windows.h>
  ../llvm/lib/Support/CrashRecoveryContext.cpp:#include <windows.h> // for GetExceptionInformation
  ../llvm/lib/Support/LockFileManager.cpp:#include <windows.h>
  ../llvm/lib/Support/Windows/WindowsSupport.h:#include <windows.h>
  ../llvm/lib/Support/Windows/WindowsSupport.h:// Must be included after windows.h

One side effect of doing things as you have done here is that this will affect the compiler-rt build. I don't expect there will be any negative consequences, but it could be surprising.

I guess my preference would be to hoist WindowsSupport.h up to llvm/include/llvm/Suppport, and make llvm-ar use that, so that we don't need to make build system changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74599