concurrent accesses well-defined

CMakeLists.txt

@@ -2,11 +2,9 @@ cmake_minimum_required(VERSION 3.20)
 
 project(class_loader)
 
-# Default to C++17
-if(NOT CMAKE_CXX_STANDARD)
-  set(CMAKE_CXX_STANDARD 17)
-  set(CMAKE_CXX_STANDARD_REQUIRED ON)
-endif()
+# The C++/C standard comes from ament_ros_defaults (cxx_std_20 / c_std_17),
+# linked PRIVATE below so it applies to this library's own translation units
+# only and is not propagated to downstream consumers.
 if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
   add_compile_options(-Wall -Wextra -Wpedantic)
 endif()
@@ -15,6 +13,7 @@ set(CLASS_LOADER_IGNORE_AMENT FALSE CACHE BOOL
   "Do not use ament when building this package.")
 if(NOT CLASS_LOADER_IGNORE_AMENT)
   find_package(ament_cmake REQUIRED)
+  find_package(ament_cmake_ros_core REQUIRED)
   set(explicit_library_type "SHARED")
 else()
   set(explicit_library_type "SHARED")
@@ -44,11 +43,18 @@ if(ament_cmake_FOUND)
   target_link_libraries(${PROJECT_NAME} PUBLIC
     console_bridge::console_bridge
     rcpputils::rcpputils)
+  # ament_ros_defaults is an INTERFACE target providing cxx_std_20 / c_std_17.
+  # Linked PRIVATE so the standard applies to our own sources without leaking
+  # into the exported interface (a SHARED library does not propagate PRIVATE
+  # link requirements to consumers).
+  target_link_libraries(${PROJECT_NAME} PRIVATE ament_cmake_ros_core::ament_ros_defaults)
   ament_export_targets(${PROJECT_NAME})
 else()
   target_include_directories(${PROJECT_NAME}
     PUBLIC ${console_bridge_INCLUDE_DIRS})
   target_link_libraries(${PROJECT_NAME} ${console_bridge_LIBRARIES})
+  # Standalone (non-ament) build: request C++20 for our own translation units.
+  target_compile_features(${PROJECT_NAME} PRIVATE cxx_std_20)
 endif()
 if(WIN32)
   # Causes the visibility macros to use dllexport rather than dllimport

include/class_loader/class_loader.hpp

@@ -33,6 +33,7 @@
 #define CLASS_LOADER__CLASS_LOADER_HPP_
 
 #include <algorithm>
+#include <atomic>
 #include <cassert>
 #include <cstddef>
 #include <functional>
@@ -108,7 +109,7 @@ class ClassLoader
    * @return vector of strings indicating names of instantiable classes derived from <Base>
    */
   template<class Base>
-  std::vector<std::string> getAvailableClasses() const
+  [[nodiscard]] std::vector<std::string> getAvailableClasses() const
   {
     return class_loader::impl::getAvailableClasses<Base>(this);
   }
@@ -124,13 +125,14 @@ class ClassLoader
    * by InterfaceTraits of the Base class)
    * @return A std::shared_ptr<Base> to newly created plugin object
    */
-  template<class Base, class ... Args,
-    std::enable_if_t<is_interface_constructible_v<Base, Args...>, bool> = true>
-  std::shared_ptr<Base> createInstance(const std::string & derived_class_name, Args &&... args)
+  template<class Base, class ... Args>
+  requires InterfaceConstructible<Base, Args...>
+  [[nodiscard]] std::shared_ptr<Base> createInstance(
+    const std::string & derived_class_name, Args &&... args)
   {
     return std::shared_ptr<Base>(
       createRawInstance<Base>(derived_class_name, true, std::forward<Args>(args)...),
-      std::bind(&ClassLoader::onPluginDeletion<Base>, this, std::placeholders::_1)
+      [this](Base * p) {onPluginDeletion<Base>(p);}
     );
   }
 
