ros / roscpp_core

ros distribution sandbox
89 stars 116 forks source link

[C++11/C++14] remove explicit -std=c++11 #94

Closed moriarty closed 6 years ago

moriarty commented 6 years ago

From REP-0003

Melodic As of ROS Melodic, we are using the C++14 (ISO/IEC 14882:2014) standard.

Related to discussion on ros-planning/moveit#1146

This should work with both Kinetic & Melodic:

For Kinetic, on 16.04 the default is gcc-5, which will use -std=c++11 https://www.gnu.org/software/gcc/gcc-5/changes.html

For Kinetc the other target platform was 15.10 reached EOL on July 28, 2016. https://wiki.ubuntu.com/Releases

moriarty commented 6 years ago

:disappointed: it looks like I read https://www.gnu.org/software/gcc/gcc-5/changes.html to quickly... They bumped the default C standard, not C++ standard

The default mode for C is now -std=gnu11 instead of -std=gnu89.

moriarty commented 6 years ago

@dirk-thomas

Is Lunar Loggerhead only supported on Xenial?

This is only for a test, so I don't think it really matters much. But I was looking for a way to use the same single kinetic-devel branch, but use C++14 for Melodic, else use C++11...

The hackish solution would be to check for C++17 support, because if you check for C++14 support it's true for both... But this only works if Lunar Loggerhead is only supported on Xenial and not still supported on Yakkety and Zesty, because gcc-6 has C++17.

@@ -38,7 +38,6 @@ if(CATKIN_ENABLE_TESTING)
   endif()
   catkin_add_gtest(${PROJECT_NAME}-test_time test/time.cpp)
   if(TARGET ${PROJECT_NAME}-test_time)
+    include(CheckCXXCompilerFlag)
+    CHECK_CXX_COMPILER_FLAG("-std=c++17" COMPILER_SUPPORTS_CXX17)
+    CHECK_CXX_COMPILER_FLAG("-std=c++11" COMPILER_SUPPORTS_CXX11)
+    if(COMPILER_SUPPORTS_CXX17)
+      set_property(TARGET ${PROJECT_NAME}-test_time APPEND_STRING PROPERTY COMPILE_FLAGS "-std=c++14")
+    elseif(COMPILER_SUPPORTS_CXX11)
-    set_property(TARGET ${PROJECT_NAME}-test_time APPEND_STRING PROPERTY COMPILE_FLAGS "-std=c++11")
+      set_property(TARGET ${PROJECT_NAME}-test_time APPEND_STRING PROPERTY COMPILE_FLAGS "-std=c++11")
+    endif()

     target_link_libraries(${PROJECT_NAME}-test_time ${catkin_LIBRARIES} rostime)
   endif()
 endif()
moriarty commented 6 years ago

I just saw the most recent commit in this repo has a nicer way. Use CHECK_CXX_SOURCE_COMPILES( to check if a C++11 feature is available, if it does not compile then set -std=c++11 otherwise don't explicitly set it.

dirk-thomas commented 6 years ago

What is the purpose of removing the C++11 option? Just because Melodic could leverage C++14 doesn't mean it has to.

moriarty commented 6 years ago

For Clarity and consistency, I gave a longer answer here:

https://github.com/ros/ros_comm/pull/1525#issuecomment-434113475

dirk-thomas commented 6 years ago

Since you mentioned in https://github.com/ros/ros_comm/pull/1525#issuecomment-434442230 that there is no technical reason to change this I will close this PR since the extra effort to check the supported compiler version (since kinetic-devel is used for multiple ROS distros) has no advantage.