Fix Compiling with Clang for CMake #1272
Conversation
executable. This is then used to determine if clang (clang++) is being used. If clang is being used then the target compile features call is skipped. The justification is that if clang is being used then using `target_compile_features` would fail as clang doesn't provide this.
|
Might be a reasonably idea to throw in a check to see if |
|
@sum01, could you review this pull request, since I don't have enough knowledge about CMake. Thanks! |
| string(REGEX MATCH "[^/\\]*$" _outStr ${CMAKE_CXX_COMPILER}) | ||
| string(REGEX MATCH "[^\.]+" _outStr ${_outStr}) | ||
| set(${out_Name} ${_outStr}) | ||
| endmacro() |
There was a problem hiding this comment.
Why is this needed? Can't you just do
if (NOT CMAKE_CXX_COMPILER MATCHES "clang_regex")
target_compile_features(...)
endif()
| target_compile_features(${PROJECT_NAME} ${_INTERFACE_OR_PUBLIC} | ||
| cxx_std_11 | ||
| cxx_nullptr | ||
| cxx_lambdas | ||
| cxx_override | ||
| cxx_defaulted_functions | ||
| cxx_attribute_deprecated | ||
| cxx_auto_type | ||
| cxx_decltype | ||
| cxx_deleted_functions | ||
| cxx_range_for | ||
| cxx_sizeof_member | ||
| ) |
There was a problem hiding this comment.
Anyway I don't see why using target_compile_features() with the low-level compile features is needed; why isn't cxx_std_11 enough? And disabling the call when using Clang would introduce new issues, because CMake wouldn't know that C++11 is required.
In my opinion the low level compile features should be simply dropped. Even CMake's documentation recommends against their use: https://cmake.org/cmake/help/latest/prop_gbl/CMAKE_CXX_KNOWN_FEATURES.html#low-level-individual-compile-features
cc @sum01
|
@JonathanCline what clang version are you using? I can't reproduce your issue. |
|
Yeah I wasn't aware it was not recommended. It could instead be edited so only Regardless, @Tachi107 is right that trying to detect Clang and disable feature settings if it's used isn't a good idea. Also, I don't have issues using Clang++ on v13.0.1 either, for both header-only and compiled versions of the build. Not sure how @JonathanCline was calling the build function, but using this works just fine on my end. cd build
# Adding -DHTTPLIB_COMPILE=ON also runs as expected.
cmake -DCMAKE_CXX_COMPILER="clang++" ..
cmake --build .If needed I can submit a small patch to delete those few unnecessary feature lines, or anyone else can feel free to as well. |
- Enable THREADS_PREFER_PTHREAD_FLAG to use -pthread where supported - Remove low-level compile features (closes yhirose#1272) - Remove unneeded DESTINATION options where possible
- Enable THREADS_PREFER_PTHREAD_FLAG to use -pthread where supported - Remove low-level compile features (closes #1272) - Remove unneeded DESTINATION options where possible
|
@JonathanCline this was closed as I believe that your issues was caused by the low-level compile features. Could you please check if your issue has been actually solved? |
- Enable THREADS_PREFER_PTHREAD_FLAG to use -pthread where supported - Remove low-level compile features (closes yhirose#1272) - Remove unneeded DESTINATION options where possible
I noticed that compiling the library with clang and cmake simply failed due to clang not providing the C++ compile features whatsoever at least with the version of clang I am using. I threw together a quick fix described below:
Added a CMake macro to determine the name of the C++ compiler executable. This is then used to determine if clang (clang++) is being used.
If clang is being used then the target compile features call is skipped.
The justification is that if clang is being used then
using
target_compile_featureswould fail as clang doesn't provide this.