Make WindowsSupport.h a public header

[zturner]

Summary

I sort of hesitate to do this, but I honestly don't have a better solution. Currently WindowsSupport.h is kept private by putting it in llvm\lib\Support\Windows so that it's only available to use from within the support library itself, and only from implementation files.

However, it's not just the thing that sets up and invokes #include <windows.h>, it also has various utility functions and classes that wrap Windows specific types (e.g. a ScopedHandle). It would be nice to be able to share this kind of thing across other sub-projects and/or libraries (basically, anything not Support) when they have to invoke some platform specific code and end up needing the same thing. It would be nice if LLVM had some means of exposing platform specific utility functions, classes, and helpers in the same way that it exposes platform-independent ones.

The alternative is to just copy the stuff I need from WindowsSupport.h into another project-specific header. This would duplicate some code, but I won't blame anyone if they think that's better than making this header public.

[amccarth]

Will making it public cause more TUs to transitively include <windows.h>, or does that happen anyway?

I have no objections if the toothpaste is already out of the tube. But if it's not, I'd prefer to keep <windows.h> as contained as possible, since it defines so many macros and global names that can easily conflict.

[Bigcheese]

Is there a good reason the use cases you want this for can't be added to libSupport? I'm a bit worried about continuing to add platform specific code outside of libSupport.

[zturner]

In D48859#1151696, @Bigcheese wrote:

Is there a good reason the use cases you want this for can't be added to libSupport? I'm a bit worried about continuing to add platform specific code outside of libSupport.

Mostly that it's quite large. See my recent post on llvm-dev about RFC: libtrace. I don't want to develop an entire library inside of libSupport, and something like a libtrace will unavoidably have a lot of platform specific code. As another example, consider lldb. There's no way to avoid a bunch of platform specific code, and it would be nice if LLVM had a solution that allowed LLDB to re-use some of LLVM's own platform specific code.

[Bigcheese]

In D48859#1151697, @zturner wrote:

In D48859#1151696, @Bigcheese wrote:

Is there a good reason the use cases you want this for can't be added to libSupport? I'm a bit worried about continuing to add platform specific code outside of libSupport.

Mostly that it's quite large. See my recent post on llvm-dev about RFC: libtrace. I don't want to develop an entire library inside of libSupport, and something like a libtrace will unavoidably have a lot of platform specific code. As another example, consider lldb. There's no way to avoid a bunch of platform specific code, and it would be nice if LLVM had a solution that allowed LLDB to re-use some of LLVM's own platform specific code.

Yeah, thought that may be the reason. LGTM, although would it be reasonable to add a comment to the header saying not to include it in another non-windows-specific header?

llvm/include/llvm/Support/WindowsSupport.h

 #define LLVM_SUPPORT_WINDOWSSUPPORT_H
 
 // mingw-w64 tends to define it as 0x0502 in its headers.
 #undef _WIN32_WINNT
 #undef _WIN32_IE
 
 // Require at least Windows 7 API.
 #define _WIN32_WINNT 0x0601
-#define _WIN32_IE    0x0800 // MinGW at it again. FIXME: verify if still needed.
+#define _WIN32_IE 0x0800 // MinGW at it again. FIXME: verify if still needed.
 #define WIN32_LEAN_AND_MEAN
 #ifndef NOMINMAX
 #define NOMINMAX
 #endif
 
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
inline bool MakeErrMsg(std::string *ErrMsg, const std::string &prefix) {
   else
     *ErrMsg = prefix + ": Unknown error";
   *ErrMsg += " (0x" + llvm::utohexstr(LastError) + ")";
 
   LocalFree(buffer);
   return R != 0;
 }
 
