[llvm-commits] _WIN32_WINNT as predefined (was Re: [Review request][Win64] Patches for Mingw-w64(and mingw64-clang))

[Eric Christopher]

On Feb 6, 2011, at 6:06 PM, NAKAMURA Takumi wrote:

> Anton gave me a comment on the github for:
> * 0004-lib-Support-Windows-Windows.h-Autoconf-provides-.patch.txt
>> View Commit: https://github.com/chapuni/LLVM/commit/45be38ca409cbbdf2b63be2eaf7bf230aa9d6b38
> 
>> I don't think this is a good way of doing thing. You're passing platform-dependent macros to all files, even those which do not include windows.h. These #define's should be localized in windows-only places (e.g. libSupport)
> 
> In the case of mingw-w64, _WIN32_WINNT would be defined (as 0x0502) by
> system headers even if we did not provide predefined _WIN32_WINNT. In
> reverse, predefined _WIN32_WINNT might affect to system headers on
> mingw-w64.
> 
> IMHO, _WIN32_WINNT (and similar) is not "platform-dependent macro". I
> think it could be done even if _WIN32_WINNT were defined on command
> line (-D_WIN32_WINNT=0x0500).
> 
> Similarly, I think, also, _GNU_SOURCE could be defined in config.h
> (not llvm-config.h).
> and definitions in command line might be reduced as possible.
> 

I don't like that way of doing it necessarily either, though it's not any worse than having a windows only include. Is there some reason why you'd need to have all of those defines in every file that includes config.h as opposed to just doing some work in libSupport as Anton suggests?


> May I reopen PR7809 - TargetSelect.h shouldn't include llvm/Config/config.h ?
> http://llvm.org/bugs/show_bug.cgi?id=7809
> In TOT, a few headers depend on llvm/Config/config.h.

No, don't reopen that bug. It isn't related as far as I know unless it's a public header including config.h.

-eric