From 4e3437a483891acfb0b61eb7fb664259e9d67095 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Wed, 4 Oct 2023 10:09:57 -0500 Subject: [PATCH] Cleanups related to stricter warnings in C++ (#2180) Signed-off-by: Michael Carroll --- include/gz/sim/System.hh | 20 +++++++++++++++++++ .../gz/sim/detail/EntityComponentManager.hh | 16 +++------------ .../CiVctCascadePrivate.cc | 2 +- .../GlobalIlluminationCiVct.cc | 2 +- src/network/NetworkManager.cc | 1 + .../afsm/include/afsm/detail/transitions.hpp | 8 ++++---- .../JointTrajectoryController.cc | 8 ++++---- src/systems/physics/EntityFeatureMap_TEST.cc | 3 ++- src/systems/physics/Physics.cc | 3 ++- src/systems/thruster/Thruster.cc | 7 ++----- test/integration/ModelPhotoShootTest.hh | 3 ++- test/integration/thruster.cc | 1 - test/integration/tracked_vehicle_system.cc | 3 ++- 13 files changed, 44 insertions(+), 33 deletions(-) diff --git a/include/gz/sim/System.hh b/include/gz/sim/System.hh index cc0139161e..51be1708b1 100644 --- a/include/gz/sim/System.hh +++ b/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,6 +125,9 @@ namespace gz class ISystemReset { + /// \brief Destructor + public: virtual ~ISystemReset () = default; + public: virtual void Reset(const UpdateInfo &_info, EntityComponentManager &_ecm) = 0; }; @@ -124,6 +135,9 @@ namespace gz /// \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; }; @@ -131,6 +145,9 @@ namespace gz /// \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; }; @@ -138,6 +155,9 @@ namespace gz /// \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; }; diff --git a/include/gz/sim/detail/EntityComponentManager.hh b/include/gz/sim/detail/EntityComponentManager.hh index 7ce3096e4a..f21c8c8cfe 100644 --- a/include/gz/sim/detail/EntityComponentManager.hh +++ b/include/gz/sim/detail/EntityComponentManager.hh @@ -42,9 +42,7 @@ inline namespace GZ_SIM_VERSION_NAMESPACE { namespace traits { /// \brief Helper struct to determine if an equality operator is present. - struct TestEqualityOperator - { - }; + struct TestEqualityOperator {}; template TestEqualityOperator operator == (const T&, const T&); @@ -52,20 +50,12 @@ namespace traits template 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::value // NOLINT + value = !std::is_same() == std::declval()), 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 diff --git a/src/gui/plugins/global_illumination_civct/CiVctCascadePrivate.cc b/src/gui/plugins/global_illumination_civct/CiVctCascadePrivate.cc index de9149b5b1..71524fb471 100644 --- a/src/gui/plugins/global_illumination_civct/CiVctCascadePrivate.cc +++ b/src/gui/plugins/global_illumination_civct/CiVctCascadePrivate.cc @@ -70,7 +70,7 @@ void CiVctCascadePrivate::UpdateAreaHalfSize(int _axis, float _halfSize) std::lock_guard lock(this->serviceMutex); math::Vector3d areaHalfSize = this->cascade->AreaHalfSize(); - areaHalfSize[(size_t)_axis] = static_cast(_halfSize); + areaHalfSize[static_cast(_axis)] = static_cast(_halfSize); this->cascade->SetAreaHalfSize(areaHalfSize); } diff --git a/src/gui/plugins/global_illumination_civct/GlobalIlluminationCiVct.cc b/src/gui/plugins/global_illumination_civct/GlobalIlluminationCiVct.cc index d2b8f4c20c..38ac3dad12 100644 --- a/src/gui/plugins/global_illumination_civct/GlobalIlluminationCiVct.cc +++ b/src/gui/plugins/global_illumination_civct/GlobalIlluminationCiVct.cc @@ -796,7 +796,7 @@ void GlobalIlluminationCiVct::PopCascade() QObject *GlobalIlluminationCiVct::GetCascade(int _idx) const { std::lock_guard lock(this->dataPtr->serviceMutex); - return this->dataPtr->cascades[(size_t)_idx].get(); + return this->dataPtr->cascades[static_cast(_idx)].get(); } ////////////////////////////////////////////////// diff --git a/src/network/NetworkManager.cc b/src/network/NetworkManager.cc index 070264a2f3..e1cde9935e 100644 --- a/src/network/NetworkManager.cc +++ b/src/network/NetworkManager.cc @@ -81,6 +81,7 @@ std::unique_ptr NetworkManager::Create( case NetworkRole::ReadOnly: // \todo(mjcarroll): Enable ReadOnly gzwarn << "ReadOnly role not currently supported" << std::endl; + break; case NetworkRole::None: break; default: diff --git a/src/systems/elevator/vender/afsm/include/afsm/detail/transitions.hpp b/src/systems/elevator/vender/afsm/include/afsm/detail/transitions.hpp index e4fe1fa821..1619b4b721 100644 --- a/src/systems/elevator/vender/afsm/include/afsm/detail/transitions.hpp +++ b/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) diff --git a/src/systems/joint_traj_control/JointTrajectoryController.cc b/src/systems/joint_traj_control/JointTrajectoryController.cc index d6882f9796..49e9295489 100644 --- a/src/systems/joint_traj_control/JointTrajectoryController.cc +++ b/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(_jointIndex) < _targetPoint.positions_size()) { this->target.position = _targetPoint.positions(_jointIndex); } - if ((signed)_jointIndex < _targetPoint.velocities_size()) + if (static_cast(_jointIndex) < _targetPoint.velocities_size()) { this->target.velocity = _targetPoint.velocities(_jointIndex); } - if ((signed)_jointIndex < _targetPoint.accelerations_size()) + if (static_cast(_jointIndex) < _targetPoint.accelerations_size()) { this->target.acceleration = _targetPoint.accelerations(_jointIndex); } - if ((signed)_jointIndex < _targetPoint.effort_size()) + if (static_cast(_jointIndex) < _targetPoint.effort_size()) { this->target.effort = _targetPoint.effort(_jointIndex); } diff --git a/src/systems/physics/EntityFeatureMap_TEST.cc b/src/systems/physics/EntityFeatureMap_TEST.cc index 948d83851a..0a8eb2ad06 100644 --- a/src/systems/physics/EntityFeatureMap_TEST.cc +++ b/src/systems/physics/EntityFeatureMap_TEST.cc @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -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()) diff --git a/src/systems/physics/Physics.cc b/src/systems/physics/Physics.cc index 047e209db9..823e38b764 100644 --- a/src/systems/physics/Physics.cc +++ b/src/systems/physics/Physics.cc @@ -61,6 +61,7 @@ #include #include #include +#include #include #include #include @@ -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); diff --git a/src/systems/thruster/Thruster.cc b/src/systems/thruster/Thruster.cc index e7663924b2..6b69a2508d 100644 --- a/src/systems/thruster/Thruster.cc +++ b/src/systems/thruster/Thruster.cc @@ -694,9 +694,6 @@ 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); @@ -704,8 +701,8 @@ void Thruster::PreUpdate( auto unitVector = jointWorldPose.Rot().RotateVector(this->dataPtr->jointAxis).Normalize(); - double desiredThrust; - double desiredPropellerAngVel; + double desiredThrust {0.0}; + double desiredPropellerAngVel {0.0}; { std::lock_guard lock(this->dataPtr->mtx); desiredThrust = this->dataPtr->thrust; diff --git a/test/integration/ModelPhotoShootTest.hh b/test/integration/ModelPhotoShootTest.hh index 5636036267..9cb0961004 100644 --- a/test/integration/ModelPhotoShootTest.hh +++ b/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(equalPixels)/static_cast(totalPixels), 0.99); // Deleting files so they do not affect future tests EXPECT_EQ(remove(imageFilePath.c_str()), 0); diff --git a/test/integration/thruster.cc b/test/integration/thruster.cc index 3a365ea69f..f07f2c2f3c 100644 --- a/test/integration/thruster.cc +++ b/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") { diff --git a/test/integration/tracked_vehicle_system.cc b/test/integration/tracked_vehicle_system.cc index 69b7155f07..9ee6c2f6e1 100644 --- a/test/integration/tracked_vehicle_system.cc +++ b/test/integration/tracked_vehicle_system.cc @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -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())