Fix -Wmaybe-uninitialized warning (#2081)

* Fix -Wmaybe-uninitialized warning

gcc 12 warned about `callback_list_ptr` potentially being uninitialized
when compiling in release mode (i.e., with `-O2`). Since `shutdown_type`
is a compile-time parameter, we fix the warning by enforcing the
decision at compile time.

Signed-off-by: Alexander Hans <[email protected]>

Author: ahans

rclcpp/include/rclcpp/context.hpp

@@ -398,20 +398,22 @@ class Context : public std::enable_shared_from_this<Context>
 
   using ShutdownCallback = ShutdownCallbackHandle::ShutdownCallbackType;
 
+  template<ShutdownType shutdown_type>
   RCLCPP_LOCAL
   ShutdownCallbackHandle
   add_shutdown_callback(
-    ShutdownType shutdown_type,
     ShutdownCallback callback);
 
+  template<ShutdownType shutdown_type>
   RCLCPP_LOCAL
   bool
   remove_shutdown_callback(
-    ShutdownType shutdown_type,
     const ShutdownCallbackHandle & callback_handle);
 
+  template<ShutdownType shutdown_type>
+  RCLCPP_LOCAL
   std::vector<rclcpp::Context::ShutdownCallback>
-  get_shutdown_callback(ShutdownType shutdown_type) const;
+  get_shutdown_callback() const;
 };
 
 /// Return a copy of the list of context shared pointers.

rclcpp/src/rclcpp/context.cpp

@@ -365,123 +365,110 @@ Context::on_shutdown(OnShutdownCallback callback)
 rclcpp::OnShutdownCallbackHandle
 Context::add_on_shutdown_callback(OnShutdownCallback callback)
 {
-  return add_shutdown_callback(ShutdownType::on_shutdown, callback);
+  return add_shutdown_callback<ShutdownType::on_shutdown>(callback);
 }
 
 bool
 Context::remove_on_shutdown_callback(const OnShutdownCallbackHandle & callback_handle)
 {
-  return remove_shutdown_callback(ShutdownType::on_shutdown, callback_handle);
+  return remove_shutdown_callback<ShutdownType::on_shutdown>(callback_handle);
 }
 
 rclcpp::PreShutdownCallbackHandle
 Context::add_pre_shutdown_callback(PreShutdownCallback callback)
 {
-  return add_shutdown_callback(ShutdownType::pre_shutdown, callback);
+  return add_shutdown_callback<ShutdownType::pre_shutdown>(callback);
 }
 
 bool
 Context::remove_pre_shutdown_callback(
   const PreShutdownCallbackHandle & callback_handle)
 {
-  return remove_shutdown_callback(ShutdownType::pre_shutdown, callback_handle);
+  return remove_shutdown_callback<ShutdownType::pre_shutdown>(callback_handle);
 }
 
