_WIN32 vs. _MSC_VER

[joto]

The libosmium code contains some places with #ifdef _WIN32 and others with #ifdef _MSC_VER. According to cxong/tinydir#18, which I just stumbled upon, this is probably false.
Shall we change all _MSC_VER to _WIN32 or are there some cases where _MSC_VER is actually correct?

/cc @BergWerkGIS @alex85k

[wilhelmberg]

Well, there is no right or wrong.
Both are correct - each for their purpose.

• _WIN32 is for generally checking if the app is built for/on Windows.
• _MSC_VER is specifically targeted towards the Microsoft compiler (aka Visual Studio or C++ Build Tools) and checking the version thereof as each version might have different bugs aehm features 😏

MSVC++ 4.x  _MSC_VER == 1000
MSVC++ 5.0  _MSC_VER == 1100
MSVC++ 6.0  _MSC_VER == 1200
MSVC++ 7.0  _MSC_VER == 1300
MSVC++ 7.1  _MSC_VER == 1310 (Visual Studio 2003)
MSVC++ 8.0  _MSC_VER == 1400 (Visual Studio 2005)
MSVC++ 9.0  _MSC_VER == 1500 (Visual Studio 2008)
MSVC++ 10.0 _MSC_VER == 1600 (Visual Studio 2010)
MSVC++ 11.0 _MSC_VER == 1700 (Visual Studio 2012)
MSVC++ 12.0 _MSC_VER == 1800 (Visual Studio 2013)
MSVC++ 14.0 _MSC_VER == 1900 (Visual Studio 2015)
MSVC++ 14.1 _MSC_VER == 1910 (Visual Studio 2017)

[joto]

Sure, but how would I know? All the Windows code I have been writing came either from somebody else or I have been guessing from docs and snippets here or there. Can we try compiling with clang or gcc on Windows and see what works and what doesn't?

[alex85k]

As far as I see, there are no _MSC_VER specific version checks now (we assume Visual Studio 2015 or later to have base C++ 11).
Many of the checks are including unistd.h file with POSIX api, not available on Windows but having some equivalents in io.h or C++ 11.
I can check with MSYS2 (latest GCC on Windows).

[joto]

@alex85k Not sure what you mean with "no _MSC_VER specific version checks now" ? I count 20 mentionings of the _MSC_VER macro in libosmium code. Many of these are because of the unistd.h vs. io.h thing, but it is inconsistently used.

[mloskot]

Can we try compiling with clang or gcc on Windows and see what works and what doesn't?

Yes, Visual Studio VS 2015 Update 1 and 2017 deploy 'flavour' of clang (Clang with Microsoft CodeGen).

Why not add clang-based build job to the AppVeyor where clang 3.8 is available. Boost Hana can provide example of config: https://github.com/boostorg/hana/blob/master/.appveyor.yml

[alex85k]

I mean there are no checking of compiler version. The reason to use _MSC_VER was tht thing like unistd existed for mingw but not for Visual Studio. Most of checks can be with _WIN32 but not all.
Gcc from MinGW does not need many code tweaks for compiler but the binary is usually slower.

[alex85k]

To check vs+clang on Appveyor we'll most likely need to rebuild dependency pack with clang...

[joto]

Now that we are testing with GCC in the Msys2 environment, I think we can close this issue. If somebody wants to provide a clang build script for Appveyor, please go ahead. But the _MSC_VER vs. _WIN32 issue seems to be fixed now and is tested going forward with the different builds on Appveyor.