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

Merged
merged 1 commit into from
Feb 9, 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 clalancette requested a review from tfoote as a code owner February 1, 2023 23:00
@clalancette
Copy link
Contributor Author

CI:

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

Copy link
Member

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The windows test failure appears to be unrelated to this change.

@clalancette
Copy link
Contributor Author

The windows test failure appears to be unrelated to this change.

Yeah, that is one of the unfortunate flakes. Thanks for the review, merging.

@clalancette clalancette merged commit d2bff0d into rolling Feb 9, 2023
@delete-merged-branch delete-merged-branch bot deleted the clalancette/update-to-c++17 branch February 9, 2023 17:45
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.

2 participants