From 04425ff406c878b7ad08cac140caf19ce107d13c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-No=C3=ABl=20Grad?= Date: Wed, 26 Apr 2023 18:34:53 +0200 Subject: [PATCH] core: Fix Clang-Tidy 16.0.0 diagnostics Addressed modernize-use-emplace, readability-simplify-boolean-expr, clang-diagnostic-logical-op-parentheses. --- src/core/bond_breakage/bond_breakage.cpp | 2 +- src/core/collision.cpp | 28 +++++++++---------- src/core/electrostatics/p3m.cpp | 8 +++--- src/core/magnetostatics/dp3m.cpp | 4 +-- src/core/p3m/common.hpp | 7 ++--- src/core/particle_node.cpp | 2 +- .../auto_parameters/AutoParameters.hpp | 2 +- .../integrators/IntegratorHandle.cpp | 10 +++---- .../interactions/NonBondedInteractions.hpp | 2 +- .../particle_data/ParticleList.cpp | 11 +++----- src/utils/tests/for_each_pair_test.cpp | 2 +- 11 files changed, 37 insertions(+), 41 deletions(-) diff --git a/src/core/bond_breakage/bond_breakage.cpp b/src/core/bond_breakage/bond_breakage.cpp index c8ec9babf2a..42632731682 100644 --- a/src/core/bond_breakage/bond_breakage.cpp +++ b/src/core/bond_breakage/bond_breakage.cpp @@ -173,7 +173,7 @@ static void remove_pair_bonds_to(Particle &p, int other_pid) { std::vector> to_delete; for (auto b : p.bonds()) { if (b.partner_ids().size() == 1 and b.partner_ids()[0] == other_pid) - to_delete.emplace_back(std::pair{b.bond_id(), other_pid}); + to_delete.emplace_back(b.bond_id(), other_pid); } for (auto const &b : to_delete) { remove_bond(p, BondView(b.first, {&b.second, 1})); diff --git a/src/core/collision.cpp b/src/core/collision.cpp index 981d5c191c0..a32dec37243 100644 --- a/src/core/collision.cpp +++ b/src/core/collision.cpp @@ -93,7 +93,7 @@ static bool bind_centers() { // but does so later in the process. This is needed to guarantee that // a particle can only be glued once, even if queued twice in a single // time step - return collision_params.mode != CollisionModeType::OFF && + return collision_params.mode != CollisionModeType::OFF and collision_params.mode != CollisionModeType::GLUE_TO_SURF; } @@ -130,7 +130,7 @@ void Collision_parameters::initialize() { #ifdef VIRTUAL_SITES // Check vs placement parameter if (collision_params.mode == CollisionModeType::BIND_VS) { - if ((collision_params.vs_placement < 0.) || + if ((collision_params.vs_placement < 0.) or (collision_params.vs_placement > 1.)) { throw std::domain_error( "Parameter 'vs_placement' must be between 0 and 1"); @@ -139,13 +139,13 @@ void Collision_parameters::initialize() { #endif // Check if bonded ia exist - if ((collision_params.mode == CollisionModeType::BIND_CENTERS) && + if ((collision_params.mode == CollisionModeType::BIND_CENTERS) and !bonded_ia_params.contains(collision_params.bond_centers)) { throw std::runtime_error( "Bond in parameter 'bond_centers' was not added to the system"); } - if ((collision_params.mode == CollisionModeType::BIND_VS) && + if ((collision_params.mode == CollisionModeType::BIND_VS) and !bonded_ia_params.contains(collision_params.bond_vs)) { throw std::runtime_error( "Bond in parameter 'bond_vs' was not added to the system"); @@ -153,16 +153,16 @@ void Collision_parameters::initialize() { // If the bond type to bind particle centers is not a pair bond... // Check that the bonds have the right number of partners - if ((collision_params.mode == CollisionModeType::BIND_CENTERS) && + if ((collision_params.mode == CollisionModeType::BIND_CENTERS) and (get_bond_num_partners(collision_params.bond_centers) != 1)) { throw std::runtime_error("The bond type to be used for binding particle " "centers needs to be a pair bond"); } // The bond between the virtual sites can be pair or triple - if ((collision_params.mode == CollisionModeType::BIND_VS) && - !(get_bond_num_partners(collision_params.bond_vs) == 1 || - get_bond_num_partners(collision_params.bond_vs) == 2)) { + if ((collision_params.mode == CollisionModeType::BIND_VS) and + (get_bond_num_partners(collision_params.bond_vs) != 1 and + get_bond_num_partners(collision_params.bond_vs) != 2)) { throw std::runtime_error("The bond type to be used for binding virtual " "sites needs to be a pair or three-particle bond"); } @@ -242,10 +242,10 @@ const Particle &glue_to_surface_calc_vs_pos(const Particle &p1, const double dist_betw_part = vec21.norm(); // Find out, which is the particle to be glued. - if ((p1.type() == collision_params.part_type_to_be_glued) && + if ((p1.type() == collision_params.part_type_to_be_glued) and (p2.type() == collision_params.part_type_to_attach_vs_to)) { c = 1 - collision_params.dist_glued_part_to_vs / dist_betw_part; - } else if ((p2.type() == collision_params.part_type_to_be_glued) && + } else if ((p2.type() == collision_params.part_type_to_be_glued) and (p1.type() == collision_params.part_type_to_attach_vs_to)) { c = collision_params.dist_glued_part_to_vs / dist_betw_part; } else { @@ -299,8 +299,8 @@ void coldet_do_three_particle_bond(Particle &p, Particle const &p1, id2 = p2.id()](BondView const &bond) { auto const partner_ids = bond.partner_ids(); - return ((partner_ids[0] == id1) && (partner_ids[1] == id2)) || - ((partner_ids[0] == id2) && (partner_ids[1] == id1)); + return ((partner_ids[0] == id1) and (partner_ids[1] == id2)) or + ((partner_ids[0] == id2) and (partner_ids[1] == id1)); }; auto const &bonds = p.bonds(); @@ -450,7 +450,7 @@ void three_particle_binding_domain_decomposition( for (auto &c : gathered_queue) { // If we have both particles, at least as ghosts, Get the corresponding cell // indices - if (cell_structure.get_local_particle(c.pp1) && + if (cell_structure.get_local_particle(c.pp1) and cell_structure.get_local_particle(c.pp2)) { Particle &p1 = *cell_structure.get_local_particle(c.pp1); Particle &p2 = *cell_structure.get_local_particle(c.pp2); @@ -459,7 +459,7 @@ void three_particle_binding_domain_decomposition( if (cell1) three_particle_binding_do_search(cell1, p1, p2); - if (cell2 && cell1 != cell2) + if (cell2 and cell1 != cell2) three_particle_binding_do_search(cell2, p1, p2); } // If local particles exist diff --git a/src/core/electrostatics/p3m.cpp b/src/core/electrostatics/p3m.cpp index 1569d88702c..c954125b5ee 100644 --- a/src/core/electrostatics/p3m.cpp +++ b/src/core/electrostatics/p3m.cpp @@ -751,10 +751,10 @@ void CoulombP3M::sanity_checks_boxl() const { } if (p3m.params.epsilon != P3M_EPSILON_METALLIC) { - if (!((box_geo.length()[0] == box_geo.length()[1]) and - (box_geo.length()[1] == box_geo.length()[2])) or - !((p3m.params.mesh[0] == p3m.params.mesh[1]) and - (p3m.params.mesh[1] == p3m.params.mesh[2]))) { + if ((box_geo.length()[0] != box_geo.length()[1]) or + (box_geo.length()[1] != box_geo.length()[2]) or + (p3m.params.mesh[0] != p3m.params.mesh[1]) or + (p3m.params.mesh[1] != p3m.params.mesh[2])) { throw std::runtime_error( "CoulombP3M: non-metallic epsilon requires cubic box"); } diff --git a/src/core/magnetostatics/dp3m.cpp b/src/core/magnetostatics/dp3m.cpp index 3149ced7c73..2b714c12461 100644 --- a/src/core/magnetostatics/dp3m.cpp +++ b/src/core/magnetostatics/dp3m.cpp @@ -874,14 +874,14 @@ void DipolarP3M::sanity_checks_boxl() const { } } - if ((box_geo.length()[0] != box_geo.length()[1]) || + if ((box_geo.length()[0] != box_geo.length()[1]) or (box_geo.length()[1] != box_geo.length()[2])) { throw std::runtime_error("DipolarP3M: requires a cubic box"); } } void DipolarP3M::sanity_checks_periodicity() const { - if (!box_geo.periodic(0) || !box_geo.periodic(1) || !box_geo.periodic(2)) { + if (!box_geo.periodic(0) or !box_geo.periodic(1) or !box_geo.periodic(2)) { throw std::runtime_error( "DipolarP3M: requires periodicity (True, True, True)"); } diff --git a/src/core/p3m/common.hpp b/src/core/p3m/common.hpp index 4ba660aab3f..247a23c1bfe 100644 --- a/src/core/p3m/common.hpp +++ b/src/core/p3m/common.hpp @@ -136,9 +136,8 @@ struct P3MParameters { } if (not(mesh >= Utils::Vector3i::broadcast(1) or - ((mesh[0] >= 1) and - (mesh == Utils::Vector3i{{mesh[0], -1, -1}}))) and - not(tuning and mesh == Utils::Vector3i::broadcast(-1))) { + ((mesh[0] >= 1) and (mesh == Utils::Vector3i{{mesh[0], -1, -1}})) or + (tuning and mesh == Utils::Vector3i::broadcast(-1)))) { throw std::domain_error("Parameter 'mesh' must be > 0"); } @@ -151,7 +150,7 @@ struct P3MParameters { } } - if ((cao < 1 or cao > 7) and not(tuning and cao == -1)) { + if ((cao < 1 or cao > 7) and (not tuning or cao != -1)) { throw std::domain_error("Parameter 'cao' must be >= 1 and <= 7"); } diff --git a/src/core/particle_node.cpp b/src/core/particle_node.cpp index 31360907adc..8c8d46d37a6 100644 --- a/src/core/particle_node.cpp +++ b/src/core/particle_node.cpp @@ -477,7 +477,7 @@ void remove_particle(int p_id) { } else if (this_node == 0) { ::comm_cart.recv(boost::mpi::any_source, 42, p_type); } - assert(not(this_node == 0 and p_type == -1)); + assert(this_node != 0 or p_type != -1); boost::mpi::broadcast(::comm_cart, p_type, 0); remove_id_from_map(p_id, p_type); } diff --git a/src/script_interface/auto_parameters/AutoParameters.hpp b/src/script_interface/auto_parameters/AutoParameters.hpp index 4fec3064859..f770398c05d 100644 --- a/src/script_interface/auto_parameters/AutoParameters.hpp +++ b/src/script_interface/auto_parameters/AutoParameters.hpp @@ -115,7 +115,7 @@ class AutoParameters : public Base { if (m_parameters.count(p.name)) { m_parameters.erase(p.name); } - m_parameters.emplace(std::make_pair(p.name, std::move(p))); + m_parameters.emplace(p.name, std::move(p)); } } diff --git a/src/script_interface/integrators/IntegratorHandle.cpp b/src/script_interface/integrators/IntegratorHandle.cpp index 2a485360572..9668a3b0c5b 100644 --- a/src/script_interface/integrators/IntegratorHandle.cpp +++ b/src/script_interface/integrators/IntegratorHandle.cpp @@ -86,15 +86,15 @@ static bool checkpoint_filter(typename VariantMap::value_type const &kv) { /* When loading from a checkpoint file, defer the integrator object to last, * and skip the time_step if it is -1 to avoid triggering sanity checks. */ - return kv.first != "integrator" and - not(kv.first == "time_step" and ::integ_switch == INTEG_METHOD_NVT and - get_time_step() == -1. and is_type(kv.second) and - get_value(kv.second) == -1.); + return kv.first == "integrator" or + (kv.first == "time_step" and ::integ_switch == INTEG_METHOD_NVT and + get_time_step() == -1. and is_type(kv.second) and + get_value(kv.second) == -1.); } void IntegratorHandle::do_construct(VariantMap const ¶ms) { for (auto const &kv : params) { - if (checkpoint_filter(kv)) { + if (not checkpoint_filter(kv)) { do_set_parameter(kv.first, kv.second); } } diff --git a/src/script_interface/interactions/NonBondedInteractions.hpp b/src/script_interface/interactions/NonBondedInteractions.hpp index 92b4dcdc9e2..c9ad70fe9b5 100644 --- a/src/script_interface/interactions/NonBondedInteractions.hpp +++ b/src/script_interface/interactions/NonBondedInteractions.hpp @@ -41,7 +41,7 @@ namespace Interactions { class NonBondedInteractions : public ObjectHandle { using container_type = - std::unordered_map>; + std::unordered_map>; auto make_interaction(int i, int j) { assert(i <= j); diff --git a/src/script_interface/particle_data/ParticleList.cpp b/src/script_interface/particle_data/ParticleList.cpp index 900c3bbd8c6..e2f2c7ab961 100644 --- a/src/script_interface/particle_data/ParticleList.cpp +++ b/src/script_interface/particle_data/ParticleList.cpp @@ -76,15 +76,12 @@ std::string ParticleList::get_internal_state() const { auto state = Utils::unpack(packed_state); state.name = "Particles::ParticleHandle"; auto const bonds_view = p_handle.call_method("get_bonds_view", {}); - state.params.emplace_back( - std::pair{"bonds", pack(bonds_view)}); + state.params.emplace_back(std::string{"bonds"}, pack(bonds_view)); #ifdef EXCLUSIONS auto const exclusions = p_handle.call_method("get_exclusions", {}); - state.params.emplace_back( - std::pair{"exclusions", pack(exclusions)}); + state.params.emplace_back(std::string{"exclusions"}, pack(exclusions)); #endif // EXCLUSIONS - state.params.emplace_back( - std::pair{"__cpt_sentinel", pack(None{})}); + state.params.emplace_back(std::string{"__cpt_sentinel"}, pack(None{})); return Utils::pack(state); }); @@ -157,7 +154,7 @@ static void auto_exclusions(boost::mpi::communicator const &comm, for (auto const &partner : partners[pid1]) if (partner.first == pid2) return; - partners[pid1].emplace_back(std::pair{pid2, n_bonds}); + partners[pid1].emplace_back(pid2, n_bonds); }; for (auto it = bonded_pairs.begin(); it != bonded_pairs.end(); it += 2) { diff --git a/src/utils/tests/for_each_pair_test.cpp b/src/utils/tests/for_each_pair_test.cpp index d1cc4de3448..6f9fa9de1eb 100644 --- a/src/utils/tests/for_each_pair_test.cpp +++ b/src/utils/tests/for_each_pair_test.cpp @@ -95,7 +95,7 @@ void check_cartesian_pairs(std::size_t n_values_1, std::size_t n_values_2, auto it = pairs.begin(); for (std::size_t i = 0; i < n_values_1; ++i) { for (std::size_t j = 0; j < n_values_2; ++j) { - if (!(exclude_diagonal and i == j)) { + if (!exclude_diagonal or i != j) { BOOST_CHECK((it->first == i) && (it->second == j)); ++it; }