Add clang-tidy performance checks.

.clang-tidy

@@ -1,4 +1,4 @@
-Checks: '-*,bugprone-*,modernize-*,-modernize-use-equals-delete,-modernize-use-emplace,readability-identifier-naming'
+Checks: '-*,bugprone-*,modernize-*,performance-*,-modernize-use-equals-delete,-modernize-use-emplace,readability-identifier-naming'
 CheckOptions:
   - { key: readability-identifier-naming.NamespaceCase,         value: lower_case }
   - { key: readability-identifier-naming.ClassCase,             value: CamelCase  }

include/ignition/gazebo/SystemLoader.hh

@@ -53,7 +53,8 @@ namespace ignition
       /// \brief Load and instantiate system plugin from an SDF element.
       /// \param[in] _sdf SDF Element describing plugin instance to be loaded.
       /// \returns Shared pointer to system instance or nullptr.
-      public: std::optional<SystemPluginPtr> LoadPlugin(sdf::ElementPtr _sdf);
+      public: std::optional<SystemPluginPtr> LoadPlugin(
+                  const sdf::ElementPtr &_sdf);
 
       /// \brief Load and instantiate system plugin from name/filename.
       /// \param[in] _filename Shared library filename to load plugin from.
@@ -63,7 +64,7 @@ namespace ignition
       public: std::optional<SystemPluginPtr> LoadPlugin(
                   const std::string &_filename,
                   const std::string &_name,
-                  sdf::ElementPtr _sdf);
+                  const sdf::ElementPtr &_sdf);
 
       /// \brief Makes a printable string with info about systems
       /// \returns A pretty string

include/ignition/gazebo/components/SimpleWrapper.hh

@@ -18,6 +18,7 @@
 #define IGNITION_GAZEBO_COMPONENTS_SIMPLEWRAPPER_HH_
 
 #include <memory>
+#include <utility>
 
 #include <ignition/gazebo/config.hh>
 #include <ignition/gazebo/Export.hh>
@@ -59,15 +60,16 @@ namespace components
 
     /// \brief Move Constructor
     /// \param[in] _simpleWrapper SimpleWrapper component to move.
-    public: SimpleWrapper(SimpleWrapper &&_simpleWrapper) = default;
+    public: SimpleWrapper(SimpleWrapper &&_simpleWrapper) noexcept = default;
 
     /// \brief Destructor.
     public: virtual ~SimpleWrapper() = default;
 
     /// \brief Move assignment operator.
     /// \param[in] _simpleWrapper SimpleWrapper component to move.
     /// \return Reference to this.
-    public: SimpleWrapper &operator=(SimpleWrapper &&_simpleWrapper) = default;
+    public: SimpleWrapper &operator=(
+                SimpleWrapper &&_simpleWrapper) noexcept = default;
 
     /// \brief Copy assignment operator.
     /// \param[in] _simpleWrapper SimpleWrapper component to copy.

src/Conversions_TEST.cc

@@ -99,15 +99,15 @@ TEST(Conversions, Gui)
   EXPECT_TRUE(guiMsg.fullscreen());
   ASSERT_EQ(2, guiMsg.plugin_size());
 
-  auto plugin1 = guiMsg.plugin(0);
+  const auto &plugin1 = guiMsg.plugin(0);
   EXPECT_EQ("plugin-file-1", plugin1.filename());
   EXPECT_EQ("plugin-1", plugin1.name());
 
   EXPECT_NE(plugin1.innerxml().find(
       "<banana>3</banana>"),
       std::string::npos);
 
