Catch exceptions by reference

[nlimpert]

GCC 8 raises errors on exceptions caught by value.

[DLu]

Hey Nicolas, thanks for the contribution.

1) Can you tell me more about what your compilation environment is so I can perhaps replicate the error. i.e. what's your OS/ROS Distro/compile command? 2) On the style front, would you mind changing the spacing on the references so that it matches the existing style, i.e. catch (nav_core2::PlannerException& e) instead of catch (nav_core2::PlannerException &e)

[nlimpert]

Hi David,

thanks for your reply!

Can you tell me more about what your compilation environment is so I can perhaps replicate the error. i.e. what's your OS/ROS Distro/compile command?

My system looks as follows:

• OS: Fedora 28
• ROS Distro: kinetic
• compile command: catkin build

Additional info:

$ catkin --version
catkin_tools 0.4.4 (C) 2014-2019 Open Source Robotics Foundation
catkin_tools is released under the Apache License, Version 2.0 (http://www.apache.org/licenses/LICENSE-2.0)
\---
Using Python 2.7.15 (default, Oct 15 2018, 15:24:06) [GCC 8.1.1 20180712 (Red Hat 8.1.1-5)]`

$ gcc --version
gcc (GCC) 8.2.1 20181215 (Red Hat 8.2.1-6)
[...]

The actual errors raised by GCC (e.g. for the global_planner_tests package) are as follows:

Errors     << global_planner_tests:make /home/nlimpert/dev/tmp_ws/logs/global_planner_tests/build.make.001.log                        
/home/nlimpert/dev/tmp_ws/src/robot_navigation/global_planner_tests/src/global_planner_tests.cpp: In function ‘bool global_planner_tests::planExists(nav_core2::GlobalPlanner&, nav_2d_msgs::Pose2DStamped, nav_2d_msgs::Pose2DStamped)’:
/home/nlimpert/dev/tmp_ws/src/robot_navigation/global_planner_tests/src/global_planner_tests.cpp:131:38: error: catching polymorphic type ‘class nav_core2::PlannerException’ by value [-Werror=catch-value=]
   catch (nav_core2::PlannerException e)
                                      ^
/home/nlimpert/dev/tmp_ws/src/robot_navigation/global_planner_tests/src/global_planner_tests.cpp: In function ‘bool global_planner_tests::checkOccupiedPathCoverage(nav_core2::GlobalPlanner&, const PoseList&, const PoseList&, const string&, bool, bool, bool, bool)’:
/home/nlimpert/dev/tmp_ws/src/robot_navigation/global_planner_tests/src/global_planner_tests.cpp:192:48: error: catching polymorphic type ‘class nav_core2::OccupiedStartException’ by value [-Werror=catch-value=]
       catch (nav_core2::OccupiedStartException e)
                                                ^
/home/nlimpert/dev/tmp_ws/src/robot_navigation/global_planner_tests/src/global_planner_tests.cpp:197:47: error: catching polymorphic type ‘class nav_core2::OccupiedGoalException’ by value [-Werror=catch-value=]
       catch (nav_core2::OccupiedGoalException e)
                                               ^
/home/nlimpert/dev/tmp_ws/src/robot_navigation/global_planner_tests/src/global_planner_tests.cpp:202:42: error: catching polymorphic type ‘class nav_core2::PlannerException’ by value [-Werror=catch-value=]
       catch (nav_core2::PlannerException e)
                                          ^
/home/nlimpert/dev/tmp_ws/src/robot_navigation/global_planner_tests/src/global_planner_tests.cpp: In function ‘bool global_planner_tests::checkOutOfBoundsPathCoverage(nav_core2::GlobalPlanner&, const PoseList&, const PoseList&, const string&, bool, bool, bool, bool)’:
/home/nlimpert/dev/tmp_ws/src/robot_navigation/global_planner_tests/src/global_planner_tests.cpp:249:46: error: catching polymorphic type ‘class nav_core2::StartBoundsException’ by value [-Werror=catch-value=]
       catch (nav_core2::StartBoundsException e)
                                              ^
/home/nlimpert/dev/tmp_ws/src/robot_navigation/global_planner_tests/src/global_planner_tests.cpp:254:45: error: catching polymorphic type ‘class nav_core2::GoalBoundsException’ by value [-Werror=catch-value=]
       catch (nav_core2::GoalBoundsException e)
                                             ^
/home/nlimpert/dev/tmp_ws/src/robot_navigation/global_planner_tests/src/global_planner_tests.cpp:259:42: error: catching polymorphic type ‘class nav_core2::PlannerException’ by value [-Werror=catch-value=]
       catch (nav_core2::PlannerException e)
                                          ^
/home/nlimpert/dev/tmp_ws/src/robot_navigation/global_planner_tests/src/global_planner_tests.cpp: In function ‘bool global_planner_tests::hasNoPaths(nav_core2::GlobalPlanner&, const nav_core2::Costmap&, bool, bool, bool)’:
/home/nlimpert/dev/tmp_ws/src/robot_navigation/global_planner_tests/src/global_planner_tests.cpp:372:47: error: catching polymorphic type ‘class nav_core2::NoGlobalPathException’ by value [-Werror=catch-value=]
       catch (nav_core2::NoGlobalPathException e)
                                               ^
/home/nlimpert/dev/tmp_ws/src/robot_navigation/global_planner_tests/src/global_planner_tests.cpp:376:42: error: catching polymorphic type ‘class nav_core2::PlannerException’ by value [-Werror=catch-value=]
       catch (nav_core2::PlannerException e)
                                          ^
cc1plus: all warnings being treated as errors
make[2]: *** [CMakeFiles/global_planner_tests.dir/build.make:63: CMakeFiles/global_planner_tests.dir/src/global_planner_tests.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:1998: CMakeFiles/global_planner_tests.dir/all] Error 2
make: *** [Makefile:141: all] Error 2

---

On the style front, would you mind changing the spacing on the references so that it matches the existing style, i.e. catch (nav_core2::PlannerException& e) instead of catch (nav_core2::PlannerException &e)

Thanks for the note on the existing style - I missed that. I have updated the commits accordingly.

[mintar]

@ros-pull-request-builder retest this please

[mintar]

Tests should work now that #30 is merged. Let's wait and see.

[nlimpert]

Thanks for taking a look @mintar.

[DLu]

Thanks for the contribution! Sorry it took me awhile to test it.