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

For clarity and consistency. The lack of triggered discussions on the moveit issue which I linked to above. But @clalancette has recently answered on that issue.

I opened ros/pluginlib#131, ros/roscpp_core#94, and this issue to create consistency throughout the ros_comm stack.

The REP reads:

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

And in downstream packages when we bump to C++14 (and actually want to take advantage of the features) there is discussion if we can safely use C++14 while some things were still being compiled as C++11.

To summarize the other issue

@gavanderhoorn wrote:

@davetcoleman wrote:

Melodic is C++14
http://www.ros.org/reps/rep-0003.html#melodic-morenia-may-2018-may-2023

Bit late, but I don't think you can say this.

The language used in the REP is:

Targeted Languages:

• C++14

but, if we look at how roscpp is compiled (for instance, there are many other packages that do similar things):

[ 33%] Building CXX object CMakeFiles/roscpp.dir/src/libros/master.cpp.o
/usr/bin/c++  -DROSCONSOLE_BACKEND_LOG4CXX -DROS_PACKAGE_NAME=\"roscpp\" -Droscpp_EXPORTS -I/tmp/binarydeb/ros-melodic-roscpp-1.14.3/obj-x86_64-linux-gnu/devel/include -I/tmp/binarydeb/ros-melodic-roscpp-1.14.3/include -I/tmp/binarydeb/ros-melodic-roscpp-1.14.3/obj-x86_64-linux-gnu/devel/include/ros -I/opt/ros/melodic/include -I/opt/ros/melodic/share/xmlrpcpp/cmake/../../../include/xmlrpcpp -I/tmp/binarydeb/ros-melodic-roscpp-1.14.3/obj-x86_64-linux-gnu  -g -O2 -fdebug-prefix-map=/tmp/binarydeb/ros-melodic-roscpp-1.14.3=. -fstack-protector-strong -Wformat -Werror=format-security -DNDEBUG -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC   -std=c++11 -Wall -Wextra -o CMakeFiles/roscpp.dir/src/libros/master.cpp.o -c /tmp/binarydeb/ros-melodic-roscpp-1.14.3/src/libros/master.cpp

Note the -std=c++11 in there.

Edit: C++11 explicitly set here.

• [C++14] Use C++14 in Melodic moveit/moveit#1146 (comment)

@clalancette wrote:

So, I originally wrote the update to REP-0003 for Melodic. And looking at it again, I agree that the wording is somewhat ambiguous. @gavanderhoorn interpretation is correct; packages are free to use C++14 in Melodic, but are not required to do so. I'd happily review and merge any changes to REP-0003 that makes this less ambiguous.

• ([C++14] Use C++14 in Melodic moveit/moveit#1146 (comment))

I think the REP-0003 is clear:

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

These PRs (ros/pluginlib#131, ros/roscpp_core#94, and this #1525) are not actually using any C++14 features, they are just removing any doubts or concerns about actually using C++14 when there are 4 packages within the ros_comm stack which are compiled with -std=c++11

./ros_comm/xmlrpcpp/test/CMakeLists.txt:set_target_properties(test_base64 PROPERTIES COMPILE_FLAGS -std=c++11)
./ros_comm/roscpp/CMakeLists.txt:  set_directory_properties(PROPERTIES COMPILE_OPTIONS "-std=c++11;-Wall;-Wextra")
./pluginlib/CMakeLists.txt:  check_cxx_compiler_flag("-std=c++11" COMPILER_SUPPORTS_CXX11)
./pluginlib/CMakeLists.txt:      set_target_properties(${PROJECT_NAME}_unique_ptr_test PROPERTIES COMPILE_FLAGS -std=c++11 LINK_FLAGS -std=c++11)
./roscpp_core/rostime/CMakeLists.txt:    set_property(TARGET ${PROJECT_NAME}-test_time APPEND_STRING PROPERTY COMPILE_FLAGS "-std=c++11")

This type of clarity isn't too important with minor bumps to C++ standards, C++11->C++14 but C++11->C++17 and C++20 will be large bumps to the C++ standards... And for those future bumps I think it is important that ros_comm sets clear precedent and removes ambiguity by compiling with the standard which is being used by that release.

Edit: I linked to the wrong PR ros/roscpp_core#9 -> ros/roscpp_core#94

Is there any technical reason why "consistency" is needed? Are you aware of any incompatibilities between C++11 and C++14 libraries?

No, there is no technical reason and I'm not aware of any incompatibilities between C++11 -> C++14.

Internally, with regards to compatibility issues, we were going to switch to C++14 and it turned out to be as simple as changing the compile option.
So we soon after went straight to C++17 for some internal components. We did come across one interesting edge cases, but it was caught during the unit/integration tests. The code compiled under all, but only passed the tests when compiled with C++11 & C++14... It was a few months ago, but if I remember it involved some the new std:basic_string_view and type deduction + lambda capture + gcc optimization... it's not something I would be worried about here.

@ros-pull-request-builder retest this please

Thanks for re-triggering the tests, is there a list of known unstable tests?

is there a list of known unstable tests?

No, there is not. The recent non-green devel and PR builds for the package give an idea which tests are flaky though.

@ros-pull-request-builder retest this please