rclcpp icon indicating copy to clipboard operation
rclcpp copied to clipboard

Add RCLCPP_*_FORMAT logging macros

Open nwn opened this issue 2 years ago • 6 comments

Similar to the RCLCPP_*_STREAM family of macros, which enable the use of C++'s stream-based output, these new logging macros allow users to conveniently invoke the new C++20 std::format() function for generating formatted text.

It should be noted that rclcpp (as of Iron) still has a minimum requirement of C++17, which predates std::format. Moreover, many compilers only recently implemented support for this feature. (See the entry for P0645R10 here.)

To account for this, the relevant standard library header is only included conditionally if it exists. This should guarantee that the include doesn't introduce errors for compilers without or with partial C++20 support, unless the user actually invokes the RCLCPP_*_FORMAT macros.

These macros are not strictly necessary. As seen in the implementation, one can mechanically replace any call to a RCLCPP_*_FORMAT_* macro with the corresponding printf-style RCLCPP_*_* macro, passing in the variadic arguments "%s", std::format(...).c_str(). However, this is less readable and more error prone.

nwn avatar Aug 03 '23 22:08 nwn

My 2 cents is that we shouldn't take this PR until we can at least reasonably compile it (that is, we switch to C++20). Otherwise this is just dead code.

But I'd like to hear opinions from @wjwwood @fujitatomoya @alsora @iuhilnehc-ynos, and anyone else on @ros2/team

clalancette avatar Aug 04 '23 12:08 clalancette

@clalancette: My 2 cents is that we shouldn't take this PR until we can at least reasonably compile it (that is, we switch to C++20). Otherwise this is just dead code.

That's fair. When you say "switch to C++20", does that mean upgrading the build system to be able to test the code in CI, or bumping the minimum language requirement to C++20? In either case, is there a timeline or idea for when that might happen? On Ubuntu at least, 24.04 (which I assume Jazzy will target) should ship with ~full C++20 support in GCC.

nwn avatar Aug 08 '23 17:08 nwn

That's fair. When you say "switch to C++20", does that mean upgrading the build system to be able to test the code in CI, or bumping the minimum language requirement to C++20?

Both, essentially.

In either case, is there a timeline or idea for when that might happen? On Ubuntu at least, 24.04 (which I assume Jazzy will target) should ship with ~full C++20 support in GCC.

Unfortunately I don't have a timeline for it. While we almost certainly will target Ubuntu 24.04, there is always a bunch of work necessary to switch C++ versions, and there is no allocated time in the schedule for doing that right now. If you are interested in trying to make it happen, I can probably come up with list of things that I think will need to happen in order to do it.

clalancette avatar Aug 08 '23 17:08 clalancette

@clalancette: If you are interested in trying to make it happen, I can probably come up with list of things that I think will need to happen in order to do it.

At least I would be interested. Would it be possible to get such a list?

Grave avatar Aug 09 '23 12:08 Grave

At least I would be interested. Would it be possible to get such a list?

The biggest issue is that while we will be switching to Ubuntu 24.04, which probably has good C++20 support, we still have to support older platforms. In particular, we will still be supporting RHEL-9 (which has gcc 11.3.1), and Windows via MSVC 2019. I have no idea what the C++20 support is like in those compilers. So the list looks something like:

  1. Try out C++20 mode in both MSVC 2019 and gcc 11.3.1, and see what doesn't work.
  2. Potentially upgrade MSVC 2019 to MSVC 2022.
  3. We can't update the compiler on RHEL-9, so if it doesn't support some C++20 features, we'll have to find other ways to workaround it.
  4. Change the top stanza of all of the CMakeLists.txt in all 350 of the core packages to reference C++20 instead of C++17.
  5. Compile the entire core with C++20, and fix all the new warnings and errors that will pop up, across all of the supported platforms. This usually also involves updating at least some of our ~24 vendored dependencies.

clalancette avatar Aug 09 '23 13:08 clalancette

... 3. We can't update the compiler on RHEL-9, so if it doesn't support some C++20 features, we'll have to find other ways to workaround it. ...

All of the C++20 language features are in GCC11, except full support for modules. The C++20 library features on the other hand are missing a few things. The C++20 format library is for example not available until GCC13. Using fmtlib for this PR would enable backwards compatibility with more or less the same API as the forthcoming standard without fragmenting the userbase.

On a side-note, C++20 also introduces the feature test macros, which can be helpful when maintaining backwards compatibility with older compilers.

oysstu avatar Nov 08 '23 11:11 oysstu