@@ -149,14 +151,15 @@ class ClassLoader
    * by InterfaceTraits of the Base class)
    * @return A std::unique_ptr<Base> to newly created plugin object.
    */
-  template<class Base, class ... Args,
-    std::enable_if_t<is_interface_constructible_v<Base, Args...>, bool> = true>
-  UniquePtr<Base> createUniqueInstance(const std::string & derived_class_name, Args &&... args)
+  template<class Base, class ... Args>
+  requires InterfaceConstructible<Base, Args...>
+  [[nodiscard]] UniquePtr<Base> createUniqueInstance(
+    const std::string & derived_class_name, Args &&... args)
   {
     Base * raw = createRawInstance<Base>(derived_class_name, true, std::forward<Args>(args)...);
     return std::unique_ptr<Base, DeleterType<Base>>(
       raw,
-      std::bind(&ClassLoader::onPluginDeletion<Base>, this, std::placeholders::_1)
+      [this](Base * p) {onPluginDeletion<Base>(p);}
     );
   }
 
@@ -175,9 +178,10 @@ class ClassLoader
    * by InterfaceTraits of the Base class)
    * @return An unmanaged (i.e. not a shared_ptr) Base* to newly created plugin object.
    */
-  template<class Base, class ... Args,
-    std::enable_if_t<is_interface_constructible_v<Base, Args...>, bool> = true>
-  Base * createUnmanagedInstance(const std::string & derived_class_name, Args &&... args)
+  template<class Base, class ... Args>
+  requires InterfaceConstructible<Base, Args...>
+  [[nodiscard]] Base * createUnmanagedInstance(
+    const std::string & derived_class_name, Args &&... args)
   {
     return createRawInstance<Base>(derived_class_name, false, std::forward<Args>(args)...);
   }
@@ -190,19 +194,18 @@ class ClassLoader
    * @return true if yes it is available, false otherwise
    */
   template<class Base>
-  bool isClassAvailable(const std::string & class_name) const
+  [[nodiscard]] bool isClassAvailable(const std::string & class_name) const
   {
     std::vector<std::string> available_classes = getAvailableClasses<Base>();
-    return std::find(
-      available_classes.begin(), available_classes.end(), class_name) != available_classes.end();
+    return std::ranges::find(available_classes, class_name) != available_classes.end();
   }
 
   /**
    * @brief Gets the full-qualified path and name of the library associated with this class loader
    *
    * @return the full-qualified path and name of the library
    */
-  CLASS_LOADER_PUBLIC
+  [[nodiscard]] CLASS_LOADER_PUBLIC
   const std::string & getLibraryPath() const;
 
   /**
@@ -215,7 +218,7 @@ class ClassLoader
    * @param library_path The path to the library to load
    * @return true if library is loaded within this ClassLoader object's scope, otherwise false
    */
-  CLASS_LOADER_PUBLIC
+  [[nodiscard]] CLASS_LOADER_PUBLIC
   bool isLibraryLoaded() const;
 
   /**
@@ -224,7 +227,7 @@ class ClassLoader
    *
    * @return true if library is loaded within the scope of the plugin system, otherwise false
    */
-  CLASS_LOADER_PUBLIC
+  [[nodiscard]] CLASS_LOADER_PUBLIC
   bool isLibraryLoadedByAnyClassloader() const;
 
   /**
@@ -234,7 +237,7 @@ class ClassLoader
    *
    * @return true if ondemand load and unload is active, otherwise false
    */
-  CLASS_LOADER_PUBLIC
+  [[nodiscard]] CLASS_LOADER_PUBLIC
   bool isOnDemandLoadUnloadEnabled() const;
 
   /**
@@ -311,8 +314,8 @@ class ClassLoader
    * by InterfaceTraits of the Base class)
    * @return A Base* to newly created plugin object.
    */
