Cpp20 part2

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

@@ -108,7 +108,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);
   }
@@ -126,11 +126,12 @@ class ClassLoader
    */
   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)
+  [[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);}
     );
   }
 
@@ -151,12 +152,13 @@ class ClassLoader
    */
   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)
+  [[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);}
     );
   }
 
@@ -177,7 +179,8 @@ class ClassLoader
    */
   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)
+  [[nodiscard]] Base * createUnmanagedInstance(
+    const std::string & derived_class_name, Args &&... args)
   {
     return createRawInstance<Base>(derived_class_name, false, std::forward<Args>(args)...);
   }
@@ -190,7 +193,7 @@ 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(
@@ -202,7 +205,7 @@ class ClassLoader
    *
    * @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;
 
   /**

include/class_loader/class_loader_core.hpp

@@ -75,14 +75,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:

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>

include/class_loader/multi_library_class_loader.hpp

@@ -56,9 +56,9 @@
 namespace class_loader
 {
 
-typedef std::string LibraryPath;
-typedef std::map<LibraryPath, class_loader::ClassLoader *> LibraryToClassLoaderMap;
-typedef std::vector<ClassLoader *> ClassLoaderVector;
+using LibraryPath = std::string;
+using LibraryToClassLoaderMap = std::map<LibraryPath, class_loader::ClassLoader *>;
+using ClassLoaderVector = std::vector<ClassLoader *>;
 
 class MultiLibraryClassLoaderImpl;
 
@@ -94,7 +94,8 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
    */
   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 & class_name, Args &&... args)
