Cmake mod #1
Cmake mod #1
Conversation
…nd languages used.
CMakeLists.txt
Outdated
| target_include_directories (imgui PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}") | ||
|
|
||
| set_target_properties(imgui PROPERTIES CXX_STANDARD 11) |
There was a problem hiding this comment.
Would using compiler features be better here? I've never quite understood which one was the officially recommended way to do it.
There was a problem hiding this comment.
Yeah, compiler features are preferred and idiomatic way.
I realised (thanks to Mike-Devel ocornut#3027 (comment)) that setting the CXX_STANDARD property directly is a bad idea as it ignores CMAKE_CXX_STANDARD set by the parent project. Features such as "cxx_std_XX" respect CMAKE_CXX_STANDARD but require cmake 3.8.2 or greater if you want to specify cxx_std_XX instead of specific language features (e.g. cxx_decltype). I initially assumed that you wanted to stick to cmake 3.2 as to not impose undue requirements on consumers.
I can go ahead and amend my pull request in an hour so if you wish to instead use a compiler feature to set cxx_std_11. Furthermore isn't imgui currently only a C++98 library? Should C++11 be required to consume the imgui target?
There was a problem hiding this comment.
Yeah that'd be great :)
And I'm not sure of the exact requirement, but at the least we should require c++0x. That's just my opinion though.
There was a problem hiding this comment.
Well doesn't Imgui currently require only C++98 and that's an currently an advertised aspect of Imgui. Furthermore do you want me to bump the minimum Cmake version to 3.8.2?
There was a problem hiding this comment.
Yeah you're right:
Dear ImGui does NOT require C++11 so it can be used with most old C++ compilers.
We should be able to remove it altogether, then. It's up to Imgui to make sure compatibility among archaic compilers, not cmake's, since that is a project choice made by the maintainer.
Good catch :)
What would the reasoning for bumping to 3.8.2 be? I'm not opposed per se, it just implies there's a reason for it :)
There was a problem hiding this comment.
CMake 3.8.2 is when then cxx_std_XX compiler features were introduced. Prior to that, you could only specify what specific language features you required and CMake would use that to figure out what C++ standard version it needed to use on a given compiler.
There was a problem hiding this comment.
Ah okay, then yes let's bump it and then target cxx_std_98.
|
Thanks for doing this. Quick question before I merge (see above). |
update cmake_minimum_required to 3.8.2 and switched from setting set_target_properties to target_compile_features cxx_std_98
I just made some minor cleanups as per ocornut#3027 (comment).