search

ros2 / rmw_implementation

CMake infrastructure and dependencies for rmw implementations
Apache License 2.0
21 stars 48 forks source link

Update rmw_implementation to C++17. #214

Closed clalancette closed 1 year ago

clalancette commented 1 year ago

The two reasons to do this are:

  1. So that we can compile rmw_implementation with the clang static analyzer. As of clang++-14 (what is in Ubuntu 22.04), the default still seems to be C++14, so we need to specify C++17 so that new things in the rclcpp headers work properly.
  2. So we can build with a newer version of gtest which requires this.

Further, due to reasons I don't fully understand, I needed to set CMAKE_CXX_STANDARD_REQUIRED in order for clang to really use that version. So set this as well.

Signed-off-by: Chris Lalancette clalancette@openrobotics.org

clalancette commented 1 year ago

quick question, are you going to apply the same change to other projects for clang static analyzer?

Yeah, I have a whole series of these queued up. See for example https://github.com/ros2/rosidl_typesupport/pull/131 , https://github.com/ros2/rosidl_typesupport_fastrtps/pull/94 , etc (and there are more that I haven't even opened yet).

why there 2 projects are picked up?

Sorry, I don't quite understand this question. Can you rephrase?

fujitatomoya commented 1 year ago

Sorry, I don't quite understand this question. Can you rephrase?

I just saw the two PRs, so that I came up with that question.

Yeah, I have a whole series of these queued up. See for example https://github.com/ros2/rosidl_typesupport/pull/131 , https://github.com/ros2/rosidl_typesupport_fastrtps/pull/94 , etc (and there are more that I haven't even opened yet).

this answers my question too! thanks.

clalancette commented 1 year ago

this answers my question too! thanks.

You are welcome!

CI for this is in https://github.com/ros2/rosidl_typesupport/pull/131#issuecomment-1406672534, and since that is green and this is approved, I'm going to merge it in.