C++11 std::shared_ptr in public API

This is a follow-up from our previous discussion about public API and C++11. I would like to propose in ROS-M we remove the no std::shared_ptrs in the public API. Remove from REP-003:

The API of the packages included in desktop-full will not use any C++11-specific feature

This proposal is motivated by annoying shims that have been put in place to allow MoveIt! to use std::shared_ptrs but still hide them externally. Currently in Lunar we are having the issue with urdf_model still using boost but urdfdom_headers using std::shared_ptr.

I’m not saying it is mandatory everyone switch to shared_ptrs, but it would be nice for authors/maintainers to have the option even if their package is in desktop-full. Thoughts?

2 Likes

Been there, done that. Both with MoveIt and ros_control suffers/suffered (?) from this.

The referred lines were added by me precisely with the purpose of changing those macros.

There were plenty of code using non-typedef-ed shared_ptr, the above two projects got through the trouble by adapting the macros, I have no knowledge of others.

There is an effort for finally brinding the urdf:: together.

I think that C++11-compatible compilers are wide-spread enough now (at least on Linux) that using C++11 in the external API should not be a problem.

@bmagyar the effort you link to does not address boost vs std shared pointers, or C++11, but rather only splitting out the packages into separate repos.

Yep, that’s the main goal there. Getting changes through there hasn’t been easy as there are many different things that share the release cycle and dependencies. urdf hasn’t been moved yet, you could push the changes in now.

The previous comment from @tfoote still applies: Std::shared_ptr in the geometric_shapes public API

Changing from boost to std shared_ptr in e.g. ROS-M would require all consumers of that API to update their code. Each package needs to make that decision on its own. But for many “core” ROS packages that might simply not be feasible or desired. So this downside needs to be taken into account when considering such a change.

That was a thorough comment @tfoote wrote and a good reminder of the rationale. I don’t know if ROS-M will still target Ubuntu 16.04, but if not, gcc6 will be on all Ubuntu distros already (starting with 16.10) so the possibility is there.

Following the previous targeted platforms, ROS-Melodic will target ubuntu 17.10 (last LTS - 1) and ubuntu 18.04 (last LTS) so they will both have gcc >=6.3.0.
The compiler support is not going to be a blocker here. I think a discussion with the community on the next iteration of REP3 for ROS Melodic is required to find a good compromise between leveraging new features v.s. stability of the APIs for all downstream packages (as pointed out by @tfoote and @dirk-thomas).