diff --git a/CMakeLists.txt b/CMakeLists.txt index 0eafb86..67df721 100644 --- a/CMakeLists.txt +++ b/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 diff --git a/include/class_loader/class_loader.hpp b/include/class_loader/class_loader.hpp index 06dd412..5a5e077 100644 --- a/include/class_loader/class_loader.hpp +++ b/include/class_loader/class_loader.hpp @@ -108,7 +108,7 @@ class ClassLoader * @return vector of strings indicating names of instantiable classes derived from */ template - std::vector getAvailableClasses() const + [[nodiscard]] std::vector getAvailableClasses() const { return class_loader::impl::getAvailableClasses(this); } @@ -126,11 +126,12 @@ class ClassLoader */ template, bool> = true> - std::shared_ptr createInstance(const std::string & derived_class_name, Args &&... args) + [[nodiscard]] std::shared_ptr createInstance( + const std::string & derived_class_name, Args &&... args) { return std::shared_ptr( createRawInstance(derived_class_name, true, std::forward(args)...), - std::bind(&ClassLoader::onPluginDeletion, this, std::placeholders::_1) + [this](Base * p) {onPluginDeletion(p);} ); } @@ -151,12 +152,13 @@ class ClassLoader */ template, bool> = true> - UniquePtr createUniqueInstance(const std::string & derived_class_name, Args &&... args) + [[nodiscard]] UniquePtr createUniqueInstance( + const std::string & derived_class_name, Args &&... args) { Base * raw = createRawInstance(derived_class_name, true, std::forward(args)...); return std::unique_ptr>( raw, - std::bind(&ClassLoader::onPluginDeletion, this, std::placeholders::_1) + [this](Base * p) {onPluginDeletion(p);} ); } @@ -177,7 +179,8 @@ class ClassLoader */ template, 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(derived_class_name, false, std::forward(args)...); } @@ -190,7 +193,7 @@ class ClassLoader * @return true if yes it is available, false otherwise */ template - bool isClassAvailable(const std::string & class_name) const + [[nodiscard]] bool isClassAvailable(const std::string & class_name) const { std::vector available_classes = getAvailableClasses(); 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; /** diff --git a/include/class_loader/class_loader_core.hpp b/include/class_loader/class_loader_core.hpp index 1e402f6..917d9f4 100644 --- a/include/class_loader/class_loader_core.hpp +++ b/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 FactoryMap; -typedef std::map BaseToFactoryMapMap; -typedef std::pair> LibraryPair; -typedef std::vector LibraryVector; -typedef std::vector MetaObjectVector; +using LibraryPath = std::string; +using ClassName = std::string; +using BaseClassName = std::string; +using FactoryMap = std::map; +using BaseToFactoryMapMap = std::map; +using LibraryPair = std::pair>; +using LibraryVector = std::vector; +using MetaObjectVector = std::vector; class MetaObjectGraveyardVector : public MetaObjectVector { public: diff --git a/include/class_loader/meta_object.hpp b/include/class_loader/meta_object.hpp index 0b41620..dc7ec4b 100644 --- a/include/class_loader/meta_object.hpp +++ b/include/class_loader/meta_object.hpp @@ -34,6 +34,7 @@ #ifndef CLASS_LOADER__META_OBJECT_HPP_ #define CLASS_LOADER__META_OBJECT_HPP_ +#include #include #include #include @@ -151,7 +152,7 @@ class CLASS_LOADER_PUBLIC AbstractMetaObjectBase */ virtual void dummyMethod() {} - AbstractMetaObjectBaseImpl * impl_; + std::unique_ptr impl_; }; template diff --git a/include/class_loader/multi_library_class_loader.hpp b/include/class_loader/multi_library_class_loader.hpp index d1ac035..4b70fd8 100644 --- a/include/class_loader/multi_library_class_loader.hpp +++ b/include/class_loader/multi_library_class_loader.hpp @@ -56,9 +56,9 @@ namespace class_loader { -typedef std::string LibraryPath; -typedef std::map LibraryToClassLoaderMap; -typedef std::vector ClassLoaderVector; +using LibraryPath = std::string; +using LibraryToClassLoaderMap = std::map; +using ClassLoaderVector = std::vector; class MultiLibraryClassLoaderImpl; @@ -94,7 +94,8 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader */ template, bool> = true> - std::shared_ptr createInstance(const std::string & class_name, Args &&... args) + [[nodiscard]] std::shared_ptr createInstance( + const std::string & class_name, Args &&... args) { CONSOLE_BRIDGE_logDebug( "class_loader::MultiLibraryClassLoader: " @@ -125,7 +126,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader */ template, bool> = true> - std::shared_ptr createInstance( + [[nodiscard]] std::shared_ptr 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, bool> = true> - ClassLoader::UniquePtr createUniqueInstance(const std::string & class_name, Args &&... args) + [[nodiscard]] ClassLoader::UniquePtr 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, bool> = true> - ClassLoader::UniquePtr + [[nodiscard]] ClassLoader::UniquePtr createUniqueInstance( const std::string & class_name, const std::string & library_path, Args &&... args) @@ -209,7 +211,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader */ template, bool> = true> - Base * createUnmanagedInstance(const std::string & class_name, Args &&... args) + [[nodiscard]] Base * createUnmanagedInstance(const std::string & class_name, Args &&... args) { ClassLoader * loader = getClassLoaderForClass(class_name); if (nullptr == loader) { @@ -232,7 +234,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader */ template, 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 - bool isClassAvailable(const std::string & class_name) const + [[nodiscard]] bool isClassAvailable(const std::string & class_name) const { std::vector available_classes = getAvailableClasses(); 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 of the available classes */ template - std::vector getAvailableClasses() const + [[nodiscard]] std::vector getAvailableClasses() const { std::vector available_classes; for (auto & loader : getAllAvailableClassLoaders()) { @@ -294,7 +296,8 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader * @return A vector of the available classes in the passed library */ template - std::vector getAvailableClassesForLibrary(const std::string & library_path) const + [[nodiscard]] std::vector 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 getRegisteredLibraries() const; + [[nodiscard]] std::vector 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 impl_; }; diff --git a/package.xml b/package.xml index d39ad95..258ca9c 100644 --- a/package.xml +++ b/package.xml @@ -23,6 +23,7 @@ libconsole-bridge-dev ament_cmake + ament_cmake_ros_core console_bridge_vendor libconsole-bridge-dev diff --git a/src/class_loader_core.cpp b/src/class_loader_core.cpp index 7bfb53a..e808953 100644 --- a/src/class_loader_core.cpp +++ b/src/class_loader_core.cpp @@ -30,6 +30,7 @@ #include "class_loader/class_loader_core.hpp" #include "class_loader/class_loader.hpp" +#include #include #include #include @@ -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 getAllLibrariesUsedByClassLoader(const ClassLoader * lo std::vector 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); } } diff --git a/src/meta_object.cpp b/src/meta_object.cpp index 002b55e..17a49de 100644 --- a/src/meta_object.cpp +++ b/src/meta_object.cpp @@ -27,6 +27,8 @@ * POSSIBILITY OF SUCH DAMAGE. */ +#include +#include #include #include @@ -38,7 +40,7 @@ namespace class_loader namespace impl { -typedef std::vector ClassLoaderVector; +using ClassLoaderVector = std::vector; class AbstractMetaObjectBaseImpl { @@ -53,7 +55,7 @@ class AbstractMetaObjectBaseImpl AbstractMetaObjectBase::AbstractMetaObjectBase( const std::string & class_name, const std::string & base_class_name, const std::string & typeid_base_class_name) -: impl_(new AbstractMetaObjectBaseImpl()) +: impl_(std::make_unique()) { impl_->associated_library_path_ = "Unknown"; impl_->base_class_name_ = base_class_name; @@ -71,7 +73,6 @@ AbstractMetaObjectBase::~AbstractMetaObjectBase() "class_loader.impl.AbstractMetaObjectBase: " "Destroying MetaObject %p (base = %s, derived = %s, library path = %s)", this, baseClassName().c_str(), className().c_str(), getAssociatedLibraryPath().c_str()); - delete impl_; } const std::string & AbstractMetaObjectBase::className() const @@ -102,7 +103,7 @@ void AbstractMetaObjectBase::setAssociatedLibraryPath(const std::string & librar void AbstractMetaObjectBase::addOwningClassLoader(ClassLoader * loader) { ClassLoaderVector & v = impl_->associated_class_loaders_; - if (std::find(v.begin(), v.end(), loader) == v.end()) { + if (std::ranges::find(v, loader) == v.end()) { v.push_back(loader); } } @@ -110,8 +111,7 @@ void AbstractMetaObjectBase::addOwningClassLoader(ClassLoader * loader) void AbstractMetaObjectBase::removeOwningClassLoader(const ClassLoader * loader) { ClassLoaderVector & v = impl_->associated_class_loaders_; - ClassLoaderVector::iterator itr = std::find(v.begin(), v.end(), loader); - if (itr != v.end()) { + if (auto itr = std::ranges::find(v, loader); itr != v.end()) { v.erase(itr); } } @@ -119,8 +119,7 @@ void AbstractMetaObjectBase::removeOwningClassLoader(const ClassLoader * loader) bool AbstractMetaObjectBase::isOwnedBy(const ClassLoader * loader) const { const ClassLoaderVector & v = impl_->associated_class_loaders_; - auto it = std::find(v.begin(), v.end(), loader); - return it != v.end(); + return std::ranges::find(v, loader) != v.end(); } bool AbstractMetaObjectBase::isOwnedByAnybody() const diff --git a/src/multi_library_class_loader.cpp b/src/multi_library_class_loader.cpp index 7ff7d09..36f4aa0 100644 --- a/src/multi_library_class_loader.cpp +++ b/src/multi_library_class_loader.cpp @@ -29,7 +29,9 @@ #include "class_loader/multi_library_class_loader.hpp" +#include #include +#include #include #include #include @@ -37,7 +39,7 @@ namespace class_loader { -typedef std::vector> ClassLoaderPtrVector; +using ClassLoaderPtrVector = std::vector>; class ClassLoaderDependency { @@ -79,7 +81,7 @@ ClassLoaderPtrVectorImpl & getClassLoaderPtrVectorImpl() } MultiLibraryClassLoader::MultiLibraryClassLoader(bool enable_ondemand_loadunload) -: impl_(new MultiLibraryClassLoaderImpl()) +: impl_(std::make_unique()) { impl_->enable_ondemand_loadunload_ = enable_ondemand_loadunload; } @@ -87,7 +89,6 @@ MultiLibraryClassLoader::MultiLibraryClassLoader(bool enable_ondemand_loadunload MultiLibraryClassLoader::~MultiLibraryClassLoader() { shutdownAllClassLoaders(); - delete impl_; } std::vector MultiLibraryClassLoader::getRegisteredLibraries() const @@ -128,8 +129,7 @@ ClassLoaderVector MultiLibraryClassLoader::getAllAvailableClassLoaders() const bool MultiLibraryClassLoader::isLibraryAvailable(const std::string & library_name) const { std::vector available_libraries = getRegisteredLibraries(); - return available_libraries.end() != std::find( - available_libraries.begin(), available_libraries.end(), library_name); + return std::ranges::find(available_libraries, library_name) != available_libraries.end(); } void MultiLibraryClassLoader::loadLibrary(const std::string & library_path) @@ -161,11 +161,11 @@ int MultiLibraryClassLoader::unloadLibrary(const std::string & library_path) impl_->active_class_loaders_[library_path] = nullptr; std::lock_guard lock(getClassLoaderPtrVectorImpl().loader_mutex_); auto & class_loader_ptrs = getClassLoaderPtrVectorImpl().class_loader_ptrs_; - for (auto iter = class_loader_ptrs.begin(); iter != class_loader_ptrs.end(); ++iter) { - if (iter->get() == loader) { - class_loader_ptrs.erase(iter); - break; - } + if (auto iter = std::ranges::find( + class_loader_ptrs, loader, [](const auto & p) {return p.get();}); + iter != class_loader_ptrs.end()) + { + class_loader_ptrs.erase(iter); } } } diff --git a/test/unique_ptr_test.cpp b/test/unique_ptr_test.cpp index eb9f8a1..1aa4578 100644 --- a/test/unique_ptr_test.cpp +++ b/test/unique_ptr_test.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include "gtest/gtest.h" @@ -119,7 +120,8 @@ TEST(ClassLoaderUniquePtrTest, basicLoadFailures) { TEST(ClassLoaderUniquePtrTest, MultiLibraryClassLoaderFailures) { class_loader::MultiLibraryClassLoader loader(true); loader.loadLibrary(LIBRARY_1); - EXPECT_THROW(loader.createUniqueInstance("Cat2"), class_loader::ClassLoaderException); + EXPECT_THROW(std::ignore = loader.createUniqueInstance("Cat2"), + class_loader::ClassLoaderException); } TEST(ClassLoaderUniquePtrTest, LibrariesUsedByClassLoader) { diff --git a/test/utest.cpp b/test/utest.cpp index 2870f6b..f4e13ee 100644 --- a/test/utest.cpp +++ b/test/utest.cpp @@ -38,6 +38,7 @@ #include #include #include +#include #include #include "class_loader/class_loader.hpp" @@ -163,7 +164,8 @@ TEST(ClassLoaderUniquePtrTest, basicLoadFailures) { TEST(ClassLoaderUniquePtrTest, MultiLibraryClassLoaderFailures) { class_loader::MultiLibraryClassLoader loader(true); loader.loadLibrary(LIBRARY_1); - EXPECT_THROW(loader.createUniqueInstance("Cat2"), class_loader::ClassLoaderException); + EXPECT_THROW(std::ignore = loader.createUniqueInstance("Cat2"), + class_loader::ClassLoaderException); } TEST(ClassLoaderTest, correctNonLazyLoadUnload) {