Bring melodic-devel closer to ros2 branch (#76)

* comply with extra and pedantic compiler flags

* use c++11 nullptr instead of NULL

* make ABI breaking change for explicit constructors

* make linters happy

* no need to support console_bridge < 0.3.0 anymore

* remove obsolete todo

* add virtual destructor in test

* vector size() returns size_t

* simplify branching

CMakeLists.txt

@@ -1,5 +1,13 @@
-cmake_minimum_required(VERSION 2.8.3)
-project(class_loader)
+cmake_minimum_required(VERSION 3.5)
+project(class_loader CXX)
+
+# Default to C++14
+if(NOT CMAKE_CXX_STANDARD)
+  set(CMAKE_CXX_STANDARD 14)
+endif()
+if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+  add_compile_options(-Wall -Wextra -Wpedantic)
+endif()
 
 find_package(Boost REQUIRED COMPONENTS thread system)
 
@@ -42,7 +50,6 @@ set(${PROJECT_NAME}_HDRS
   include/class_loader/class_loader_core.h
   include/class_loader/class_loader_exceptions.h
   include/class_loader/class_loader_register_macro.h
-  include/class_loader/console_bridge_compatibility.h
   include/class_loader/meta_object.h
   include/class_loader/multi_library_class_loader.h
 )

include/class_loader/class_loader.h

@@ -33,22 +33,20 @@
 #include <boost/bind.hpp>
 #include <boost/shared_ptr.hpp>
 #include <boost/thread/recursive_mutex.hpp>
+#include <cstddef>
 #include <string>
 #include <vector>
 
 #include "console_bridge/console.h"
 
 #include "class_loader/class_loader_core.h"
 #include "class_loader/class_loader_register_macro.h"
-#include "class_loader/console_bridge_compatibility.h"
 
 #if __cplusplus >= 201103L
 #include <memory>
 #include <functional>
 #endif
 
-// TODO(mikaelarguedas) : replace no lints with the explicit keyword in an ABI breaking release
-
 namespace class_loader
 {
 
@@ -77,7 +75,7 @@ class ClassLoader
    * @param library_path - The path of the runtime library to load
    * @param ondemand_load_unload - Indicates if on-demand (lazy) unloading/loading of libraries occurs as plugins are created/destroyed
    */
-  ClassLoader(const std::string & library_path, bool ondemand_load_unload = false);  // NOLINT
+  explicit ClassLoader(const std::string & library_path, bool ondemand_load_unload = false);
 
   /**
    * @brief  Destructor for ClassLoader. All libraries opened by this ClassLoader are unloaded automatically.
@@ -210,23 +208,25 @@ class ClassLoader
   void onPluginDeletion(Base * obj)
   {
     CONSOLE_BRIDGE_logDebug(
-      "class_loader::ClassLoader: Calling onPluginDeletion() for obj ptr = %p.\n", obj);
-    if (obj) {
-      boost::recursive_mutex::scoped_lock lock(plugin_ref_count_mutex_);
-      delete (obj);
-      plugin_ref_count_ = plugin_ref_count_ - 1;
-      assert(plugin_ref_count_ >= 0);
-      if (0 == plugin_ref_count_ && isOnDemandLoadUnloadEnabled()) {
-        if (!ClassLoader::hasUnmanagedInstanceBeenCreated()) {
-          unloadLibraryInternal(false);
-        } else {
-          CONSOLE_BRIDGE_logWarn(
-            "class_loader::ClassLoader: "
-            "Cannot unload library %s even though last shared pointer went out of scope. "
-            "This is because createUnmanagedInstance was used within the scope of this process,"
-            " perhaps by a different ClassLoader. Library will NOT be closed.",
-            getLibraryPath().c_str());
-        }
+      "class_loader::ClassLoader: Calling onPluginDeletion() for obj ptr = %p.\n",
+      reinterpret_cast<void *>(obj));
+    if (nullptr == obj) {
+      return;
+    }
+    boost::recursive_mutex::scoped_lock lock(plugin_ref_count_mutex_);
+    delete (obj);
+    plugin_ref_count_ = plugin_ref_count_ - 1;
+    assert(plugin_ref_count_ >= 0);
+    if (0 == plugin_ref_count_ && isOnDemandLoadUnloadEnabled()) {
+      if (!ClassLoader::hasUnmanagedInstanceBeenCreated()) {
+        unloadLibraryInternal(false);
+      } else {
+        CONSOLE_BRIDGE_logWarn(
+          "class_loader::ClassLoader: "
+          "Cannot unload library %s even though last shared pointer went out of scope. "
+          "This is because createUnmanagedInstance was used within the scope of this process,"
+          " perhaps by a different ClassLoader. Library will NOT be closed.",
+          getLibraryPath().c_str());
       }
     }
   }
@@ -267,7 +267,7 @@ class ClassLoader
 
     Base * obj =
       class_loader::class_loader_private::createInstance<Base>(derived_class_name, this);
-    assert(obj != NULL);  // Unreachable assertion if createInstance() throws on failure
+    assert(obj != nullptr);  // Unreachable assertion if createInstance() throws on failure
 
     if (managed) {
       boost::recursive_mutex::scoped_lock lock(plugin_ref_count_mutex_);

include/class_loader/class_loader_core.h

@@ -31,6 +31,7 @@
 #define CLASS_LOADER__CLASS_LOADER_CORE_H_
 
 #include <boost/thread/recursive_mutex.hpp>
+#include <cstddef>
 #include <cstdio>
 #include <map>
 #include <string>
@@ -56,7 +57,6 @@ namespace class_loader_private
 {
 
 // Typedefs
-/*****************************************************************************/
 typedef std::string LibraryPath;
 typedef std::string ClassName;
 typedef std::string BaseClassName;
@@ -67,11 +67,9 @@ typedef std::vector<LibraryPair> LibraryVector;
 typedef std::vector<AbstractMetaObjectBase *> MetaObjectVector;
 
 // Debug
-/*****************************************************************************/
 void printDebugInfoToScreen();
 
 // Global storage
-/*****************************************************************************/
 
 /**
  * @brief Gets a handle to a global data structure that holds a map of base class names (Base class describes plugin interface) to a FactoryMap which holds the factories for the various different concrete classes that can be instantiated. Note that the Base class is NOT THE LITERAL CLASSNAME, but rather the result of typeid(Base).name() which sometimes is the literal class name (as on Windows) but is often in mangled form (as on Linux).
@@ -147,7 +145,6 @@ bool hasANonPurePluginLibraryBeenOpened();
 void hasANonPurePluginLibraryBeenOpened(bool hasIt);
 
 // Plugin Functions
-/*****************************************************************************/
 
 /**
  * @brief This function is called by the CLASS_LOADER_REGISTER_CLASS macro in plugin_register_macro.h to register factories.
@@ -168,7 +165,7 @@ void registerPlugin(const std::string & class_name, const std::string & base_cla
     class_name.c_str(), getCurrentlyActiveClassLoader(),
     getCurrentlyLoadingLibraryName().c_str());
 
-  if (NULL == getCurrentlyActiveClassLoader()) {
+  if (nullptr == getCurrentlyActiveClassLoader()) {
     CONSOLE_BRIDGE_logDebug("%s",
       "class_loader.impl: ALERT!!! "
       "A library containing plugins has been opened through a means other than through the "
@@ -215,7 +212,7 @@ void registerPlugin(const std::string & class_name, const std::string & base_cla
   CONSOLE_BRIDGE_logDebug(
     "class_loader.class_loader_private: "
     "Registration of %s complete (Metaobject Address = %p)",
-    class_name.c_str(), new_factory);
+    class_name.c_str(), reinterpret_cast<void *>(new_factory));
 }
 
 /**
@@ -227,7 +224,7 @@ void registerPlugin(const std::string & class_name, const std::string & base_cla
 template<typename Base>
 Base * createInstance(const std::string & derived_class_name, ClassLoader * loader)
 {
-  AbstractMetaObject<Base> * factory = NULL;
+  AbstractMetaObject<Base> * factory = nullptr;
 
   getPluginBaseToFactoryMapMapMutex().lock();
   FactoryMap & factoryMap = getFactoryMapForBaseClass<Base>();
@@ -241,13 +238,13 @@ Base * createInstance(const std::string & derived_class_name, ClassLoader * load
   }
   getPluginBaseToFactoryMapMapMutex().unlock();
 
-  Base * obj = NULL;
-  if (factory != NULL && factory->isOwnedBy(loader)) {
+  Base * obj = nullptr;
+  if (factory != nullptr && factory->isOwnedBy(loader)) {
     obj = factory->create();
   }
 
-  if (NULL == obj) {  // Was never created
-    if (factory && factory->isOwnedBy(NULL)) {
+  if (nullptr == obj) {  // Was never created
+    if (factory && factory->isOwnedBy(nullptr)) {
       CONSOLE_BRIDGE_logDebug("%s",
         "class_loader.impl: ALERT!!! "
         "A metaobject (i.e. factory) exists for desired class, but has no owner. "
@@ -269,7 +266,7 @@ Base * createInstance(const std::string & derived_class_name, ClassLoader * load
   CONSOLE_BRIDGE_logDebug(
     "class_loader.class_loader_private: "
     "Created instance of type %s and object pointer = %p",
-    (typeid(obj).name()), obj);
+    (typeid(obj).name()), reinterpret_cast<void *>(obj));
 
   return obj;
 }
@@ -292,7 +289,7 @@ std::vector<std::string> getAvailableClasses(ClassLoader * loader)
     AbstractMetaObjectBase * factory = itr->second;
     if (factory->isOwnedBy(loader)) {
       classes.push_back(itr->first);
-    } else if (factory->isOwnedBy(NULL)) {
+    } else if (factory->isOwnedBy(nullptr)) {
       classes_with_no_owner.push_back(itr->first);
     }
   }

include/class_loader/class_loader_exceptions.h

@@ -33,8 +33,6 @@
 #include <exception>
 #include <string>
 
-// TODO(mikaelarguedas) : replace no lints with the explicit keyword in an ABI breaking release
-
 namespace class_loader
 {
 
@@ -45,7 +43,7 @@ namespace class_loader
 class ClassLoaderException : public std::runtime_error
 {
 public:
-  ClassLoaderException(const std::string error_desc)    // NOLINT(runtime/explicit)
+  explicit ClassLoaderException(const std::string error_desc)
   : std::runtime_error(error_desc) {}
 };
 
@@ -56,7 +54,7 @@ class ClassLoaderException : public std::runtime_error
 class LibraryLoadException : public ClassLoaderException
 {
 public:
-  LibraryLoadException(const std::string error_desc)    // NOLINT(runtime/explicit)
+  explicit LibraryLoadException(const std::string error_desc)
   : ClassLoaderException(error_desc) {}
 };
 
@@ -67,7 +65,7 @@ class LibraryLoadException : public ClassLoaderException
 class LibraryUnloadException : public ClassLoaderException
 {
 public:
-  LibraryUnloadException(const std::string error_desc)    // NOLINT(runtime/explicit)
+  explicit LibraryUnloadException(const std::string error_desc)
   : ClassLoaderException(error_desc) {}
 };
 
@@ -78,7 +76,7 @@ class LibraryUnloadException : public ClassLoaderException
 class CreateClassException : public ClassLoaderException
 {
 public:
-  CreateClassException(const std::string error_desc)    // NOLINT(runtime/explicit)
+  explicit CreateClassException(const std::string error_desc)
   : ClassLoaderException(error_desc) {}
 };
 
@@ -89,7 +87,7 @@ class CreateClassException : public ClassLoaderException
 class NoClassLoaderExistsException : public ClassLoaderException
 {
 public:
-  NoClassLoaderExistsException(const std::string error_desc)    // NOLINT(runtime/explicit)
+  explicit NoClassLoaderExistsException(const std::string error_desc)
   : ClassLoaderException(error_desc) {}
 };
 

include/class_loader/class_loader_register_macro.h

@@ -33,7 +33,6 @@
 #include <string>
 
 #include "class_loader/class_loader_core.h"
-#include "class_loader/console_bridge_compatibility.h"
 
 #include "console_bridge/console.h"
 

include/class_loader/meta_object.h

@@ -38,8 +38,6 @@
 #include <string>
 #include <vector>
 
-#include "class_loader/console_bridge_compatibility.h"
-
 namespace class_loader
 {