Support compiling on MSYS2 #814
Conversation
| @@ -1,6 +1,8 @@ | |||
|
|
|||
| if("${CMAKE_C_SIMULATE_ID}" STREQUAL "MSVC") | |||
| if("${CMAKE_C_SIMULATE_ID}" STREQUAL "MSVC" OR MINGW) | |||
| # TODO: someone needs to provide the msvc equivalent warning flags | |||
There was a problem hiding this comment.
I believe this project compiles on Windows using clang-cl. In that case, all of the compiler options defined below can be passed to the clang-cl compiler. That surfaces a bunch of warnings, which could be fixed in a follow-up PR.
There was a problem hiding this comment.
IIRC, -fms-extensions are non-conforming extensions and we should not be enabling them. If MinGW requires that, then it should be done under a MINGW specific clause.
There was a problem hiding this comment.
You need -fms-extensions because __popcnt64 is used in hw_config.h. That's a Microsoft-specific extension.
The source code, as it stands, requires this Microsoft-specific extension when targeting the Windows platform. I assume you don't need to make it explicit on Windows because you're using clang-cl (which enables -fms-extensions by default), or the MSVC compiler.
I think the proper condition is "when targeting Windows but not using a MSVC-compatible compiler`, let me try to refine that if clause.
qmfrederik
force-pushed
the
msys2-minimal
branch
from
451b3dd to
5460cef
Compare
|
@MaxDesiatov Roger that, removed the GitHub actions. |
| @@ -1,6 +1,8 @@ | |||
|
|
|||
| if("${CMAKE_C_SIMULATE_ID}" STREQUAL "MSVC") | |||
| if("${CMAKE_C_SIMULATE_ID}" STREQUAL "MSVC" OR MINGW) | |||
| # TODO: someone needs to provide the msvc equivalent warning flags | |||
There was a problem hiding this comment.
IIRC, -fms-extensions are non-conforming extensions and we should not be enabling them. If MinGW requires that, then it should be done under a MINGW specific clause.
os/generic_win_base.h
Outdated
| @@ -24,7 +24,10 @@ | |||
| // Unices provide `howmany` via sys/param.h | |||
| #define howmany(x, y) (((x) + ((y) - 1)) / (y)) | |||
|
|
|||
| #ifndef _MODE_T_ | |||
| #define _MODE_T_ | |||
There was a problem hiding this comment.
Is there no generic way to avoid this? I'd rather not encode the macro that MinGW uses for this into the header. Why not hoist this into a check in CMake and use HAVE_MODE_T instead?
tests/generic_win_port.h
Outdated
| @@ -7,7 +7,9 @@ | |||
| #include <Windows.h> | |||
|
|
|||
| typedef int kern_return_t; | |||
| #ifndef _PID_T_ | |||
MaxDesiatov
dismissed
their stale review
Dismissing the change request as feedback was addressed.
|
@compnerd I've used CMake checks for |
|
@swift-ci please test |
|
OK - looks like CI for Windows passed: https://ci-external.swift.org/job/swift-corelibs-libdispatch-PR-windows/55/consoleFull |
|
@swift-ci test windows |
This patch consists of two commits:
-fms-extensionsvariable is passed to clang to enable the__popcnt64intrinsicIf this project doesn't want to use GitHub actions, I can remove the second commit.