feat: Switch to c++20 and remove resulting compile warnings (#3124)

Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
Co-authored-by: Janosch Machowinski <J.Machowinski@cellumation.com>

Author: jmachowinski

rclcpp/CMakeLists.txt

@@ -5,6 +5,7 @@ project(rclcpp)
 find_package(Threads REQUIRED)
 
 find_package(ament_cmake_ros REQUIRED)
+find_package(ament_cmake_ros_core REQUIRED)
 find_package(ament_index_cpp REQUIRED)
 find_package(builtin_interfaces REQUIRED)
 find_package(libstatistics_collector REQUIRED)
@@ -24,12 +25,6 @@ find_package(rosidl_typesupport_cpp REQUIRED)
 find_package(statistics_msgs REQUIRED)
 find_package(tracetools REQUIRED)
 
-# TODO(wjwwood): remove this when gtest can build on its own, when using target_compile_features()
-# Default to C++17
-if(NOT CMAKE_CXX_STANDARD)
-  set(CMAKE_CXX_STANDARD 17)
-  set(CMAKE_CXX_STANDARD_REQUIRED ON)
-endif()
 if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
   # About -Wno-sign-conversion: With Clang, -Wconversion implies -Wsign-conversion. There are a number of
   # implicit sign conversions in rclcpp and gtest.cc, see https://ci.ros2.org/job/ci_osx/9265/.
@@ -193,7 +188,6 @@ foreach(interface_file ${interface_files})
 endforeach()
 
 add_library(${PROJECT_NAME} ${${PROJECT_NAME}_SRCS})
-target_compile_features(${PROJECT_NAME} PUBLIC cxx_std_17)
 # TODO(wjwwood): address all deprecation warnings and then remove this
 if(WIN32)
   target_compile_definitions(${PROJECT_NAME} PUBLIC "_SILENCE_ALL_CXX17_DEPRECATION_WARNINGS")
@@ -219,6 +213,9 @@ target_link_libraries(${PROJECT_NAME} PUBLIC
   statistics_msgs::statistics_msgs
   tracetools::tracetools
   ${CMAKE_THREAD_LIBS_INIT}
+  # Note we link public on purpose, as we want to export
+  # the used C++ version as minimum to all consuming packages
+  ament_cmake_ros_core::ament_ros_defaults
 )
 
 target_link_libraries(${PROJECT_NAME} PRIVATE

rclcpp/package.xml

@@ -20,6 +20,7 @@
   <buildtool_depend>python3-empy</buildtool_depend>
 
   <build_depend>ament_index_cpp</build_depend>
+  <build_depend>ament_index_cpp_core</build_depend>
   <build_depend>builtin_interfaces</build_depend>
   <build_depend>rcl_interfaces</build_depend>
   <build_depend>rosgraph_msgs</build_depend>

rclcpp/src/rclcpp/parameter_event_handler.cpp

@@ -83,7 +83,7 @@ ParameterEventHandler::configure_nodes_filter(const std::vector<std::string> & n
 
   if (node_names.empty()) {
     // Clear content filter
-    event_subscription_->set_content_filter("");
+    event_subscription_->set_content_filter(std::string());
     if (event_subscription_->is_cft_enabled()) {
       return false;
     }
@@ -101,8 +101,9 @@ ParameterEventHandler::configure_nodes_filter(const std::vector<std::string> & n
 
   // Enclose each node name in "'".
   std::vector<std::string> quoted_node_names;
+  const std::string delim("'");
   for (const auto & name : node_names) {
-    quoted_node_names.push_back("'" + resolve_path(name) + "'");
+    quoted_node_names.push_back(delim + resolve_path(name) + delim);
   }
 
   event_subscription_->set_content_filter(filter_expression, quoted_node_names);
@@ -229,20 +230,21 @@ ParameterEventHandler::Callbacks::event_callback(const rcl_interfaces::msg::Para
 std::string
 ParameterEventHandler::resolve_path(const std::string & path)
 {
-  std::string full_path;
+  if (path.empty()) {
+    return node_base_->get_fully_qualified_name();
+  }
 
-  if (path == "") {
-    full_path = node_base_->get_fully_qualified_name();
-  } else {
-    full_path = path;
-    if (*path.begin() != '/') {
-      auto ns = node_base_->get_namespace();
-      const std::vector<std::string> paths{ns, path};
-      full_path = (ns == std::string("/")) ? ns + path : rcpputils::join(paths, "/");
+  if (*path.begin() != '/') {
+    auto ns = node_base_->get_namespace();
+    const std::vector<std::string> paths{ns, path};
+    if(ns == std::string("/")) {
+      return ns + path;
     }
+
+    return rcpputils::join(paths, "/");
   }
 
-  return full_path;
+  return path;
 }
 
 }  // namespace rclcpp

rclcpp/test/rclcpp/executors/test_executors.cpp

@@ -650,7 +650,7 @@ TYPED_TEST(TestExecutors, testRaceConditionAddNode)
           if (should_cancel) {
             break;
           }
-          total += k * (i + 42);
+          total = total + k * (i + 42);
           (void)total;
         }
       });