Variable name in macro shadows other variable

[felixendres]

Hi,

The logging code here triggers a lot of warnings on building. It is because a macro uses the very simple variable name ret, which is also used by the rclcpp function borrow_serialized_message (on my system in /opt/ros/dashing/include/rclcpp/message_memory_strategy.hpp L103) which uses that macro.

I would suggest to skip the assignment to the variable, as it is only used once. Alternatively, use a more unique name that is not likely to be used outside of the macro.

The full warning in question:

/opt/ros/dashing/include/rclcpp/message_memory_strategy.hpp:100:16: warning: declaration of ‘ret’ shadows a previous local [-Wshadow]
           auto ret = rmw_serialized_message_fini(msg);
                ^~~
/opt/ros/dashing/include/rclcpp/message_memory_strategy.hpp:93:10: note: shadowed declaration is here
     auto ret = rmw_serialized_message_init(msg, capacity, &rcutils_allocator_);
          ^~~
In file included from /opt/ros/dashing/include/rmw/types.h:28:0,
                 from /opt/ros/dashing/include/rcl/types.h:18,
                 from /opt/ros/dashing/include/rcl/arguments.h:20,
                 from /opt/ros/dashing/include/rcl/context.h:26,
                 from /opt/ros/dashing/include/rcl/guard_condition.h:24,
                 from /opt/ros/dashing/include/rclcpp/executor.hpp:29,
                 from /opt/ros/dashing/include/rclcpp/executors/multi_threaded_executor.hpp:24,
                 from /opt/ros/dashing/include/rclcpp/executors.hpp:21,
                 from /opt/ros/dashing/include/rclcpp/rclcpp.hpp:144,
                 from ../../../../../xxx.h:12,
                 from ../../../../../xxx.cpp:5:
/opt/ros/dashing/include/rcutils/logging.h:543:19: warning: declaration of ‘ret’ shadows a previous local [-Wshadow]
     rcutils_ret_t ret = rcutils_logging_initialize(); \
                   ^
/opt/ros/dashing/include/rcutils/logging_macros.h:68:5: note: in expansion of macro ‘RCUTILS_LOGGING_AUTOINIT’
     RCUTILS_LOGGING_AUTOINIT \
     ^~~~~~~~~~~~~~~~~~~~~~~~
/opt/ros/dashing/include/rcutils/logging_macros.h:994:3: note: in expansion of macro ‘RCUTILS_LOG_COND_NAMED’
   RCUTILS_LOG_COND_NAMED( \
   ^~~~~~~~~~~~~~~~~~~~~~
/opt/ros/dashing/include/rclcpp/message_memory_strategy.hpp:103:13: note: in expansion of macro ‘RCUTILS_LOG_ERROR_NAMED’
             RCUTILS_LOG_ERROR_NAMED(
             ^~~~~~~~~~~~~~~~~~~~~~~
In file included from /opt/ros/dashing/include/rclcpp/subscription.hpp:40:0,
                 from /opt/ros/dashing/include/rclcpp/callback_group.hpp:26,
                 from /opt/ros/dashing/include/rclcpp/any_executable.hpp:20,
                 from /opt/ros/dashing/include/rclcpp/memory_strategy.hpp:24,
                 from /opt/ros/dashing/include/rclcpp/memory_strategies.hpp:18,
                 from /opt/ros/dashing/include/rclcpp/executor.hpp:33,
                 from /opt/ros/dashing/include/rclcpp/executors/multi_threaded_executor.hpp:24,
                 from /opt/ros/dashing/include/rclcpp/executors.hpp:21,
                 from /opt/ros/dashing/include/rclcpp/rclcpp.hpp:144,
                 from ../../../../../xxx.h:12,
                 from ../../../../../xxx.cpp:5:
/opt/ros/dashing/include/rclcpp/message_memory_strategy.hpp:100:16: note: shadowed declaration is here
           auto ret = rmw_serialized_message_fini(msg);
                ^~~

[clalancette]

I would suggest to skip the assignment to the variable, as it is only used once.

Yep, that seems totally reasonable to me. We should also consider wrapping the whole thing in a do..while(0) loop to avoid other macro badness. Can you submit a pull request to do that? Thanks.

[fujitatomoya]

@clalancette I think you can close this.

[clalancette]

@fujitatomoya Good call, fixed by #290, closing. Thanks.