-  template<class Base, class ... Args,
-    std::enable_if_t<is_interface_constructible_v<Base, Args...>, bool> = true>
+  template<class Base, class ... Args>
+  requires InterfaceConstructible<Base, Args...>
   Base * createRawInstance(const std::string & derived_class_name, bool managed, Args &&... args)
   {
     if (!managed) {
@@ -377,7 +380,7 @@ class ClassLoader
   std::recursive_mutex load_ref_count_mutex_;
   int plugin_ref_count_;
   std::recursive_mutex plugin_ref_count_mutex_;
-  static bool has_unmanaged_instance_been_created_;
+  static std::atomic<bool> has_unmanaged_instance_been_created_;
 };
 
 }  // namespace class_loader

include/class_loader/class_loader_core.hpp

@@ -32,6 +32,7 @@
 #ifndef CLASS_LOADER__CLASS_LOADER_CORE_HPP_
 #define CLASS_LOADER__CLASS_LOADER_CORE_HPP_
 
+#include <algorithm>
 #include <cstddef>
 #include <cstdio>
 #include <functional>
@@ -75,14 +76,14 @@ namespace impl
 {
 
 // Typedefs
-typedef std::string LibraryPath;
-typedef std::string ClassName;
-typedef std::string BaseClassName;
-typedef std::map<ClassName, impl::AbstractMetaObjectBase *> FactoryMap;
-typedef std::map<BaseClassName, FactoryMap> BaseToFactoryMapMap;
-typedef std::pair<LibraryPath, std::shared_ptr<rcpputils::SharedLibrary>> LibraryPair;
-typedef std::vector<LibraryPair> LibraryVector;
-typedef std::vector<AbstractMetaObjectBase *> MetaObjectVector;
+using LibraryPath = std::string;
+using ClassName = std::string;
+using BaseClassName = std::string;
+using FactoryMap = std::map<ClassName, impl::AbstractMetaObjectBase *>;
+using BaseToFactoryMapMap = std::map<BaseClassName, FactoryMap>;
+using LibraryPair = std::pair<LibraryPath, std::shared_ptr<rcpputils::SharedLibrary>>;
+using LibraryVector = std::vector<LibraryPair>;
+using MetaObjectVector = std::vector<AbstractMetaObjectBase *>;
 class MetaObjectGraveyardVector : public MetaObjectVector
 {
 public:
@@ -277,25 +278,16 @@ registerPlugin(const std::string & class_name, const std::string & base_class_na
     [](AbstractMetaObjectBase * p) {
       getPluginBaseToFactoryMapMapMutex().lock();
       MetaObjectGraveyardVector & graveyard = getMetaObjectGraveyard();
-      for (auto iter = graveyard.begin(); iter != graveyard.end(); ++iter) {
-        if (*iter == p) {
-          graveyard.erase(iter);
-          break;
-        }
+      if (auto iter = std::ranges::find(graveyard, p); iter != graveyard.end()) {
+        graveyard.erase(iter);
       }
 
       BaseToFactoryMapMap & factory_map_map = getGlobalPluginBaseToFactoryMapMap();
-      bool erase_flag = false;
-      for (auto & factory_map_item : factory_map_map) {
-        FactoryMap & factory_map = factory_map_item.second;
-        for (auto iter = factory_map.begin(); iter != factory_map.end(); ++iter) {
-          if (iter->second == p) {
-            factory_map.erase(iter);
-            erase_flag = true;
-            break;
-          }
-        }
-        if (erase_flag) {
+      for (auto & [base_class_name, factory_map] : factory_map_map) {
+        if (auto iter = std::ranges::find(factory_map, p, &FactoryMap::value_type::second);
+        iter != factory_map.end())
+        {
+          factory_map.erase(iter);
           break;
         }
       }
@@ -318,7 +310,7 @@ registerPlugin(const std::string & class_name, const std::string & base_class_na
   // Add it to global factory map map
   getPluginBaseToFactoryMapMapMutex().lock();
   FactoryMap & factoryMap = getFactoryMapForBaseClass<Base>();
-  if (factoryMap.find(class_name) != factoryMap.end()) {
+  if (factoryMap.contains(class_name)) {
     CONSOLE_BRIDGE_logWarn(
       "class_loader.impl: SEVERE WARNING!!! "
       "A namespace collision has occurred with plugin factory for class %s. "
@@ -349,16 +341,16 @@ registerPlugin(const std::string & class_name, const std::string & base_class_na
  * by InterfaceTraits of the Base class)
  * @return A pointer to newly created plugin, note caller is responsible for object destruction
  */
-template<typename Base, class ... Args,
-  std::enable_if_t<is_interface_constructible_v<Base, Args...>, bool> = true>
+template<typename Base, class ... Args>
+requires InterfaceConstructible<Base, Args...>
 Base * createInstance(const std::string & derived_class_name, ClassLoader * loader, Args &&... args)
 {
   AbstractMetaObject<Base> * factory = nullptr;
 
   getPluginBaseToFactoryMapMapMutex().lock();
   FactoryMap & factoryMap = getFactoryMapForBaseClass<Base>();
-  if (factoryMap.find(derived_class_name) != factoryMap.end()) {
-    factory = dynamic_cast<impl::AbstractMetaObject<Base> *>(factoryMap[derived_class_name]);
+  if (auto it = factoryMap.find(derived_class_name); it != factoryMap.end()) {
+    factory = dynamic_cast<impl::AbstractMetaObject<Base> *>(it->second);
   } else {
     CONSOLE_BRIDGE_logError(
       "class_loader.impl: No metaobject exists for class type %s.", derived_class_name.c_str());

include/class_loader/interface_traits.hpp

@@ -103,15 +103,17 @@ struct InterfaceTraits
 namespace impl
 {
 
-template<class T, class = void>
+template<class T>
 struct interface_constructor_parameters_impl
 {
   using type = ConstructorParameters<>;
 };
 
+// Constrained partial specialization replaces the std::void_t detection idiom:
+// it is selected when InterfaceTraits<T> defines a `constructor_parameters` member.
 template<class T>
-struct interface_constructor_parameters_impl<T,
-  std::void_t<typename InterfaceTraits<T>::constructor_parameters>>
+requires requires {typename InterfaceTraits<T>::constructor_parameters;}
+struct interface_constructor_parameters_impl<T>
 {
   using type = typename InterfaceTraits<T>::constructor_parameters;
 };
@@ -190,6 +192,17 @@ template<class Base, class ... Args>
 constexpr bool is_interface_constructible_v =
   is_interface_constructible<Base, Args...>::value;
 
+/**
+ * @brief Concept satisfied when a plugin deriving from @p Base can be constructed
+ * from @p Args, as declared by its InterfaceTraits.
+ *
+ * Used in place of std::enable_if to constrain the create*Instance() factories,
+ * yielding clearer diagnostics ("constraint not satisfied") on a mismatch.
+ * @see is_interface_constructible
+ */
+template<class Base, class ... Args>
+concept InterfaceConstructible = is_interface_constructible_v<Base, Args...>;
+
 }  // namespace class_loader
 
 #endif  // CLASS_LOADER__INTERFACE_TRAITS_HPP_

include/class_loader/meta_object.hpp

@@ -34,6 +34,7 @@
 #ifndef CLASS_LOADER__META_OBJECT_HPP_
 #define CLASS_LOADER__META_OBJECT_HPP_
 
+#include <memory>
 #include <string>
 #include <typeinfo>
 #include <utility>
@@ -151,7 +152,7 @@ class CLASS_LOADER_PUBLIC AbstractMetaObjectBase
    */
   virtual void dummyMethod() {}
 
-  AbstractMetaObjectBaseImpl * impl_;
+  std::unique_ptr<AbstractMetaObjectBaseImpl> impl_;
 };
 
 template<class ... Ts>