-  auto plugin2 = guiMsg.plugin(1);
+  const auto &plugin2 = guiMsg.plugin(1);
   EXPECT_EQ("plugin-file-2", plugin2.filename());
   EXPECT_EQ("plugin-2", plugin2.name());
   EXPECT_NE(plugin2.innerxml().find(

src/Model_TEST.cc

@@ -37,7 +37,9 @@ TEST(ModelTest, CopyConstructor)
   ignition::gazebo::Entity id(3);
   ignition::gazebo::Model model(id);
 
-  ignition::gazebo::Model modelCopy(model);
+  // Marked nolint because we are specifically testing copy
+  // constructor here (clang wants unnecessary copies removed)
+  ignition::gazebo::Model modelCopy(model); // NOLINT
   EXPECT_EQ(model.Entity(), modelCopy.Entity());
 }
 

src/ServerPrivate.cc

@@ -135,7 +135,7 @@ bool ServerPrivate::WorldsService(ignition::msgs::StringMsg_V &_res)
 
   _res.Clear();
 
-  for (auto name : this->worldNames)
+  for (const auto &name : this->worldNames)
   {
     _res.add_data(name);
   }

src/SystemLoader.cc

@@ -57,7 +57,7 @@ class ignition::gazebo::SystemLoaderPrivate
   //////////////////////////////////////////////////
   public: bool InstantiateSystemPlugin(const std::string &_filename,
               const std::string &_name,
-              sdf::ElementPtr /*_sdf*/,
+              const sdf::ElementPtr &/*_sdf*/,
               ignition::plugin::PluginPtr &_plugin)
   {
     ignition::common::SystemPaths systemPaths;
@@ -149,7 +149,7 @@ void SystemLoader::AddSystemPluginPath(const std::string &_path)
 std::optional<SystemPluginPtr> SystemLoader::LoadPlugin(
   const std::string &_filename,
   const std::string &_name,
-  sdf::ElementPtr _sdf)
+  const sdf::ElementPtr &_sdf)
 {
   ignition::plugin::PluginPtr plugin;
 
@@ -173,7 +173,8 @@ std::optional<SystemPluginPtr> SystemLoader::LoadPlugin(
 }
 
 //////////////////////////////////////////////////
-std::optional<SystemPluginPtr> SystemLoader::LoadPlugin(sdf::ElementPtr _sdf)
+std::optional<SystemPluginPtr> SystemLoader::LoadPlugin(
+  const sdf::ElementPtr &_sdf)
 {
   if (nullptr == _sdf)
   {

src/gui_main.cc

@@ -14,17 +14,17 @@
  * limitations under the License.
  *
 */
+
 #include <gflags/gflags.h>
-#include <csignal>
 #include <tinyxml2.h>
 
-#include <ignition/common/Console.hh>
+#include <csignal>
+#include <iostream>
 
+#include <ignition/common/Console.hh>
 #include <ignition/gui/Application.hh>
 #include <ignition/gui/MainWindow.hh>
 
-#include <iostream>
-
 #include "ignition/gazebo/config.hh"
 #include "ignition/gazebo/gui/TmpIface.hh"
 
@@ -210,8 +210,8 @@ int main(int _argc, char **_argv)
 
     for (int p = 0; p < res.plugin_size(); ++p)
     {
-      auto plugin = res.plugin(p);
-      auto fileName = plugin.filename();
+      const auto &plugin = res.plugin(p);
+      const auto &fileName = plugin.filename();
       std::string pluginStr = "<plugin filename='" + fileName + "'>" +
           plugin.innerxml() + "</plugin>";
 

src/main.cc

@@ -14,19 +14,18 @@
  * limitations under the License.
  *
 */
-#include <csignal>
+#include <gflags/gflags.h>
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <unistd.h>
-#include <gflags/gflags.h>
+
+#include <csignal>
+#include <iostream>
 
 #include <ignition/common/Console.hh>
 #include <ignition/common/SignalHandler.hh>
 #include <ignition/common/Time.hh>
 
-#include <csignal>
-#include <iostream>
-
 #include "ignition/gazebo/config.hh"
 
 // Gflag command line argument definitions

test/integration/diff_drive_system.cc

@@ -63,19 +63,19 @@ class Relay
 
   public: Relay &OnPreUpdate(MockSystem::CallbackType _cb)
   {
-    this->mockSystem->preUpdateCallback = _cb;
+    this->mockSystem->preUpdateCallback = std::move(_cb);
     return *this;
   }
 
   public: Relay &OnUpdate(MockSystem::CallbackType _cb)
   {
-    this->mockSystem->updateCallback = _cb;
+    this->mockSystem->updateCallback = std::move(_cb);
     return *this;
   }
 
   public: Relay &OnPostUpdate(MockSystem::CallbackTypeConst _cb)
   {
-    this->mockSystem->postUpdateCallback = _cb;
+    this->mockSystem->postUpdateCallback = std::move(_cb);
     return *this;
   }
 
@@ -121,7 +121,7 @@ TEST_P(DiffDriveTest, PublishCmd)
 
   EXPECT_EQ(1000u, poses.size());
 
-  for (auto pose : poses)
+  for (const auto &pose : poses)
   {
     EXPECT_EQ(poses[0], pose);
   }

test/integration/each_new_erased.cc

@@ -56,19 +56,19 @@ class Relay
 
   public: Relay &OnPreUpdate(gazebo::MockSystem::CallbackType _cb)
   {
-    this->mockSystem->preUpdateCallback = _cb;
+    this->mockSystem->preUpdateCallback = std::move(_cb);
     return *this;
   }
 
   public: Relay &OnUpdate(gazebo::MockSystem::CallbackType _cb)
   {
-    this->mockSystem->updateCallback = _cb;
+    this->mockSystem->updateCallback = std::move(_cb);
     return *this;
   }
 
   public: Relay &OnPostUpdate(gazebo::MockSystem::CallbackTypeConst _cb)
   {
-    this->mockSystem->postUpdateCallback = _cb;
+    this->mockSystem->postUpdateCallback = std::move(_cb);
     return *this;
   }
 

test/integration/examples_build.cc

@@ -144,7 +144,8 @@ void ExamplesBuild::Build(const std::string &_type)
     auto base = ignition::common::basename(*dirIter);
 
     // Source directory for this example
-    auto sourceDir = examplesDir + "/" + base;
+    auto sourceDir = examplesDir;
+    sourceDir += "/" + base;
     ASSERT_TRUE(ignition::common::exists(sourceDir));
     igndbg << "Source: " << sourceDir << std::endl;
 

test/integration/physics_system.cc

@@ -76,19 +76,19 @@ class Relay
 
   public: Relay & OnPreUpdate(gazebo::MockSystem::CallbackType _cb)
   {
-    this->mockSystem->preUpdateCallback = _cb;
+    this->mockSystem->preUpdateCallback = std::move(_cb);
     return *this;
   }
 
   public: Relay & OnUpdate(gazebo::MockSystem::CallbackType _cb)
   {
-    this->mockSystem->updateCallback = _cb;
+    this->mockSystem->updateCallback = std::move(_cb);
     return *this;
   }
 
   public: Relay & OnPostUpdate(gazebo::MockSystem::CallbackTypeConst _cb)
   {
-    this->mockSystem->postUpdateCallback = _cb;
+    this->mockSystem->postUpdateCallback = std::move(_cb);
     return *this;
   }