[ros-users] [Discourse.ros.org] [General] RFC: Proposal to reconcile ROS 1 and ROS 2 C++ style guides

[Mikael Arguedas]

Hello ROS users!

We've been starting to migrate some core packages to ROS 2 and in the process found a few conflicts between the [ROS 1](https://wiki.ros.org/CppStyleGuide) and [ROS 2](https://github.com/ros2/ros2/wiki/Developer-Guide#c-1) C++ style guides.
These differences cause some friction when either trying to have a single branch be both ROS 1 and ROS 2 or when just maintaining patches between the ROS 1 and ROS 2 versions.
So, in order to minimize this friction, we'd like to eliminate any conflicts that we can.
After looking at the conflicts, it seems that relaxing the ROS 1 style's in a few places is the best solution, but we would like some feedback on the proposed changes.
All of the changes would not require any changes in existing ROS 1 packages, but would allow ROS 1 packages to make changes to their style to comply with both the ROS 2 style and the newly-relaxed ROS 1 style.

These are the changes we're proposing to the ROS 1 C++ style guide:

- Header file names:
  - Existing ROS 1 style: header files should always use the `.h` extension
  - Proposed ROS 1 style: header files should use either `.h` or `.hpp`, if you are planning on supporting both ROS 1 and ROS 2, the `.hpp` extension is recommended.
    - Rationale: differentiating between C and C++ header files provides a way to have both a C and a C++ interface to a library without having to modify the filenames arbitrarily
    - Additional benefit: it gives humans and tools a hint about which language is used in the given header file
- Header define guards:
  - Existing ROS 1 style: `<PACKAGE>_<PATH>_<FILE>_H`
  - Proposed ROS 1 style: `<PACKAGE>[_]_<PATH>[_]_<FILE>_{H,<EXT>}[_]` and optionally leave `<PACKAGE>` off when `<PATH>` is the same
    - Allow an optional trailing underscore
      - Rationale: matches [Google C++ style guide recommendation](https://google.github.io/styleguide/cppguide.html#The__define_Guard) (i.e. `<PROJECT>_<PATH>_<FILE>_H_`)
    - Allow for double underscore as separator between PATH tokens, the PROJECT token, and the FILE token of the guard name
      - Rationale: reduces the chance of guard name collision, though it doesn't prevent it, see: https://github.com/google/styleguide/issues/78
    - Optionally use actual file extension, e.g. if the header is `pluginlib/pluginlib.hpp` then use `PLUGINLIB__PLUGINLIB_HPP_` rather than `PLUGINLIB__PLUGINLIB_H_`
      - Rationale: allows for coexisting C and C++ headers to have different guard names
- Braces:
  - Existing ROS 1 style: open braces (i.e curly braces should always be on their own line)
  - Proposed ROS 1 style: allow any consistent braces style, but continue to recommend open braces for ROS 1 only code (i.e. don't change unless trying to coexist with other style guides)
    - Rationale: This is the most subjective change, but by relaxing the requirement in the ROS 1 guide it allows us to have the ROS 1 style coexist with the ROS 2 style and others more easily.
      - Closer to [Google C++ Style Guide recommendation](https://google.github.io/styleguide/cppguide.html#Function_Declarations_and_Definitions): e.g. "The open curly brace is always on the end of the last line of the function declaration, not the start of the next line.", but it defines brace style for each case separately.
      - Minimizes vertical whitespace use: The more code that fits on one screen, the easier it is to follow and understand the control flow of the program.
      - A preference was given to use the newer ROS 2 style in dual-homed packages and add an option to the ROS 1 style checkers to ignore the brace style violations, though the inverse is also possible.

These are our preferred changes, but we're interested in the feedback from everyone.

Once we reach some consensus well help update any ROS 1 linters with changes and/or options.

Your friendly ROS team





---
[Visit Topic](https://discourse.ros.org/t/rfc-proposal-to-reconcile-ros-1-and-ros-2-c-style-guides/4616/1) or reply to this email to respond.