-template <typename HandleTraits>
-class ScopedHandle {
+template <typename HandleTraits> class ScopedHandle {
   typedef typename HandleTraits::handle_type handle_type;
   handle_type Handle;
 
-  ScopedHandle(const ScopedHandle &other); // = delete;
+  ScopedHandle(const ScopedHandle &other);   // = delete;
   void operator=(const ScopedHandle &other); // = delete;
 public:
-  ScopedHandle()
-    : Handle(HandleTraits::GetInvalid()) {}
+  ScopedHandle() : Handle(HandleTraits::GetInvalid()) {}
 
-  explicit ScopedHandle(handle_type h)
-    : Handle(h) {}
+  explicit ScopedHandle(handle_type h) : Handle(h) {}
 
   ~ScopedHandle() {
     if (HandleTraits::IsValid(Handle))
       HandleTraits::Close(Handle);
   }
 
   handle_type take() {
     handle_type t = Handle;
     Handle = HandleTraits::GetInvalid();
     return t;
   }
 
   ScopedHandle &operator=(handle_type h) {
     if (HandleTraits::IsValid(Handle))
       HandleTraits::Close(Handle);
     Handle = h;
     return *this;
   }
 
   // True if Handle is valid.
   explicit operator bool() const {
     return HandleTraits::IsValid(Handle) ? true : false;
   }
 
-  operator handle_type() const {
-    return Handle;
-  }
+  operator handle_type() const { return Handle; }
 };
 
 struct CommonHandleTraits {
   typedef HANDLE handle_type;
 
-  static handle_type GetInvalid() {
-    return INVALID_HANDLE_VALUE;
-  }
+  static handle_type GetInvalid() { return INVALID_HANDLE_VALUE; }
 
-  static void Close(handle_type h) {
-    ::CloseHandle(h);
-  }
+  static void Close(handle_type h) { ::CloseHandle(h); }
 
-  static bool IsValid(handle_type h) {
-    return h != GetInvalid();
-  }
+  static bool IsValid(handle_type h) { return h != GetInvalid(); }
 };
 
 struct JobHandleTraits : CommonHandleTraits {
-  static handle_type GetInvalid() {
-    return NULL;
-  }
+  static handle_type GetInvalid() { return NULL; }
 };
 
 struct CryptContextTraits : CommonHandleTraits {
   typedef HCRYPTPROV handle_type;
 
-  static handle_type GetInvalid() {
-    return 0;
-  }
+  static handle_type GetInvalid() { return 0; }
 
-  static void Close(handle_type h) {
-    ::CryptReleaseContext(h, 0);
-  }
+  static void Close(handle_type h) { ::CryptReleaseContext(h, 0); }
 
-  static bool IsValid(handle_type h) {
-    return h != GetInvalid();
-  }
+  static bool IsValid(handle_type h) { return h != GetInvalid(); }
 };
 
 struct RegTraits : CommonHandleTraits {
   typedef HKEY handle_type;
 
-  static handle_type GetInvalid() {
-    return NULL;
-  }
+  static handle_type GetInvalid() { return NULL; }
 
-  static void Close(handle_type h) {
-    ::RegCloseKey(h);
-  }
+  static void Close(handle_type h) { ::RegCloseKey(h); }
 
-  static bool IsValid(handle_type h) {
-    return h != GetInvalid();
-  }
+  static bool IsValid(handle_type h) { return h != GetInvalid(); }
 };
 
 struct FindHandleTraits : CommonHandleTraits {
-  static void Close(handle_type h) {
-    ::FindClose(h);
-  }
+  static void Close(handle_type h) { ::FindClose(h); }
 };
 
 struct FileHandleTraits : CommonHandleTraits {};
 
 typedef ScopedHandle<CommonHandleTraits> ScopedCommonHandle;
-typedef ScopedHandle<FileHandleTraits>   ScopedFileHandle;
+typedef ScopedHandle<FileHandleTraits> ScopedFileHandle;
 typedef ScopedHandle<CryptContextTraits> ScopedCryptContext;
-typedef ScopedHandle<RegTraits>          ScopedRegHandle;
-typedef ScopedHandle<FindHandleTraits>   ScopedFindHandle;
-typedef ScopedHandle<JobHandleTraits>    ScopedJobHandle;
+typedef ScopedHandle<RegTraits> ScopedRegHandle;
+typedef ScopedHandle<FindHandleTraits> ScopedFindHandle;
+typedef ScopedHandle<JobHandleTraits> ScopedJobHandle;
 
 namespace llvm {
-template <class T>
-class SmallVectorImpl;
+template <class T> class SmallVectorImpl;
 
 template <class T>
-typename SmallVectorImpl<T>::const_pointer
-c_str(SmallVectorImpl<T> &str) {
+typename SmallVectorImpl<T>::const_pointer c_str(SmallVectorImpl<T> &str) {
   str.push_back(0);
   str.pop_back();
   return str.data();
 }
 
 namespace sys {
 
 inline std::chrono::nanoseconds toDuration(FILETIME Time) {

llvm/lib/Support/CrashRecoveryContext.cpp

 // SetUnhandledExceptionFilter. SetUnhandledExceptionFilter also doesn't have
 // any native support for chaining exception handlers, but VEH allows more than
 // one.
 //
 // The vectored exception handler functionality was added in Windows
 // XP, so if support for older versions of Windows is required,
 // it will have to be added.
 
-#include "Windows/WindowsSupport.h"
+#include "llvm/Support/WindowsSupport.h"
 
 static LONG CALLBACK ExceptionHandler(PEXCEPTION_POINTERS ExceptionInfo)
 {
   // DBG_PRINTEXCEPTION_WIDE_C is not properly defined on all supported
   // compilers and platforms, so we define it manually.
   constexpr ULONG DbgPrintExceptionWideC = 0x4001000AL;
   switch (ExceptionInfo->ExceptionRecord->ExceptionCode)
   {

llvm/lib/Support/InitLLVM.cpp

 #include "llvm/Support/Error.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include <string>
 
 #ifdef _WIN32
-#include "Windows/WindowsSupport.h"
+#include "llvm/Support/WindowsSupport.h"
 #endif
 
 using namespace llvm;
 using namespace llvm::sys;
 
 InitLLVM::InitLLVM(int &Argc, const char **&Argv) : StackPrinter(Argc, Argv) {
   sys::PrintStackTraceOnErrorSignal(Argv[0]);
 

llvm/lib/Support/RandomNumberGenerator.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/RandomNumberGenerator.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 #ifdef _WIN32
-#include "Windows/WindowsSupport.h"
+#include "llvm/Support/WindowsSupport.h"
 #else
 #include "Unix/Unix.h"
 #endif
 
 using namespace llvm;
 
 #define DEBUG_TYPE "rng"
 

llvm/lib/Support/raw_ostream.cpp

 # define STDOUT_FILENO 1
 #endif
 #ifndef STDERR_FILENO
 # define STDERR_FILENO 2
 #endif
 #endif
 
 #ifdef _WIN32
-#include "Windows/WindowsSupport.h"
+#include "llvm/Support/WindowsSupport.h"
 #endif
 
 using namespace llvm;
 
 raw_ostream::~raw_ostream() {
   // raw_ostream's subclasses should take care to flush the buffer
   // in their destructors.
   assert(OutBufCur == OutBufStart &&