From 62bc4ebac5efa4e95117242ebd324b076984b5a2 Mon Sep 17 00:00:00 2001 From: ejjordan Date: Mon, 19 Apr 2021 20:54:55 +0200 Subject: [PATCH] clang-tidy-11 fixes mdrun tests Split off from !1433 --- src/programs/mdrun/tests/.clang-tidy | 91 +++++++++++++++++++ src/programs/mdrun/tests/energycomparison.cpp | 6 +- src/programs/mdrun/tests/energyreader.cpp | 4 +- src/programs/mdrun/tests/ewaldsurfaceterm.cpp | 4 +- .../mdrun/tests/initialconstraints.cpp | 4 +- src/programs/mdrun/tests/moduletest.cpp | 12 ++- src/programs/mdrun/tests/moduletest.h | 8 +- src/programs/mdrun/tests/simple_mdrun.cpp | 4 +- 8 files changed, 115 insertions(+), 18 deletions(-) create mode 100644 src/programs/mdrun/tests/.clang-tidy diff --git a/src/programs/mdrun/tests/.clang-tidy b/src/programs/mdrun/tests/.clang-tidy new file mode 100644 index 0000000000..0adf51e3ee --- /dev/null +++ b/src/programs/mdrun/tests/.clang-tidy @@ -0,0 +1,91 @@ +# List of rationales for check suppressions (where known). +# This have to precede the list because inline comments are not +# supported by clang-tidy. +# +# -cppcoreguidelines-non-private-member-variables-in-classes, +# -misc-non-private-member-variables-in-classes, +# We intend a gradual transition to conform to this guideline, but it +# is not practical to implement yet. +# +# -readability-isolate-declaration, +# Declarations like "int a, b;" are readable. Some forms are not, and +# those might reasonably be suggested against during code review. +# +# -cppcoreguidelines-avoid-c-arrays, +# C arrays are still necessary in many places with legacy code +# +# -cppcoreguidelines-avoid-magic-numbers, +# -readability-magic-numbers, +# We have many legitimate use cases for magic numbers +# +# -cppcoreguidelines-macro-usage, +# We do use too many macros, and we should fix many of them, but there +# is no reasonable way to suppress the check e.g. in src/config.h and +# configuring the build is a major legitimate use of macros. +# +# -cppcoreguidelines-narrowing-conversions, +# -bugprone-narrowing-conversions +# 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 +# +# -misc-no-recursion +# We have way too many functions and methods relying on recursion +# +# -cppcoreguidelines-avoid-non-const-global-variables +# There are quite a lot of static variables in the test code that +# can not be replaced. +# +# -modernize-avoid-bind +# Some code needs to use std::bind and can't be modernized quickly. +Checks: clang-diagnostic-*,-clang-analyzer-*,-clang-analyzer-security.insecureAPI.strcpy, + bugprone-*,misc-*,readability-*,performance-*,mpi-*, + -readability-inconsistent-declaration-parameter-name, + -readability-function-size,-readability-else-after-return, + modernize-use-nullptr,modernize-use-emplace, + modernize-make-unique,modernize-make-shared, + modernize-avoid-bind, + modernize-use-override, + modernize-redundant-void-arg,modernize-use-bool-literals, + cppcoreguidelines-*,-cppcoreguidelines-pro-*,-cppcoreguidelines-owning-memory, + -cppcoreguidelines-no-malloc,-cppcoreguidelines-special-member-functions, + -cppcoreguidelines-avoid-goto, + google-*,-google-build-using-namespace,-google-explicit-constructor, + -google-readability-function-size,-google-readability-todo,-google-runtime-int, + -cppcoreguidelines-non-private-member-variables-in-classes, + -misc-non-private-member-variables-in-classes, + -readability-isolate-declaration, + -cppcoreguidelines-avoid-c-arrays, + -cppcoreguidelines-avoid-magic-numbers, + -readability-magic-numbers, + -cppcoreguidelines-macro-usage, + -cppcoreguidelines-narrowing-conversions, + -bugprone-narrowing-conversions, + -google-readability-avoid-underscore-in-googletest-name, + -cppcoreguidelines-init-variables, + -misc-no-recursion, + -cppcoreguidelines-avoid-non-const-global-variables, + -modernize-avoid-bind +HeaderFilterRegex: .* +CheckOptions: + - key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor + value: 1 + - key: modernize-make-unique.IncludeStyle + value: google + - key: modernize-make-shared.IncludeStyle + value: google + - key: readability-implicit-bool-conversion.AllowIntegerConditions + value: 1 + - key: readability-implicit-bool-conversion.AllowPointerConditions + value: 1 + - key: bugprone-dangling-handle.HandleClasses + value: std::basic_string_view; nonstd::sv_lite::basic_string_view +# Permit passing shard pointers by value for sink parameters + - key: performance-unnecessary-copy-initialization.AllowedTypes + value: shared_ptr + - key: performance-unnecessary-value-param.AllowedTypes + value: shared_ptr diff --git a/src/programs/mdrun/tests/energycomparison.cpp b/src/programs/mdrun/tests/energycomparison.cpp index dee0ba828a..fa9823675a 100644 --- a/src/programs/mdrun/tests/energycomparison.cpp +++ b/src/programs/mdrun/tests/energycomparison.cpp @@ -107,13 +107,13 @@ void EnergyComparison::operator()(const EnergyFrame& reference, const EnergyFram + test.frameName()); for (auto referenceIt = reference.begin(); referenceIt != reference.end(); ++referenceIt) { - auto& energyName = referenceIt->first; + const auto& energyName = referenceIt->first; SCOPED_TRACE("Comparing " + energyName + " between frames"); auto testIt = test.find(energyName); if (testIt != test.end()) { - auto& energyValueInReference = referenceIt->second; - auto& energyValueInTest = testIt->second; + const auto& energyValueInReference = referenceIt->second; + const auto& energyValueInTest = testIt->second; EXPECT_REAL_EQ_TOL( energyValueInReference, energyValueInTest, energyTermsToCompare_.at(energyName)); } diff --git a/src/programs/mdrun/tests/energyreader.cpp b/src/programs/mdrun/tests/energyreader.cpp index 858051ffa5..6f72f626b0 100644 --- a/src/programs/mdrun/tests/energyreader.cpp +++ b/src/programs/mdrun/tests/energyreader.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,2021, 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. @@ -106,7 +106,7 @@ EnergyFrameReaderPtr openEnergyFileToReadTerms(const std::string& f { std::string requiredEnergiesNotFound = "Did not find the following required energies in mdrun output:\n"; - for (auto& name : namesOfRequiredEnergyTerms) + for (const auto& name : namesOfRequiredEnergyTerms) { auto possibleIndex = indicesOfEnergyTerms.find(name); if (possibleIndex == indicesOfEnergyTerms.end()) diff --git a/src/programs/mdrun/tests/ewaldsurfaceterm.cpp b/src/programs/mdrun/tests/ewaldsurfaceterm.cpp index df6c75b82e..6c5102744d 100644 --- a/src/programs/mdrun/tests/ewaldsurfaceterm.cpp +++ b/src/programs/mdrun/tests/ewaldsurfaceterm.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2020, by the GROMACS development team, led by + * Copyright (c) 2020,2021, 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 @@ TEST_P(EwaldSurfaceTermTest, WithinTolerances) auto frame = reader.frame(); auto force = frame.f(); int atom = 0; - for (auto& f : force) + for (const auto& f : force) { std::string forceName = frame.frameName() + " F[" + toString(atom) + "]"; diff --git a/src/programs/mdrun/tests/initialconstraints.cpp b/src/programs/mdrun/tests/initialconstraints.cpp index 66bbeb797e..ad18331849 100644 --- a/src/programs/mdrun/tests/initialconstraints.cpp +++ b/src/programs/mdrun/tests/initialconstraints.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2017,2018,2019,2020, by the GROMACS development team, led by + * Copyright (c) 2017,2018,2019,2020,2021, 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. @@ -76,7 +76,7 @@ TEST_P(InitialConstraintsTest, Works) { const int nsteps = 1; const float timestep = 0.001; - auto integrator = GetParam(); + const auto* integrator = GetParam(); const std::string integratorName(integrator); SCOPED_TRACE("Integrating with " + integratorName); const std::string theMdpFile = formatString( diff --git a/src/programs/mdrun/tests/moduletest.cpp b/src/programs/mdrun/tests/moduletest.cpp index 7442deacb1..1a15c721a9 100644 --- a/src/programs/mdrun/tests/moduletest.cpp +++ b/src/programs/mdrun/tests/moduletest.cpp @@ -2,7 +2,7 @@ * This file is part of the GROMACS molecular simulation package. * * Copyright (c) 2013,2014,2015,2016,2017 by the GROMACS development team. - * Copyright (c) 2018,2019,2020, by the GROMACS development team, led by + * Copyright (c) 2018,2019,2020,2021, 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. @@ -83,9 +83,11 @@ namespace #if GMX_OPENMP || defined(DOXYGEN) //! Number of OpenMP threads for child mdrun call. +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) int g_numOpenMPThreads = 1; #endif //! \cond +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) GMX_TEST_OPTIONS(MdrunTestOptions, options) { GMX_UNUSED_VALUE(options); @@ -148,7 +150,7 @@ void SimulationRunner::useStringAsMdpFile(const std::string& mdpString) mdpInputContents_ = mdpString; } -void SimulationRunner::useStringAsNdxFile(const char* ndxString) +void SimulationRunner::useStringAsNdxFile(const char* ndxString) const { gmx::TextWriter::writeFileFromString(ndxFileName_, ndxString); } @@ -245,7 +247,7 @@ int SimulationRunner::callGrompp() return callGrompp(CommandLine()); } -int SimulationRunner::changeTprNsteps(int nsteps) +int SimulationRunner::changeTprNsteps(int nsteps) const { CommandLine caller; caller.append("convert-tpr"); @@ -258,7 +260,7 @@ int SimulationRunner::changeTprNsteps(int nsteps) return gmx::test::CommandLineTestHelper::runModuleFactory(&gmx::ConvertTprInfo::create, &caller); } -int SimulationRunner::callNmeig() +int SimulationRunner::callNmeig() const { /* Conforming to style guide by not passing a non-const reference to this function. Passing a non-const reference might make it @@ -330,8 +332,10 @@ int SimulationRunner::callMdrun() // ==== // static +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) MPI_Comm MdrunTestFixtureBase::communicator_ = MPI_COMM_NULL; // static +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) std::unique_ptr MdrunTestFixtureBase::hwinfo_; // static diff --git a/src/programs/mdrun/tests/moduletest.h b/src/programs/mdrun/tests/moduletest.h index 516bb8b5c0..72261b69fe 100644 --- a/src/programs/mdrun/tests/moduletest.h +++ b/src/programs/mdrun/tests/moduletest.h @@ -118,7 +118,7 @@ public: //! Use a given string as input to grompp void useStringAsMdpFile(const std::string& mdpString); //! Use a string as -n input to grompp - void useStringAsNdxFile(const char* ndxString); + void useStringAsNdxFile(const char* ndxString) const; //! Use a standard .top and .g96 file as input to grompp void useTopG96AndNdxFromDatabase(const std::string& name); //! Use a standard .top and .gro file as input to grompp @@ -136,14 +136,14 @@ public: //! Convenience wrapper for a default call to \c callGromppOnThisRank int callGromppOnThisRank(); //! Calls nmeig for testing - int callNmeig(); + int callNmeig() const; //! Calls mdrun for testing with a customized command line int callMdrun(const CommandLine& callerRef); /*! \brief Convenience wrapper for calling mdrun for testing * with default command line */ int callMdrun(); //! Calls convert-tpr on this rank to set a new number of steps in the tpr. - int changeTprNsteps(int nsteps); + int changeTprNsteps(int nsteps) const; //@{ /*! \name Names for frequently used grompp and mdrun output files @@ -208,11 +208,13 @@ public: ~MdrunTestFixtureBase() override; //! Communicator over which the test fixture works + // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) static MPI_Comm communicator_; /*! \brief Hardware information object * * Detected within \c communicator_ and available to re-use * over all tests in the test case of this text fixture. */ + // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) static std::unique_ptr hwinfo_; }; diff --git a/src/programs/mdrun/tests/simple_mdrun.cpp b/src/programs/mdrun/tests/simple_mdrun.cpp index 66cc3acdee..271b6232e0 100644 --- a/src/programs/mdrun/tests/simple_mdrun.cpp +++ b/src/programs/mdrun/tests/simple_mdrun.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2019,2020, by the GROMACS development team, led by + * Copyright (c) 2019,2020,2021, 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. @@ -160,7 +160,7 @@ TEST_P(SimpleMdrunTest, WithinTolerances) auto frame = reader.frame(); auto force = frame.f(); int atom = 0; - for (auto& f : force) + for (const auto& f : force) { std::string forceName = frame.frameName() + " F[" + toString(atom) + "]"; -- 2.22.0