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

Fix compilation with clang on Windows. #209

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JKot-Coder
Copy link

@JKot-Coder JKot-Coder commented Jul 13, 2023

Description

This change addresses an issue where compilation fails on Windows with Clang due to unknown options "/W4 /WX" being passed to the Clang compiler.

The solution

The rationale behind this change is that CMAKE_CXX_SIMULATE_ID is intended to specify only the ABI of the compiler, whereas CMAKE_CXX_COMPILER_FRONTEND_VARIANT is used to specify the command-line argument style. By using CMAKE_CXX_COMPILER_FRONTEND_VARIANT, we ensure that the correct options are passed to the Clang compiler on Windows, resolving the compilation failure.

For both clang.exe and clang-cl.exe CMAKE_CXX_SIMULATE_ID is defined as MVSC, but CMAKE_CXX_COMPILER_FRONTEND_VARIANT is defined "GNU" in first case and "MVSC" in second.

@claremacrae
Copy link
Collaborator

Hi, many thanks for offering this change.

Before I merge this, I would like to set up a failing build in order to see the problem in action.

For this reason, I spent today getting most of the GitHub Actions builds back to green here.

In fact, I was hoping I would see a failure that would be fixed by merging this...

@claremacrae
Copy link
Collaborator

We do have a CI build that uses Windows VS-latest and clangcl.

Do you happen to have a build in GitHub Actions that I could check, to see how it is set up, and add it to the GitHub Actions here, please?

Then I can see it fail, and go ahead and merge your fix and see it pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants