`<windows.h>` should not be exposed to users

[milleniumbug]

<windows.h> is included on Windows in the header file. This is problematic for users, as <windows.h> has a bad reputation - for example, your calls to std::min() will stop working if it's included. The specific problem of non-working std::min be fixed by defining NOMINMAX, but there are many more #defines in there. The general problem can be fixed by using PIMPL idiom and moving away the #include <windows.h> to the implementation file.

[eidheim]

The best solution here is to deftype id_type, fd_type and string_type manually for Windows in process.hpp maybe?

[eidheim]

Something like this:

  typedef unsigned long id_type
  typedef void* fd_type
#ifdef UNICODE
  typedef std::wstring string_type
#else
  typedef std::string string_type
#endif

?

[milleniumbug]

That will do. I assume Microsoft won't ever change what HANDLE is typedef'd to, but you can protect yourself even from that scenario: In the implementation file have static_assert(std::is_same<fd_type, HANDLE>::value, "change your fd_type");, so if the type is ever different, you can get clear compile time message.

[nabijaczleweli]

namespace windooze {
    #include <windows.h>
}

[eidheim]

@nabijaczleweli good suggestion, but I'm not 100% sure this works with the clang compiler.

[milleniumbug]

@nabijaczleweli No, that's a terrible suggestion. How does that protect your codebase from shitty macros? Also it doesn't really work for functions with extern "C" linkage, which is most of the <windows.h>.

[nabijaczleweli]

#undef min, #undef max

[milleniumbug]

Now, generalize that for all non-hygienic macros (that means, using at least one lowercase letter) defined there, and we can start considering this as a viable solution (there are a LOT of them). I actually have an project idea of a header <unwindows.h> which undefines all of them, but it's unfinished (inspired by POCO's Unwindows.h, but it undefines only a selected subset of it which they use)

It's simpler to just compile-firewall it.

[eidheim]

We could move fd_type out of the process.hpp header completely (having std::unique_ptr<fd_type> stdout_fd, stderr_fd, stdin_fd; in each implementation (through PIMPL), though id_type is still needed. Does including IntSafe.h have the same problems (https://msdn.microsoft.com/en-us/library/windows/desktop/aa383751(v=vs.85).aspx, search for DWORD)?

[eidheim]

I decided to go with my first suggestion, hoping Windows goes POSIX before changing the HANDLE typedef!

[nabijaczleweli]

The API will be stable for at least the next 20 years, so no problems there