-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Simplyfy c++ version check #11997
Simplyfy c++ version check #11997
Conversation
Hi @autoantwort Thanks! May I ask what prompted you to make this change, though? Did it not work? The |
The motivation was that I currently get:
but I use the most recent apple clang version ...
But why do a try_compile if cmake can do it natively. |
target_compile_features(main PRIVATE cxx_std_14)
target_compile_features(main PRIVATE cxx_std_11) # it is still 14
target_compile_features(main PRIVATE cxx_std_17) # now it is 17 set(CMAKE_CXX_STANDARD 17)
add_library(main main.cpp)
target_compile_features(main PRIVATE cxx_std_14) # now compiled with C++17 because of CMAKE_CXX_STANDARD set(CMAKE_CXX_STANDARD 11)
add_library(main main.cpp)
target_compile_features(main PRIVATE cxx_std_14) # compiled with C++14
But the clients must at least choose C++11 right? Now C++11 is used when nothing is specified and if they specify something this is used (C++11 is used if they specify something smaller than c++11) |
I think, as of clang 16, It's funny, because of all the builds in our CI it was the Mac build that failed... All the rest of your comments make sense, but we need to be careful here. Let's see what the CI says... |
No. Ci also seems to be happy. Now only |
@maloel any inputs regarding this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problems that I can see...
But it needs a rebase to solve the conflict -- I've done this and will try to push.
Looks like newer code in rsutils requires C++14. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @autoantwort
This has the same effect then the code before.