+  [[nodiscard]] std::shared_ptr<Base> createInstance(
+    const std::string & class_name, Args &&... args)
   {
     CONSOLE_BRIDGE_logDebug(
       "class_loader::MultiLibraryClassLoader: "
@@ -125,7 +126,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
    */
   template<class Base, class ... Args,
     std::enable_if_t<is_interface_constructible_v<Base, Args...>, bool> = true>
-  std::shared_ptr<Base> createInstance(
+  [[nodiscard]] std::shared_ptr<Base> createInstance(
     const std::string & class_name, const std::string & library_path, Args &&... args)
   {
     ClassLoader * loader = getClassLoaderForLibrary(library_path);
@@ -151,7 +152,8 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
    */
   template<class Base, class ... Args,
     std::enable_if_t<is_interface_constructible_v<Base, Args...>, bool> = true>
-  ClassLoader::UniquePtr<Base> createUniqueInstance(const std::string & class_name, Args &&... args)
+  [[nodiscard]] ClassLoader::UniquePtr<Base> createUniqueInstance(
+    const std::string & class_name, Args &&... args)
   {
     CONSOLE_BRIDGE_logDebug(
       "class_loader::MultiLibraryClassLoader: Attempting to create instance of class type %s.",
@@ -180,7 +182,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
    */
   template<class Base, class ... Args,
     std::enable_if_t<is_interface_constructible_v<Base, Args...>, bool> = true>
-  ClassLoader::UniquePtr<Base>
+  [[nodiscard]] ClassLoader::UniquePtr<Base>
   createUniqueInstance(
     const std::string & class_name, const std::string & library_path,
     Args &&... args)
@@ -209,7 +211,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
    */
   template<class Base, class ... Args,
     std::enable_if_t<is_interface_constructible_v<Base, Args...>, bool> = true>
-  Base * createUnmanagedInstance(const std::string & class_name, Args &&... args)
+  [[nodiscard]] Base * createUnmanagedInstance(const std::string & class_name, Args &&... args)
   {
     ClassLoader * loader = getClassLoaderForClass<Base>(class_name);
     if (nullptr == loader) {
@@ -232,7 +234,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
    */
   template<class Base, class ... Args,
     std::enable_if_t<is_interface_constructible_v<Base, Args...>, bool> = true>
-  Base * createUnmanagedInstance(
+  [[nodiscard]] Base * createUnmanagedInstance(
     const std::string & class_name, const std::string & library_path,
     Args &&... args)
   {
@@ -254,7 +256,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
    * @return true if loaded, 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 available_classes.end() != std::find(
@@ -267,7 +269,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
    * @param library_path - The full qualified path to the runtime library
    * @return true if library is loaded, false otherwise
    */
-  bool isLibraryAvailable(const std::string & library_path) const;
+  [[nodiscard]] bool isLibraryAvailable(const std::string & library_path) const;
 
   /**
    * @brief Gets a list of all classes that are loaded by the class loader
@@ -276,7 +278,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
    * @return A vector<string> of the available classes
    */
   template<class Base>
-  std::vector<std::string> getAvailableClasses() const
+  [[nodiscard]] std::vector<std::string> getAvailableClasses() const
   {
     std::vector<std::string> available_classes;
     for (auto & loader : getAllAvailableClassLoaders()) {
@@ -294,7 +296,8 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
    * @return A vector<string> of the available classes in the passed library
    */
   template<class Base>
-  std::vector<std::string> getAvailableClassesForLibrary(const std::string & library_path) const
+  [[nodiscard]] std::vector<std::string> getAvailableClassesForLibrary(
+    const std::string & library_path) const
   {
     const ClassLoader * loader = getClassLoaderForLibrary(library_path);
     if (nullptr == loader) {
@@ -311,7 +314,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
    *
    * @return A list of libraries opened by this class loader
    */
-  std::vector<std::string> getRegisteredLibraries() const;
+  [[nodiscard]] std::vector<std::string> getRegisteredLibraries() const;
 
   /**
    * @brief Loads a library into memory for this class loader
@@ -379,7 +382,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
    */
   void shutdownAllClassLoaders();
 
-  MultiLibraryClassLoaderImpl * impl_;
+  std::unique_ptr<MultiLibraryClassLoaderImpl> impl_;
 };
 
 

package.xml

@@ -23,6 +23,7 @@
   <build_depend>libconsole-bridge-dev</build_depend>
 
   <buildtool_depend>ament_cmake</buildtool_depend>
+  <buildtool_depend>ament_cmake_ros_core</buildtool_depend>
 
   <exec_depend>console_bridge_vendor</exec_depend>
   <exec_depend>libconsole-bridge-dev</exec_depend>

src/class_loader_core.cpp

@@ -30,6 +30,7 @@
 #include "class_loader/class_loader_core.hpp"
 #include "class_loader/class_loader.hpp"
 
+#include <algorithm>
 #include <cassert>
 #include <cstddef>
 #include <memory>
@@ -63,13 +64,9 @@ BaseToFactoryMapMap & getGlobalPluginBaseToFactoryMapMap()
 
 FactoryMap & getFactoryMapForBaseClass(const std::string & typeid_base_class_name)
 {
-  BaseToFactoryMapMap & factoryMapMap = getGlobalPluginBaseToFactoryMapMap();
-  std::string base_class_name = typeid_base_class_name;
-  if (factoryMapMap.find(base_class_name) == factoryMapMap.end()) {
-    factoryMapMap[base_class_name] = FactoryMap();
-  }
-
-  return factoryMapMap[base_class_name];
+  // std::map::operator[] value-initializes (an empty FactoryMap) when the key is
+  // absent, so a single lookup both finds and inserts as needed.
+  return getGlobalPluginBaseToFactoryMapMap()[typeid_base_class_name];
 }
 
 MetaObjectGraveyardVector & getMetaObjectGraveyard()
@@ -275,12 +272,7 @@ bool areThereAnyExistingMetaObjectsForLibrary(const std::string & library_path)
 LibraryVector::iterator findLoadedLibrary(const std::string & library_path)
 {
   LibraryVector & open_libraries = getLoadedLibraryVector();
-  for (auto it = open_libraries.begin(); it != open_libraries.end(); ++it) {
-    if (it->first == library_path) {
-      return it;
-    }
-  }
-  return open_libraries.end();
+  return std::ranges::find(open_libraries, library_path, &LibraryPair::first);
 }
 
 bool isLibraryLoadedByAnybody(const std::string & library_path)
@@ -317,7 +309,7 @@ std::vector<std::string> getAllLibrariesUsedByClassLoader(const ClassLoader * lo
   std::vector<std::string> all_libs;
   for (auto & meta_obj : all_loader_meta_objs) {
     std::string lib_path = meta_obj->getAssociatedLibraryPath();
-    if (std::find(all_libs.begin(), all_libs.end(), lib_path) == all_libs.end()) {
+    if (std::ranges::find(all_libs, lib_path) == all_libs.end()) {
       all_libs.push_back(lib_path);
     }
   }