Skip to content

👩‍🌾 [ros2] Fix clang warning about hidden overloads#196

Merged
clalancette merged 1 commit into
ros-perception:ros2from
chapulina:chapulina/ros2/overload_warning
Jul 1, 2021
Merged

👩‍🌾 [ros2] Fix clang warning about hidden overloads#196
clalancette merged 1 commit into
ros-perception:ros2from
chapulina:chapulina/ros2/overload_warning

Conversation

@chapulina
Copy link
Copy Markdown
Contributor

Clang is overly cautions about virtual overloads. After #195 was merged, we started getting this warning in the macOS nightlies:

01:56:54 In file included from /Users/osrf/jenkins-agent/workspace/nightly_osx_release/ws/src/ros-perception/image_common/image_transport/src/manifest.cpp:38:
01:56:54 /Users/osrf/jenkins-agent/workspace/nightly_osx_release/ws/src/ros-perception/image_common/image_transport/include/image_transport/raw_subscriber.hpp:73:8: warning: 'image_transport::RawSubscriber::subscribeImpl' hides overloaded virtual function [-Woverloaded-virtual]
01:56:54   void subscribeImpl(
01:56:54        ^
01:56:54 /Users/osrf/jenkins-agent/workspace/nightly_osx_release/ws/src/ros-perception/image_common/image_transport/include/image_transport/simple_subscriber_plugin.hpp:115:8: note: hidden overloaded virtual function 'image_transport::SimpleSubscriberPlugin<sensor_msgs::msg::Image_<std::__1::allocator<void> > >::subscribeImpl' declared here: different number of parameters (4 vs 5)
01:56:54   void subscribeImpl(
01:56:54        ^
01:56:54 1 warning generated.

I tested locally on Ubuntu Focal with clang 10 that adding this line gives clang some peace of mind.


https://github.com/osrf/buildfarmer/issues/207

Signed-off-by: Louise Poubel <louise@openrobotics.org>
Copy link
Copy Markdown
Contributor

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

LGTM

@chapulina
Copy link
Copy Markdown
Contributor Author

Can we get this in before the next nightly, @mjcarroll ? Thanks!

@clalancette
Copy link
Copy Markdown
Contributor

Here's CI for this:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@clalancette
Copy link
Copy Markdown
Contributor

OK, CI is green here. I'm going to go ahead and merge this one to try and make our nightly CI happier.

@clalancette clalancette merged commit 5b6f1b9 into ros-perception:ros2 Jul 1, 2021
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.

4 participants