Skip to content

Clang-msvc warns of _WIN32_WINNT is reserved#111

Closed
denchat wants to merge 4 commits intolewissbaker:masterfrom
denchat:patch-1
Closed

Clang-msvc warns of _WIN32_WINNT is reserved#111
denchat wants to merge 4 commits intolewissbaker:masterfrom
denchat:patch-1

Conversation

@denchat
Copy link
Contributor

@denchat denchat commented May 16, 2019

I'm not sure if users are supposed to suppress certain warnings themselves.

@lewissbaker
Copy link
Owner

Is defining _WIN32_WINNT in a header file is somewhat of an anti-pattern?
Perhaps it should be defined only the command-line?
Maybe config.hpp should require that the build system specifies it?

@denchat
Copy link
Contributor Author

denchat commented May 30, 2019

Is it okey to use __has_include?

#if defined(_WIN32_WINNT) || defined(_WIN32)
# if !defined(_WIN32_WINNT)
#  if __has_include(<sdkddkver.h>)
#   // By default, we just use SDKDDKVer.h in the include paths
#   // for _WIN32_WINNT value
#   include <sdkddkver.h>
#   define CPPCORO_HAS_SDKDDKVER_H 1
#  else
#   define CPPCORO_HAS_SDKDDKVER_H 0
#  endif
   static_assert(CPPCORO_HAS_SDKDDKVER_H,
                 "Ensure sdkddkver.h header from WindowsSDK of your target"
                 " exists in the include paths, or otherwise explicitly"
                 " define _WIN32_WINNT as corresponding value of"
                 " your target via compile command.");
# endif
# define CPPCORO_OS_WINNT _WIN32_WINNT
#else
# define CPPCORO_OS_WINNT 0
#endif

@denchat
Copy link
Contributor Author

denchat commented Jun 10, 2019

Maybe config.hpp should require that the build system specifies it?

Typically _WIN32_WINNT definition depends on #include <sdkddkver.h>.

Do you intend that the build system like cake or CMAKE should be the one who defines _WIN32_WINNT without #include <sdkddkver.h> in source code?

@denchat
Copy link
Contributor Author

denchat commented Jun 10, 2019

CMAKE's CMAKE_SYSTEM_VERSION might not reflect the WindowsSDK version that being used.

https://stackoverflow.com/questions/9742003/platform-detection-in-cmake#comment96056421_40217291

user2019765 Feb 11 at 13:55

If CMAKE_SYSTEM_VERSION happens to be newer than the SDK version (e.g. CMAKE_SYSTEM_VERSION corresponds to Windows 10 but you only have the Windows SDK installed with Visual Studio 2010), this may lead to defining _WIN32_WINNT to a value that is not explicitly supported by the SDK. This is probably not a problem as such, but if you happen to do further checks based on the value of _WIN32_WINNT, the corresponding features might not be available during the build.

@denchat
Copy link
Contributor Author

denchat commented Jun 10, 2019

Well, it seems Falkon from KDE adopts this exact _WIN32_WINNT definition using CMAKE

https://phabricator.kde.org/D8079

denchat added a commit to denchat/cppcoro that referenced this pull request Jun 10, 2019
according to comments in
lewissbaker#111

The same CMAKE's _WIN32_WINNT detection is also used in Falkon

https://phabricator.kde.org/D8079
@lewissbaker
Copy link
Owner

The intention was to let the build system specify _WIN32_WINNT to allow the application to specify the minimum version of Windows that they want to target and so restrict the set of APIs that cppcoro can use to only those that are available on that OS version or earlier, regardless of what version of the SDK the program is being built against.

If _WIN32_WINNT is not specified by the build system then we need to pick some default OS version to target. We could pick some minimum OS version for maximum compatibility or we could pick the latest version supported by the SDK or we could pick something in-between.

@denchat
Copy link
Contributor Author

denchat commented Jun 13, 2019

Okey then now this should be closed. Let the build system do the work.

@denchat denchat closed this Jun 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants