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/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..7861830 100644 --- a/include/class_loader/multi_library_class_loader.hpp +++ b/include/class_loader/multi_library_class_loader.hpp @@ -379,7 +379,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); } } }