From b40144642658a2c8fb37d27163b26715bf846fb5 Mon Sep 17 00:00:00 2001 From: Mark Abraham Date: Thu, 19 Mar 2020 09:42:47 +0100 Subject: [PATCH] Fixes for clang-tidy-9 Mostly suggestions for risky repeat branches and more readable use of methods that use no member variables and could be static. This means that some bad, incompletely implemented, or excessively implemented code is now gone or uglier than it was before. The CPU-build classes for GPU management objects didn't use the only member variable impl_, so they were also recommended to become static. That doesn't work for our use case, but swapping the assertion to be !impl_ does all jobs well enough. eg2cHartree_e probably has had the wrong behavior ever since it was introduced, but I don't know and it's not a big problem. LJ12 Encad table is probably fixed, but it's unused, so who cares. --- src/.clang-tidy | 9 ++- .../analysisdata/datamodulemanager.cpp | 6 +- src/gromacs/analysisdata/tests/datatest.h | 4 +- .../tests/densityfittingoptions.cpp | 4 +- .../applied_forces/tests/electricfield.cpp | 4 +- src/gromacs/awh/biasparams.cpp | 5 +- .../commandline/cmdlinemodulemanager.cpp | 2 +- src/gromacs/commandline/cmdlineparser.cpp | 6 +- .../coordinateio/outputadapters/setatoms.cpp | 4 +- .../coordinateio/outputadapters/setatoms.h | 4 +- src/gromacs/coordinateio/tests/requirements.h | 7 ++- .../correlationfunctions/tests/autocorr.cpp | 16 +++--- src/gromacs/domdec/gpuhaloexchange_impl.cpp | 8 +-- .../pme_coordinate_receiver_gpu_impl.cpp | 8 +-- .../ewald/pme_force_sender_gpu_impl.cpp | 6 +- src/gromacs/ewald/pme_pp_comm_gpu_impl.cpp | 12 ++-- src/gromacs/fileio/checkpoint.cpp | 7 +-- src/gromacs/fileio/pdbio.cpp | 10 +--- src/gromacs/fileio/tests/xvgio.cpp | 55 ++++++++----------- src/gromacs/fileio/tpxio.cpp | 34 +++--------- src/gromacs/fileio/xvgr.cpp | 6 +- src/gromacs/gmxana/anadih.cpp | 6 +- src/gromacs/gmxana/dlist.cpp | 2 +- src/gromacs/gmxana/gmx_angle.cpp | 2 +- src/gromacs/gmxana/gmx_do_dssp.cpp | 12 +--- src/gromacs/gmxpreprocess/gen_ad.cpp | 30 ++-------- src/gromacs/gmxpreprocess/grompp.cpp | 2 +- src/gromacs/gmxpreprocess/nm2type.cpp | 2 +- src/gromacs/gmxpreprocess/pdb2top.cpp | 6 +- src/gromacs/gmxpreprocess/topio.cpp | 6 +- src/gromacs/gmxpreprocess/toputil.cpp | 10 ++-- src/gromacs/gpu_utils/device_stream.cpp | 1 + src/gromacs/gpu_utils/hostallocator.h | 3 +- .../gpu_utils/tests/device_stream_manager.cpp | 43 ++++++++------- src/gromacs/listed_forces/gpubonded_impl.cpp | 2 +- src/gromacs/math/densityfit.cpp | 4 +- src/gromacs/math/multidimarray.h | 3 +- src/gromacs/math/units.cpp | 4 +- src/gromacs/mdlib/energyoutput.h | 4 +- src/gromacs/mdlib/mdebin_bar.cpp | 9 +-- src/gromacs/mdlib/tests/constr.cpp | 12 ++-- src/gromacs/mdlib/tests/leapfrog.cpp | 8 +-- src/gromacs/mdlib/tests/settle.cpp | 12 ++-- src/gromacs/mdlib/tests/shake.cpp | 12 ++-- .../mdlib/update_constrain_gpu_impl.cpp | 12 ++-- src/gromacs/mdlib/updategroups.cpp | 2 +- src/gromacs/mdlib/vcm.cpp | 3 +- src/gromacs/mdrun/md.cpp | 8 ++- src/gromacs/mdrun/mimic.cpp | 20 +++---- src/gromacs/mdrun/minimize.cpp | 14 ++--- src/gromacs/mdrun/rerun.cpp | 4 +- src/gromacs/mdrun/runner.cpp | 3 +- .../mdrunutility/tests/threadaffinity_mpi.cpp | 12 ++-- .../mdrunutility/tests/threadaffinitytest.h | 3 +- src/gromacs/mdtypes/inputrec.cpp | 2 +- src/gromacs/mdtypes/inputrec.h | 6 +- .../state_propagator_data_gpu_impl.cpp | 46 ++++++++-------- src/gromacs/mimic/communicator.cpp | 4 +- src/gromacs/mimic/communicator.h | 16 +++--- src/gromacs/options/filenameoptionmanager.cpp | 12 ++-- src/gromacs/options/tests/treesupport.cpp | 4 +- src/gromacs/pulling/pull_rotation.cpp | 9 +-- src/gromacs/pulling/tests/pull.cpp | 2 +- src/gromacs/restraint/restraintmdmodule.cpp | 10 ---- .../restraint/restraintmdmodule_impl.h | 12 +--- src/gromacs/selection/compiler.cpp | 5 +- src/gromacs/selection/compiler.h | 14 +---- src/gromacs/selection/evaluate.cpp | 2 + src/gromacs/selection/poscalc.cpp | 2 +- src/gromacs/selection/scanner_internal.cpp | 6 +- src/gromacs/selection/selectioncollection.cpp | 8 +-- src/gromacs/selection/selectioncollection.h | 2 +- .../selection/selectioncollection_impl.h | 4 +- .../selection/selectionoptionbehavior.cpp | 4 +- src/gromacs/selection/selhelp.cpp | 8 +-- src/gromacs/selection/tests/nbsearch.cpp | 47 ++++++++-------- src/gromacs/selection/tests/poscalc.cpp | 7 ++- src/gromacs/simd/tests/simd_math.cpp | 6 +- src/gromacs/tables/forcetable.cpp | 4 +- src/gromacs/taskassignment/taskassignment.h | 26 ++++----- src/gromacs/tools/mk_angndx.cpp | 4 +- src/gromacs/tools/tests/dump.cpp | 4 +- .../trajectoryanalysis/analysismodule.cpp | 6 +- .../trajectoryanalysis/analysismodule.h | 4 +- .../trajectoryanalysis/modules/angle.cpp | 4 +- .../trajectoryanalysis/modules/distance.cpp | 4 +- .../trajectoryanalysis/modules/freevolume.cpp | 2 +- .../trajectoryanalysis/modules/pairdist.cpp | 4 +- .../trajectoryanalysis/modules/rdf.cpp | 4 +- .../trajectoryanalysis/modules/sasa.cpp | 8 +-- .../trajectoryanalysis/modules/select.cpp | 2 +- .../trajectoryanalysis/modules/trajectory.cpp | 4 +- src/gromacs/utility/filestream.cpp | 12 +--- src/gromacs/utility/filestream.h | 11 +--- src/gromacs/utility/keyvaluetree.cpp | 4 +- src/gromacs/utility/smalloc.h | 4 +- .../utility/tests/inmemoryserializer.cpp | 8 +-- .../utility/tests/keyvaluetreeserializer.cpp | 2 +- 98 files changed, 374 insertions(+), 478 deletions(-) diff --git a/src/.clang-tidy b/src/.clang-tidy index 6efdf2d87c..6a7556b89e 100644 --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -28,7 +28,11 @@ # We have many cases where int is converted to float and we don't care # enough about such potential loss of precision to use explicit casts # in large numbers of places. - +# +# -google-readability-avoid-underscore-in-googletest-name +# We need to use underscores for readability for our legacy types +# and command-line parameter names +# Checks: clang-diagnostic-*,-clang-analyzer-*,-clang-analyzer-security.insecureAPI.strcpy, bugprone-*,misc-*,readability-*,performance-*,mpi-*, -readability-inconsistent-declaration-parameter-name, @@ -51,7 +55,8 @@ Checks: clang-diagnostic-*,-clang-analyzer-*,-clang-analyzer-security.insecureA -readability-magic-numbers, -cppcoreguidelines-macro-usage, -cppcoreguidelines-narrowing-conversions, - -bugprone-narrowing-conversions + -bugprone-narrowing-conversions, + -google-readability-avoid-underscore-in-googletest-name HeaderFilterRegex: .* CheckOptions: - key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor diff --git a/src/gromacs/analysisdata/datamodulemanager.cpp b/src/gromacs/analysisdata/datamodulemanager.cpp index a68bb85459..bd51891b02 100644 --- a/src/gromacs/analysisdata/datamodulemanager.cpp +++ b/src/gromacs/analysisdata/datamodulemanager.cpp @@ -2,7 +2,7 @@ * This file is part of the GROMACS molecular simulation package. * * Copyright (c) 2010-2018, The GROMACS development team. - * Copyright (c) 2019, by the GROMACS development team, led by + * Copyright (c) 2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -107,7 +107,7 @@ public: * \param[in] bSet Value of the property to check against. * \throws APIError if \p module is not compatible with the data. */ - void checkModuleProperty(const IAnalysisDataModule& module, DataProperty property, bool bSet) const; + static void checkModuleProperty(const IAnalysisDataModule& module, DataProperty property, bool bSet); /*! \brief * Checks whether a module is compatible with the data properties. * @@ -175,7 +175,7 @@ AnalysisDataModuleManager::Impl::Impl() : void AnalysisDataModuleManager::Impl::checkModuleProperty(const IAnalysisDataModule& module, DataProperty property, - bool bSet) const + bool bSet) { bool bOk = true; const int flags = module.flags(); diff --git a/src/gromacs/analysisdata/tests/datatest.h b/src/gromacs/analysisdata/tests/datatest.h index 1d32c2d0ae..1e1766b58b 100644 --- a/src/gromacs/analysisdata/tests/datatest.h +++ b/src/gromacs/analysisdata/tests/datatest.h @@ -102,7 +102,7 @@ public: //! Returns the error in column \p i. real error(int i) const { return values_[i].error; } //! Returns whether the value in column \p i is present. - bool present(int /*i*/) const { return true; } + static bool present(int /*i*/) { return true; } //! Returns an AnalysisDataValue for column \p i. AnalysisDataValue value(int i) const { @@ -158,7 +158,7 @@ public: //! Returns x coordinate for the frame. real x() const { return x_; } //! Returns error in the x coordinate for the frame. - real dx() const { return 0.0; } + static real dx() { return 0.0; } //! Number of individual point sets in the frame. int pointSetCount() const { return pointSets_.size(); } diff --git a/src/gromacs/applied_forces/tests/densityfittingoptions.cpp b/src/gromacs/applied_forces/tests/densityfittingoptions.cpp index 4dcc3c5873..6580498ba5 100644 --- a/src/gromacs/applied_forces/tests/densityfittingoptions.cpp +++ b/src/gromacs/applied_forces/tests/densityfittingoptions.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2019, by the GROMACS development team, led by + * Copyright (c) 2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -91,7 +91,7 @@ public: assignOptionsFromKeyValueTree(&densityFittingModuleOptions, transformedMdpValues.object(), nullptr); } - KeyValueTreeObject densityFittingSetActiveAsMdpValues() + static KeyValueTreeObject densityFittingSetActiveAsMdpValues() { // Prepare MDP inputs KeyValueTreeBuilder mdpValueBuilder; diff --git a/src/gromacs/applied_forces/tests/electricfield.cpp b/src/gromacs/applied_forces/tests/electricfield.cpp index bef644480f..62c5b3199f 100644 --- a/src/gromacs/applied_forces/tests/electricfield.cpp +++ b/src/gromacs/applied_forces/tests/electricfield.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2015,2016,2017,2018,2019, by the GROMACS development team, led by + * Copyright (c) 2015,2016,2017,2018,2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -81,7 +81,7 @@ namespace class ElectricFieldTest : public ::testing::Test { public: - void test(int dim, real E0, real omega, real t0, real sigma, real expectedValue) + static void test(int dim, real E0, real omega, real t0, real sigma, real expectedValue) { // Make the electric field module auto module = createElectricFieldModule(); diff --git a/src/gromacs/awh/biasparams.cpp b/src/gromacs/awh/biasparams.cpp index c28d7c615d..d5afec9f5f 100644 --- a/src/gromacs/awh/biasparams.cpp +++ b/src/gromacs/awh/biasparams.cpp @@ -184,12 +184,13 @@ double gaussianGeometryFactor(gmx::ArrayRef xArray) zetaTable = zetaTable1d; } else if (xArray.size() == 2) - { + { // NOLINT bugprone-branch-clone zetaTable = zetaTable2d; } else { - /* TODO... but this is anyway a rough estimate and > 2 dimensions is not so popular. */ + /* TODO... but this is anyway a rough estimate and > 2 dimensions is not so popular. + * Remove the above NOLINT when addressing this */ zetaTable = zetaTable2d; } diff --git a/src/gromacs/commandline/cmdlinemodulemanager.cpp b/src/gromacs/commandline/cmdlinemodulemanager.cpp index 60522e4158..066495cd67 100644 --- a/src/gromacs/commandline/cmdlinemodulemanager.cpp +++ b/src/gromacs/commandline/cmdlinemodulemanager.cpp @@ -277,7 +277,7 @@ public: char*** argv); //! Prints the footer at the end of execution. - void printThanks(FILE* fp); + static void printThanks(FILE* fp); /*! \brief * Maps module names to module objects. diff --git a/src/gromacs/commandline/cmdlineparser.cpp b/src/gromacs/commandline/cmdlineparser.cpp index 8f5b50e798..5f33ed552f 100644 --- a/src/gromacs/commandline/cmdlineparser.cpp +++ b/src/gromacs/commandline/cmdlineparser.cpp @@ -2,7 +2,7 @@ * This file is part of the GROMACS molecular simulation package. * * Copyright (c) 2010-2018, The GROMACS development team. - * Copyright (c) 2019, by the GROMACS development team, led by + * Copyright (c) 2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -79,7 +79,7 @@ public: * \returns The beginning of the option name in \p arg, or NULL if * \p arg does not look like an option. */ - const char* toOptionName(const char* arg) const; + static const char* toOptionName(const char* arg); //! Helper object for assigning the options. OptionsAssigner assigner_; @@ -100,7 +100,7 @@ CommandLineParser::Impl::Impl(Options* options) : assigner_.setAcceptBooleanNoPrefix(true); } -const char* CommandLineParser::Impl::toOptionName(const char* arg) const +const char* CommandLineParser::Impl::toOptionName(const char* arg) { // Lone '-' or '--' is not an option. if (arg[0] != '-' || arg[1] == '\0' || (arg[1] == '-' && arg[2] == '\0')) diff --git a/src/gromacs/coordinateio/outputadapters/setatoms.cpp b/src/gromacs/coordinateio/outputadapters/setatoms.cpp index a450cf6379..5a65e621c7 100644 --- a/src/gromacs/coordinateio/outputadapters/setatoms.cpp +++ b/src/gromacs/coordinateio/outputadapters/setatoms.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2019, by the GROMACS development team, led by + * Copyright (c) 2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -100,7 +100,7 @@ void SetAtoms::processFrame(const int /*framenumber*/, t_trxframe* input) } } -bool SetAtoms::haveFrameAtoms(const t_trxframe& input) const +bool SetAtoms::haveFrameAtoms(const t_trxframe& input) { return input.bAtoms; } diff --git a/src/gromacs/coordinateio/outputadapters/setatoms.h b/src/gromacs/coordinateio/outputadapters/setatoms.h index 58e8a7870d..00d7d1af37 100644 --- a/src/gromacs/coordinateio/outputadapters/setatoms.h +++ b/src/gromacs/coordinateio/outputadapters/setatoms.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2019, by the GROMACS development team, led by + * Copyright (c) 2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -125,7 +125,7 @@ private: * * \param[in] input t_trxframe before we start modifying it. */ - bool haveFrameAtoms(const t_trxframe& input) const; + static bool haveFrameAtoms(const t_trxframe& input); //! Test if the atoms data is available for writing. bool haveAtoms(const t_trxframe& input) const { diff --git a/src/gromacs/coordinateio/tests/requirements.h b/src/gromacs/coordinateio/tests/requirements.h index da4dd48b15..9afbfbf243 100644 --- a/src/gromacs/coordinateio/tests/requirements.h +++ b/src/gromacs/coordinateio/tests/requirements.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2019, by the GROMACS development team, led by + * Copyright (c) 2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -126,7 +126,10 @@ public: * \param[in] options Container for options. * \param[in] type Need to know type of entries. */ - void setModuleFlag(const std::string& optionName, const std::string& optionValues, Options* options, TestEnums type) + static void setModuleFlag(const std::string& optionName, + const std::string& optionValues, + Options* options, + TestEnums type) { OptionsAssigner assigner(options); assigner.start(); diff --git a/src/gromacs/correlationfunctions/tests/autocorr.cpp b/src/gromacs/correlationfunctions/tests/autocorr.cpp index e23ace8fc1..eb6e17e1c1 100644 --- a/src/gromacs/correlationfunctions/tests/autocorr.cpp +++ b/src/gromacs/correlationfunctions/tests/autocorr.cpp @@ -137,18 +137,20 @@ protected: checker_.checkReal(testResult, "Integral"); } - int getDim(unsigned long type) + static int getDim(unsigned long type) { switch (type) { case eacNormal: return 1; case eacVector: return 3; - case eacCos: return 1; - case eacRcross: return 3; - case eacP0: return 3; - case eacP1: return 3; - case eacP2: return 3; - case eacP3: return 3; + case eacCos: + return 1; + // Several intended fall-throughs follow + case eacRcross: + case eacP0: + case eacP1: + case eacP2: + case eacP3: case eacP4: return 3; case eacIden: return 1; default: GMX_RELEASE_ASSERT(false, "Invalid auto correlation option"); return -1; diff --git a/src/gromacs/domdec/gpuhaloexchange_impl.cpp b/src/gromacs/domdec/gpuhaloexchange_impl.cpp index 1e22b4016e..e9106b7c89 100644 --- a/src/gromacs/domdec/gpuhaloexchange_impl.cpp +++ b/src/gromacs/domdec/gpuhaloexchange_impl.cpp @@ -78,7 +78,7 @@ GpuHaloExchange::~GpuHaloExchange() = default; void GpuHaloExchange::reinitHalo(DeviceBuffer /* d_coordinatesBuffer */, DeviceBuffer /* d_forcesBuffer */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for GPU Halo Exchange was called insted of the correct implementation."); } @@ -86,7 +86,7 @@ void GpuHaloExchange::reinitHalo(DeviceBuffer /* d_coordinatesBuffer */, void GpuHaloExchange::communicateHaloCoordinates(const matrix /* box */, GpuEventSynchronizer* /*coordinatesOnDeviceEvent*/) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for GPU Halo Exchange exchange was called insted of the correct " "implementation."); } @@ -94,14 +94,14 @@ void GpuHaloExchange::communicateHaloCoordinates(const matrix /* box */, /*!\brief apply F halo exchange stub. */ void GpuHaloExchange::communicateHaloForces(bool gmx_unused accumulateForces) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for GPU Halo Exchange was called insted of the correct implementation."); } /*!\brief get forces ready on device event stub. */ GpuEventSynchronizer* GpuHaloExchange::getForcesReadyOnDeviceEvent() { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for GPU Halo Exchange was called insted of the correct implementation."); return nullptr; } diff --git a/src/gromacs/ewald/pme_coordinate_receiver_gpu_impl.cpp b/src/gromacs/ewald/pme_coordinate_receiver_gpu_impl.cpp index b0da71cc47..e377cb8731 100644 --- a/src/gromacs/ewald/pme_coordinate_receiver_gpu_impl.cpp +++ b/src/gromacs/ewald/pme_coordinate_receiver_gpu_impl.cpp @@ -67,7 +67,7 @@ PmeCoordinateReceiverGpu::PmeCoordinateReceiverGpu(const DeviceStream& /* pmeStr gmx::ArrayRef /* ppRanks */) : impl_(nullptr) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for PME-PP GPU communication was called instead of the correct " "implementation."); } @@ -77,21 +77,21 @@ PmeCoordinateReceiverGpu::~PmeCoordinateReceiverGpu() = default; /*!\brief init PME-PP GPU communication stub */ void PmeCoordinateReceiverGpu::sendCoordinateBufferAddressToPpRanks(DeviceBuffer /* d_x */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for PME-PP GPU communication initialization was called instead of the " "correct implementation."); } void PmeCoordinateReceiverGpu::launchReceiveCoordinatesFromPpCudaDirect(int /* ppRank */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for PME-PP GPU communication was called instead of the correct " "implementation."); } void PmeCoordinateReceiverGpu::enqueueWaitReceiveCoordinatesFromPpCudaDirect() { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for PME-PP GPU communication was called instead of the correct " "implementation."); } diff --git a/src/gromacs/ewald/pme_force_sender_gpu_impl.cpp b/src/gromacs/ewald/pme_force_sender_gpu_impl.cpp index 3ae502ccd8..aa046ee718 100644 --- a/src/gromacs/ewald/pme_force_sender_gpu_impl.cpp +++ b/src/gromacs/ewald/pme_force_sender_gpu_impl.cpp @@ -66,7 +66,7 @@ PmeForceSenderGpu::PmeForceSenderGpu(const DeviceStream& /*pmeStream */, gmx::ArrayRef /* ppRanks */) : impl_(nullptr) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for PME-PP GPU communication was called instead of the correct " "implementation."); } @@ -76,14 +76,14 @@ PmeForceSenderGpu::~PmeForceSenderGpu() = default; /*!\brief init PME-PP GPU communication stub */ void PmeForceSenderGpu::sendForceBufferAddressToPpRanks(rvec* /* d_f */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for PME-PP GPU communication initialization was called instead of the " "correct implementation."); } void PmeForceSenderGpu::sendFToPpCudaDirect(int /* ppRank */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for PME-PP GPU communication was called instead of the correct " "implementation."); } diff --git a/src/gromacs/ewald/pme_pp_comm_gpu_impl.cpp b/src/gromacs/ewald/pme_pp_comm_gpu_impl.cpp index b53ce94ada..d194b73c20 100644 --- a/src/gromacs/ewald/pme_pp_comm_gpu_impl.cpp +++ b/src/gromacs/ewald/pme_pp_comm_gpu_impl.cpp @@ -68,7 +68,7 @@ PmePpCommGpu::PmePpCommGpu(MPI_Comm /* comm */, const DeviceStream& /* deviceStream */) : impl_(nullptr) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for PME-PP GPU communication was called instead of the correct " "implementation."); } @@ -78,7 +78,7 @@ PmePpCommGpu::~PmePpCommGpu() = default; /*!\brief init PME-PP GPU communication stub */ void PmePpCommGpu::reinit(int /* size */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for PME-PP GPU communication initialization was called instead of the " "correct implementation."); } @@ -87,7 +87,7 @@ void PmePpCommGpu::receiveForceFromPmeCudaDirect(void* /* recvPtr */, int /* recvSize */, bool /* receivePmeForceToGpu */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for PME-PP GPU communication was called instead of the correct " "implementation."); } @@ -97,14 +97,14 @@ void PmePpCommGpu::sendCoordinatesToPmeCudaDirect(void* /* sendPtr */, bool /* sendPmeCoordinatesFromGpu */, GpuEventSynchronizer* /* coordinatesOnDeviceEvent */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for PME-PP GPU communication was called instead of the correct " "implementation."); } void* PmePpCommGpu::getGpuForceStagingPtr() { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for PME-PP GPU communication was called instead of the correct " "implementation."); return nullptr; @@ -112,7 +112,7 @@ void* PmePpCommGpu::getGpuForceStagingPtr() void* PmePpCommGpu::getForcesReadySynchronizer() { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for PME-PP GPU communication was called instead of the correct " "implementation."); return nullptr; diff --git a/src/gromacs/fileio/checkpoint.cpp b/src/gromacs/fileio/checkpoint.cpp index ee9c72daa0..351d7574af 100644 --- a/src/gromacs/fileio/checkpoint.cpp +++ b/src/gromacs/fileio/checkpoint.cpp @@ -1272,16 +1272,11 @@ static int do_cpt_state(XDR* xd, int fflags, t_state* state, FILE* list) break; /* The RNG entries are no longer written, * the next 4 lines are only for reading old files. + * It's OK that three case statements fall through. */ case estLD_RNG_NOTSUPPORTED: - ret = do_cpte_ints(xd, part, i, sflags, 0, nullptr, list); - break; case estLD_RNGI_NOTSUPPORTED: - ret = do_cpte_ints(xd, part, i, sflags, 0, nullptr, list); - break; case estMC_RNG_NOTSUPPORTED: - ret = do_cpte_ints(xd, part, i, sflags, 0, nullptr, list); - break; case estMC_RNGI_NOTSUPPORTED: ret = do_cpte_ints(xd, part, i, sflags, 0, nullptr, list); break; diff --git a/src/gromacs/fileio/pdbio.cpp b/src/gromacs/fileio/pdbio.cpp index 74f776ed90..8e170a04df 100644 --- a/src/gromacs/fileio/pdbio.cpp +++ b/src/gromacs/fileio/pdbio.cpp @@ -730,15 +730,7 @@ gmx_bool is_hydrogen(const char* nm) std::strcpy(buf, nm); trim(buf); - if (buf[0] == 'H') - { - return TRUE; - } - else if ((std::isdigit(buf[0])) && (buf[1] == 'H')) - { - return TRUE; - } - return FALSE; + return ((buf[0] == 'H') || ((std::isdigit(buf[0]) != 0) && (buf[1] == 'H'))); } gmx_bool is_dummymass(const char* nm) diff --git a/src/gromacs/fileio/tests/xvgio.cpp b/src/gromacs/fileio/tests/xvgio.cpp index 865d4da734..20d82b537c 100644 --- a/src/gromacs/fileio/tests/xvgio.cpp +++ b/src/gromacs/fileio/tests/xvgio.cpp @@ -56,10 +56,10 @@ namespace gmx namespace test { -class XvgioTester +class XvgioTest : public ::testing::Test { public: - XvgioTester() { referenceFilename_ = fileManager_.getTemporaryFilePath("ref.xvg"); } + XvgioTest() { referenceFilename_ = fileManager_.getTemporaryFilePath("ref.xvg"); } const std::string& referenceFilename() const { return referenceFilename_; } @@ -72,8 +72,8 @@ public: gmx::TextWriter::writeFileFromString(referenceFilename(), referenceContents()); } - void compareValues(basic_mdspan ref, - basic_mdspan test) + static void compareValues(basic_mdspan ref, + basic_mdspan test) { // The xvg reading routines use a column-major layout, while we would // like to enforce row major behaviour everywhere else. This requires @@ -97,34 +97,30 @@ private: std::string referenceContents_; }; -TEST(XvgioTest, readXvgIntWorks) +TEST_F(XvgioTest, readXvgIntWorks) { - XvgioTester xvgioTester; - xvgioTester.useStringAsXvgFile( + useStringAsXvgFile( "1 2 3\n" "4 5 6\n"); - xvgioTester.writeXvgFile(); - MultiDimArray, dynamicExtents2D> xvgTestData = - readXvgData(xvgioTester.referenceFilename()); + writeXvgFile(); + MultiDimArray, dynamicExtents2D> xvgTestData = readXvgData(referenceFilename()); const int numRows = 2; const int numColumns = 3; MultiDimArray, dynamicExtents2D> xvgRefData(numRows, numColumns); std::iota(begin(xvgRefData), end(xvgRefData), 1); - xvgioTester.compareValues(xvgRefData.asConstView(), xvgTestData.asConstView()); + compareValues(xvgRefData.asConstView(), xvgTestData.asConstView()); } -TEST(XvgioTest, readXvgRealWorks) +TEST_F(XvgioTest, readXvgRealWorks) { - XvgioTester xvgioTester; - xvgioTester.useStringAsXvgFile( + useStringAsXvgFile( "1.1 2.2\n" "3.3 4.4\n" "5.5 6.6\n"); - xvgioTester.writeXvgFile(); - MultiDimArray, dynamicExtents2D> xvgTestData = - readXvgData(xvgioTester.referenceFilename()); + writeXvgFile(); + MultiDimArray, dynamicExtents2D> xvgTestData = readXvgData(referenceFilename()); const int numRows = 3; const int numColumns = 2; @@ -133,42 +129,39 @@ TEST(XvgioTest, readXvgRealWorks) n += 1.1; return n; }); - xvgioTester.compareValues(xvgRefData.asConstView(), xvgTestData.asConstView()); + compareValues(xvgRefData.asConstView(), xvgTestData.asConstView()); } -TEST(XvgioTest, readXvgIgnoreCommentLineWorks) +TEST_F(XvgioTest, readXvgIgnoreCommentLineWorks) { - XvgioTester xvgioTester; - xvgioTester.useStringAsXvgFile( + useStringAsXvgFile( "1 2 3\n" "#comment\n" "4 5 6\n"); - xvgioTester.writeXvgFile(); + writeXvgFile(); - MultiDimArray, dynamicExtents2D> xvgTestData = - readXvgData(xvgioTester.referenceFilename()); + MultiDimArray, dynamicExtents2D> xvgTestData = readXvgData(referenceFilename()); const int numRows = 2; const int numColumns = 3; MultiDimArray, dynamicExtents2D> xvgRefData(numRows, numColumns); std::iota(begin(xvgRefData), end(xvgRefData), 1); - xvgioTester.compareValues(xvgRefData.asConstView(), xvgTestData.asConstView()); + compareValues(xvgRefData.asConstView(), xvgTestData.asConstView()); } -// Todo: Remove this test once all calls to read_xvg have been ported to readXvgData -TEST(XvgioTest, readXvgDeprecatedWorks) +// TODO Remove this test once all calls to read_xvg have been ported to readXvgData +TEST_F(XvgioTest, readXvgDeprecatedWorks) { - XvgioTester xvgioTester; - xvgioTester.useStringAsXvgFile( + useStringAsXvgFile( "1 2 3\n" "4 5 6\n"); - xvgioTester.writeXvgFile(); + writeXvgFile(); std::vector> xvgData = { { 1, 4 }, { 2, 5 }, { 3, 6 } }; double** xvgTestData = nullptr; int testNumColumns; - int testNumRows = read_xvg(xvgioTester.referenceFilename().c_str(), &xvgTestData, &testNumColumns); + int testNumRows = read_xvg(referenceFilename().c_str(), &xvgTestData, &testNumColumns); double** xvgRefData = nullptr; int refNumColumns = 3; diff --git a/src/gromacs/fileio/tpxio.cpp b/src/gromacs/fileio/tpxio.cpp index 0e5ac9ce39..68b4f3e95c 100644 --- a/src/gromacs/fileio/tpxio.cpp +++ b/src/gromacs/fileio/tpxio.cpp @@ -1156,20 +1156,14 @@ static void do_inputrec(gmx::ISerializer* serializer, t_inputrec* ir, int file_v real dummy_rlistlong = -1; serializer->doReal(&dummy_rlistlong); - if (ir->rlist > 0 && (dummy_rlistlong == 0 || dummy_rlistlong > ir->rlist)) - { - // Get mdrun to issue an error (regardless of - // ir->cutoff_scheme). - ir->useTwinRange = true; - } - else - { - // grompp used to set rlistlong actively. Users were - // probably also confused and set rlistlong == rlist. - // However, in all remaining cases, it is safe to let - // mdrun proceed normally. - ir->useTwinRange = false; - } + ir->useTwinRange = (ir->rlist > 0 && (dummy_rlistlong == 0 || dummy_rlistlong > ir->rlist)); + // When true, this forces mdrun to issue an error (regardless of + // ir->cutoff_scheme). + // + // Otherwise, grompp used to set rlistlong actively. Users + // were probably also confused and set rlistlong == rlist. + // However, in all remaining cases, it is safe to let + // mdrun proceed normally. } } else @@ -1352,14 +1346,6 @@ static void do_inputrec(gmx::ISerializer* serializer, t_inputrec* ir, int file_v if (file_version >= 79) { serializer->doBool(&ir->bExpanded); - if (ir->bExpanded) - { - ir->bExpanded = TRUE; - } - else - { - ir->bExpanded = FALSE; - } } if (ir->bExpanded) { @@ -1937,9 +1923,7 @@ static void do_iparams(gmx::ISerializer* serializer, t_functype ftype, t_iparams break; case F_CBTDIHS: serializer->doRealArray(iparams->cbtdihs.cbtcA, NR_CBTDIHS); break; case F_RBDIHS: - serializer->doRealArray(iparams->rbdihs.rbcA, NR_RBDIHS); - serializer->doRealArray(iparams->rbdihs.rbcB, NR_RBDIHS); - break; + // Fall-through intended case F_FOURDIHS: /* Fourier dihedrals are internally represented * as Ryckaert-Bellemans since those are faster to compute. diff --git a/src/gromacs/fileio/xvgr.cpp b/src/gromacs/fileio/xvgr.cpp index 90e2074137..4cc0794db2 100644 --- a/src/gromacs/fileio/xvgr.cpp +++ b/src/gromacs/fileio/xvgr.cpp @@ -512,11 +512,7 @@ static int wordcount(char* ptr) for (i = 0; (ptr[i] != '\0'); i++) { is[cur] = std::isspace(ptr[i]); - if ((0 == i) && !is[cur]) - { - n++; - } - else if ((i > 0) && (!is[cur] && is[prev])) + if (((0 == i) && !is[cur]) || ((i > 0) && (!is[cur] && is[prev]))) { n++; } diff --git a/src/gromacs/gmxana/anadih.cpp b/src/gromacs/gmxana/anadih.cpp index a015a329b8..3a3a952f31 100644 --- a/src/gromacs/gmxana/anadih.cpp +++ b/src/gromacs/gmxana/anadih.cpp @@ -724,11 +724,7 @@ static real calc_fraction(const real angles[], int nangles) { trans += 1.0; } - else if (angle > 270 && angle < 330) - { - gauche += 1.0; - } - else if (angle < 90 && angle > 30) + else if ((angle > 270 && angle < 330) || (angle < 90 && angle > 30)) { gauche += 1.0; } diff --git a/src/gromacs/gmxana/dlist.cpp b/src/gromacs/gmxana/dlist.cpp index 2cc84009d2..908f0fa8ac 100644 --- a/src/gromacs/gmxana/dlist.cpp +++ b/src/gromacs/gmxana/dlist.cpp @@ -127,7 +127,7 @@ t_dlist* mk_dlist(FILE* log, || (std::strcmp(*(atoms->atomname[i]), "SD") == 0) || (std::strcmp(*(atoms->atomname[i]), "OD1") == 0) || (std::strcmp(*(atoms->atomname[i]), "ND1") == 0)) - { + { // NOLINT bugprone-branch-clone atm.Cn[4] = i; } /* by grs - split the Cn[4] into 2 bits to check allowing dih to H */ diff --git a/src/gromacs/gmxana/gmx_angle.cpp b/src/gromacs/gmxana/gmx_angle.cpp index e3de075ad2..70f006512a 100644 --- a/src/gromacs/gmxana/gmx_angle.cpp +++ b/src/gromacs/gmxana/gmx_angle.cpp @@ -212,7 +212,7 @@ int gmx_g_angle(int argc, char* argv[]) mult = 3; maxang = 180.0; break; - case 'd': break; + case 'd': // Intended fall through case 'i': break; case 'r': bRb = TRUE; break; } diff --git a/src/gromacs/gmxana/gmx_do_dssp.cpp b/src/gromacs/gmxana/gmx_do_dssp.cpp index f030505f71..5dda614a9c 100644 --- a/src/gromacs/gmxana/gmx_do_dssp.cpp +++ b/src/gromacs/gmxana/gmx_do_dssp.cpp @@ -268,15 +268,9 @@ static void check_oo(t_atoms* atoms) for (i = 0; (i < atoms->nr); i++) { - if (std::strcmp(*(atoms->atomname[i]), "OXT") == 0) - { - *atoms->atomname[i] = OOO; - } - else if (std::strcmp(*(atoms->atomname[i]), "O1") == 0) - { - *atoms->atomname[i] = OOO; - } - else if (std::strcmp(*(atoms->atomname[i]), "OC1") == 0) + if ((std::strcmp(*(atoms->atomname[i]), "OXT") == 0) + || (std::strcmp(*(atoms->atomname[i]), "O1") == 0) + || (std::strcmp(*(atoms->atomname[i]), "OC1") == 0)) { *atoms->atomname[i] = OOO; } diff --git a/src/gromacs/gmxpreprocess/gen_ad.cpp b/src/gromacs/gmxpreprocess/gen_ad.cpp index 224057a312..bdc4bcc8f2 100644 --- a/src/gromacs/gmxpreprocess/gen_ad.cpp +++ b/src/gromacs/gmxpreprocess/gen_ad.cpp @@ -78,11 +78,7 @@ static bool acomp(const InteractionOfType& a1, const InteractionOfType& a2) { int ac; - if ((ac = (a1.aj() - a2.aj())) != 0) - { - return ac < 0; - } - else if ((ac = (a1.ai() - a2.ai())) != 0) + if (((ac = (a1.aj() - a2.aj())) != 0) || ((ac = (a1.ai() - a2.ai())) != 0)) { return ac < 0; } @@ -111,12 +107,8 @@ static bool dcomp(const InteractionOfType& d1, const InteractionOfType& d2) int dc; /* First sort by J & K (the two central) atoms */ - if ((dc = (d1.aj() - d2.aj())) != 0) - { - return dc < 0; - } - else if ((dc = (d1.ak() - d2.ak())) != 0) - { + if (((dc = (d1.aj() - d2.aj())) != 0) || ((dc = (d1.ak() - d2.ak())) != 0)) + { // NOLINT bugprone-branch-clone return dc < 0; } /* Then make sure to put rtp dihedrals before generated ones */ @@ -129,11 +121,7 @@ static bool dcomp(const InteractionOfType& d1, const InteractionOfType& d2) return false; } /* Then sort by I and J (two outer) atoms */ - else if ((dc = (d1.ai() - d2.ai())) != 0) - { - return dc < 0; - } - else if ((dc = (d1.al() - d2.al())) != 0) + else if (((dc = (d1.ai() - d2.ai())) != 0) || ((dc = (d1.al() - d2.al())) != 0)) { return dc < 0; } @@ -194,15 +182,7 @@ static bool idcomp(const InteractionOfType& a, const InteractionOfType& b) { int d; - if ((d = (a.ai() - b.ai())) != 0) - { - return d < 0; - } - else if ((d = (a.al() - b.al())) != 0) - { - return d < 0; - } - else if ((d = (a.aj() - b.aj())) != 0) + if (((d = (a.ai() - b.ai())) != 0) || ((d = (a.al() - b.al())) != 0) || ((d = (a.aj() - b.aj())) != 0)) { return d < 0; } diff --git a/src/gromacs/gmxpreprocess/grompp.cpp b/src/gromacs/gmxpreprocess/grompp.cpp index 20ad419a74..93e2fc4c00 100644 --- a/src/gromacs/gmxpreprocess/grompp.cpp +++ b/src/gromacs/gmxpreprocess/grompp.cpp @@ -1131,7 +1131,7 @@ static int nrdf_internal(const t_atoms* atoms) } switch (nmass) { - case 0: nrdf = 0; break; + case 0: // Fall through intended case 1: nrdf = 0; break; case 2: nrdf = 1; break; default: nrdf = nmass * 3 - 6; break; diff --git a/src/gromacs/gmxpreprocess/nm2type.cpp b/src/gromacs/gmxpreprocess/nm2type.cpp index a4c50af094..035daabf2a 100644 --- a/src/gromacs/gmxpreprocess/nm2type.cpp +++ b/src/gromacs/gmxpreprocess/nm2type.cpp @@ -173,7 +173,7 @@ enum static int match_str(const char* atom, const char* template_string) { if (!atom || !template_string) - { + { // NOLINT bugprone-branch-clone return ematchNone; } else if (gmx_strcasecmp(atom, template_string) == 0) diff --git a/src/gromacs/gmxpreprocess/pdb2top.cpp b/src/gromacs/gmxpreprocess/pdb2top.cpp index b58dd35433..d4df1c2718 100644 --- a/src/gromacs/gmxpreprocess/pdb2top.cpp +++ b/src/gromacs/gmxpreprocess/pdb2top.cpp @@ -840,11 +840,7 @@ static bool pcompar(const InteractionOfType& a, const InteractionOfType& b) { int d; - if ((d = a.ai() - b.ai()) != 0) - { - return d < 0; - } - else if ((d = a.aj() - b.aj()) != 0) + if (((d = a.ai() - b.ai()) != 0) || ((d = a.aj() - b.aj()) != 0)) { return d < 0; } diff --git a/src/gromacs/gmxpreprocess/topio.cpp b/src/gromacs/gmxpreprocess/topio.cpp index c6115e8e07..8237ed1ea0 100644 --- a/src/gromacs/gmxpreprocess/topio.cpp +++ b/src/gromacs/gmxpreprocess/topio.cpp @@ -634,9 +634,7 @@ static char** read_topol(const char* infile, bGenPairs ? &pair : nullptr, wi); break; - case Directive::d_bondtypes: - push_bt(d, interactions, 2, nullptr, &bondAtomType, pline, wi); - break; + case Directive::d_bondtypes: // Intended to fall through case Directive::d_constrainttypes: push_bt(d, interactions, 2, nullptr, &bondAtomType, pline, wi); break; @@ -662,7 +660,7 @@ static char** read_topol(const char* infile, push_nbt(d, nbparam, atypes, pline, nb_funct, wi); break; - case Directive::d_implicit_genborn_params: + case Directive::d_implicit_genborn_params: // NOLINT bugprone-branch-clone // Skip this line, so old topologies with // GB parameters can be read. break; diff --git a/src/gromacs/gmxpreprocess/toputil.cpp b/src/gromacs/gmxpreprocess/toputil.cpp index 9a573af613..10ebbfce3e 100644 --- a/src/gromacs/gmxpreprocess/toputil.cpp +++ b/src/gromacs/gmxpreprocess/toputil.cpp @@ -90,7 +90,7 @@ static void print_bt(FILE* out, int f = 0; switch (ftype) { - case F_G96ANGLES: f = 1; break; + case F_G96ANGLES: // Intended to fall through case F_G96BONDS: f = 1; break; case F_MORSE: f = 2; break; case F_CUBICBONDS: f = 3; break; @@ -99,18 +99,18 @@ static void print_bt(FILE* out, case F_CROSS_BOND_ANGLES: f = 2; break; case F_CROSS_BOND_BONDS: f = 3; break; case F_UREY_BRADLEY: f = 4; break; - case F_PDIHS: - case F_RBDIHS: + case F_PDIHS: // Intended to fall through + case F_RBDIHS: // Intended to fall through case F_FOURDIHS: bDih = TRUE; break; case F_IDIHS: f = 1; bDih = TRUE; break; - case F_CONSTRNC: f = 1; break; + case F_CONSTRNC: // Intended to fall through case F_VSITE3FD: f = 1; break; case F_VSITE3FAD: f = 2; break; case F_VSITE3OUT: f = 3; break; - case F_VSITE4FDN: f = 1; break; + case F_VSITE4FDN: // Intended to fall through case F_CMAP: f = 1; break; default: bDih = FALSE; diff --git a/src/gromacs/gpu_utils/device_stream.cpp b/src/gromacs/gpu_utils/device_stream.cpp index bfbe049235..ecc8f09af0 100644 --- a/src/gromacs/gpu_utils/device_stream.cpp +++ b/src/gromacs/gpu_utils/device_stream.cpp @@ -54,6 +54,7 @@ void DeviceStream::init(const DeviceContext& /* deviceContext */, DeviceStream::~DeviceStream() = default; +// NOLINTNEXTLINE readability-convert-member-functions-to-static bool DeviceStream::isValid() const { return false; diff --git a/src/gromacs/gpu_utils/hostallocator.h b/src/gromacs/gpu_utils/hostallocator.h index 888c86062e..f483f0cf28 100644 --- a/src/gromacs/gpu_utils/hostallocator.h +++ b/src/gromacs/gpu_utils/hostallocator.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2017,2018,2019, by the GROMACS development team, led by + * Copyright (c) 2017,2018,2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -186,6 +186,7 @@ public: //! Propagate for move using propagate_on_container_swap = std::true_type; //! Use default allocator for copy (same as construct+copy) + // NOLINTNEXTLINE readability-convert-member-functions-to-static HostAllocationPolicy select_on_container_copy_construction() const { return {}; } private: diff --git a/src/gromacs/gpu_utils/tests/device_stream_manager.cpp b/src/gromacs/gpu_utils/tests/device_stream_manager.cpp index e75c80bc07..3698f290a8 100644 --- a/src/gromacs/gpu_utils/tests/device_stream_manager.cpp +++ b/src/gromacs/gpu_utils/tests/device_stream_manager.cpp @@ -69,35 +69,36 @@ const EnumerationArray c_deviceStreamNames = { { "non-bonded local", "non-bonded non-local", "PME", "PME-PP transfer", "update" } }; -//! Test fixture -class DeviceStreamManagerTest : public GpuTest +/*! \brief Non-GPU builds return nullptr instead of streams, + * so we have to expect that in such build configurations. */ +const bool c_canExpectValidStreams = (GMX_GPU != GMX_GPU_NONE); + +//! Helper function to implement readable testing +void expectValidStreams(DeviceStreamManager* manager, std::initializer_list types) { -public: - //! Helper function to implement readable testing - void expectValidStreams(DeviceStreamManager* manager, std::initializer_list types) - { - if (canExpectValidStreams_) - { - for (const DeviceStreamType type : types) - { - SCOPED_TRACE("Testing " + c_deviceStreamNames[type] + " stream."); - EXPECT_TRUE(manager->streamIsValid(type)); - } - } - } - //! Helper function to implement readable testing - void expectInvalidStreams(DeviceStreamManager* manager, std::initializer_list types) + if (c_canExpectValidStreams) { for (const DeviceStreamType type : types) { SCOPED_TRACE("Testing " + c_deviceStreamNames[type] + " stream."); - EXPECT_FALSE(manager->streamIsValid(type)); + EXPECT_TRUE(manager->streamIsValid(type)); } } +} +//! Helper function to implement readable testing +void expectInvalidStreams(DeviceStreamManager* manager, std::initializer_list types) +{ + for (const DeviceStreamType type : types) + { + SCOPED_TRACE("Testing " + c_deviceStreamNames[type] + " stream."); + EXPECT_FALSE(manager->streamIsValid(type)); + } +} - /*! \brief Non-GPU builds return nullptr instead of streams, - * so we have to expect that in such build configurations. */ - const bool canExpectValidStreams_ = (GMX_GPU != GMX_GPU_NONE); +//! Test fixture +class DeviceStreamManagerTest : public GpuTest +{ +public: }; TEST_F(DeviceStreamManagerTest, CorrectStreamsAreReturnedOnNonbondedDevice) diff --git a/src/gromacs/listed_forces/gpubonded_impl.cpp b/src/gromacs/listed_forces/gpubonded_impl.cpp index ec8e43323e..67eec92cac 100644 --- a/src/gromacs/listed_forces/gpubonded_impl.cpp +++ b/src/gromacs/listed_forces/gpubonded_impl.cpp @@ -180,7 +180,7 @@ void GpuBonded::updateInteractionListsAndDeviceBuffers(ArrayRef /* nb bool GpuBonded::haveInteractions() const { - return false; + return !impl_; } void GpuBonded::launchKernel(const t_forcerec* /* fr */, diff --git a/src/gromacs/math/densityfit.cpp b/src/gromacs/math/densityfit.cpp index a9ae79c5e7..3301fc73df 100644 --- a/src/gromacs/math/densityfit.cpp +++ b/src/gromacs/math/densityfit.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2019, by the GROMACS development team, led by + * Copyright (c) 2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -283,7 +283,7 @@ public: } private: - real evaluatePrefactor(real comparisonSquaredSum, real referenceSquaredSum) + static real evaluatePrefactor(real comparisonSquaredSum, real referenceSquaredSum) { GMX_ASSERT(comparisonSquaredSum > 0, "Squared sum of comparison values needs to be larger than zero."); diff --git a/src/gromacs/math/multidimarray.h b/src/gromacs/math/multidimarray.h index 2e1a9e2eb6..8f6047cc40 100644 --- a/src/gromacs/math/multidimarray.h +++ b/src/gromacs/math/multidimarray.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2019, by the GROMACS development team, led by + * Copyright (c) 2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -69,6 +69,7 @@ struct is_resizable().resize(size_t()))>> : s //! Type has a resize member function callable with size_t argument template +// NOLINTNEXTLINE misc-definitions-in-headers constexpr bool is_resizable_v = is_resizable::value; } // namespace detail diff --git a/src/gromacs/math/units.cpp b/src/gromacs/math/units.cpp index 367234a7c2..6881111e60 100644 --- a/src/gromacs/math/units.cpp +++ b/src/gromacs/math/units.cpp @@ -49,7 +49,7 @@ double convert2gmx(double x, int unit) case eg2cNm: return x; case eg2cBohr: return x * BOHR2NM; case eg2cKcal_Mole: return x / CAL2JOULE; - case eg2cHartree: return x * ONE_4PI_EPS0 / BOHR2NM; + case eg2cHartree: return x * ONE_4PI_EPS0 / BOHR2NM; // NOLINT bugprone-branch-clone case eg2cHartree_e: return x * ONE_4PI_EPS0 / BOHR2NM; case eg2cAngstrom3: return x * A2NM * A2NM * A2NM; case eg2cCoulomb: return x / E_CHARGE; @@ -69,7 +69,7 @@ double gmx2convert(double x, int unit) case eg2cNm: return x; case eg2cBohr: return x / BOHR2NM; case eg2cKcal_Mole: return x * CAL2JOULE; - case eg2cHartree: return x / (ONE_4PI_EPS0 / BOHR2NM); + case eg2cHartree: return x / (ONE_4PI_EPS0 / BOHR2NM); // NOLINT bugprone-branch-clone case eg2cHartree_e: return x / (ONE_4PI_EPS0 / BOHR2NM); case eg2cAngstrom3: return x / (A2NM * A2NM * A2NM); case eg2cCoulomb: return x * E_CHARGE; diff --git a/src/gromacs/mdlib/energyoutput.h b/src/gromacs/mdlib/energyoutput.h index fe9b06f311..6bfff750de 100644 --- a/src/gromacs/mdlib/energyoutput.h +++ b/src/gromacs/mdlib/energyoutput.h @@ -216,7 +216,7 @@ public: * \param[in] opts Atom temperature coupling groups options * (annealing is done by groups). */ - void printAnnealingTemperatures(FILE* log, const SimulationGroups* groups, t_grpopts* opts); + static void printAnnealingTemperatures(FILE* log, const SimulationGroups* groups, t_grpopts* opts); /*! \brief Prints average values to log file. * @@ -256,7 +256,7 @@ public: void restoreFromEnergyHistory(const energyhistory_t& enerhist); //! Print an output header to the log file. - void printHeader(FILE* log, int64_t steps, double time); + static void printHeader(FILE* log, int64_t steps, double time); private: //! Timestep diff --git a/src/gromacs/mdlib/mdebin_bar.cpp b/src/gromacs/mdlib/mdebin_bar.cpp index 2c9ddb69f0..394c225dde 100644 --- a/src/gromacs/mdlib/mdebin_bar.cpp +++ b/src/gromacs/mdlib/mdebin_bar.cpp @@ -759,12 +759,5 @@ void mde_delta_h_coll_restore_energyhistory(t_mde_delta_h_coll* dhc, const delta { dhc->start_lambda = deltaH->start_lambda; } - if (dhc->dh[0].ndh > 0) - { - dhc->start_time_set = TRUE; - } - else - { - dhc->start_time_set = FALSE; - } + dhc->start_time_set = (dhc->dh[0].ndh > 0); } diff --git a/src/gromacs/mdlib/tests/constr.cpp b/src/gromacs/mdlib/tests/constr.cpp index c8654f1136..88f5e76c8c 100644 --- a/src/gromacs/mdlib/tests/constr.cpp +++ b/src/gromacs/mdlib/tests/constr.cpp @@ -167,7 +167,9 @@ public: * \param[in] testData Test data structure. * \param[in] pbc Periodic boundary data. */ - void checkConstrainsLength(FloatingPointTolerance tolerance, const ConstraintsTestData& testData, t_pbc pbc) + static void checkConstrainsLength(FloatingPointTolerance tolerance, + const ConstraintsTestData& testData, + t_pbc pbc) { // Test if all the constraints are satisfied @@ -208,7 +210,7 @@ public: * \param[in] testData Test data structure. * \param[in] pbc Periodic boundary data. */ - void checkConstrainsDirection(const ConstraintsTestData& testData, t_pbc pbc) + static void checkConstrainsDirection(const ConstraintsTestData& testData, t_pbc pbc) { for (index c = 0; c < ssize(testData.constraints_) / 3; c++) @@ -248,7 +250,7 @@ public: * \param[in] tolerance Allowed tolerance in COM coordinates. * \param[in] testData Test data structure. */ - void checkCOMCoordinates(FloatingPointTolerance tolerance, const ConstraintsTestData& testData) + static void checkCOMCoordinates(FloatingPointTolerance tolerance, const ConstraintsTestData& testData) { RVec comPrime0({ 0.0, 0.0, 0.0 }); @@ -278,7 +280,7 @@ public: * \param[in] tolerance Allowed tolerance in COM velocity components. * \param[in] testData Test data structure. */ - void checkCOMVelocity(FloatingPointTolerance tolerance, const ConstraintsTestData& testData) + static void checkCOMVelocity(FloatingPointTolerance tolerance, const ConstraintsTestData& testData) { RVec comV0({ 0.0, 0.0, 0.0 }); @@ -307,7 +309,7 @@ public: * \param[in] tolerance Tolerance for the tensor values. * \param[in] testData Test data structure. */ - void checkVirialTensor(FloatingPointTolerance tolerance, const ConstraintsTestData& testData) + static void checkVirialTensor(FloatingPointTolerance tolerance, const ConstraintsTestData& testData) { for (int i = 0; i < DIM; i++) { diff --git a/src/gromacs/mdlib/tests/leapfrog.cpp b/src/gromacs/mdlib/tests/leapfrog.cpp index abfe31c3ab..1a03709ac6 100644 --- a/src/gromacs/mdlib/tests/leapfrog.cpp +++ b/src/gromacs/mdlib/tests/leapfrog.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2019, by the GROMACS development team, led by + * Copyright (c) 2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -165,9 +165,9 @@ public: * \param[in] testData Test data object * \param[in] totalTime Total numerical integration time */ - void testAgainstAnalyticalSolution(FloatingPointTolerance tolerance, - const LeapFrogTestData& testData, - const real totalTime) + static void testAgainstAnalyticalSolution(FloatingPointTolerance tolerance, + const LeapFrogTestData& testData, + const real totalTime) { for (int i = 0; i < testData.numAtoms_; i++) { diff --git a/src/gromacs/mdlib/tests/settle.cpp b/src/gromacs/mdlib/tests/settle.cpp index bc7025c5b1..3a08231685 100644 --- a/src/gromacs/mdlib/tests/settle.cpp +++ b/src/gromacs/mdlib/tests/settle.cpp @@ -202,9 +202,9 @@ public: * \param[in] tolerance Tolerance to compare floating point numbers. * \param[in] testData An object, containing all the data structures needed by SETTLE. */ - void checkConstrainsSatisfied(const int numSettles, - const FloatingPointTolerance tolerance, - const SettleTestData& testData) + static void checkConstrainsSatisfied(const int numSettles, + const FloatingPointTolerance tolerance, + const SettleTestData& testData) { for (int i = 0; i < numSettles; ++i) { @@ -234,9 +234,9 @@ public: * \param[in] tolerance Tolerance to compare floating point numbers. * \param[in] testData An object, containing all the data structures needed by SETTLE. */ - void checkVirialSymmetric(const bool calcVirial, - const FloatingPointTolerance tolerance, - const SettleTestData& testData) + static void checkVirialSymmetric(const bool calcVirial, + const FloatingPointTolerance tolerance, + const SettleTestData& testData) { for (int d = 0; d < DIM; ++d) { diff --git a/src/gromacs/mdlib/tests/shake.cpp b/src/gromacs/mdlib/tests/shake.cpp index 25da551d89..4d29a38f91 100644 --- a/src/gromacs/mdlib/tests/shake.cpp +++ b/src/gromacs/mdlib/tests/shake.cpp @@ -147,12 +147,12 @@ public: } //! Run the test - void runTest(size_t gmx_unused numAtoms, - size_t numConstraints, - const std::vector& iatom, - const std::vector& constrainedDistances, - const std::vector& inverseMasses, - const std::vector& positions) + static void runTest(size_t gmx_unused numAtoms, + size_t numConstraints, + const std::vector& iatom, + const std::vector& constrainedDistances, + const std::vector& inverseMasses, + const std::vector& positions) { // Check the test input is consistent assert(numConstraints * constraintStride == iatom.size()); diff --git a/src/gromacs/mdlib/update_constrain_gpu_impl.cpp b/src/gromacs/mdlib/update_constrain_gpu_impl.cpp index 76f3a0eedf..e290939c09 100644 --- a/src/gromacs/mdlib/update_constrain_gpu_impl.cpp +++ b/src/gromacs/mdlib/update_constrain_gpu_impl.cpp @@ -62,7 +62,7 @@ UpdateConstrainGpu::UpdateConstrainGpu(const t_inputrec& /* ir */, GpuEventSynchronizer* /* xUpdatedOnDevice */) : impl_(nullptr) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for UpdateConstrain was called instead of the correct implementation."); } @@ -79,13 +79,13 @@ void UpdateConstrainGpu::integrate(GpuEventSynchronizer* /* fReadyOnDevice */, const float /* dtPressureCouple */, const matrix /* prVelocityScalingMatrix*/) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for UpdateConstrain was called instead of the correct implementation."); } void UpdateConstrainGpu::scaleCoordinates(const matrix /* scalingMatrix */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for UpdateConstrain was called instead of the correct implementation."); } @@ -96,19 +96,19 @@ void UpdateConstrainGpu::set(DeviceBuffer /* d_x */, const t_mdatoms& /* md */, const int /* numTempScaleValues */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for UpdateConstrain was called instead of the correct implementation."); } void UpdateConstrainGpu::setPbc(const PbcType /* pbcType */, const matrix /* box */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for UpdateConstrain was called instead of the correct implementation."); } GpuEventSynchronizer* UpdateConstrainGpu::getCoordinatesReadySync() { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub for UpdateConstrain was called instead of the correct implementation."); return nullptr; } diff --git a/src/gromacs/mdlib/updategroups.cpp b/src/gromacs/mdlib/updategroups.cpp index f33aa4cb5c..cf4856825c 100644 --- a/src/gromacs/mdlib/updategroups.cpp +++ b/src/gromacs/mdlib/updategroups.cpp @@ -308,7 +308,7 @@ static int detectGroup(int firstAtom, { AtomIndexExtremes extremes = vsiteConstructRange(lastAtom + 1, moltype); if (extremes.minAtom < firstAtom) - { + { // NOLINT bugprone-branch-clone /* Constructing atom precedes the group */ return 0; } diff --git a/src/gromacs/mdlib/vcm.cpp b/src/gromacs/mdlib/vcm.cpp index b34bbdef35..63a3df664a 100644 --- a/src/gromacs/mdlib/vcm.cpp +++ b/src/gromacs/mdlib/vcm.cpp @@ -280,7 +280,7 @@ static void doStopComMotionLinear(const t_mdatoms& mdatoms, gmx::ArrayRefefep != efepNO) @@ -1544,7 +1545,8 @@ void gmx::LegacySimulator::do_md() if (doSimulatedAnnealing) { - energyOutput.printAnnealingTemperatures(do_log ? fplog : nullptr, groups, &(ir->opts)); + gmx::EnergyOutput::printAnnealingTemperatures(do_log ? fplog : nullptr, groups, + &(ir->opts)); } if (do_log || do_ene || do_dr || do_or) { @@ -1671,7 +1673,7 @@ void gmx::LegacySimulator::do_md() { if (ir->nstcalcenergy > 0) { - energyOutput.printAnnealingTemperatures(fplog, groups, &(ir->opts)); + gmx::EnergyOutput::printAnnealingTemperatures(fplog, groups, &(ir->opts)); energyOutput.printAverages(fplog, groups); } } diff --git a/src/gromacs/mdrun/mimic.cpp b/src/gromacs/mdrun/mimic.cpp index 2cfa7705a0..653dae9656 100644 --- a/src/gromacs/mdrun/mimic.cpp +++ b/src/gromacs/mdrun/mimic.cpp @@ -208,13 +208,11 @@ void gmx::LegacySimulator::do_mimic() int nstglobalcomm = 1; const bool bNS = true; - // Communicator to interact with MiMiC - MimicCommunicator mimicCommunicator{}; if (MASTER(cr)) { - mimicCommunicator.init(); - mimicCommunicator.sendInitData(top_global, state_global->x); - ir->nsteps = mimicCommunicator.getStepNumber(); + MimicCommunicator::init(); + MimicCommunicator::sendInitData(top_global, state_global->x); + ir->nsteps = MimicCommunicator::getStepNumber(); } ir->nstxout_compressed = 0; @@ -364,7 +362,7 @@ void gmx::LegacySimulator::do_mimic() if (MASTER(cr)) { - mimicCommunicator.getCoords(&state_global->x, state_global->natoms); + MimicCommunicator::getCoords(&state_global->x, state_global->natoms); } if (ir->efep != efepNO) @@ -396,7 +394,7 @@ void gmx::LegacySimulator::do_mimic() if (MASTER(cr)) { - energyOutput.printHeader(fplog, step, t); /* can we improve the information printed here? */ + EnergyOutput::printHeader(fplog, step, t); /* can we improve the information printed here? */ } if (ir->efep != efepNO) @@ -493,8 +491,8 @@ void gmx::LegacySimulator::do_mimic() if (MASTER(cr)) { - mimicCommunicator.sendEnergies(enerd->term[F_EPOT]); - mimicCommunicator.sendForces(ftemp, state_global->natoms); + MimicCommunicator::sendEnergies(enerd->term[F_EPOT]); + MimicCommunicator::sendForces(ftemp, state_global->natoms); } } @@ -526,7 +524,7 @@ void gmx::LegacySimulator::do_mimic() const bool do_dr = ir->nstdisreout != 0; const bool do_or = ir->nstorireout != 0; - energyOutput.printAnnealingTemperatures(do_log ? fplog : nullptr, groups, &(ir->opts)); + EnergyOutput::printAnnealingTemperatures(do_log ? fplog : nullptr, groups, &(ir->opts)); energyOutput.printStepToEnergyFile(mdoutf_get_fp_ene(outf), do_ene, do_dr, do_or, do_log ? fplog : nullptr, step, t, fcd, awh); @@ -570,7 +568,7 @@ void gmx::LegacySimulator::do_mimic() if (MASTER(cr)) { - mimicCommunicator.finalize(); + MimicCommunicator::finalize(); } if (!thisRankHasDuty(cr, DUTY_PME)) diff --git a/src/gromacs/mdrun/minimize.cpp b/src/gromacs/mdrun/minimize.cpp index 9329d5040b..c6959e3643 100644 --- a/src/gromacs/mdrun/minimize.cpp +++ b/src/gromacs/mdrun/minimize.cpp @@ -1123,7 +1123,7 @@ void LegacySimulator::do_cg() enerd, nullptr, nullptr, nullptr, nullBox, nullptr, nullptr, vir, pres, nullptr, mu_tot, constr); - energyOutput.printHeader(fplog, step, step); + EnergyOutput::printHeader(fplog, step, step); energyOutput.printStepToEnergyFile(mdoutf_get_fp_ene(outf), TRUE, FALSE, FALSE, fplog, step, step, fcd, nullptr); } @@ -1537,7 +1537,7 @@ void LegacySimulator::do_cg() if (do_log) { - energyOutput.printHeader(fplog, step, step); + EnergyOutput::printHeader(fplog, step, step); } energyOutput.printStepToEnergyFile(mdoutf_get_fp_ene(outf), do_ene, FALSE, FALSE, do_log ? fplog : nullptr, step, step, fcd, nullptr); @@ -1577,7 +1577,7 @@ void LegacySimulator::do_cg() if (!do_log) { /* Write final value to log since we didn't do anything the last step */ - energyOutput.printHeader(fplog, step, step); + EnergyOutput::printHeader(fplog, step, step); } if (!do_ene || !do_log) { @@ -1772,7 +1772,7 @@ void LegacySimulator::do_lbfgs() enerd, nullptr, nullptr, nullptr, nullBox, nullptr, nullptr, vir, pres, nullptr, mu_tot, constr); - energyOutput.printHeader(fplog, step, step); + EnergyOutput::printHeader(fplog, step, step); energyOutput.printStepToEnergyFile(mdoutf_get_fp_ene(outf), TRUE, FALSE, FALSE, fplog, step, step, fcd, nullptr); } @@ -2263,7 +2263,7 @@ void LegacySimulator::do_lbfgs() if (do_log) { - energyOutput.printHeader(fplog, step, step); + EnergyOutput::printHeader(fplog, step, step); } energyOutput.printStepToEnergyFile(mdoutf_get_fp_ene(outf), do_ene, FALSE, FALSE, do_log ? fplog : nullptr, step, step, fcd, nullptr); @@ -2303,7 +2303,7 @@ void LegacySimulator::do_lbfgs() */ if (!do_log) /* Write final value to log since we didn't do anythin last step */ { - energyOutput.printHeader(fplog, step, step); + EnergyOutput::printHeader(fplog, step, step); } if (!do_ene || !do_log) /* Write final energy file entries */ { @@ -2441,7 +2441,7 @@ void LegacySimulator::do_steep() if (MASTER(cr)) { - energyOutput.printHeader(fplog, count, count); + EnergyOutput::printHeader(fplog, count, count); } if (count == 0) diff --git a/src/gromacs/mdrun/rerun.cpp b/src/gromacs/mdrun/rerun.cpp index dfa8d39ae4..e3709ab459 100644 --- a/src/gromacs/mdrun/rerun.cpp +++ b/src/gromacs/mdrun/rerun.cpp @@ -510,7 +510,7 @@ void gmx::LegacySimulator::do_rerun() if (MASTER(cr)) { - energyOutput.printHeader(fplog, step, t); /* can we improve the information printed here? */ + EnergyOutput::printHeader(fplog, step, t); /* can we improve the information printed here? */ } if (ir->efep != efepNO) @@ -618,7 +618,7 @@ void gmx::LegacySimulator::do_rerun() const bool do_dr = ir->nstdisreout != 0; const bool do_or = ir->nstorireout != 0; - energyOutput.printAnnealingTemperatures(do_log ? fplog : nullptr, groups, &(ir->opts)); + EnergyOutput::printAnnealingTemperatures(do_log ? fplog : nullptr, groups, &(ir->opts)); energyOutput.printStepToEnergyFile(mdoutf_get_fp_ene(outf), do_ene, do_dr, do_or, do_log ? fplog : nullptr, step, t, fcd, awh); diff --git a/src/gromacs/mdrun/runner.cpp b/src/gromacs/mdrun/runner.cpp index 55e3444572..0656ac7bfe 100644 --- a/src/gromacs/mdrun/runner.cpp +++ b/src/gromacs/mdrun/runner.cpp @@ -1132,8 +1132,7 @@ int Mdrunner::mdrunner() } // Produce the task assignment for this rank - done after DD is constructed - GpuTaskAssignmentsBuilder gpuTaskAssignmentsBuilder; - GpuTaskAssignments gpuTaskAssignments = gpuTaskAssignmentsBuilder.build( + GpuTaskAssignments gpuTaskAssignments = GpuTaskAssignmentsBuilder::build( gpuIdsToUse, userGpuTaskAssignment, *hwinfo, communicator, physicalNodeComm, nonbondedTarget, pmeTarget, bondedTarget, updateTarget, useGpuForNonbonded, useGpuForPme, thisRankHasDuty(cr, DUTY_PP), diff --git a/src/gromacs/mdrunutility/tests/threadaffinity_mpi.cpp b/src/gromacs/mdrunutility/tests/threadaffinity_mpi.cpp index bc72cfaaed..9cd90ccd1e 100644 --- a/src/gromacs/mdrunutility/tests/threadaffinity_mpi.cpp +++ b/src/gromacs/mdrunutility/tests/threadaffinity_mpi.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2016,2017,2019, by the GROMACS development team, led by + * Copyright (c) 2016,2017,2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -114,17 +114,17 @@ TEST(ThreadAffinityMultiRankTest, HandlesTooManyThreadsWithForce) class ThreadAffinityHeterogeneousNodesTest : public ::testing::Test { public: - int currentNode() const { return gmx_node_rank() / 2; } - int indexInNode() const { return gmx_node_rank() % 2; } - bool isMaster() const { return gmx_node_rank() == 0; } + static int currentNode() { return gmx_node_rank() / 2; } + static int indexInNode() { return gmx_node_rank() % 2; } + static bool isMaster() { return gmx_node_rank() == 0; } - void setupNodes(ThreadAffinityTestHelper* helper, std::array cores) + static void setupNodes(ThreadAffinityTestHelper* helper, std::array cores) { const int node = currentNode(); helper->setPhysicalNodeId(node); helper->setLogicalProcessorCount(cores[node]); } - void expectNodeAffinitySet(ThreadAffinityTestHelper* helper, int node, int core) + static void expectNodeAffinitySet(ThreadAffinityTestHelper* helper, int node, int core) { if (currentNode() == node) { diff --git a/src/gromacs/mdrunutility/tests/threadaffinitytest.h b/src/gromacs/mdrunutility/tests/threadaffinitytest.h index 6720e7a682..d760c6d4b1 100644 --- a/src/gromacs/mdrunutility/tests/threadaffinitytest.h +++ b/src/gromacs/mdrunutility/tests/threadaffinitytest.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2016,2017,2018,2019, by the GROMACS development team, led by + * Copyright (c) 2016,2017,2018,2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -108,6 +108,7 @@ public: expectAffinitySet(core); } } + // NOLINTNEXTLINE readability-convert-member-functions-to-static void expectAffinitySetThatFails(int core) { using ::testing::Return; diff --git a/src/gromacs/mdtypes/inputrec.cpp b/src/gromacs/mdtypes/inputrec.cpp index fa25fa4875..616abbdd0b 100644 --- a/src/gromacs/mdtypes/inputrec.cpp +++ b/src/gromacs/mdtypes/inputrec.cpp @@ -1458,7 +1458,7 @@ bool inputrecPbcXY2Walls(const t_inputrec* ir) bool integratorHasConservedEnergyQuantity(const t_inputrec* ir) { if (!EI_MD(ir->eI)) - { + { // NOLINT bugprone-branch-clone // Energy minimization or stochastic integrator: no conservation return false; } diff --git a/src/gromacs/mdtypes/inputrec.h b/src/gromacs/mdtypes/inputrec.h index 8dc7b07c03..ef4302a590 100644 --- a/src/gromacs/mdtypes/inputrec.h +++ b/src/gromacs/mdtypes/inputrec.h @@ -639,7 +639,11 @@ gmx_bool inputrecNphTrotter(const t_inputrec* ir); bool inputrecPbcXY2Walls(const t_inputrec* ir); /*! \brief Returns true for MD integator with T and/or P-coupling that supports - * calculating the conserved energy quantity. + * calculating a conserved energy quantity. + * + * Note that dynamical integrators without T and P coupling (ie NVE) + * return false, i.e. the return value refers to whether there + * is a conserved quantity different than the total energy. */ bool integratorHasConservedEnergyQuantity(const t_inputrec* ir); diff --git a/src/gromacs/mdtypes/state_propagator_data_gpu_impl.cpp b/src/gromacs/mdtypes/state_propagator_data_gpu_impl.cpp index 269b6eb377..1600c7a062 100644 --- a/src/gromacs/mdtypes/state_propagator_data_gpu_impl.cpp +++ b/src/gromacs/mdtypes/state_propagator_data_gpu_impl.cpp @@ -79,14 +79,14 @@ StatePropagatorDataGpu::~StatePropagatorDataGpu() = default; void StatePropagatorDataGpu::reinit(int /* numAtomsLocal */, int /* numAtomsAll */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub method from GPU state propagator data was called instead of one from " "GPU implementation."); } std::tuple StatePropagatorDataGpu::getAtomRangesFromAtomLocality(AtomLocality /* atomLocality */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub method from GPU state propagator data was called instead of one from " "GPU implementation."); return std::make_tuple(0, 0); @@ -94,7 +94,7 @@ std::tuple StatePropagatorDataGpu::getAtomRangesFromAtomLocality(AtomL DeviceBuffer StatePropagatorDataGpu::getCoordinates() { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub method from GPU state propagator data was called instead of one from " "GPU implementation."); return {}; @@ -105,7 +105,7 @@ GpuEventSynchronizer* StatePropagatorDataGpu::getCoordinatesReadyOnDeviceEvent( const SimulationWorkload& /* simulationWork */, const StepWorkload& /* stepWork */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub method from GPU state propagator data was called instead of one from " "GPU implementation."); return nullptr; @@ -113,14 +113,14 @@ GpuEventSynchronizer* StatePropagatorDataGpu::getCoordinatesReadyOnDeviceEvent( void StatePropagatorDataGpu::waitCoordinatesCopiedToDevice(AtomLocality /* atomLocality */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub method from GPU state propagator data was called instead of one from " "GPU implementation."); } GpuEventSynchronizer* StatePropagatorDataGpu::xUpdatedOnDevice() { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub method from GPU state propagator data was called instead of one from " "GPU implementation."); return nullptr; @@ -129,14 +129,14 @@ GpuEventSynchronizer* StatePropagatorDataGpu::xUpdatedOnDevice() void StatePropagatorDataGpu::copyCoordinatesToGpu(const gmx::ArrayRef /* h_x */, AtomLocality /* atomLocality */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub method from GPU state propagator data was called instead of one from " "GPU implementation."); } void StatePropagatorDataGpu::waitCoordinatesReadyOnHost(AtomLocality /* atomLocality */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub method from GPU state propagator data was called instead of one from " "GPU implementation."); } @@ -144,7 +144,7 @@ void StatePropagatorDataGpu::waitCoordinatesReadyOnHost(AtomLocality /* atomLoca void StatePropagatorDataGpu::copyCoordinatesFromGpu(gmx::ArrayRef /* h_x */, AtomLocality /* atomLocality */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub method from GPU state propagator data was called instead of one from " "GPU implementation."); } @@ -152,7 +152,7 @@ void StatePropagatorDataGpu::copyCoordinatesFromGpu(gmx::ArrayRef /* DeviceBuffer StatePropagatorDataGpu::getVelocities() { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub method from GPU state propagator data was called instead of one from " "GPU implementation."); return {}; @@ -161,14 +161,14 @@ DeviceBuffer StatePropagatorDataGpu::getVelocities() void StatePropagatorDataGpu::copyVelocitiesToGpu(const gmx::ArrayRef /* h_v */, AtomLocality /* atomLocality */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub method from GPU state propagator data was called instead of one from " "GPU implementation."); } GpuEventSynchronizer* StatePropagatorDataGpu::getVelocitiesReadyOnDeviceEvent(AtomLocality /* atomLocality */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub method from GPU state propagator data was called instead of one from " "GPU implementation."); return nullptr; @@ -177,14 +177,14 @@ GpuEventSynchronizer* StatePropagatorDataGpu::getVelocitiesReadyOnDeviceEvent(At void StatePropagatorDataGpu::copyVelocitiesFromGpu(gmx::ArrayRef /* h_v */, AtomLocality /* atomLocality */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub method from GPU state propagator data was called instead of one from " "GPU implementation."); } void StatePropagatorDataGpu::waitVelocitiesReadyOnHost(AtomLocality /* atomLocality */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub method from GPU state propagator data was called instead of one from " "GPU implementation."); } @@ -192,7 +192,7 @@ void StatePropagatorDataGpu::waitVelocitiesReadyOnHost(AtomLocality /* atomLocal DeviceBuffer StatePropagatorDataGpu::getForces() { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub method from GPU state propagator data was called instead of one from " "GPU implementation."); return {}; @@ -201,7 +201,7 @@ DeviceBuffer StatePropagatorDataGpu::getForces() void StatePropagatorDataGpu::copyForcesToGpu(const gmx::ArrayRef /* h_f */, AtomLocality /* atomLocality */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub method from GPU state propagator data was called instead of one from " "GPU implementation."); } @@ -209,7 +209,7 @@ void StatePropagatorDataGpu::copyForcesToGpu(const gmx::ArrayRef /* h_f */, AtomLocality /* atomLocality */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub method from GPU state propagator data was called instead of one from " "GPU implementation."); } void StatePropagatorDataGpu::waitForcesReadyOnHost(AtomLocality /* atomLocality */) { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub method from GPU state propagator data was called instead of one from " "GPU implementation."); } @@ -241,7 +241,7 @@ void StatePropagatorDataGpu::waitForcesReadyOnHost(AtomLocality /* atomLocality const DeviceStream* StatePropagatorDataGpu::getUpdateStream() { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub method from GPU state propagator data was called instead of one from " "GPU implementation."); return nullptr; @@ -249,7 +249,7 @@ const DeviceStream* StatePropagatorDataGpu::getUpdateStream() int StatePropagatorDataGpu::numAtomsLocal() { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub method from GPU state propagator data was called instead of one from " "GPU implementation."); return 0; @@ -257,7 +257,7 @@ int StatePropagatorDataGpu::numAtomsLocal() int StatePropagatorDataGpu::numAtomsAll() { - GMX_ASSERT(false, + GMX_ASSERT(!impl_, "A CPU stub method from GPU state propagator data was called instead of one from " "GPU implementation."); return 0; diff --git a/src/gromacs/mimic/communicator.cpp b/src/gromacs/mimic/communicator.cpp index 7c2b89ff74..12dcf874d3 100644 --- a/src/gromacs/mimic/communicator.cpp +++ b/src/gromacs/mimic/communicator.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2018,2019, by the GROMACS development team, led by + * Copyright (c) 2018,2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -102,7 +102,7 @@ void gmx::MimicCommunicator::init() { char path[GMX_PATH_MAX]; gmx_getcwd(path, GMX_PATH_MAX); - return MCL_init_client(path); + MCL_init_client(path); } void gmx::MimicCommunicator::sendInitData(gmx_mtop_t* mtop, PaddedHostVector coords) diff --git a/src/gromacs/mimic/communicator.h b/src/gromacs/mimic/communicator.h index 033a11d482..6d2b9c2761 100644 --- a/src/gromacs/mimic/communicator.h +++ b/src/gromacs/mimic/communicator.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2018,2019, by the GROMACS development team, led by + * Copyright (c) 2018,2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -60,7 +60,7 @@ public: /*! \brief * Initializes the communicator */ - void init(); + static void init(); /*! \brief * Sends the data needed for MiMiC initialization @@ -73,14 +73,14 @@ public: * @param mtop global topology data * @param coords coordinates of all atoms */ - void sendInitData(gmx_mtop_t* mtop, PaddedHostVector coords); + static void sendInitData(gmx_mtop_t* mtop, PaddedHostVector coords); /*! \brief * Gets the number of MD steps to perform from MiMiC * * @return nsteps the number of MD steps to perform */ - int64_t getStepNumber(); + static int64_t getStepNumber(); /*! \brief * Receive and array of updated atomic coordinates from MiMiC @@ -88,14 +88,14 @@ public: * @param x array of coordinates to fill * @param natoms number of atoms in the system */ - void getCoords(PaddedHostVector* x, int natoms); + static void getCoords(PaddedHostVector* x, int natoms); /*! \brief * Send the potential energy value to MiMiC * * @param energy energy value to send */ - void sendEnergies(real energy); + static void sendEnergies(real energy); /*! \brief * Send classical forces acting on all atoms in the system @@ -104,12 +104,12 @@ public: * @param forces array of forces to send * @param natoms number of atoms in the system */ - void sendForces(ArrayRef forces, int natoms); + static void sendForces(ArrayRef forces, int natoms); /*! \brief * Finish communications and disconnect from the server */ - void finalize(); + static void finalize(); }; } // namespace gmx diff --git a/src/gromacs/options/filenameoptionmanager.cpp b/src/gromacs/options/filenameoptionmanager.cpp index 9d15b56c83..54a4d407ff 100644 --- a/src/gromacs/options/filenameoptionmanager.cpp +++ b/src/gromacs/options/filenameoptionmanager.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2014,2015,2017,2019, by the GROMACS development team, led by + * Copyright (c) 2014,2015,2017,2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -198,12 +198,12 @@ std::string FileNameOptionManager::completeFileName(const std::string& value, co return processedValue; } if (bAllowMissing) - { + { // NOLINT bugprone-branch-clone return value + option.defaultExtension(); } else if (option.isLibraryFile()) { - // TODO: Treat also library files here. + // TODO: Treat also library files here and remove the NOLINT. return value + option.defaultExtension(); } else @@ -262,12 +262,12 @@ std::string FileNameOptionManager::completeDefaultFileName(const std::string& return completedName; } if (option.allowMissing()) - { + { // NOLINT bugprone-branch-clone return realPrefix + option.defaultExtension(); } - else if (option.isLibraryFile()) + else if (option.isLibraryFile()) // NOLINT bugprone-branch-clone { - // TODO: Treat also library files here. + // TODO: Treat also library files here and remove the NOLINT return realPrefix + option.defaultExtension(); } else if (option.isSet()) diff --git a/src/gromacs/options/tests/treesupport.cpp b/src/gromacs/options/tests/treesupport.cpp index 8fb7069e63..ef4e480267 100644 --- a/src/gromacs/options/tests/treesupport.cpp +++ b/src/gromacs/options/tests/treesupport.cpp @@ -347,14 +347,14 @@ public: gmx::KeyValueTreeBuilder builder_; private: - std::vector serializeTree(const gmx::KeyValueTreeObject& tree) + static std::vector serializeTree(const gmx::KeyValueTreeObject& tree) { gmx::InMemorySerializer serializer; gmx::serializeKeyValueTree(tree, &serializer); return serializer.finishAndGetBuffer(); } - std::string formatBuffer(const std::vector& buffer) + static std::string formatBuffer(const std::vector& buffer) { return gmx::formatAndJoin(buffer, " ", [](char c) { return gmx::formatString("%02x", static_cast(c)); diff --git a/src/gromacs/pulling/pull_rotation.cpp b/src/gromacs/pulling/pull_rotation.cpp index 9a95f2430d..17cb7bdd8c 100644 --- a/src/gromacs/pulling/pull_rotation.cpp +++ b/src/gromacs/pulling/pull_rotation.cpp @@ -3636,14 +3636,7 @@ std::unique_ptr init_rot(FILE* fplo er->restartWithAppending = (startingBehavior == gmx::StartingBehavior::RestartWithAppending); /* When appending, skip first output to avoid duplicate entries in the data files */ - if (er->restartWithAppending) - { - er->bOut = FALSE; - } - else - { - er->bOut = TRUE; - } + er->bOut = er->restartWithAppending; if (MASTER(cr) && er->bOut) { diff --git a/src/gromacs/pulling/tests/pull.cpp b/src/gromacs/pulling/tests/pull.cpp index 21956445be..fe8e604d6c 100644 --- a/src/gromacs/pulling/tests/pull.cpp +++ b/src/gromacs/pulling/tests/pull.cpp @@ -70,7 +70,7 @@ class PullTest : public ::testing::Test protected: PullTest() {} - void test(PbcType pbcType, matrix box) + static void test(PbcType pbcType, matrix box) { t_pbc pbc; diff --git a/src/gromacs/restraint/restraintmdmodule.cpp b/src/gromacs/restraint/restraintmdmodule.cpp index d940a45667..b1dcb03d09 100644 --- a/src/gromacs/restraint/restraintmdmodule.cpp +++ b/src/gromacs/restraint/restraintmdmodule.cpp @@ -158,16 +158,6 @@ RestraintMDModuleImpl::RestraintMDModuleImpl(std::shared_ptrimpl_->sc_; gmx_sel_evaluate_t evaldata; diff --git a/src/gromacs/selection/compiler.h b/src/gromacs/selection/compiler.h index 08ff2c51a6..6602835a75 100644 --- a/src/gromacs/selection/compiler.h +++ b/src/gromacs/selection/compiler.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2009,2010,2011,2013,2019, by the GROMACS development team, led by + * Copyright (c) 2009,2010,2011,2013,2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -51,7 +51,7 @@ class SelectionCollection; * \brief * Implements selection compilation. * - * This class is used to implement SelectionCollection::compile(). + * This function is used to implement SelectionCollection::compile(). * It prepares the selections in a selection collection for evaluation and * performs some optimizations. * @@ -59,15 +59,7 @@ class SelectionCollection; * * \ingroup module_selection */ -class SelectionCompiler -{ -public: - //! Creates a selection compiler. - SelectionCompiler(); - - //! Compiles the given selection collection. - void compile(SelectionCollection* coll); -}; +void compileSelection(SelectionCollection* coll); } // namespace gmx diff --git a/src/gromacs/selection/evaluate.cpp b/src/gromacs/selection/evaluate.cpp index bf211d5668..fd2be14434 100644 --- a/src/gromacs/selection/evaluate.cpp +++ b/src/gromacs/selection/evaluate.cpp @@ -419,6 +419,7 @@ SelectionEvaluator::SelectionEvaluator() {} * This is the only function that user code should call if they want to * evaluate a selection for a new frame. */ +// NOLINTNEXTLINE readability-convert-member-functions-to-static void SelectionEvaluator::evaluate(SelectionCollection* coll, t_trxframe* fr, t_pbc* pbc) { gmx_ana_selcollection_t* sc = &coll->impl_->sc_; @@ -464,6 +465,7 @@ void SelectionEvaluator::evaluate(SelectionCollection* coll, t_trxframe* fr, t_p * \param[in,out] coll The selection collection to evaluate. * \param[in] nframes Total number of frames. */ +// NOLINTNEXTLINE readability-convert-member-functions-to-static void SelectionEvaluator::evaluateFinal(SelectionCollection* coll, int nframes) { gmx_ana_selcollection_t* sc = &coll->impl_->sc_; diff --git a/src/gromacs/selection/poscalc.cpp b/src/gromacs/selection/poscalc.cpp index fafc654a05..38b8bb67ea 100644 --- a/src/gromacs/selection/poscalc.cpp +++ b/src/gromacs/selection/poscalc.cpp @@ -255,7 +255,7 @@ static e_index_t index_type_for_poscalc(e_poscalc_t type) case POS_ATOM: return INDEX_ATOM; case POS_RES: return INDEX_RES; case POS_MOL: return INDEX_MOL; - case POS_ALL: return INDEX_ALL; + case POS_ALL: // Intended fall through case POS_ALL_PBC: return INDEX_ALL; } return INDEX_UNKNOWN; diff --git a/src/gromacs/selection/scanner_internal.cpp b/src/gromacs/selection/scanner_internal.cpp index b1c345ee06..ff040fe854 100644 --- a/src/gromacs/selection/scanner_internal.cpp +++ b/src/gromacs/selection/scanner_internal.cpp @@ -2,7 +2,7 @@ * This file is part of the GROMACS molecular simulation package. * * Copyright (c) 2009-2018, The GROMACS development team. - * Copyright (c) 2019, by the GROMACS development team, led by + * Copyright (c) 2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -167,7 +167,7 @@ static int init_method_token(YYSTYPE* yylval, } switch (method->type) { - case INT_VALUE: return METHOD_NUMERIC; + case INT_VALUE: // Intended fall through case REAL_VALUE: return METHOD_NUMERIC; case POS_VALUE: return METHOD_POS; case GROUP_VALUE: return METHOD_GROUP; @@ -304,7 +304,7 @@ int _gmx_sel_lexer_process_identifier(YYSTYPE* yylval, YYLTYPE* yylloc, char* yy yylval->sel = new gmx::SelectionTreeElementPointer(var); switch (var->v.type) { - case INT_VALUE: return VARIABLE_NUMERIC; + case INT_VALUE: // Intended fall through case REAL_VALUE: return VARIABLE_NUMERIC; case POS_VALUE: return VARIABLE_POS; case GROUP_VALUE: return VARIABLE_GROUP; diff --git a/src/gromacs/selection/selectioncollection.cpp b/src/gromacs/selection/selectioncollection.cpp index 197f0962c9..a9c6e6825a 100644 --- a/src/gromacs/selection/selectioncollection.cpp +++ b/src/gromacs/selection/selectioncollection.cpp @@ -503,7 +503,7 @@ bool SelectionCollection::Impl::areForcesRequested() const SelectionTopologyProperties -SelectionCollection::Impl::requiredTopologyPropertiesForPositionType(const std::string& post, bool forces) const +SelectionCollection::Impl::requiredTopologyPropertiesForPositionType(const std::string& post, bool forces) { SelectionTopologyProperties props; if (!post.empty()) @@ -687,7 +687,8 @@ bool SelectionCollection::requiresIndexGroups() const SelectionList SelectionCollection::parseFromStdin(int count, bool bInteractive, const std::string& context) { - return parseInteractive(count, &StandardInputStream::instance(), + StandardInputStream inputStream; + return parseInteractive(count, &inputStream, bInteractive ? &TextOutputFile::standardError() : nullptr, context); } @@ -764,8 +765,7 @@ void SelectionCollection::compile() printTree(stderr, false); } - SelectionCompiler compiler; - compiler.compile(this); + compileSelection(this); if (impl_->debugLevel_ != Impl::DebugLevel::None) { diff --git a/src/gromacs/selection/selectioncollection.h b/src/gromacs/selection/selectioncollection.h index 7e3d3daef5..886bb898a0 100644 --- a/src/gromacs/selection/selectioncollection.h +++ b/src/gromacs/selection/selectioncollection.h @@ -419,7 +419,7 @@ private: /*! \brief * Needed for the compiler to freely modify the collection. */ - friend class SelectionCompiler; + friend void compileSelection(SelectionCollection* /*coll*/); /*! \brief * Needed for the evaluator to freely modify the collection. */ diff --git a/src/gromacs/selection/selectioncollection_impl.h b/src/gromacs/selection/selectioncollection_impl.h index 4c79e66c6f..f5cec6b3a0 100644 --- a/src/gromacs/selection/selectioncollection_impl.h +++ b/src/gromacs/selection/selectioncollection_impl.h @@ -160,8 +160,8 @@ public: /*! \brief * Returns topology properties needed for a certain position type. */ - SelectionTopologyProperties requiredTopologyPropertiesForPositionType(const std::string& post, - bool forces) const; + static SelectionTopologyProperties requiredTopologyPropertiesForPositionType(const std::string& post, + bool forces); //! Describes the available debugging levels enum class DebugLevel : int diff --git a/src/gromacs/selection/selectionoptionbehavior.cpp b/src/gromacs/selection/selectionoptionbehavior.cpp index c6c68d8805..5419a6bb87 100644 --- a/src/gromacs/selection/selectionoptionbehavior.cpp +++ b/src/gromacs/selection/selectionoptionbehavior.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2015,2016,2017,2018,2019, by the GROMACS development team, led by + * Copyright (c) 2015,2016,2017,2018,2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -92,7 +92,7 @@ public: void promptSelections() { - const bool isInteractive = StandardInputStream::instance().isInteractive(); + const bool isInteractive = StandardInputStream::isInteractive(); initIndexGroups(); manager_.parseRequestedFromStdin(isInteractive); doneIndexGroups(); diff --git a/src/gromacs/selection/selhelp.cpp b/src/gromacs/selection/selhelp.cpp index e93a3cc129..ec96ec50fd 100644 --- a/src/gromacs/selection/selhelp.cpp +++ b/src/gromacs/selection/selhelp.cpp @@ -588,11 +588,11 @@ private: /*! \brief * Prints markup for starting a list of keywords. */ - void writeKeywordListStart(const HelpWriterContext& context, const char* heading) const; + static void writeKeywordListStart(const HelpWriterContext& context, const char* heading); /*! \brief * Prints markup for ending a list of keywords. */ - void writeKeywordListEnd(const HelpWriterContext& context, const char* extraInfo) const; + static void writeKeywordListEnd(const HelpWriterContext& context, const char* extraInfo); /*! \brief * Prints a brief list of keywords (selection methods) available. @@ -667,7 +667,7 @@ void KeywordsHelpTopic::writeHelp(const HelpWriterContext& context) const writeKeywordSubTopics(context); } -void KeywordsHelpTopic::writeKeywordListStart(const HelpWriterContext& context, const char* heading) const +void KeywordsHelpTopic::writeKeywordListStart(const HelpWriterContext& context, const char* heading) { context.paragraphBreak(); std::string fullHeading("* "); @@ -681,7 +681,7 @@ void KeywordsHelpTopic::writeKeywordListStart(const HelpWriterContext& context, } } -void KeywordsHelpTopic::writeKeywordListEnd(const HelpWriterContext& context, const char* extraInfo) const +void KeywordsHelpTopic::writeKeywordListEnd(const HelpWriterContext& context, const char* extraInfo) { if (context.outputFormat() == eHelpOutputFormat_Rst) { diff --git a/src/gromacs/selection/tests/nbsearch.cpp b/src/gromacs/selection/tests/nbsearch.cpp index ab8d19566e..d0f61f30f7 100644 --- a/src/gromacs/selection/tests/nbsearch.cpp +++ b/src/gromacs/selection/tests/nbsearch.cpp @@ -149,13 +149,13 @@ public: GMX_RELEASE_ASSERT(testPos_.empty(), "Cannot add positions after testPositions() call"); testPositions_.emplace_back(x); } - gmx::RVec generateRandomPosition(); - std::vector generateIndex(int count, uint64_t seed) const; - void generateRandomRefPositions(int count); - void generateRandomTestPositions(int count); - void useRefPositionsAsTestPositions(); - void computeReferences(t_pbc* pbc) { computeReferencesInternal(pbc, false); } - void computeReferencesXY(t_pbc* pbc) { computeReferencesInternal(pbc, true); } + gmx::RVec generateRandomPosition(); + static std::vector generateIndex(int count, uint64_t seed); + void generateRandomRefPositions(int count); + void generateRandomTestPositions(int count); + void useRefPositionsAsTestPositions(); + void computeReferences(t_pbc* pbc) { computeReferencesInternal(pbc, false); } + void computeReferencesXY(t_pbc* pbc) { computeReferencesInternal(pbc, true); } bool containsPair(int testIndex, const RefPair& pair) const { @@ -217,7 +217,7 @@ gmx::RVec NeighborhoodSearchTestData::generateRandomPosition() return x; } -std::vector NeighborhoodSearchTestData::generateIndex(int count, uint64_t seed) const +std::vector NeighborhoodSearchTestData::generateIndex(int count, uint64_t seed) { gmx::DefaultRandomEngine rngIndex(seed); gmx::UniformRealDistribution dist; @@ -389,21 +389,24 @@ void ExclusionsHelper::generateExclusions() class NeighborhoodSearchTest : public ::testing::Test { public: - void testIsWithin(gmx::AnalysisNeighborhoodSearch* search, const NeighborhoodSearchTestData& data); - void testMinimumDistance(gmx::AnalysisNeighborhoodSearch* search, + static void testIsWithin(gmx::AnalysisNeighborhoodSearch* search, const NeighborhoodSearchTestData& data); - void testNearestPoint(gmx::AnalysisNeighborhoodSearch* search, const NeighborhoodSearchTestData& data); - void testPairSearch(gmx::AnalysisNeighborhoodSearch* search, const NeighborhoodSearchTestData& data); - void testPairSearchIndexed(gmx::AnalysisNeighborhood* nb, - const NeighborhoodSearchTestData& data, - uint64_t seed); - void testPairSearchFull(gmx::AnalysisNeighborhoodSearch* search, - const NeighborhoodSearchTestData& data, - const gmx::AnalysisNeighborhoodPositions& pos, - const gmx::ListOfLists* excls, - const gmx::ArrayRef& refIndices, - const gmx::ArrayRef& testIndices, - bool selfPairs); + static void testMinimumDistance(gmx::AnalysisNeighborhoodSearch* search, + const NeighborhoodSearchTestData& data); + static void testNearestPoint(gmx::AnalysisNeighborhoodSearch* search, + const NeighborhoodSearchTestData& data); + static void testPairSearch(gmx::AnalysisNeighborhoodSearch* search, + const NeighborhoodSearchTestData& data); + static void testPairSearchIndexed(gmx::AnalysisNeighborhood* nb, + const NeighborhoodSearchTestData& data, + uint64_t seed); + static void testPairSearchFull(gmx::AnalysisNeighborhoodSearch* search, + const NeighborhoodSearchTestData& data, + const gmx::AnalysisNeighborhoodPositions& pos, + const gmx::ListOfLists* excls, + const gmx::ArrayRef& refIndices, + const gmx::ArrayRef& testIndices, + bool selfPairs); gmx::AnalysisNeighborhood nb_; }; diff --git a/src/gromacs/selection/tests/poscalc.cpp b/src/gromacs/selection/tests/poscalc.cpp index 63698881e2..9748909eb1 100644 --- a/src/gromacs/selection/tests/poscalc.cpp +++ b/src/gromacs/selection/tests/poscalc.cpp @@ -123,8 +123,11 @@ private: typedef std::vector PositionTestList; - void setTopologyIfRequired(); - void checkPositions(gmx::test::TestReferenceChecker* checker, const char* name, gmx_ana_pos_t* p, bool bCoordinates); + void setTopologyIfRequired(); + static void checkPositions(gmx::test::TestReferenceChecker* checker, + const char* name, + gmx_ana_pos_t* p, + bool bCoordinates); std::vector pcList_; PositionTestList posList_; diff --git a/src/gromacs/simd/tests/simd_math.cpp b/src/gromacs/simd/tests/simd_math.cpp index f5e9f74e38..20d7b916db 100644 --- a/src/gromacs/simd/tests/simd_math.cpp +++ b/src/gromacs/simd/tests/simd_math.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2014,2015,2017,2018,2019, by the GROMACS development team, led by + * Copyright (c) 2014,2015,2017,2018,2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -149,11 +149,11 @@ public: * to avoid altering any value the user has set on the command line; since * it's a static member, changing it would have permanent effect. */ - std::vector generateTestPoints(Range range, std::size_t points); + static std::vector generateTestPoints(Range range, std::size_t points); /*! \brief Test routine for the test point vector generation */ - void generateTestPointsTest(); + static void generateTestPointsTest(); }; /*! \brief Test approximate equality of SIMD vs reference version of a function. diff --git a/src/gromacs/tables/forcetable.cpp b/src/gromacs/tables/forcetable.cpp index 920966ebd0..20aa4bd3e9 100644 --- a/src/gromacs/tables/forcetable.cpp +++ b/src/gromacs/tables/forcetable.cpp @@ -1031,8 +1031,8 @@ static void fill_table(t_tabledata* td, int tp, const interaction_const_t* ic, g case etabLJ12Encad: if (r < rc) { - Vtab = -(r6 - 6.0 * (rc - r) * rc6 / rc - rc6); - Ftab = -(6.0 * r6 / r - 6.0 * rc6 / rc); + Vtab = -(r12 - 12.0 * (rc - r) * rc12 / rc - rc12); + Ftab = -(12.0 * r12 / r - 12.0 * rc12 / rc); } else /* r>rc */ { diff --git a/src/gromacs/taskassignment/taskassignment.h b/src/gromacs/taskassignment/taskassignment.h index 7e661e2166..f709f24e40 100644 --- a/src/gromacs/taskassignment/taskassignment.h +++ b/src/gromacs/taskassignment/taskassignment.h @@ -159,19 +159,19 @@ public: * \throws std::bad_alloc If out of memory. * InconsistentInputError If user and/or detected inputs are inconsistent. */ - GpuTaskAssignments build(const std::vector& gpuIdsToUse, - const std::vector& userGpuTaskAssignment, - const gmx_hw_info_t& hardwareInfo, - MPI_Comm gromacsWorldComm, - const PhysicalNodeCommunicator& physicalNodeComm, - TaskTarget nonbondedTarget, - TaskTarget pmeTarget, - TaskTarget bondedTarget, - TaskTarget updateTarget, - bool useGpuForNonbonded, - bool useGpuForPme, - bool rankHasPpTask, - bool rankHasPmeTask); + static GpuTaskAssignments build(const std::vector& gpuIdsToUse, + const std::vector& userGpuTaskAssignment, + const gmx_hw_info_t& hardwareInfo, + MPI_Comm gromacsWorldComm, + const PhysicalNodeCommunicator& physicalNodeComm, + TaskTarget nonbondedTarget, + TaskTarget pmeTarget, + TaskTarget bondedTarget, + TaskTarget updateTarget, + bool useGpuForNonbonded, + bool useGpuForPme, + bool rankHasPpTask, + bool rankHasPmeTask); }; /*! \libinternal diff --git a/src/gromacs/tools/mk_angndx.cpp b/src/gromacs/tools/mk_angndx.cpp index e6ddbce86a..ad7a15459c 100644 --- a/src/gromacs/tools/mk_angndx.cpp +++ b/src/gromacs/tools/mk_angndx.cpp @@ -120,9 +120,7 @@ static void fill_ft_ind(int nft, const int* ft, const t_idef* idef, int ft_ind[] sprintf(buf, "RB-A1=%.2f", idef->iparams[i].rbdihs.rbcA[1]); break; case F_RESTRANGLES: - sprintf(buf, "Theta=%.1f_%.2f", idef->iparams[i].harmonic.rA, - idef->iparams[i].harmonic.krA); - break; + // Fall through intended case F_RESTRDIHS: sprintf(buf, "Theta=%.1f_%.2f", idef->iparams[i].harmonic.rA, idef->iparams[i].harmonic.krA); diff --git a/src/gromacs/tools/tests/dump.cpp b/src/gromacs/tools/tests/dump.cpp index 05982fa110..26f07d25bb 100644 --- a/src/gromacs/tools/tests/dump.cpp +++ b/src/gromacs/tools/tests/dump.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2019, by the GROMACS development team, led by + * Copyright (c) 2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -59,7 +59,7 @@ class DumpTest : public ::testing::Test { public: //! Run test case. - void runTest(CommandLine* cmdline); + static void runTest(CommandLine* cmdline); protected: // TODO this is changed in newer googletest versions diff --git a/src/gromacs/trajectoryanalysis/analysismodule.cpp b/src/gromacs/trajectoryanalysis/analysismodule.cpp index 06e289a4a4..263fa1cbe8 100644 --- a/src/gromacs/trajectoryanalysis/analysismodule.cpp +++ b/src/gromacs/trajectoryanalysis/analysismodule.cpp @@ -2,7 +2,7 @@ * This file is part of the GROMACS molecular simulation package. * * Copyright (c) 2010-2018, The GROMACS development team. - * Copyright (c) 2019, by the GROMACS development team, led by + * Copyright (c) 2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -105,7 +105,7 @@ public: const SelectionCollection& selections); //! Checks whether the given AnalysisData has been initialized. - bool isInitialized(const AnalysisData& data) const; + static bool isInitialized(const AnalysisData& data); //! Keeps a data handle for each AnalysisData object. HandleContainer handles_; @@ -130,7 +130,7 @@ TrajectoryAnalysisModuleData::Impl::Impl(TrajectoryAnalysisModule* modu } } -bool TrajectoryAnalysisModuleData::Impl::isInitialized(const AnalysisData& data) const +bool TrajectoryAnalysisModuleData::Impl::isInitialized(const AnalysisData& data) { for (int i = 0; i < data.dataSetCount(); ++i) { diff --git a/src/gromacs/trajectoryanalysis/analysismodule.h b/src/gromacs/trajectoryanalysis/analysismodule.h index d53d24143b..7462f19bad 100644 --- a/src/gromacs/trajectoryanalysis/analysismodule.h +++ b/src/gromacs/trajectoryanalysis/analysismodule.h @@ -130,7 +130,7 @@ public: * * Does not throw. */ - Selection parallelSelection(const Selection& selection); + static Selection parallelSelection(const Selection& selection); /*! \brief * Returns a set of selection that corresponds to the given selections. * @@ -140,7 +140,7 @@ public: * * \see parallelSelection() */ - SelectionList parallelSelections(const SelectionList& selections); + static SelectionList parallelSelections(const SelectionList& selections); protected: /*! \brief diff --git a/src/gromacs/trajectoryanalysis/modules/angle.cpp b/src/gromacs/trajectoryanalysis/modules/angle.cpp index f5ab6c7944..d4a8f18c02 100644 --- a/src/gromacs/trajectoryanalysis/modules/angle.cpp +++ b/src/gromacs/trajectoryanalysis/modules/angle.cpp @@ -684,8 +684,8 @@ void calc_vec(int natoms, rvec x[], t_pbc* pbc, rvec xout, rvec cout) void Angle::analyzeFrame(int frnr, const t_trxframe& fr, t_pbc* pbc, TrajectoryAnalysisModuleData* pdata) { AnalysisDataHandle dh = pdata->dataHandle(angles_); - const SelectionList& sel1 = pdata->parallelSelections(sel1_); - const SelectionList& sel2 = pdata->parallelSelections(sel2_); + const SelectionList& sel1 = TrajectoryAnalysisModuleData::parallelSelections(sel1_); + const SelectionList& sel2 = TrajectoryAnalysisModuleData::parallelSelections(sel2_); checkSelections(sel1, sel2); diff --git a/src/gromacs/trajectoryanalysis/modules/distance.cpp b/src/gromacs/trajectoryanalysis/modules/distance.cpp index b23cd8ff27..6acf73ab1d 100644 --- a/src/gromacs/trajectoryanalysis/modules/distance.cpp +++ b/src/gromacs/trajectoryanalysis/modules/distance.cpp @@ -2,7 +2,7 @@ * This file is part of the GROMACS molecular simulation package. * * Copyright (c) 2010-2018, The GROMACS development team. - * Copyright (c) 2019, by the GROMACS development team, led by + * Copyright (c) 2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -324,7 +324,7 @@ void Distance::analyzeFrame(int frnr, const t_trxframe& fr, t_pbc* pbc, Trajecto { AnalysisDataHandle distHandle = pdata->dataHandle(distances_); AnalysisDataHandle xyzHandle = pdata->dataHandle(xyz_); - const SelectionList& sel = pdata->parallelSelections(sel_); + const SelectionList& sel = TrajectoryAnalysisModuleData::parallelSelections(sel_); checkSelections(sel); diff --git a/src/gromacs/trajectoryanalysis/modules/freevolume.cpp b/src/gromacs/trajectoryanalysis/modules/freevolume.cpp index 3077ea136f..2597e359ea 100644 --- a/src/gromacs/trajectoryanalysis/modules/freevolume.cpp +++ b/src/gromacs/trajectoryanalysis/modules/freevolume.cpp @@ -303,7 +303,7 @@ void FreeVolume::initAnalysis(const TrajectoryAnalysisSettings& settings, const void FreeVolume::analyzeFrame(int frnr, const t_trxframe& fr, t_pbc* pbc, TrajectoryAnalysisModuleData* pdata) { AnalysisDataHandle dh = pdata->dataHandle(data_); - const Selection& sel = pdata->parallelSelection(sel_); + const Selection& sel = TrajectoryAnalysisModuleData::parallelSelection(sel_); gmx::UniformRealDistribution dist; GMX_RELEASE_ASSERT(nullptr != pbc, "You have no periodic boundary conditions"); diff --git a/src/gromacs/trajectoryanalysis/modules/pairdist.cpp b/src/gromacs/trajectoryanalysis/modules/pairdist.cpp index d3f36ff5be..f1d470779a 100644 --- a/src/gromacs/trajectoryanalysis/modules/pairdist.cpp +++ b/src/gromacs/trajectoryanalysis/modules/pairdist.cpp @@ -410,8 +410,8 @@ TrajectoryAnalysisModuleDataPointer PairDistance::startFrames(const AnalysisData void PairDistance::analyzeFrame(int frnr, const t_trxframe& fr, t_pbc* pbc, TrajectoryAnalysisModuleData* pdata) { AnalysisDataHandle dh = pdata->dataHandle(distances_); - const Selection& refSel = pdata->parallelSelection(refSel_); - const SelectionList& sel = pdata->parallelSelections(sel_); + const Selection& refSel = TrajectoryAnalysisModuleData::parallelSelection(refSel_); + const SelectionList& sel = TrajectoryAnalysisModuleData::parallelSelections(sel_); PairDistanceModuleData& frameData = *static_cast(pdata); std::vector& distArray = frameData.distArray_; std::vector& countArray = frameData.countArray_; diff --git a/src/gromacs/trajectoryanalysis/modules/rdf.cpp b/src/gromacs/trajectoryanalysis/modules/rdf.cpp index 438ba85a87..bc088be33f 100644 --- a/src/gromacs/trajectoryanalysis/modules/rdf.cpp +++ b/src/gromacs/trajectoryanalysis/modules/rdf.cpp @@ -478,8 +478,8 @@ void Rdf::analyzeFrame(int frnr, const t_trxframe& fr, t_pbc* pbc, TrajectoryAna { AnalysisDataHandle dh = pdata->dataHandle(pairDist_); AnalysisDataHandle nh = pdata->dataHandle(normFactors_); - const Selection& refSel = pdata->parallelSelection(refSel_); - const SelectionList& sel = pdata->parallelSelections(sel_); + const Selection& refSel = TrajectoryAnalysisModuleData::parallelSelection(refSel_); + const SelectionList& sel = TrajectoryAnalysisModuleData::parallelSelections(sel_); RdfModuleData& frameData = *static_cast(pdata); const bool bSurface = !frameData.surfaceDist2_.empty(); diff --git a/src/gromacs/trajectoryanalysis/modules/sasa.cpp b/src/gromacs/trajectoryanalysis/modules/sasa.cpp index 9b581dad9d..502f51d073 100644 --- a/src/gromacs/trajectoryanalysis/modules/sasa.cpp +++ b/src/gromacs/trajectoryanalysis/modules/sasa.cpp @@ -116,12 +116,12 @@ struct t_conect void add_rec(t_conect c[], int i, int j, real d2) { if (c[i].aa == -1) - { + { // NOLINT bugprone-branch-clone c[i].aa = j; c[i].d2a = d2; } else if (c[i].ab == -1) - { + { // NOLINT bugprone-branch-clone c[i].ab = j; c[i].d2b = d2; } @@ -906,8 +906,8 @@ void Sasa::analyzeFrame(int frnr, const t_trxframe& fr, t_pbc* pbc, TrajectoryAn AnalysisDataHandle aah = pdata->dataHandle(atomArea_); AnalysisDataHandle rah = pdata->dataHandle(residueArea_); AnalysisDataHandle vh = pdata->dataHandle(volume_); - const Selection& surfaceSel = pdata->parallelSelection(surfaceSel_); - const SelectionList& outputSel = pdata->parallelSelections(outputSel_); + const Selection& surfaceSel = TrajectoryAnalysisModuleData::parallelSelection(surfaceSel_); + const SelectionList& outputSel = TrajectoryAnalysisModuleData::parallelSelections(outputSel_); SasaModuleData& frameData = *static_cast(pdata); const bool bResAt = !frameData.res_a_.empty(); diff --git a/src/gromacs/trajectoryanalysis/modules/select.cpp b/src/gromacs/trajectoryanalysis/modules/select.cpp index f3aefd8c4a..77fec1d469 100644 --- a/src/gromacs/trajectoryanalysis/modules/select.cpp +++ b/src/gromacs/trajectoryanalysis/modules/select.cpp @@ -626,7 +626,7 @@ void Select::analyzeFrame(int frnr, const t_trxframe& fr, t_pbc* /* pbc */, Traj AnalysisDataHandle cdh = pdata->dataHandle(cdata_); AnalysisDataHandle idh = pdata->dataHandle(idata_); AnalysisDataHandle mdh = pdata->dataHandle(mdata_); - const SelectionList& sel = pdata->parallelSelections(sel_); + const SelectionList& sel = TrajectoryAnalysisModuleData::parallelSelections(sel_); sdh.startFrame(frnr, fr.time); for (size_t g = 0; g < sel.size(); ++g) diff --git a/src/gromacs/trajectoryanalysis/modules/trajectory.cpp b/src/gromacs/trajectoryanalysis/modules/trajectory.cpp index 303d5dd792..1247ea9b8b 100644 --- a/src/gromacs/trajectoryanalysis/modules/trajectory.cpp +++ b/src/gromacs/trajectoryanalysis/modules/trajectory.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2016,2018,2019, by the GROMACS development team, led by + * Copyright (c) 2016,2018,2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -253,7 +253,7 @@ void analyzeFrameImpl(int frnr, const t_trxframe& fr, AnalysisDataHandle* dh, co void Trajectory::analyzeFrame(int frnr, const t_trxframe& fr, t_pbc* /* pbc */, TrajectoryAnalysisModuleData* pdata) { AnalysisDataHandle dh = pdata->dataHandle(xdata_); - const SelectionList& sel = pdata->parallelSelections(sel_); + const SelectionList& sel = TrajectoryAnalysisModuleData::parallelSelections(sel_); analyzeFrameImpl(frnr, fr, &dh, sel, [](const SelectionPosition& pos) { return pos.x(); }); if (fr.bV) { diff --git a/src/gromacs/utility/filestream.cpp b/src/gromacs/utility/filestream.cpp index 4893c509fe..4773b5c2c8 100644 --- a/src/gromacs/utility/filestream.cpp +++ b/src/gromacs/utility/filestream.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2015,2017,2018,2019, by the GROMACS development team, led by + * Copyright (c) 2015,2017,2018,2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -153,7 +153,8 @@ using internal::FileStreamImpl; * StandardInputStream */ -bool StandardInputStream::isInteractive() const +// static +bool StandardInputStream::isInteractive() { #ifdef HAVE_UNISTD_H return isatty(fileno(stdin)) != 0; @@ -167,13 +168,6 @@ bool StandardInputStream::readLine(std::string* line) return readLineImpl(stdin, line); } -// static -StandardInputStream& StandardInputStream::instance() -{ - static StandardInputStream stdinObject; - return stdinObject; -} - /******************************************************************** * TextInputFile */ diff --git a/src/gromacs/utility/filestream.h b/src/gromacs/utility/filestream.h index 26c240a6bf..5309fb2178 100644 --- a/src/gromacs/utility/filestream.h +++ b/src/gromacs/utility/filestream.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2015,2018,2019, by the GROMACS development team, led by + * Copyright (c) 2015,2018,2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -78,18 +78,11 @@ public: * * Does not throw. */ - bool isInteractive() const; + static bool isInteractive(); // From TextInputStream bool readLine(std::string* line) override; void close() override {} - - /*! \brief - * Returns a stream for accessing `stdin`. - * - * Does not throw. - */ - static StandardInputStream& instance(); }; /*! \libinternal \brief diff --git a/src/gromacs/utility/keyvaluetree.cpp b/src/gromacs/utility/keyvaluetree.cpp index 3d5b32abf0..2d49c09448 100644 --- a/src/gromacs/utility/keyvaluetree.cpp +++ b/src/gromacs/utility/keyvaluetree.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2016,2017,2018,2019, by the GROMACS development team, led by + * Copyright (c) 2016,2017,2018,2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -275,7 +275,7 @@ private: writer_->writeLine(message); } - std::string formatValueForMissingMessage(const KeyValueTreeValue& value) + static std::string formatValueForMissingMessage(const KeyValueTreeValue& value) { if (value.isObject() || value.isArray()) { diff --git a/src/gromacs/utility/smalloc.h b/src/gromacs/utility/smalloc.h index adb5c9a0ba..bf7fa34c9c 100644 --- a/src/gromacs/utility/smalloc.h +++ b/src/gromacs/utility/smalloc.h @@ -3,7 +3,7 @@ * * Copyright (c) 1991-2000, University of Groningen, The Netherlands. * Copyright (c) 2001-2004, The GROMACS development team. - * Copyright (c) 2013,2014,2015,2018,2019, by the GROMACS development team, led by + * Copyright (c) 2013,2014,2015,2018,2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -203,6 +203,7 @@ template static inline void gmx_snew_impl(const char* name, const char* file, int line, T*& ptr, size_t nelem) { static_assert(std::is_pod::value, "snew() called on C++ type"); + // NOLINTNEXTLINE bugprone-sizeof-expression ptr = static_cast(save_calloc(name, file, line, nelem, sizeof(T))); } /** C++ helper for srenew(). */ @@ -210,6 +211,7 @@ template static inline void gmx_srenew_impl(const char* name, const char* file, int line, T*& ptr, size_t nelem) { static_assert(std::is_pod::value, "srenew() called on C++ type"); + // NOLINTNEXTLINE bugprone-sizeof-expression ptr = static_cast(save_realloc(name, file, line, ptr, nelem, sizeof(T))); } /** C++ helper for smalloc(). */ diff --git a/src/gromacs/utility/tests/inmemoryserializer.cpp b/src/gromacs/utility/tests/inmemoryserializer.cpp index 06b3d0e236..1a45090003 100644 --- a/src/gromacs/utility/tests/inmemoryserializer.cpp +++ b/src/gromacs/utility/tests/inmemoryserializer.cpp @@ -109,13 +109,13 @@ public: real realValue_; }; - void serialize(ISerializer* serializer, SerializerValues* values) + static void serialize(ISerializer* serializer, SerializerValues* values) { EXPECT_FALSE(serializer->reading()); doValues(serializer, values); } - SerializerValues deserialize(ISerializer* serializer) + static SerializerValues deserialize(ISerializer* serializer) { EXPECT_TRUE(serializer->reading()); SerializerValues result; @@ -123,7 +123,7 @@ public: return result; } - void checkSerializerValuesforEquality(const SerializerValues& lhs, const SerializerValues& rhs) + static void checkSerializerValuesforEquality(const SerializerValues& lhs, const SerializerValues& rhs) { EXPECT_EQ(lhs.boolValue_, rhs.boolValue_); EXPECT_EQ(lhs.unsignedCharValue_, rhs.unsignedCharValue_); @@ -138,7 +138,7 @@ public: } private: - void doValues(ISerializer* serializer, SerializerValues* values) + static void doValues(ISerializer* serializer, SerializerValues* values) { serializer->doBool(&values->boolValue_); serializer->doUChar(&values->unsignedCharValue_); diff --git a/src/gromacs/utility/tests/keyvaluetreeserializer.cpp b/src/gromacs/utility/tests/keyvaluetreeserializer.cpp index dea592d38f..4d24c608ad 100644 --- a/src/gromacs/utility/tests/keyvaluetreeserializer.cpp +++ b/src/gromacs/utility/tests/keyvaluetreeserializer.cpp @@ -108,7 +108,7 @@ public: gmx::KeyValueTreeBuilder builder_; private: - std::vector serializeTree(const gmx::KeyValueTreeObject& tree) + static std::vector serializeTree(const gmx::KeyValueTreeObject& tree) { gmx::InMemorySerializer serializer; gmx::serializeKeyValueTree(tree, &serializer); -- 2.22.0