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

Removing some boost usage #1331

Merged
merged 29 commits into from
Jun 15, 2022
Merged

Removing some boost usage #1331

merged 29 commits into from
Jun 15, 2022

Conversation

henrygerardmoore
Copy link
Contributor

@henrygerardmoore henrygerardmoore commented Jun 9, 2022

Description

Taking over from #1118, this is part 1 of the work that will be done. More boost usage can be (somewhat) easily removed, but the PR is already quite large so that will take place in another PR. The changes included in this PR are

  • boost::array -> std::array
  • boost::function -> std::function
  • boost::mutex and similar objects (boost::shared_lock etc) -> std::mutex
  • boost::starts_with(name, object.first) -> name.rfind(object.first, 0) == 0
  • boost::math::tools::epsilon<double>() -> std::numeric_limits<double>::min()
  • boost::filesystem -> std::filesystem
  • boost::thread -> std::thread
  • Adding now-necessary includes
  • Changing some calls that differ between boost and the std equivalent

In a future PR, the following will be done:

  • replace boost::regex
  • replace boost::noncopyable
  • replace boost::lexical_cast
  • replace boost::optional
  • replace boost::thread_group

When MoveIt is updated to C++20, any use of boost::math::constants can be replaced with std::numbers constants

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend the tutorials / documentation reference
  • Document API changes relevant to the user in the MIGRATION.md notes
  • Create tests, which fail without this PR reference
  • Include a screenshot if changing a GUI
  • While waiting for someone to review your request, please help review another open pull request to support the maintainers

@codecov
Copy link

codecov bot commented Jun 9, 2022

Codecov Report

Merging #1331 (46670a2) into main (212af0f) will increase coverage by 0.02%.
The diff coverage is 29.47%.

@@            Coverage Diff             @@
##             main    #1331      +/-   ##
==========================================
+ Coverage   61.57%   61.58%   +0.02%     
==========================================
  Files         274      274              
  Lines       24965    24965              
==========================================
+ Hits        15369    15372       +3     
+ Misses       9596     9593       -3     
Impacted Files Coverage Δ
...lude/moveit/collision_detection/collision_common.h 83.88% <ø> (ø)
...lude/moveit/collision_detection/collision_matrix.h 100.00% <ø> (ø)
...include/moveit/collision_detection/occupancy_map.h 0.00% <ø> (ø)
...tection/include/moveit/collision_detection/world.h 100.00% <ø> (ø)
..._core/collision_detection/src/collision_matrix.cpp 37.15% <ø> (ø)
...e/collision_detection_fcl/src/collision_common.cpp 73.76% <ø> (ø)
...sion_distance_field/collision_env_distance_field.h 4.77% <ø> (ø)
...it/collision_distance_field/collision_env_hybrid.h 0.00% <ø> (ø)
...e/include/moveit/kinematics_base/kinematics_base.h 75.76% <ø> (ø)
...ene/include/moveit/planning_scene/planning_scene.h 43.55% <ø> (ø)
... and 37 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c491ac1...46670a2. Read the comment docs.

@henrygerardmoore henrygerardmoore changed the title Removing boost Removing some boost usage Jun 10, 2022
@Abishalini Abishalini mentioned this pull request Jun 10, 2022
6 tasks
@henrygerardmoore henrygerardmoore marked this pull request as ready for review June 10, 2022 21:05
@mergify
Copy link

mergify bot commented Jun 13, 2022

This pull request is in conflict. Could you fix it @henrygerardmoore?

Copy link
Member

@henningkayser henningkayser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for continuing on this effort. It's a lot of changes, but I assume it's fine squash-merging this since everything is pretty straight-forward. @Abishalini should be mentioned as co-author as she has prepared a sizeable portion of these changes (this can be added with the squash-merge commit).

@henrygerardmoore Are there any packages that don't depend on boost anymore? This should be reflected in the CMakeLists.txt fiels, possibly also in the package.xml dependencies. Could you verify this?

moveit_core/collision_detection/src/world.cpp Outdated Show resolved Hide resolved
@mergify
Copy link

mergify bot commented Jun 14, 2022

This pull request is in conflict. Could you fix it @henrygerardmoore?

@henrygerardmoore
Copy link
Contributor Author

Removed boost from as many CMakeLists as I think is possible, and also made sure it wasn't unnecessarily in any package.xml files. I believe the PR should be good to go now.

Copy link
Contributor

@Abishalini Abishalini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found some headers that can be removed

Copy link
Contributor

@Abishalini Abishalini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thanks for this

Copy link
Member

@tylerjw tylerjw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for taking this on and getting it over the finish line!

@tylerjw tylerjw merged commit 5ce32f9 into moveit:main Jun 15, 2022
This was referenced Jun 16, 2022
@henrygerardmoore henrygerardmoore deleted the remove_boost branch June 17, 2022 16:37
mikeferguson added a commit to mikeferguson/moveit2 that referenced this pull request Jun 21, 2022
The noted PR swapped boost::thread to std::thread - however
std::thread will cause the program to crash with a note that
TERMINATE CALLED WITHOUT ACTIVE EXCEPTION if the thread object
goes out of scope while not joinable. Detaching the thread
restores the original workflow prior to the PR and allows
users to send more than one trajectory via the motion planning
plugin in RVIZ (otherwise, it crashes on the first trajectory).
mikeferguson added a commit that referenced this pull request Jun 21, 2022
The noted PR swapped boost::thread to std::thread - however
std::thread will cause the program to crash with a note that
TERMINATE CALLED WITHOUT ACTIVE EXCEPTION if the thread object
goes out of scope while not joinable. Detaching the thread
restores the original workflow prior to the PR and allows
users to send more than one trajectory via the motion planning
plugin in RVIZ (otherwise, it crashes on the first trajectory).
peterdavidfagan pushed a commit to peterdavidfagan/moveit2 that referenced this pull request Jul 14, 2022
Co-authored-by: Vatan Aksoy Tezer <vatan@picknik.ai>
peterdavidfagan pushed a commit to peterdavidfagan/moveit2 that referenced this pull request Jul 14, 2022
The noted PR swapped boost::thread to std::thread - however
std::thread will cause the program to crash with a note that
TERMINATE CALLED WITHOUT ACTIVE EXCEPTION if the thread object
goes out of scope while not joinable. Detaching the thread
restores the original workflow prior to the PR and allows
users to send more than one trajectory via the motion planning
plugin in RVIZ (otherwise, it crashes on the first trajectory).
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

Successfully merging this pull request may close these issues.

None yet

5 participants