ros2 / ros2/rclcpp

rclcpp warnings not showing as expected

Open
#3,176 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
805
Forks
564
Avg merge
1d 17h
Merged PRs (30d)
27

Description

Background

To my best knowledge, there are some "bad patterns" we try to avoid when using rclcpp. Two examples that come to my mind:

1. Using rclcpp logging macros without format arguments:

std::string my_std_string = "Foo";
RCLCPP_DEBUG(get_logger(), my_std_string);

This is documented in the galactic changelog: https://docs.ros.org/en/rolling/Releases/Release-Galactic-Geochelone.html#change-in-rclcpp-s-logging-macros

2. Using non-const shared_ptr in subscription callbacks:

sub_ = this->create_subscription(
    input,
    10,
    [](std::shared_ptr msg) { ... }
);

This is documented in the Jazzy changelog: https://docs.ros.org/en/rolling/Releases/Release-Jazzy-Jalisco.html#deprecated-subscription-callback-signatures-were-removed

Problem

When using these "bad patterns", I believe the expected behavior is a compiler warning/error like:

/Users/maurice/nav2_projects/nav2_ws/src/navigation2/nav2_behaviors/plugins/spin.cpp:66:27: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
   66 |     RCLCPP_ERROR(logger_, error_msg.c_str());
      |                           ^~~~~~~~~~~~~~~~~
/Users/maurice/nav2_projects/nav2_ws/.pixi/envs/default/include/rclcpp/rclcpp/logging.hpp:753:82: note: expanded from macro 'RCLCPP_ERROR'
  753 | #define RCLCPP_ERROR(logger, ...) RCLCPP_LOG(RCUTILS_LOG_SEVERITY_ERROR, logger, __VA_ARGS__)
      |                                                                                  ^~~~~~~~~~~
/Users/maurice/nav2_projects/nav2_ws/.pixi/envs/default/include/rclcpp/rclcpp/logging.hpp:48:54: note: expanded from macro 'RCLCPP_LOG'
   48 |     RCUTILS_LOG_NAMED(severity, (logger).get_name(), __VA_ARGS__); \
      |                                                      ^~~~~~~~~~~
/Users/maurice/nav2_projects/nav2_ws/.pixi/envs/default/include/rcutils/rcutils/logging_macros.h:56:73: note: expanded from macro 'RCUTILS_LOG_NAMED'
   56 |       rcutils_log_internal(&__rcutils_logging_location, severity, name, __VA_ARGS__); \

or

/Users/maurice/nav2_projects/nav2_ws/.pixi/envs/default/include/rclcpp/rclcpp/node_impl.hpp:100:18: note: in instantiation of function template specialization 'rclcpp::create_subscription<tf2_msgs::msg::TFMessage_<std::allocator<void>>, (lambda at /Users/maurice/nav2_projects/nav2_ws/src/navigation2/nav2_loopback_sim/test/test_loopback_simulator.cpp:157:5), std::allocator<void>, rclcpp::Subscription<tf2_msgs::msg::TFMessage_<std::allocator<void>>>, rclcpp::message_memory_strategy::MessageMemoryStrategy<tf2_msgs::msg::TFMessage_<std::allocator<void>>>, rclcpp::Node>' requested here
  100 |   return rclcpp::create_subscription<MessageT>(
      |                  ^
/Users/maurice/nav2_projects/nav2_ws/src/navigation2/nav2_loopback_sim/test/test_loopback_simulator.cpp:155:31: note: in instantiation of function template specialization 'rclcpp::Node::create_subscription<tf2_msgs::msg::TFMessage_<std::allocator<void>>, (lambda at /Users/maurice/nav2_projects/nav2_ws/src/navigation2/nav2_loopback_sim/test/test_loopback_simulator.cpp:157:5), std::allocator<void>, rclcpp::Subscription<tf2_msgs::msg::TFMessage_<std::allocator<void>>>, rclcpp::message_memory_strategy::MessageMemoryStrategy<tf2_msgs::msg::TFMessage_<std::allocator<void>>>>' requested here
  155 |   auto tf_sub = helper_node_->create_subscription<tf2_msgs::msg::TFMessage>(
      |                               ^
/Users/maurice/nav2_projects/nav2_ws/.pixi/envs/default/include/rclcpp/rclcpp/any_subscription_callback.hpp:456:5: note: 'set_deprecated<tf2_msgs::msg::TFMessage_<std::allocator<void>>>' has been explicitly marked deprecated here
  456 |   [[deprecated("use 'void(std::shared_ptr<const MessageT>)' instead")]]

However, these warnings/errors only showed up yesterday when I compiled on macOS (pixi + robotstack), and not in my usual workflow (e.g. pulling a fresh ros:rolling image and doing a colcon build), even with -Wall.

I also quickly searched for ::SharedPtr msg across ros2 repos and found that the pattern is still being used : https://docs.ros.org/en/rolling/Tutorials/Advanced/Writing-a-Buffer-Compatible-Conversions-Package.html#publish-and-subscribe, which makes me think CI also doesn't report the warning, same as what I see on Ubuntu.

Additional Information

I would also like to share some of my findings:

My guess (not validated yet) is that the warnings introduced in 2021 were visible at the time, but then something happened between 2021 and 2022 that accidentally hid them, and the problem persists until today.

  • When I worked on #2975, I first used static_assert to block the bad pattern, I also found the packages using it and fixed them. Then, we agreed to deprecate for one distribution first, so I switched from static_assert to [[deprecated]]. If I remember correctly, I could see the warnings on Ubuntu at that point, which is why I added:
  #define RCLCPP_AVOID_DEPRECATIONS_FOR_UNIT_TESTS 1

But I admittedly didn't carefully recheck everything after switching to [[deprecated]], e.g. testing in downstream packages. So my guess (not validated yet) is that the warning shows up inside rclcpp itself but not in downstream packages consuming it.

I am using macOS (warnings visible) and Ubuntu (no warnings at all) to refer to these two cases, but my best guess is that it might be related to compiler differences or compiler flags. I don't have much time to dig deeper right now, but I am documenting this here so I don't forget, and I hope to get some insights from the community or the PMCs :)

This issue was originally written by hand, assisted by Claude Sonnet 4.6 for formatting the logs, code blocks, and organizing the content more clearly, and finally edited by the author.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reproducing the warning behavior with a fresh ros:rolling image and colcon build, then compare it with the macOS pixi + robotstack result. Inspect rclcpp/logging.hpp, node_impl.hpp, and any_subscription_callback.hpp alongside the cited examples and history. Done means identifying why downstream warnings differ and documenting or addressing the confirmed cause.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
api, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.