Skip to content

Fix '/Zc:__cplusplus' leaking into GNU-based compilers on Windows#787

Merged
axxel merged 2 commits intozxing-cpp:masterfrom
bindreams:msvc-public-flag-guard
May 20, 2024
Merged

Fix '/Zc:__cplusplus' leaking into GNU-based compilers on Windows#787
axxel merged 2 commits intozxing-cpp:masterfrom
bindreams:msvc-public-flag-guard

Conversation

@bindreams
Copy link
Contributor

The change in #612 has introduced a bug when the library is used as a dependency on Windows with the Clang compiler.

For example, when zxing is installed via vcpkg, it is automatically compiled using MSVC and all is well. However, when a user (me) attempts to build a dependent program using a MSVC-compatible compiler with a GNU frontend, such as Clang (not mingw-based clang, the official one), the program fails to compile. This happens because zxing exports /Zc:__cplusplus publicly, even though the compiler might not support it. Example compilation error on a dependent project:

C:\PROGRA~1\LLVM\bin\CLANG_~1.EXE [...] -Wall -Wextra -pedantic -Werror /Zc:__cplusplus -MD -MT [...]
CLANG_~1: error: no such file or directory: '/Zc:__cplusplus'

I was able to fix this by using a generator expression around the flag. This change propagates to ZXingTargets.cmake and patching vcpkg to use my fork also seems to fix the issue.

@axxel
Copy link
Collaborator

axxel commented May 19, 2024

You mean the generator expression ends up in ZXingTargets.cmake and hence still gets evaluated from inside your client code cmake project?

@bindreams
Copy link
Contributor Author

Yes, exactly.

@bindreams
Copy link
Contributor Author

bindreams commented May 19, 2024

From an idealist point of view, a better solution would be to move the code affected by the flag into a cpp file, and make the flag private. Then the issue would go away altogether.

Current approach works only because MSVC does not provide a GNU-style frontend (yet), and Clang's MSVC-style frontend does not need this flag.

CMake also has CMAKE_CXX_COMPILER_FRONTEND_VARIANT which is very useful with these kinds of issues, but the variable is not supported in generator expressions.

@axxel axxel merged commit 38da404 into zxing-cpp:master May 20, 2024
@axxel
Copy link
Collaborator

axxel commented May 20, 2024

Thanks for the effort and insight. Removing the public dependency on BitHacks.h would work but maybe there is going to be another situation where we need to determine the c++ version in the future and we'd end up with the same issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants