Remove unneeded standard passing #869
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The following comments explains why it was added, and that indeed it is no longer needed, as such. jtv#851 (comment) Note that neither of these 2 cmake modules are for public use, they are internal cmake stuff. Nothing on the outside should use it. This was feedback given by kitware developers.
Revert "remove a use added for a purpose which is gone in the meantime." This reverts commit 7fdd2f8.
…: -std=c++17, -std=c++20, ...
Just before the try_compile sections are exectued, the following happens:
- CMAKE_REQUIRED_DEFINITIONS is back-up-ed
- next it gets adjusted, to contain the cmpiler option to specif the language standard, this is done by setting ${CMAKE_CXX${CMAKE_CXX_STANDARD}_STANDARD_COMPILE_OPTION}
- some exampes of that are: CMAKE_CXX17_STANDARD_COMPILE_OPTION: -std=c++17 , CMAKE_CXX20_STANDARD_COMPILE_OPTION: -std=c++20
- and that value becomes the new value for CMAKE_REQUIRED_DEFINITIONS
- next all the try_compile statements are issued
- and afterwards the CMAKE_REQUIRED_DEFINITIONS is restored
Now all of that is not needed, since the minimum cmake required is 3.8.
And in cmake 3.8 an adjustment was made so the standard selection compile option is passed to try_compile invocations.
See here : https://cmake.org/cmake/help/latest/policy/CMP0067.html#policy:CMP0067 : Honor language standard in try_compile() source-file signature.
This means that any cmake version >= 3.8 is using by default this new behavior (unless someone would overrule it and put it explicitly to old).
I have done experiments with this, toether with the changed cmake_minimum_required syntax (it can have range), whic allowed me to have newer cmake behave as 3.7 and 3.8 ... , and in 3.7 the lnaguag standard indeed was not passed on, but in 3.8 it is.
Conclusion : this entire process mentioned above is not needed, this is the default behavior of cmake (since 3.8).
|
is anything prohibiting this PR to be merged ? |
|
Life has been intervening. Trying to get to it! |
|
Sorry about the delay @killerbot242, and thanks for the fix. I hope next we can do a release. |
|
@killerbot242 I've just released 7.9.2, including this change. |
|
I have like 3 or 4 more PR coming up |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
based upon the CMAKE_CXX_STANDARD, cmake can generate an option like : -std=c++17, -std=c++20, ...
Just before the try_compile sections are exectued, the following happens:
Now all of that is not needed, since the minimum cmake required is 3.8.
And in cmake 3.8 an adjustment was made so the standard selection compile option is passed to try_compile invocations.
See here : https://cmake.org/cmake/help/latest/policy/CMP0067.html#policy:CMP0067 : Honor language standard in try_compile() source-file signature.
This means that any cmake version >= 3.8 is using by default this new behavior (unless someone would overrule it and put it explicitly to old).
I have done experiments with this, toether with the changed cmake_minimum_required syntax (it can have range), whic allowed me to have newer cmake behave as 3.7 and 3.8 ... , and in 3.7 the lnaguag standard indeed was not passed on, but in 3.8 it is.
Conclusion : this entire process mentioned above is not needed, this is the default behavior of cmake (since 3.8).