# RCLCPP logging macros not a single expression

I just discovered after a lot of confusion that the RCLCPP_ logging macros do not expand to a single expression and therefore cannot be used in brace-less if clauses and for loops. For example, this doesn't compile for me because the compiler sees an unexpected else:

if (true)
RCLCPP_INFO(node->get_logger(), "true");
else
RCLCPP_INFO(node->get_logger(), "false");


Is this expected?

Edit: added the mistakenly elided semi-colons from the original question

edit retag close merge delete

"Expected" I don't know, but yes, all RCLCPP_* macros are two-liners: see here.

Might be an oversight: perhaps they should be wrapped in do { } while(0); constructs?

( 2019-03-20 15:45:29 -0500 )edit

There was some discussion about this last fall here. I suspect that ;'s might help you in the above example, but, otherwise, ROS 2 linters require curly braces on all if statements, so it's understandable as to how this was not caught earlier.

( 2019-03-20 15:54:23 -0500 )edit

Sort by » oldest newest most voted

@gvdhoorn is correct, it's an oversight that you cannot do:

if (condition)
RCLCPP_INFO(...);
else
RCLCPP_INFO(...);


The macro's contents should be wrapped within a do { ... } while(0). This is done in the equivalent C API's, see:

https://github.com/ros2/rcutils/blob/...

But the rclcpp one's do not do this:

https://github.com/ros2/rclcpp/blob/4...

I'd recommend opening an issue on rclcpp and ideally providing a pull request to help us fix it :D

more

Thanks, I was really confused that no one seemed to be bothered that function-call semantics had been broken, knowing that many more people would be falling into this non-obvious trap. I will be able to open a PR against rclcpp shortly.

( 2019-03-21 08:52:16 -0500 )edit
( 2019-04-03 01:02:18 -0500 )edit

This is expected. As of ROS Crystal, the logging macros expect a semi-colon ; at the end. The relevant issue can be found here

Edit: As gvdhoorn pointed out in his comment, the macros are two-liners. The ticket referenced above does mention that the chosen approach is a do / while requiring a semi-colon at the end. It looks like the implementation doesn't match.

more

1

And you should always use the curly braces to make sure that you're robust to the macro being compiled out.

( 2019-03-20 15:52:19 -0500 )edit

Sorry for my sloppy question writing; my code does have the semi-colons after the macro calls and it still does not compile.

( 2019-03-20 15:59:29 -0500 )edit
1

It's not semi-colons: it's curly braces that you are missing.

so single-line conditionals are not allowed.

if (..) {
}
else {
}

( 2019-03-20 16:06:56 -0500 )edit

I don't follow why braces should be required to handle the case of an empty macro expansion given that a semi-colon is required. What's the difference between

if (true) {
;
}


and

if (true)
;


?

( 2019-03-20 16:09:27 -0500 )edit

In this case if the macros were only one line then you could just rely on the semi-colon. As a defensive programming mechanism you should use the curly braces to make sure that your code executes correctly and is readable. https://index.ros.org/doc/ros2/Contri...

( 2019-03-20 18:47:47 -0500 )edit

And you should always use the curly braces to make sure that you're robust to the macro being compiled out.

If compiled out they should be come an empty do {} while(0), not nothing.

( 2019-03-20 19:04:10 -0500 )edit