Cleanups related to stricter warnings in C++ (#2180)

Signed-off-by: Michael Carroll <[email protected]>

include/gz/sim/System.hh

@@ -88,6 +88,10 @@ namespace gz
     /// and components are loaded from the corresponding SDF world, and before
     /// simulation begins exectution.
     class ISystemConfigure {
+
+      /// \brief Destructor
+      public: virtual ~ISystemConfigure() = default;
+
       /// \brief Configure the system
       /// \param[in] _entity The entity this plugin is attached to.
       /// \param[in] _sdf The SDF Element associated with this system plugin.
@@ -108,6 +112,10 @@ namespace gz
     /// ISystemConfigureParameters::ConfigureParameters is called after
     /// ISystemConfigure::Configure.
     class ISystemConfigureParameters {
+
+      /// \brief Destructor
+      public: virtual ~ISystemConfigureParameters() = default;
+
       /// \brief Configure the parameters of the system.
       /// \param[in] _registry The parameter registry.
       public: virtual void ConfigureParameters(
@@ -117,27 +125,39 @@ namespace gz
 
 
     class ISystemReset {
+      /// \brief Destructor
+      public: virtual ~ISystemReset () = default;
+
       public: virtual void Reset(const UpdateInfo &_info,
                                  EntityComponentManager &_ecm) = 0;
     };
 
     /// \class ISystemPreUpdate ISystem.hh gz/sim/System.hh
     /// \brief Interface for a system that uses the PreUpdate phase
     class ISystemPreUpdate {
+      /// \brief Destructor
+      public: virtual ~ISystemPreUpdate() = default;
+
       public: virtual void PreUpdate(const UpdateInfo &_info,
                                      EntityComponentManager &_ecm) = 0;
     };
 
     /// \class ISystemUpdate ISystem.hh gz/sim/System.hh
     /// \brief Interface for a system that uses the Update phase
     class ISystemUpdate {
+      /// \brief Destructor
+      public: virtual ~ISystemUpdate() = default;
+
       public: virtual void Update(const UpdateInfo &_info,
                                   EntityComponentManager &_ecm) = 0;
     };
 
     /// \class ISystemPostUpdate ISystem.hh gz/sim/System.hh
     /// \brief Interface for a system that uses the PostUpdate phase
     class ISystemPostUpdate{
+      /// \brief Destructor
+      public: virtual ~ISystemPostUpdate() = default;
+
       public: virtual void PostUpdate(const UpdateInfo &_info,
                                       const EntityComponentManager &_ecm) = 0;
     };

include/gz/sim/detail/EntityComponentManager.hh

@@ -42,30 +42,20 @@ inline namespace GZ_SIM_VERSION_NAMESPACE {
 namespace traits
 {
   /// \brief Helper struct to determine if an equality operator is present.
-  struct TestEqualityOperator
-  {
-  };
+  struct TestEqualityOperator {};
   template<typename T>
   TestEqualityOperator operator == (const T&, const T&);
 
   /// \brief Type trait that determines if an operator== is defined for `T`.
   template<typename T>
   struct HasEqualityOperator
   {
-#if !defined(_MSC_VER)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wnonnull"
-#endif
     enum
     {
-      // False positive codecheck "Using C-style cast"
-      value = !std::is_same<decltype(*(T*)(0) == *(T*)(0)), TestEqualityOperator>::value // NOLINT
+      value = !std::is_same<decltype(std::declval<T>() == std::declval<T>()), TestEqualityOperator>::value
     };
-#if !defined(_MSC_VER)
-#pragma GCC diagnostic pop
-#endif
   };
-}
+}  // namespace traits
 
 //////////////////////////////////////////////////
 /// \brief Helper function to compare two objects of the same type using its

src/gui/plugins/global_illumination_civct/CiVctCascadePrivate.cc

@@ -70,7 +70,7 @@ void CiVctCascadePrivate::UpdateAreaHalfSize(int _axis, float _halfSize)
 
   std::lock_guard<std::mutex> lock(this->serviceMutex);
   math::Vector3d areaHalfSize = this->cascade->AreaHalfSize();
-  areaHalfSize[(size_t)_axis] = static_cast<double>(_halfSize);
+  areaHalfSize[static_cast<size_t>(_axis)] = static_cast<double>(_halfSize);
   this->cascade->SetAreaHalfSize(areaHalfSize);
 }
 

src/gui/plugins/global_illumination_civct/GlobalIlluminationCiVct.cc

@@ -796,7 +796,7 @@ void GlobalIlluminationCiVct::PopCascade()
 QObject *GlobalIlluminationCiVct::GetCascade(int _idx) const
 {
   std::lock_guard<std::mutex> lock(this->dataPtr->serviceMutex);
-  return this->dataPtr->cascades[(size_t)_idx].get();
+  return this->dataPtr->cascades[static_cast<size_t>(_idx)].get();
 }
 
 //////////////////////////////////////////////////

src/network/NetworkManager.cc

@@ -81,6 +81,7 @@ std::unique_ptr<NetworkManager> NetworkManager::Create(
     case NetworkRole::ReadOnly:
       // \todo(mjcarroll): Enable ReadOnly
       gzwarn << "ReadOnly role not currently supported" << std::endl;
+      break;
     case NetworkRole::None:
       break;
     default:

src/systems/elevator/vender/afsm/include/afsm/detail/transitions.hpp

@@ -444,19 +444,19 @@ class state_transition_table {
 
     state_transition_table(fsm_type& fsm, state_transition_table const& rhs)
         : fsm_{&fsm},
-          current_state_{ (::std::size_t)rhs.current_state_ },
+          current_state_{ static_cast<::std::size_t>(rhs.current_state_) },
           states_{ inner_states_constructor::copy_construct(fsm, rhs.states_) }
       {}
     state_transition_table(fsm_type& fsm, state_transition_table&& rhs)
         : fsm_{&fsm},
-          current_state_{ (::std::size_t)rhs.current_state_ },
+          current_state_{ static_cast<::std::size_t>(rhs.current_state_) },
           states_{ inner_states_constructor::move_construct(fsm, ::std::move(rhs.states_)) }
       {}
 
     state_transition_table(state_transition_table const&) = delete;
     state_transition_table(state_transition_table&& rhs)
         : fsm_{rhs.fsm_},
-          current_state_{ (::std::size_t)rhs.current_state_ },
+          current_state_{ static_cast<::std::size_t>(rhs.current_state_) },
           states_{ ::std::move(rhs.states_) }
     {}
     state_transition_table&
@@ -499,7 +499,7 @@ class state_transition_table {
 
     ::std::size_t
     current_state() const
-    { return (::std::size_t)current_state_; }
+    { return static_cast<::std::size_t>(current_state_); }
 
     void
     set_current_state(::std::size_t val)

src/systems/joint_traj_control/JointTrajectoryController.cc

@@ -922,19 +922,19 @@ void ActuatedJoint::SetTarget(
     const gz::msgs::JointTrajectoryPoint &_targetPoint,
     const size_t &_jointIndex)
 {
-  if ((signed)_jointIndex < _targetPoint.positions_size())
+  if (static_cast<int>(_jointIndex) < _targetPoint.positions_size())
   {
     this->target.position = _targetPoint.positions(_jointIndex);
   }
-  if ((signed)_jointIndex < _targetPoint.velocities_size())
+  if (static_cast<int>(_jointIndex) < _targetPoint.velocities_size())
   {
     this->target.velocity = _targetPoint.velocities(_jointIndex);
   }
-  if ((signed)_jointIndex < _targetPoint.accelerations_size())
+  if (static_cast<int>(_jointIndex) < _targetPoint.accelerations_size())
   {
     this->target.acceleration = _targetPoint.accelerations(_jointIndex);
   }
-  if ((signed)_jointIndex < _targetPoint.effort_size())
+  if (static_cast<int>(_jointIndex) < _targetPoint.effort_size())
   {
     this->target.effort = _targetPoint.effort(_jointIndex);
   }

src/systems/physics/EntityFeatureMap_TEST.cc

@@ -25,6 +25,7 @@
 #include <gz/physics/Entity.hh>
 #include <gz/physics/ForwardStep.hh>
 #include <gz/physics/Implements.hh>
+#include <gz/physics/InstallationDirectories.hh>
 #include <gz/physics/Link.hh>
 #include <gz/physics/RemoveEntities.hh>
 #include <gz/physics/config.hh>
@@ -57,7 +58,7 @@ class EntityFeatureMapFixture: public InternalFixture<::testing::Test>
     const std::string pluginLib = "gz-physics-dartsim-plugin";
 
     common::SystemPaths systemPaths;
-    systemPaths.AddPluginPaths({GZ_PHYSICS_ENGINE_INSTALL_DIR});
+    systemPaths.AddPluginPaths({gz::physics::getEngineInstallDir()});
 
     auto pathToLib = systemPaths.FindSharedLibrary(pluginLib);
     ASSERT_FALSE(pathToLib.empty())

src/systems/physics/Physics.cc

@@ -61,6 +61,7 @@
 #include <gz/physics/GetContacts.hh>
 #include <gz/physics/GetBoundingBox.hh>
 #include <gz/physics/GetEntities.hh>
+#include <gz/physics/InstallationDirectories.hh>
 #include <gz/physics/Joint.hh>
 #include <gz/physics/Link.hh>
 #include <gz/physics/RemoveEntities.hh>
@@ -806,7 +807,7 @@ void Physics::Configure(const Entity &_entity,
   // * Engines installed with gz-physics
   common::SystemPaths systemPaths;
   systemPaths.SetPluginPathEnv(this->dataPtr->pluginPathEnv);
-  systemPaths.AddPluginPaths({GZ_PHYSICS_ENGINE_INSTALL_DIR});
+  systemPaths.AddPluginPaths({gz::physics::getEngineInstallDir()});
 
   auto pathToLib = systemPaths.FindSharedLibrary(pluginLib);
 

src/systems/thruster/Thruster.cc

@@ -694,18 +694,15 @@ void Thruster::PreUpdate(
 
   gz::sim::Link link(this->dataPtr->linkEntity);
 
-
-  auto pose = worldPose(this->dataPtr->linkEntity, _ecm);
-
   // TODO(arjo129): add logic for custom coordinate frame
   // Convert joint axis to the world frame
   const auto linkWorldPose = worldPose(this->dataPtr->linkEntity, _ecm);
   auto jointWorldPose = linkWorldPose * this->dataPtr->jointPose;
   auto unitVector =
       jointWorldPose.Rot().RotateVector(this->dataPtr->jointAxis).Normalize();
 
-  double desiredThrust;
-  double desiredPropellerAngVel;
+  double desiredThrust {0.0};
+  double desiredPropellerAngVel {0.0};
   {
     std::lock_guard<std::mutex> lock(this->dataPtr->mtx);
     desiredThrust = this->dataPtr->thrust;

test/integration/ModelPhotoShootTest.hh

@@ -108,7 +108,8 @@ void testImages(const std::string &_imageFile,
       }
     }
   }
-  ASSERT_GT((float)equalPixels/(float)totalPixels, 0.99);
+  ASSERT_GT(
+    static_cast<float>(equalPixels)/static_cast<float>(totalPixels), 0.99);
 
   // Deleting files so they do not affect future tests
   EXPECT_EQ(remove(imageFilePath.c_str()), 0);

test/integration/thruster.cc

@@ -337,7 +337,6 @@ void ThrusterTest::TestWorld(const std::string &_world,
   propellerLinVels.clear();
   // Make sure that when the deadband is disabled
   // commands below the deadband should create a movement
-  auto latest_pose = modelPoses.back();
   msgs::Boolean db_msg;
   if (_namespace == "deadband")
   {

test/integration/tracked_vehicle_system.cc

@@ -33,6 +33,7 @@
 #include <gz/physics/ContactProperties.hh>
 #include <gz/physics/FeatureList.hh>
 #include <gz/physics/FeaturePolicy.hh>
+#include <gz/physics/InstallationDirectories.hh>
 #include <gz/physics/config.hh>
 #include <gz/plugin/Loader.hh>
 #include <gz/utils/ExtraTestMacros.hh>
@@ -111,7 +112,7 @@ class TrackedVehicleTest : public InternalFixture<::testing::Test>
     // modifications)
     common::SystemPaths systemPaths;
     systemPaths.SetPluginPathEnv("GZ_SIM_PHYSICS_ENGINE_PATH");
-    systemPaths.AddPluginPaths({GZ_PHYSICS_ENGINE_INSTALL_DIR});
+    systemPaths.AddPluginPaths({gz::physics::getEngineInstallDir()});
 
     auto pathToLib = systemPaths.FindSharedLibrary(*pluginLib);
     ASSERT_FALSE(pathToLib.empty())