Skip to content

Commit

Permalink
Properly handle the requirement of C++17 at the CMake exported target…
Browse files Browse the repository at this point in the history
… level (#251)

SDFormat 9 requires the use of C++17 in its public headers, however it does not correctly expose this requirement
in its installed imported targets. In particular, at the moment the requirement of C++17 is only documented via adding the
-std=c++17 flag in the `SDFormat_CXX_FLAGS` variable, but this strategy has two problems:
* It does not work unless a user explicitly pass this flags to its compilation targets, and this is definitely not obvious if it is not using directly SDFormat, but he is linking sdformat transitively through Gazebo 11.
* If a different C++ version is set at the CMake level, it is possible that the `-std=c++17` flag is ignored (it depends what is
the order with which the flags are passed to the compiler).

For CMake consumers, a better strategy is to use the target_compile_features and explicitly mark that
that this library requires the `cxx_std_17` feature, as `PUBLIC` because it also required in the public headers.

As the minimum required version of CMake is 3.10, we can use cxx_std_17 as it is available in CMake 3.10, see https://cmake.org/cmake/help/v3.10/prop_gbl/CMAKE_CXX_KNOWN_FEATURES.html#prop_gbl:CMAKE_CXX_KNOWN_FEATURES .

Signed-off-by: Silvio Traversaro <[email protected]>
  • Loading branch information
traversaro authored and scpeters committed May 12, 2020
1 parent 3bbd303 commit 6a195db
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ if (NOT WIN32)
endif()

sdf_add_library(${sdf_target} ${sources})
target_compile_features(${sdf_target} PUBLIC cxx_std_17)
target_link_libraries(${sdf_target} PUBLIC ${IGNITION-MATH_LIBRARIES})

target_include_directories(${sdf_target}
Expand Down

0 comments on commit 6a195db

Please sign in to comment.