Latest clang and dllimport

In the latest clang built from source, if a function is mismatched as to a difference in dllimport attributes clang produces an error. As in:

C:\Utilities\mingw-w64\i686-5.1.0-posix-dwarf-rt_v4-rev0\mingw32\i686-w64-mingw32\include\synchapi.h:127:26: error: redeclaration of 'Sleep' cannot add 'dllimport' attribute
   WINBASEAPI VOID WINAPI Sleep (DWORD dwMilliseconds);
                          ^
..\..\..\boost/smart_ptr/detail/yield_k.hpp:67:29: note: previous declaration is here
   extern "C" void __stdcall Sleep( unsigned long ms );

Changing the latter to:

   extern "C" __declspec(dllimport) void __stdcall Sleep( unsigned long ms );

fixes the problem for clang.

Earlier versions of clang ( 3.4.1, 3.5.2, 3.6.1 ) did not have this problem. Is there a __has_feature or __has_extension that I can use to test this change in the latest clang ? The previous versions are also happy with the __declspec(dllimport) added in the declaration so even without a __has_feature or __has_extension I can just test for clang on Windows, but I like to be as precise as possible in Boost code.

Hi Edward,

I'm using this code to reproduce the issue you're seeing:

  extern "C" void sleep(unsigned ms);
  void f() {
    sleep(1000);
  }
  extern "C" __declspec(dllimport) void sleep(unsigned ms);

Compiling with "clang -target i686-pc-win32 -c a.cc" produces the
error you describe.

This produces an error both with both 3.5, 3.6 and trunk for me. Is it
possible that something has changed in your code, or that something is
getting #ifdef'ed differently based on the clang version?

The problem here is that Clang doesn't want to change the
"dllimport"-ness of sleep() after it has been called. If you try to
add the dllimport attribute before sleep() has been used, Clang will
just warn.

- Hans

The error reported is with mingw 5.1.0 which is failry new.
Maybe previous clangs were ok using previous versions of the mingw-w64 headers?

I don’t think we have any feature test flags for diagnostic changes.

It sounds like the general problem is that two headers declare parts of the win32 API because windows.h is too much of a header dependence. Boost also isn’t sure sure whether windows.h, from mingw or Microsoft, is using dllimport or not. Past versions of mingw have been inconsistent, so you can’t just always declare it dllimport, like it should be.

I’m starting to feel like we should just downgrade this to a warning for free functions. We already make it a warning in other cases. Very few programs rely on function pointer identity, but global variable identity is often important.

Sound reasonable?

Hi Edward.

Did try to build with BOOST_USE_WINDOWS_H defined?

Last time i used boost with Mingw-w64+Clang, i remember getting these errors. I can’t test it right now, but if i’m not mistaken, it stopped when i defined BOOST_USE_WINDOWS_H.

Regards

The error reported is with mingw 5.1.0 which is failry new.
Maybe previous clangs were ok using previous versions of the mingw-w64
headers?

Actually previous clang are using mingw headers and the __declspec(dllimport) which is WINBASEAPI in mingw-64 is nothing in mingw. It looks as if Boost will have to use logic to see whether mingw or mingw-64 is being used.

Hi Edward.

Did try to build with BOOST_USE_WINDOWS_H defined?

I actually tried it but found some problems with it, which I have not had the time to pursue. Even if there are no problems Boost is supposed to work properly without it.

Last time i used boost with Mingw-w64+Clang, i remember getting these
errors. I can't test it right now, but if i'm not mistaken, it stopped
when i defined BOOST_USE_WINDOWS_H.

Feel free to report errors on the Boost trac or if you are willing to do the work create PR requests.

I do my best to fix Boost problems using clang on Windows targeting the gcc RTL, but I can't cover every Boost library but just the my own and some others in which I am very interested Unfortunately there is nobody doing official regression testing of clang on Windows, although clang gets good regression testing for Android, Darwin, and Linux. So if you, or anyone else among the clang developers, wants to contribute regression tests for clang on Windows targeting gcc RTL, I would be glad to help in any way I can. I just don't have the resources myself to do it.

I don't think we have any feature test flags for diagnostic changes.

It sounds like the general problem is that two headers declare parts of
the win32 API because windows.h is too much of a header dependence.
Boost also isn't sure sure whether windows.h, from mingw or Microsoft,
is using dllimport or not. Past versions of mingw have been
inconsistent, so you can't just always declare it dllimport, like it
should be.

I'm starting to feel like we should just downgrade this to a warning for
free functions. We already make it a warning in other cases. Very few
programs rely on function pointer identity, but global variable identity
is often important.

Please do change this if it is possible for clang.

Let me argue the case as a clang user. Neither gcc or vc++ on Windows requires the sort of attribute matching with imported Windows API functions that clang requires. In other words if a Windows API has __declspec(dllimport) as part of the declaration of a function and the end-user separately declares that function without the __declspec(dllimport), it is not an error with gcc or vc++.

By enforcing a matching of this __declspec(dllimport) you are making code match whatever the particular Windows API header files specify. But the various Windows implementations are different so now the end-user has to figure out which Windows implementation is being used at compile-time and manipulate his declaration accordingly. In particular mingw never used any __declspec(dllimport) whereas mingw-64 appears to always use it. The official Microsoft header files also use it. I have not used cygwin but that is yet another case.

You may ask why any end-user is declaring a Windows API function instead of just including windows.h. For Boost the answer is all the header file dependencies that windows.h brings in, as well as the non C++-isms and poor C++-isms which the windows header files are famous for and for which code often has to find workarounds ( the common macro names are one example of these sorts of problems ). So a Boost library will often try not to include windows.h when it is using very few Windows API functions.

By enforciong this strict function declaration matching clang is making it much harder to declare Windows API functionality than it need be.

I don't think we have any feature test flags for diagnostic changes.

It sounds like the general problem is that two headers declare parts of
the win32 API because windows.h is too much of a header dependence.
Boost also isn't sure sure whether windows.h, from mingw or Microsoft,
is using dllimport or not. Past versions of mingw have been
inconsistent, so you can't just always declare it dllimport, like it
should be.

I'm starting to feel like we should just downgrade this to a warning for
free functions. We already make it a warning in other cases. Very few
programs rely on function pointer identity, but global variable identity
is often important.

Please do change this if it is possible for clang.

Let me argue the case as a clang user. Neither gcc or vc++ on Windows
requires the sort of attribute matching with imported Windows API functions
that clang requires. In other words if a Windows API has
__declspec(dllimport) as part of the declaration of a function and the
end-user separately declares that function without the
__declspec(dllimport), it is not an error with gcc or vc++.

By enforcing a matching of this __declspec(dllimport) you are making code
match whatever the particular Windows API header files specify. But the
various Windows implementations are different so now the end-user has to
figure out which Windows implementation is being used at compile-time and
manipulate his declaration accordingly. In particular mingw never used any
__declspec(dllimport) whereas mingw-64 appears to always use it. The
official Microsoft header files also use it. I have not used cygwin but
that is yet another case.

You may ask why any end-user is declaring a Windows API function instead of
just including windows.h. For Boost the answer is all the header file
dependencies that windows.h brings in, as well as the non C++-isms and poor
C++-isms which the windows header files are famous for and for which code
often has to find workarounds ( the common macro names are one example of
these sorts of problems ). So a Boost library will often try not to include
windows.h when it is using very few Windows API functions.

By enforciong this strict function declaration matching clang is making it
much harder to declare Windows API functionality than it need be.

I've filed PR24215 about downgrading this to a warning.

Thanks,
Hans