cursey/safetyhook

Compiling broken on clang-cl

angelfor3v3r opened this issue · 4 comments

While testing #13 I've ran into the issue where the library refuses to build under clang-cl. It seems like commit 3393ced is the cause of it.

The error seems to stem from the use of -pedantic:

clang-cl: error: unknown argument ignored in clang-cl: '-pedantic' [-Werror,-Wunknown-argument]

The other thing I noticed is clang-cl gets picked up as both "msvc" and "clang" here, so both compile flags are used from these sections.
This is most likely just the fault of cmkr not having great predefined conditions: https://cmkr.build/cmake-toml/#predefined-conditions

msvc.private-compile-options = ["/WX", "/permissive-", "/W4", "/w14640"]
clang.private-compile-options = ["-Werror", "-Wall", "-Wextra", "-Wshadow", "-Wnon-virtual-dtor", "-pedantic"]

# Both of these pass on clang-cl:
if(MSVC) # msvc
if(CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR CMAKE_C_COMPILER_ID MATCHES "Clang") # clang

The final flags on my compiler look like this (this is only from Builder.cpp but it happens for all source files):

clang-cl.exe  /nologo -TP -DNOMINMAX -DZYDIS_STATIC_BUILD -I[REDACTED]\_deps\safetyhook-src\include -m32 /O2 /Ob2 /DNDEBUG -MD /WX /permissive- /W4 /w14640 -Werror -Wall -Wextra -Wshadow -Wnon-virtual-dtor -pedantic -std:c++17 /showIncludes /Fo_deps\safetyhook-build\CMakeFiles\safetyhook.dir\src\Builder.cpp.obj /Fd_deps\safetyhook-build\CMakeFiles\safetyhook.dir\safetyhook.pdb -c -- [REDACTED]\_deps\safetyhook-src\src\Builder.cpp

From what I know, clang flags passed to clang-cl must be passed like so: /clang:<flag>. However, It seems like most flags in the MSVC section would work fine. The only ones I cant find in the documentation are: /permissive and /w<warning id>. Either way, I don't think it's ideal that both MSVC and clang-cl flags are being mixed on the command line in this case since they're redundant.

I'm sure theres some work around for this but perhaps cmkr could have better predefined compiler detection conditions?

This is a rather esoteric part of CMake (https://stackoverflow.com/a/10055571/1806760). Likely the default condition for cmkr is not strict enough. You can fix this locally in your project by overwriting the clang condition:

[conditions]
clang = "CMAKE_CXX_COMPILER_ID MATCHES \"Clang\" AND NOT CMAKE_CXX_SIMULATE_ID"

Also as a general note you shouldn't set -Werror as a compiler flag. This means that if the compiler improves over time your compilation might fail, which is a pain in the ass for consumers of your library. Compiler flags in the CMake should be seen as requirements to build your target (which is why they are PUBLIC).

CMake's "solution" is to use presets for this and manually specify the CMAKE_CXX_FLAGS variables there, which isn't ideal...

Should be fixed in cmkr v0.2.19

Should be fixed in cmkr v0.2.19

You’re awesome, thank you so much for the information and cmkr changes! I’ll be giving it a try soon.