RCLCPP logging macros not a single expression

asked 2019-03-20 15:39:33 -0600

jdlangs
971 ●23 ●47 ●45

updated 2019-03-20 16:00:07 -0600

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 flag offensive close merge delete

Comments

"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?

gvdhoorn ( 2019-03-20 15:45:29 -0600 )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.

allenh1 ( 2019-03-20 15:54:23 -0600 )edit

add a comment

1

answered 2019-03-20 19:01:45 -0600

William

17335 ●101 ●182 ●262 http://wjwwood.io/

@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

edit flag offensive delete link

Comments

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.

jdlangs ( 2019-03-21 08:52:16 -0600 )edit

See https://github.com/ros2/rclcpp/pull/681

Dirk Thomas ( 2019-04-03 01:02:18 -0600 )edit

add a comment

0

answered 2019-03-20 15:48:53 -0600

marguedas

3621 ●17 ●109 ●57

updated 2019-03-20 17:54:09 -0600

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.

edit flag offensive delete link

Comments

1

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

tfoote ( 2019-03-20 15:52:19 -0600 )edit

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

jdlangs ( 2019-03-20 15:59:29 -0600 )edit

1

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

so single-line conditionals are not allowed.

if (..) {
}
else {
}

gvdhoorn ( 2019-03-20 16:06:56 -0600 )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)
  ;

?

jdlangs ( 2019-03-20 16:09:27 -0600 )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...

tfoote ( 2019-03-20 18:47:47 -0600 )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.

William ( 2019-03-20 19:04:10 -0600 )edit

add a comment

RCLCPP logging macros not a single expression

Comments

2 Answers

Comments

Comments

Question Tools

Stats

Related questions

RCLCPP logging macros not a single expression edit

Comments

2 Answers

Comments

Comments

Question Tools

Stats

Related questions

RCLCPP logging macros not a single expression