Created attachment 371740 [details] [review]
gwin32: Fix detection of MinGW32 vs MinGW-w64

__MINGW64__ is only defined on _64bit_ mingw-w64, not mingw-w64 in general.

I have both winternl.h and ntdef.h in my i686-mingw-w64. Why does the 64-bit one need to include a different header? The version of x86_64-mingw-w64 that i have laying around also has both headers.

(In reply to LRN from comment #3)
> I have both winternl.h and ntdef.h in my i686-mingw-w64. Why does the 64-bit
> one need to include a different header? The version of x86_64-mingw-w64 that
> i have laying around also has both headers.

This is not about 32-bit vs 64-bit, this is about mingw[1] (which does not contain winternl.h) vs mingw-w64[2] (which contains winternl.h).

1. https://en.wikipedia.org/wiki/MinGW
2. https://en.wikipedia.org/wiki/MinGW#MinGW-w64

The main issue is that if you `#include <ntdef.h>` on MinGW-w64, you will get errors about redefined prototypes, so you should only use winternl.h.

I suppose the proper way to do this is to check if winternl.h exists and use that. Will attach a patch that does that.

If you need to differentiate between MinGW.org and MinGW-w64, the right macro to check is __MINGW64_VERSION_MAJOR. If it's defined at all, you're building against a variant of MinGW-w64. If not, you're likely using MinGW.org.

(In reply to LRN from comment #5)
> If you need to differentiate between MinGW.org and MinGW-w64, the right
> macro to check is __MINGW64_VERSION_MAJOR. If it's defined at all, you're
> building against a variant of MinGW-w64. If not, you're likely using
> MinGW.org.

I couldn't find that define in my mingw-w64 compiler:

$ x86_64-w64-mingw32-gcc -dM -E - </dev/null | grep -i mingw
#define __MINGW32__ 1
#define __MINGW64__ 1
$ x86_64-w64-mingw32-gcc --version
x86_64-w64-mingw32-gcc (GCC) 4.7.3
Copyright (C) 2012 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Created attachment 371749 [details] [review]
gwin32: Fix detection of MinGW32 vs MinGW-w64

You're right, it's available, but in the headers. Not as a compiler define.

__MINGW32__ is defined on all MinGW variants including MinGW-w64.
__MINGW64_VERSION_MAJOR is only defined on MinGW-w64.

It's not part of the compiler, it's defined in _mingw_mac.h, which is included from _mingw.h, which is included from basic headers, like crtdefs.h and windef.h, which you have to include to compile practically anything useful. If you differentiate between MinGW.org-the-headers-and-libs and MinGW-w64-the-headers-and-libs, then this is the thing to check.

__MINGW64__ is a gcc built-in definition that is functionally equivalent to WIN64 - i.e. it's only defined for x86_64 targets (this is according to gcc-4.9 source code that i have laying around; hopefully, it still applies to newer GCCs).

(In reply to LRN from comment #8)
> It's not part of the compiler, it's defined in _mingw_mac.h, which is
> included from _mingw.h, which is included from basic headers, like crtdefs.h
> and windef.h, which you have to include to compile practically anything
> useful. If you differentiate between MinGW.org-the-headers-and-libs and
> MinGW-w64-the-headers-and-libs, then this is the thing to check.
> 
> __MINGW64__ is a gcc built-in definition that is functionally equivalent to
> WIN64 - i.e. it's only defined for x86_64 targets (this is according to
> gcc-4.9 source code that i have laying around; hopefully, it still applies
> to newer GCCs).

Thanks for the detailed explanation! And yes, __MINGW64__ behaves like that on all MinGW-w64 prefixes I have. That was my bad.

Review of attachment 371749 [details] [review]:

Can you please expand the commit message to include some details of *why* we care about differentiating between MinGW32 and MinGW-w64?

Created attachment 371839 [details] [review]
gwin32: Fix detection of MinGW32 vs MinGW-w64

__MINGW32__ is defined on all MinGW variants including MinGW-w64.
__MINGW64_VERSION_MAJOR is only defined on MinGW-w64.

This difference is important because on MinGW-w64 we must #include
winternl.h because including ntdef.h results in compiler errors
about symbol redefinition, and the header warns that it is deprecated
and may be removed in the future.

Review of attachment 371839 [details] [review]:

Thanks.

Attachment 371839 [details] pushed as a9fe62a - gwin32: Fix detection of MinGW32 vs MinGW-w64