clang-tidy-11 fixes mdrun tests
authorejjordan <ejjordan@kth.se>
Mon, 19 Apr 2021 18:54:55 +0000 (20:54 +0200)
committerAndrey Alekseenko <al42and@gmail.com>
Tue, 20 Apr 2021 06:15:03 +0000 (06:15 +0000)
Split off from !1433

src/programs/mdrun/tests/.clang-tidy [new file with mode: 0644]
src/programs/mdrun/tests/energycomparison.cpp
src/programs/mdrun/tests/energyreader.cpp
src/programs/mdrun/tests/ewaldsurfaceterm.cpp
src/programs/mdrun/tests/initialconstraints.cpp
src/programs/mdrun/tests/moduletest.cpp
src/programs/mdrun/tests/moduletest.h
src/programs/mdrun/tests/simple_mdrun.cpp

diff --git a/src/programs/mdrun/tests/.clang-tidy b/src/programs/mdrun/tests/.clang-tidy
new file mode 100644 (file)
index 0000000..0adf51e
--- /dev/null
@@ -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
index dee0ba828a514f24e4cbe1ff2f23a781fb37b699..fa9823675ab1b7b81dc4f9bc85a582ad71a2a982 100644 (file)
@@ -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));
         }
index 858051ffa53149874f8853a33aef4bebfa4295ce..6f72f626b08521838795d3f9bef630a84d765f8a 100644 (file)
@@ -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())
index df6c75b82e9fce4b76a435d279ffc9c49425ab50..6c5102744dd082a21bb1cdefcabc73b898c5ec67 100644 (file)
@@ -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) + "]";
 
index 66bbeb797e5955772f735aef084f860b92836f5e..ad18331849d0b5f44e63f01726d3dc70bd38ed3d 100644 (file)
@@ -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(
index 7442deacb1eaffde3d29891a89e001bd3928e00d..1a15c721a9db9d08296836959a08b0c7d157e9e9 100644 (file)
@@ -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<gmx_hw_info_t> MdrunTestFixtureBase::hwinfo_;
 
 // static
index 516bb8b5c09cdbbe92c978c0cab2eb64b374e055..72261b69fe5d0cec23e2186aae32545735d53962 100644 (file)
@@ -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<gmx_hw_info_t> hwinfo_;
 };
 
index 66cc3acdee93c393b650fab6771295cd17273737..271b6232e0eedffb1ace111f1d31e1b03b2b950b 100644 (file)
@@ -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) + "]";