search

appliedAI-Initiative / orb_slam_2_ros

A ROS implementation of ORB_SLAM2
Other
605 stars 282 forks source link

Implicitly-declared Frame assignment operator is deprecated because Frame has a copy constructor #124

Open clydemcqueen opened 2 years ago

clydemcqueen commented 2 years ago

In ROS2 Foxy (on the ros2 branch):

I'm getting a bunch of warnings of the form:

/home/clyde/ros2/orca3_ws/src/orb_slam_2_ros/orb_slam2/src/Tracking.cc: In member function ‘cv::Mat ORB_SLAM2::Tracking::GrabImageStereo(const cv::Mat&, const cv::Mat&, const double&)’:
/home/clyde/ros2/orca3_ws/src/orb_slam_2_ros/orb_slam2/src/Tracking.cc:180:137: warning: implicitly-declared ‘ORB_SLAM2::Frame& ORB_SLAM2::Frame::operator=(const ORB_SLAM2::Frame&)’ is deprecated [-Wdeprecated-copy]
  180 |     mCurrentFrame = Frame(mImGray,imGrayRight,timestamp,mpORBextractorLeft,mpORBextractorRight,mpORBVocabulary,mK,mDistCoef,mbf,mThDepth);
      |                                                                                                                                         ^
In file included from /home/clyde/ros2/orca3_ws/src/orb_slam_2_ros/orb_slam2/include/KeyFrame.h:29,
                 from /home/clyde/ros2/orca3_ws/src/orb_slam_2_ros/orb_slam2/include/MapPoint.h:24,
                 from /home/clyde/ros2/orca3_ws/src/orb_slam_2_ros/orb_slam2/include/FrameDrawer.h:25,
                 from /home/clyde/ros2/orca3_ws/src/orb_slam_2_ros/orb_slam2/include/Tracking.h:29,
                 from /home/clyde/ros2/orca3_ws/src/orb_slam_2_ros/orb_slam2/src/Tracking.cc:22:
/home/clyde/ros2/orca3_ws/src/orb_slam_2_ros/orb_slam2/include/Frame.h:49:5: note: because ‘ORB_SLAM2::Frame’ has user-provided ‘ORB_SLAM2::Frame::Frame(const ORB_SLAM2::Frame&)’
   49 |     Frame(const Frame &frame);
      |     ^~~~~

I'm not sure when this started appearing in the build, but it occurs because:

I looked at the Frame copy constructor and it appears to be a trivial constructor -- that is, we can delete it, and the compiler will provide an equivalent constructor based on std::memmove. I tried this for my use case (stereo camera) and I'm not seeing any adverse affects. I will submit a PR that removes the copy constructor.

The master branch doesn't define -Wextra so there is no warning, but the copy constructor is still deprecated and this patch could also be applied to that branch.