Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update common_interfaces to C++17. (#215) #151

Merged
merged 1 commit into from
Feb 10, 2023

Conversation

clalancette
Copy link
Contributor

The main reason to do this is so that we can compile common_interfaces 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.

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

The main reason to do this is so that we can compile
common_interfaces 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.

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
Copy link
Contributor Author

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@gbiggs
Copy link
Member

gbiggs commented Feb 9, 2023

Agree that this is a sensible change. Feel free to merge once CI is passing.

@clalancette
Copy link
Contributor Author

Green CI, so merging. Thanks for the review!

@clalancette clalancette merged commit 8eec885 into rolling Feb 10, 2023
@clalancette clalancette deleted the clalancette/update-to-c++17 branch February 10, 2023 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants