Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use c++11 instead of boost #21

Closed
scpeters opened this issue Sep 20, 2015 · 9 comments
Closed

Use c++11 instead of boost #21

scpeters opened this issue Sep 20, 2015 · 9 comments

Comments

@scpeters
Copy link
Contributor

This package uses just a very small amount of boost, much less than urdfdom (related issue: ros/urdfdom#62 ). So it should be easier to use this as a test case for how to start the move to c++11. I have made the necessary changes in scpeters@255abf6 but I wanted to wait before submitting a pull request to ask what the new branch should be called and what numbering to use.

Some questions:

  • Should this involve a major version bump to 1.0?
  • Should the current code that uses boost get moved to a release branch (i.e. release-0.2) and make the master branch use c++11?

@wjwwood

@scpeters
Copy link
Contributor Author

Finding the correct version will be a challenge since neither this nor urdfdom nor urdfdom_headers create cmake version config files. Also, the cmake config file for urdfdom currently calls find_package without a version specified when searching for its dependencies (urdfdom_headers and console_bridge).

In ign-math (used by gazebo and sdformat), we have been appending the major version number to the name of the pkg-config and cmake config files. I'm not sure if that's desired in this case.

@dirk-thomas
Copy link
Member

The current target platforms for ROS are defined in REP 3 (http://www.ros.org/reps/rep-0003.html). For all previous ROS releases C++03 was the minimum requirement and and that's why it can currently not use C++11.

Therefore it can only be considered for future releases. You might want to start a discussion about the minimum C++ version required for the next ROS version (k-turtle, May 2016). But please keep in mind that this implies that ROS would not be compilable anymore on platform which don't have a C++11 compatible compiler.

@scpeters
Copy link
Contributor Author

The default version of gcc in Ubuntu 12.04 (Precise) doesn't support c++11, but it is supported in 14.04 (trusty) and up. Since the minimum supported Ubuntu version for Jade is trusty, perhaps we could do it for K-turtle?

Should I start a thread on ros-users or is there a better venue?

@tfoote
Copy link
Member

tfoote commented Sep 21, 2015

I'd suggest a PR to REP 3 with a compatible proposal for kinetic and announcement of the discussion on ros-users@ to let people know the discussion is going on.

@dirk-thomas
Copy link
Member

After looking briefly in the code it looks like that console_bridge is using boost only internally in cpp files and does not expose it in the headers. If the usage of C++11 gets accepted that should be easy to update.

In other cases where the types are being exposed through the API it might be significantly more effort to update if every code using the API needs to be updated to.

@scpeters
Copy link
Contributor Author

Indeed, I have started a discussion at ros/urdfdom#62 where boost shared pointers are used in the public API.

@jacquelinekay
Copy link

In October, the ros2 branch implemented these changes:

787c487

We've also decided that C++11 compatibility will be required for Kinetic.

So it sounds like the way forward for this issue will be easy enough.

@scpeters
Copy link
Contributor Author

@jacquelinekay thanks for the pointer to the ros2 branch. I'll add those changes to #25

@scpeters
Copy link
Contributor Author

fixed by #25

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants