boostorg / winapi

Windows API declarations without <windows.h>, for internal Boost use.
62 stars 58 forks source link

Failures on msvc-8.0, msvc-10.0 #60

Closed pdimov closed 6 years ago

pdimov commented 6 years ago

In the course of looking at something else, I saw that building Boost.System fails on msvc-8.0 and msvc-10.0 with:

compile-c-c++ ..\bin.v2\libs\system\build\msvc-10.0\debug\asynch-exceptions-on\threading-multi\error_code.obj
error_code.cpp
C:\Projects\boost-git\boost\boost/system/detail/local_free_on_destruction.hpp(29) : error C2039: 'LocalFree' : is not a member of 'boost::detail::winapi'
C:\Projects\boost-git\boost\boost/system/detail/local_free_on_destruction.hpp(29) : error C3861: 'LocalFree': identifier not found
C:\Projects\boost-git\boost\boost/system/detail/error_code.ipp(426) : error C2039: 'FORMAT_MESSAGE_ALLOCATE_BUFFER_' : is not a member of 'boost::detail::winapi
'
C:\Projects\boost-git\boost\boost/system/detail/error_code.ipp(426) : error C2065: 'FORMAT_MESSAGE_ALLOCATE_BUFFER_' : undeclared identifier

msvc-11.0 and later work.

Lastique commented 6 years ago

I believe, this is caused by https://github.com/boostorg/predef/pull/57. I'm working on an update for Boost.WinAPI to accommodate that change, but due to the problem with Boost.Thread (https://github.com/boostorg/thread/pull/160) testing might be difficult.

pdimov commented 6 years ago

The Thread problem should be fixed now.

jeking3 commented 6 years ago

@pdimov you can work around it temporarily, locally, with the changes from https://github.com/boostorg/winapi/pull/59, if you want to make local progress.

Lastique commented 6 years ago

@jeking3 No, that fix is incomplete.

pdimov commented 6 years ago

Makes no sense to me; when BOOST_PLAT_WINDOWS_UWP is 0 all BOOST_WINAPI_PARTITION are made 1? How is this correct?

Lastique commented 6 years ago

The idea is that on older Windows SDKs that have no support for API families WinAPI is supposed to provide all APIs, sans the ones not available in the target Windows version. This is when BOOST_PLAT_WINDOWS_UWP is 0.

The problem is that Boost.WinAPI only halfway ported to the partition macros and in some places directly tests Boost.Predef macros, which are all 0 after https://github.com/boostorg/predef/pull/57. The right thing to do is to port all of Boost.WinAPI to the partition macros, which should account for the older SDKs. This is what I've done locally and testing now.

pdimov commented 6 years ago

But changing BOOST_PLAT_WINDOWS_RUNTIME from 1 to 0 on older SDKs is wrong. This change should be reverted as it has repercussions all over.

Lastique commented 6 years ago

BOOST_PLAT_WINDOWS_RUNTIME should be 0 on older SDKs. The breaking change is that BOOST_PLAT_WINDOWS_DESKTOP was previously 1 by default on older SDKs and now is 0. The rationale is that Boost.Predef reflects the fact that the SDK does not support API families. See https://github.com/boostorg/predef/issues/54.

pdimov commented 6 years ago

Right, BOOST_PLAT_WINDOWS_DESKTOP. :-)

jeking3 commented 6 years ago

@Lastique cool it'll be good to see the fixes here; I thought modifying config.hpp like I did in #59 was all that was needed.

pdimov commented 6 years ago

BOOST_PLAT_WINDOWS_DESKTOP needs to stay 1 on older SDKs if you ask me. #if BOOST_PLAT_WINDOWS_DESKTOP means "Desktop APIs are available" and they are.

Lastique commented 6 years ago

Both ways have its merits. I can agree with changing it back to 1 by default, although what is considered a "desktop API" is a little fuzzy since BOOST_PLAT_WINDOWS_APP and BOOST_PLAT_WINDOWS_SYSTEM families also exist on the desktop and not only there. This family stuff is a lot of mess, IMHO.

Lastique commented 6 years ago

Should be fixed in https://github.com/boostorg/winapi/commit/8091170850cc0836d24d15e38de004aee35e3ee0.

pdimov commented 6 years ago

The problem is that the old PLAT macros were mutually exclusive and indicated platforms, not API families. Changing them to mean (overlapping) API families is a breaking change, as code relies on their being mutually exclusive.

A much more prudent approach would have been to not touch these and introduce separate FAMILY macros which new code can test.

Lastique commented 6 years ago

Families are not overlapping in the sense that the user selects only one family. The Boost.Predef macros directly derive from WINAPI_FAMILY, which makes them indicate the family selected by the user or Windows SDK. In this light I don't see how these new FAMILY macros would differ from the existing ones.

pdimov commented 6 years ago

Looking at this further, it doesn't look like the Winapi config code is quite right still, as under Win10 one can pick partitions directly.

/* In Windows 10, WINAPI_PARTITIONs will be used to add additional  
 * device specific APIs to a particular WINAPI_FAMILY.  
 * For example, when writing Windows Universal apps, specifying 
 * WINAPI_FAMILY_APP will hide phone APIs from compilation.  
 * However, specifying WINAPI_PARTITION_PHONE_APP=1 additionally, will         
 * unhide any API hidden behind the partition, to the compiler.

 * The following partitions are currently defined:
 * WINAPI_PARTITION_DESKTOP            // usable for Desktop Win32 apps (but not store apps)
 * WINAPI_PARTITION_APP                // usable for Windows Universal store apps
 * WINAPI_PARTITION_PC_APP             // specific to Desktop-only store apps
 * WINAPI_PARTITION_PHONE_APP          // specific to Phone-only store apps
 * WINAPI_PARTITION_SYSTEM             // specific to System applications

 * The following partitions are indirect partitions and defined in 
 * winpackagefamily.h. These partitions are related to package based 
 * partitions. For example, specifying WINAPI_PARTITION_SERVER=1 will light up
 * any API hidden behind the package based partitions that are bound to 
 * WINAPI_PARTITION_SERVER, to the compiler.
 * WINAPI_PARTITION_SERVER             // specific to Server applications
*/
jeking3 commented 6 years ago

Ugh, why did they have to make this so insanely complicated? SDK 8.0, 8.1, and 10.0 are totally different in this respect.

I'd suggest that for the 1.66 boost release we ignore this or document that we don't support it. I think the work we've done to this point is significantly better towards supporting UWP. We don't need to cover corner cases like this right now.

Lastique commented 6 years ago

Yep, I'm not going to support overriding or adding new partitions. Anyone willing to use this feature is free to define BOOST_USE_WINDOWS_H.

pdimov commented 6 years ago

Clang says

..\../boost/detail/winapi/config.hpp:146:7: warning: extra tokens at end of #else directive [-Wextra-tokens]
#else if defined(WINAPI_FAMILY_DESKTOP_APP)
      ^
Lastique commented 6 years ago

Thanks, should be fixed now.