Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expanding CMake to other compilers other than MSVC on Windows platform #1251

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

BoHomola
Copy link

@BoHomola BoHomola requested a review from a team as a code owner September 22, 2023 13:28
@dsnopek dsnopek added bug This has been identified as a bug topic:buildsystem Related to the buildsystem or CI setup labels Oct 18, 2023
@Faless
Copy link
Contributor

Faless commented Oct 18, 2023

Thank you for looking into this.

My understanding is that TYPED_METHOD_BIND should not be defined when using mingw, and that by using the WIN32 flag all windows builds (including mingw ones) will have that flag set.

So we need to detect the specific case of Clang as a MSVC frontend, which as far as I understand means checking the CXX_COMPILER_FRONTEND_VARIANT variable.

I don't know much about CMake, but this might work:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 0ee99aa..215c719 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -55,7 +55,7 @@ endif()
 # Set some helper variables for readability
 set( compiler_is_clang "$<OR:$<CXX_COMPILER_ID:AppleClang>,$<CXX_COMPILER_ID:Clang>>" )
 set( compiler_is_gnu "$<CXX_COMPILER_ID:GNU>" )
-set( compiler_is_msvc "$<CXX_COMPILER_ID:MSVC>" )
+set( compiler_is_msvc "$<OR:$<CXX_COMPILER_ID:MSVC>,$<CXX_COMPILER_FRONTEND_VARIANT:MSVC>>" )
 
 # Default build type is Debug in the SConstruct
 if("${CMAKE_BUILD_TYPE}" STREQUAL "")

@BoHomola BoHomola force-pushed the Cmake_windows_adjustments branch 2 times, most recently from 5906866 to f1ab271 Compare October 18, 2023 20:57
@BoHomola
Copy link
Author

Hello, thank you for your feedback.

CXX_COMPILER_FRONTEND_VARIANT would not work. I am using clang with GNU frontend. So the condition for defining TYPED_METHOD_BIND would still result in false.

If we want to specifically exclude MinGW, then a simple check if WIN and NOT GNU should work.

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 0ee99aa..45b9238 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -159,7 +159,7 @@ target_compile_definitions(${PROJECT_NAME} PUBLIC
                DEBUG_ENABLED
                DEBUG_METHODS_ENABLED
        >
-       $<${compiler_is_msvc}:
+    $<$<AND:$<BOOL:${WIN32}>,$<NOT:${compiler_is_gnu}>>:
                TYPED_METHOD_BIND
        >
 )

CMakeLists.txt Outdated Show resolved Hide resolved
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
@dsnopek dsnopek added the cmake label Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This has been identified as a bug cmake topic:buildsystem Related to the buildsystem or CI setup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants