Migrating to Kinetic, MoveIt! needs C++11

[pbeeson]

Industrial Core seems to compile under Kinetic, only once you force industrial_trajectory_filters to use C++11.

[pbeeson]

14.04 / Indigo also should support C++-11, so this should be a straight forward backport.

[shaun-edwards]

Thanks for the contribution @pbeeson.

I'm not an expert here, so perhaps @Jmeyer1292 or @gavanderhoorn could chime in.

I've see this done two "other" ways:

# Check for c++11 compatibility 
include(CheckCXXCompilerFlag)
CHECK_CXX_COMPILER_FLAG("-std=c++11" COMPILER_SUPPORTS_CXX11)
if(COMPILER_SUPPORTS_CXX11)
    set(CMAKE_CXX_FLAGS "-std=c++11 -fPIC") # Additional flags as determined
else()
    message(FATAL_ERROR "The compiler ${CMAKE_CXX_COMPILER} has no C++11 support. Please use a different C++ compiler.")
endif()

OR... target_compile_options(<main> PUBLIC -std=c++11 -Wall -Wextra) (not sure what the -W's are for)

It's my understanding that the later is the desired method since it only applies to a single target. Anybody else have any thoughts?

[pbeeson]

Your CHECK_CXX method is indeed more generic (though you don't need fpic or -W flags added).

[VictorLamoine]

• A more elegant solution to enable C++11 in CMake projects: https://github.com/davetcoleman/rviz_visual_tools/blob/kinetic-devel/CMakeLists.txt#L4-L11

• Mixing different C++ standards is generally not a great idea: https://gcc.gnu.org/wiki/Cxx11AbiCompatibility http://stackoverflow.com/questions/6494101/any-issues-with-mixing-libraries-with-and-without-std-c0x

[gavanderhoorn]

@VictorLamoine: afaik, add_definitions(..) should not be used to add compiler flags (net result is the same, but semantics are different).

re: mixing C++ standards: no, that is not a good idea, but industrial_trajectory_filters is sufficiently stand-alone that it would seem possible to do it in this case. To avoid overriding CXX_FLAGS for an entire workspace (in the case of catkin_make), using target_compile_options(..) could potentially be used.

[shaun-edwards]

@pbeeson, did you to add a commit to the PR...see 3ae0527

[shaun-edwards]

Based on feedback from @gavanderhoorn and the fact that a lot of people use catkin make as opposed to catkin_tools, it appears that target_compile_options(<main> PUBLIC -std=c++11) is the preferred way to do this.

The travis jobs appear to be failing due to socket connection issues (this is a known issue).

[gavanderhoorn]

@shaun-edwards wrote:

Based on feedback from @gavanderhoorn [..]

what I forgot to factor in is that trajectory_filters is a library-only package. It doesn't have any stand-alone nodes, meaning the only way to use it is by linking it to other binaries.

We then get into the problems that @VictorLamoine hinted at: anyone using these libraries will have to compile their own sources with -std=c++11 as well, or it won't work.

With MoveIt, everything was migrated in one go. I'm guessing we will probably have to do this here as well, or we'll have to explicitly document somewhere that only trajectory_filters required C++11 for now.

[shaun-edwards]

Well, since we mostly depend on MoveIt, should we just make the leap to C++ 11?

What isn't clear to me is how this is or is not an issue with ROS itself? How is it that we aren't seen issues with a dependency on ROS (not sure if it has moved to C++ 11 yet)?

[mathias-luedtke]

@pbeeson: 14.04 / Indigo also should support C++-11

ROS indigo enforces C++03, C++11 is an option since jade (http://www.ros.org/reps/rep-0003.html)

@shaun-edwards: Well, since we mostly depend on MoveIt, should we just make the leap to C++ 11?

👍 , but not for indigo Update: moveit switched for kinetic only, jade is still using c++03.

@shaun-edwards: What isn't clear to me is how this is or is not an issue with ROS itself?

"Support for C++11 is now a compiler requirement, but the API of the packages included in desktop-full will not use any C++11-specific feature. External packages are encouraged to follow this guideline." (http://www.ros.org/reps/rep-0003.html#c)

@shaun-edwards: it appears that target_compile_options(<main> PUBLIC -std=c++11) is the preferred way to do this.

According to https://github.com/ros-planning/moveit/issues/289#issuecomment-269872254 the recommended way is add_compile_options(-std=c++11) This has to be done for all downstream packages as well, as @gavanderhoorn already pointed out.

[shaun-edwards]

Yet another way to ad C++11 option? Based on some quick searching, it appears add_compile_options is more global than a target by target setting with target_compile_options. It isn't clear (to me) if this will have an effect on the package or the entire workspace. Based on @gavanderhoorn's comment:

To avoid overriding CXX_FLAGS for an entire workspace (in the case of catkin_make), using target_compile_options(..) could potentially be used.

@ipa-mdl, do you still recommend add_compile_options? Do you know if it applies to the whole workspace?

[VictorLamoine]

It only applies the current and sub-directories; https://cmake.org/cmake/help/v3.0/command/add_compile_options.html

I think we should use the same way MoveIt enables C++11. All of the suggested approach would work.

:+1: for this fix (with add_compile_options), it enables compiling industrial_core on Kinetic (and thus, also allows to use Travis)

[mathias-luedtke]

@ipa-mdl, do you still recommend add_compile_options?

I have never tested any of the c++11 support options myself (still on c++03 code and indigo..). That's why I have referred to @davetcoleman's answer.

And I agree with @VictorLamoine:

I think we should use the same way MoveIt enables C++11.

[pbeeson]

I believe #161 is a more proper fix.