+template<Context::ShutdownType shutdown_type>
 rclcpp::ShutdownCallbackHandle
 Context::add_shutdown_callback(
-  ShutdownType shutdown_type,
   ShutdownCallback callback)
 {
   auto callback_shared_ptr =
     std::make_shared<ShutdownCallbackHandle::ShutdownCallbackType>(callback);
 
-  switch (shutdown_type) {
-    case ShutdownType::pre_shutdown:
-      {
-        std::lock_guard<std::mutex> lock(pre_shutdown_callbacks_mutex_);
-        pre_shutdown_callbacks_.emplace(callback_shared_ptr);
-      }
-      break;
-    case ShutdownType::on_shutdown:
-      {
-        std::lock_guard<std::mutex> lock(on_shutdown_callbacks_mutex_);
-        on_shutdown_callbacks_.emplace(callback_shared_ptr);
-      }
-      break;
+  static_assert(
+    shutdown_type == ShutdownType::pre_shutdown || shutdown_type == ShutdownType::on_shutdown);
+
+  if constexpr (shutdown_type == ShutdownType::pre_shutdown) {
+    std::lock_guard<std::mutex> lock(pre_shutdown_callbacks_mutex_);
+    pre_shutdown_callbacks_.emplace(callback_shared_ptr);
+  } else {
+    std::lock_guard<std::mutex> lock(on_shutdown_callbacks_mutex_);
+    on_shutdown_callbacks_.emplace(callback_shared_ptr);
   }
 
   ShutdownCallbackHandle callback_handle;
   callback_handle.callback = callback_shared_ptr;
   return callback_handle;
 }
 
+template<Context::ShutdownType shutdown_type>
 bool
 Context::remove_shutdown_callback(
-  ShutdownType shutdown_type,
   const ShutdownCallbackHandle & callback_handle)
 {
-  std::mutex * mutex_ptr = nullptr;
-  std::unordered_set<
-    std::shared_ptr<ShutdownCallbackHandle::ShutdownCallbackType>> * callback_list_ptr;
-
-  switch (shutdown_type) {
-    case ShutdownType::pre_shutdown:
-      mutex_ptr = &pre_shutdown_callbacks_mutex_;
-      callback_list_ptr = &pre_shutdown_callbacks_;
-      break;
-    case ShutdownType::on_shutdown:
-      mutex_ptr = &on_shutdown_callbacks_mutex_;
-      callback_list_ptr = &on_shutdown_callbacks_;
-      break;
-  }
-
-  std::lock_guard<std::mutex> lock(*mutex_ptr);
-  auto callback_shared_ptr = callback_handle.callback.lock();
+  const auto callback_shared_ptr = callback_handle.callback.lock();
   if (callback_shared_ptr == nullptr) {
     return false;
   }
-  return callback_list_ptr->erase(callback_shared_ptr) == 1;
+
+  const auto remove_callback = [this, &callback_shared_ptr](auto & mutex, auto & callback_set) {
+      const std::lock_guard<std::mutex> lock(mutex);
+      return callback_set.erase(callback_shared_ptr) == 1;
+    };
+
+  static_assert(
+    shutdown_type == ShutdownType::pre_shutdown || shutdown_type == ShutdownType::on_shutdown);
+
+  if constexpr (shutdown_type == ShutdownType::pre_shutdown) {
+    return remove_callback(pre_shutdown_callbacks_mutex_, pre_shutdown_callbacks_);
+  } else {
+    return remove_callback(on_shutdown_callbacks_mutex_, on_shutdown_callbacks_);
+  }
 }
 
 std::vector<rclcpp::Context::OnShutdownCallback>
 Context::get_on_shutdown_callbacks() const
 {
-  return get_shutdown_callback(ShutdownType::on_shutdown);
+  return get_shutdown_callback<ShutdownType::on_shutdown>();
 }
 
 std::vector<rclcpp::Context::PreShutdownCallback>
 Context::get_pre_shutdown_callbacks() const
 {
-  return get_shutdown_callback(ShutdownType::pre_shutdown);
+  return get_shutdown_callback<ShutdownType::pre_shutdown>();
 }
 
+template<Context::ShutdownType shutdown_type>
 std::vector<rclcpp::Context::ShutdownCallback>
-Context::get_shutdown_callback(ShutdownType shutdown_type) const
+Context::get_shutdown_callback() const
 {
-  std::mutex * mutex_ptr = nullptr;
-  const std::unordered_set<
-    std::shared_ptr<ShutdownCallbackHandle::ShutdownCallbackType>> * callback_list_ptr;
-
-  switch (shutdown_type) {
-    case ShutdownType::pre_shutdown:
-      mutex_ptr = &pre_shutdown_callbacks_mutex_;
-      callback_list_ptr = &pre_shutdown_callbacks_;
-      break;
-    case ShutdownType::on_shutdown:
-      mutex_ptr = &on_shutdown_callbacks_mutex_;
-      callback_list_ptr = &on_shutdown_callbacks_;
-      break;
-  }
+  const auto get_callback_vector = [this](auto & mutex, auto & callback_set) {
+      const std::lock_guard<std::mutex> lock(mutex);
+      std::vector<rclcpp::Context::ShutdownCallback> callbacks;
+      for (auto & callback : callback_set) {
+        callbacks.push_back(*callback);
+      }
+      return callbacks;
+    };
 
-  std::vector<rclcpp::Context::ShutdownCallback> callbacks;
-  {
-    std::lock_guard<std::mutex> lock(*mutex_ptr);
-    for (auto & iter : *callback_list_ptr) {
-      callbacks.emplace_back(*iter);
-    }
-  }
+  static_assert(
+    shutdown_type == ShutdownType::pre_shutdown || shutdown_type == ShutdownType::on_shutdown);
 
-  return callbacks;
+  if constexpr (shutdown_type == ShutdownType::pre_shutdown) {
+    return get_callback_vector(pre_shutdown_callbacks_mutex_, pre_shutdown_callbacks_);
+  } else {
+    return get_callback_vector(on_shutdown_callbacks_mutex_, on_shutdown_callbacks_);
+  }
 }
 
 std::shared_ptr<rcl_context_t>