clang-tidy-11 fixes for gmxpreprocess

diff --git a/src/gromacs/gmxpreprocess/convparm.cpp b/src/gromacs/gmxpreprocess/convparm.cpp
index 1712ec3c1658ed1d75a6435f3e853427e9c75f5d..37b59f1eb5bce38d43afe26566dab8f77d221dcf 100644 (file)
--- a/src/gromacs/gmxpreprocess/convparm.cpp
+++ b/src/gromacs/gmxpreprocess/convparm.cpp
@@ -529,7 +529,7 @@ static void enter_function(const InteractionsOfType* p,
 {
     int start = ffparams->numTypes();
 
-    for (auto& parm : p->interactionTypes)
+    for (const auto& parm : p->interactionTypes)
     {
         int type = enter_params(ffparams, ftype, parm.forceParam(), comb, reppow, start, bAppend);
         /* Type==-1 is used as a signal that this interaction is all-zero and should not be added. */

diff --git a/src/gromacs/gmxpreprocess/gen_ad.cpp b/src/gromacs/gmxpreprocess/gen_ad.cpp
index 643963f73d1748a73fae6ada581b396731f2528e..a39d5c881084e5a3e2b29c89704a49c8409d8f3b 100644 (file)
--- a/src/gromacs/gmxpreprocess/gen_ad.cpp
+++ b/src/gromacs/gmxpreprocess/gen_ad.cpp
@@ -58,6 +58,7 @@
 #include "gromacs/math/vec.h"
 #include "gromacs/topology/ifunc.h"
 #include "gromacs/utility/cstringutil.h"
+#include "gromacs/utility/enumerationhelpers.h"
 #include "gromacs/utility/fatalerror.h"
 #include "gromacs/utility/smalloc.h"
 

diff --git a/src/gromacs/gmxpreprocess/gen_vsite.cpp b/src/gromacs/gmxpreprocess/gen_vsite.cpp
index eec1e75386973c331bd110e37f199fff6c58b422..d94a14e58281182b50afbbddeadc3ade88689bb8 100644 (file)
--- a/src/gromacs/gmxpreprocess/gen_vsite.cpp
+++ b/src/gromacs/gmxpreprocess/gen_vsite.cpp
@@ -1657,7 +1657,7 @@ static bool is_vsite(int vsite_type)
     }
 }
 
-static char atomnamesuffix[] = "1234";
+static const char atomnamesuffix[] = "1234";
 
 void do_vsites(gmx::ArrayRef<const PreprocessResidue> rtpFFDB,
                PreprocessingAtomTypes*                atype,

diff --git a/src/gromacs/gmxpreprocess/genhydro.cpp b/src/gromacs/gmxpreprocess/genhydro.cpp
index 5ee31602a3bfa2be660374604f57ac2c7cc4d479..b9c8680c2ef9310cffb36646e6ffefea4f297764 100644 (file)
--- a/src/gromacs/gmxpreprocess/genhydro.cpp
+++ b/src/gromacs/gmxpreprocess/genhydro.cpp
@@ -177,7 +177,7 @@ static void expand_hackblocks_one(const MoleculePatchDatabase& newPatch,
 {
     /* we'll recursively add atoms to atoms */
     int pos = 0;
-    for (auto& singlePatch : newPatch.hack)
+    for (const auto& singlePatch : newPatch.hack)
     {
         /* first check if we're in the N- or C-terminus, then we should ignore
            all hacks involving atoms from resp. previous or next residue

diff --git a/src/gromacs/gmxpreprocess/gmxcpp.cpp b/src/gromacs/gmxpreprocess/gmxcpp.cpp
index 1a597a41012b539dfa6f4e9dffb2f2be68e11f3a..bde57d24434cb7766a6dd52788c9ff27e55acf08 100644 (file)
--- a/src/gromacs/gmxpreprocess/gmxcpp.cpp
+++ b/src/gromacs/gmxpreprocess/gmxcpp.cpp
@@ -4,7 +4,7 @@
  * Copyright (c) 1991-2000, University of Groningen, The Netherlands.
  * Copyright (c) 2001-2004, The GROMACS development team.
  * Copyright (c) 2013,2014,2015,2017,2018 by the GROMACS development team.
- * 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.
@@ -770,7 +770,7 @@ std::string checkAndWarnForUnusedDefines(const gmx_cpp& handle)
                 "The following macros were defined in the 'define' mdp field with the -D prefix, "
                 "but "
                 "were not used in the topology:\n";
-        for (auto& str : handle.unmatched_defines)
+        for (const auto& str : handle.unmatched_defines)
         {
             warning += ("    " + str + "\n");
         }

diff --git a/src/gromacs/gmxpreprocess/grompp.cpp b/src/gromacs/gmxpreprocess/grompp.cpp
index c8faf6d05b9b697207a2ff50e6383ea3655b26f3..6e9b8d476418cfd0989d4e487da31d8943dcbc00 100644 (file)
--- a/src/gromacs/gmxpreprocess/grompp.cpp
+++ b/src/gromacs/gmxpreprocess/grompp.cpp
@@ -122,7 +122,7 @@ InteractionOfType::InteractionOfType(gmx::ArrayRef<const int>  atoms,
             params.size() <= forceParam_.size(),
             gmx::formatString("Cannot have more parameters than the maximum number possible (%d)", MAXFORCEPARAM)
                     .c_str());
-    auto forceParamIt = forceParam_.begin();
+    auto* forceParamIt = forceParam_.begin();
     for (const auto param : params)
     {
         *forceParamIt++ = param;

diff --git a/src/gromacs/gmxpreprocess/hackblock.cpp b/src/gromacs/gmxpreprocess/hackblock.cpp
index 5cbc9e3d9bb3dbae290cbbf451d3350310a2daf9..956e0d72e13d43cf370dacc68164c74296c43da5 100644 (file)
--- a/src/gromacs/gmxpreprocess/hackblock.cpp
+++ b/src/gromacs/gmxpreprocess/hackblock.cpp
@@ -49,6 +49,7 @@
 #include "gromacs/topology/symtab.h"
 #include "gromacs/utility/arrayref.h"
 #include "gromacs/utility/cstringutil.h"
+#include "gromacs/utility/enumerationhelpers.h"
 #include "gromacs/utility/exceptions.h"
 #include "gromacs/utility/fatalerror.h"
 #include "gromacs/utility/gmxassert.h"

diff --git a/src/gromacs/gmxpreprocess/hizzie.cpp b/src/gromacs/gmxpreprocess/hizzie.cpp
index 67f17ada1fc80b49406d75864ae5f95855e2bf53..946ada2f392d7730f2c79414ff9b8818c7f23c4d 100644 (file)
--- a/src/gromacs/gmxpreprocess/hizzie.cpp
+++ b/src/gromacs/gmxpreprocess/hizzie.cpp
@@ -180,15 +180,16 @@ void set_histp(t_atoms* pdba, rvec* x, t_symtab* symtab, real angle, real dist)
                                       "NZ", "OG",  "OG1", "OH", "NE1", "OW" };
 #define NPD asize(prot_don)
 
-    bool *    donor, *acceptor;
-    bool*     hbond;
-    bool      bHDd, bHEd;
-    rvec      xh1, xh2;
-    int       natom;
-    int       i, j, nd, na, hisind, type = -1;
-    int       nd1, ne2, cg, cd2, ce1;
-    t_blocka* hb;
-    char*     atomnm;
+    bool *          donor, *acceptor;
+    bool*           hbond;
+    bool            bHDd, bHEd;
+    rvec            xh1, xh2;
+    int             natom;
+    int             i, j, nd, na, hisind;
+    HistidineStates type = HistidineStates::Count;
+    int             nd1, ne2, cg, cd2, ce1;
+    t_blocka*       hb;
+    char*           atomnm;
 
     natom = pdba->nr;
 
@@ -288,25 +289,28 @@ void set_histp(t_atoms* pdba, rvec* x, t_symtab* symtab, real angle, real dist)
                     {
                         if (bHEd)
                         {
-                            type = ehisH;
+                            type = HistidineStates::H;
                         }
                         else
                         {
-                            type = ehisA;
+                            type = HistidineStates::A;
                         }
                     }
                     else
                     {
-                        type = ehisB;
+                        type = HistidineStates::B;
                     }
-                    fprintf(stderr, "Will use %s for residue %d\n", hh[type], pdba->resinfo[hisind].nr);
+                    fprintf(stderr,
+                            "Will use %s for residue %d\n",
+                            enumValueToString(type),
+                            pdba->resinfo[hisind].nr);
                 }
                 else
                 {
                     gmx_fatal(FARGS, "Incomplete ring in HIS%d", pdba->resinfo[hisind].nr);
                 }
 
-                pdba->resinfo[hisind].rtp = put_symtab(symtab, hh[type]);
+                pdba->resinfo[hisind].rtp = put_symtab(symtab, enumValueToString(type));
             }
         }
     }

diff --git a/src/gromacs/gmxpreprocess/pdb2gmx.cpp b/src/gromacs/gmxpreprocess/pdb2gmx.cpp
index 5346f3804e9929747aff743218dab748f220ea49..d562476a1ebe7d20b8493d886bfd8c58e806fea4 100644 (file)
--- a/src/gromacs/gmxpreprocess/pdb2gmx.cpp
+++ b/src/gromacs/gmxpreprocess/pdb2gmx.cpp
@@ -37,6 +37,7 @@
  */
 #include "gmxpre.h"
 
+#include "gromacs/utility/enumerationhelpers.h"
 #include "pdb2gmx.h"
 
 #include <cctype>
@@ -121,17 +122,133 @@ const char* res2bb_notermini(const std::string& name, gmx::ArrayRef<const RtpRen
     return found != rr.end() ? found->main.c_str() : name.c_str();
 }
 
-const char* select_res(int                            nr,
-                       int                            resnr,
-                       const char*                    name[],
-                       const char*                    expl[],
-                       const char*                    title,
-                       gmx::ArrayRef<const RtpRename> rr)
+const char* enumValueToLongString(HistidineStates enumValue)
+{
+    constexpr gmx::EnumerationArray<HistidineStates, const char*> histidineStatesLongNames = {
+        "H on ND1 only", "H on NE2 only", "H on ND1 and NE2", "Coupled to Heme"
+    };
+    return histidineStatesLongNames[enumValue];
+}
+
+enum class AspartateStates : int
+{
+    Deprot,
+    Prot,
+    Count
+};
+
+const char* enumValueToString(AspartateStates enumValue)
+{
+    constexpr gmx::EnumerationArray<AspartateStates, const char*> aspartateStateNames = { "ASP",
+                                                                                          "ASPH" };
+    return aspartateStateNames[enumValue];
+}
+
+const char* enumValueToLongString(AspartateStates enumValue)
+{
+    constexpr gmx::EnumerationArray<AspartateStates, const char*> aspartateStateLongNames = {
+        "Not protonated (charge -1)", "Protonated (charge 0)"
+    };
+    return aspartateStateLongNames[enumValue];
+}
+
+enum class GlutamateStates : int
+{
+    Deprot,
+    Prot,
+    Count
+};
+
+const char* enumValueToString(GlutamateStates enumValue)
+{
+    constexpr gmx::EnumerationArray<GlutamateStates, const char*> glutamateStateNames = { "GLU",
+                                                                                          "GLUH" };
+    return glutamateStateNames[enumValue];
+}
+
+const char* enumValueToLongString(GlutamateStates enumValue)
+{
+    constexpr gmx::EnumerationArray<GlutamateStates, const char*> glutamateStateLongNames = {
+        "Not protonated (charge -1)", "Protonated (charge 0)"
+    };
+    return glutamateStateLongNames[enumValue];
+}
+
+enum class GlutamineStates : int
+{
+    Deprot,
+    Prot,
+    Count
+};
+
+const char* enumValueToString(GlutamineStates enumValue)
+{
+    constexpr gmx::EnumerationArray<GlutamineStates, const char*> glutamineStateNames = { "GLN",
+                                                                                          "QLN" };
+    return glutamineStateNames[enumValue];
+}
+
+const char* enumValueToLongString(GlutamineStates enumValue)
+{
+    constexpr gmx::EnumerationArray<GlutamineStates, const char*> glutamineStateLongNames = {
+        "Not protonated (charge 0)", "Protonated (charge +1)"
+    };
+    return glutamineStateLongNames[enumValue];
+}
+
+enum class LysineStates : int
+{
+    Deprot,
+    Prot,
+    Count
+};
+
+const char* enumValueToString(LysineStates enumValue)
+{
+    constexpr gmx::EnumerationArray<LysineStates, const char*> lysineStateNames = { "LYSN", "LYS" };
+    return lysineStateNames[enumValue];
+}
+
+const char* enumValueToLongString(LysineStates enumValue)
+{
+    constexpr gmx::EnumerationArray<LysineStates, const char*> lysineStateLongNames = {
+        "Not protonated (charge 0)", "Protonated (charge +1)"
+    };
+    return lysineStateLongNames[enumValue];
+}
+
+enum class ArginineStates : int
+{
+    Deprot,
+    Prot,
+    Count
+};
+
+const char* enumValueToString(ArginineStates enumValue)
+{
+    constexpr gmx::EnumerationArray<ArginineStates, const char*> arginineStatesNames = { "ARGN",
+                                                                                         "ARG" };
+    return arginineStatesNames[enumValue];
+}
+
+const char* enumValueToLongString(ArginineStates enumValue)
+{
+    constexpr gmx::EnumerationArray<ArginineStates, const char*> arginineStatesLongNames = {
+        "Not protonated (charge 0)", "Protonated (charge +1)"
+    };
+    return arginineStatesLongNames[enumValue];
+}
+
+template<typename EnumType>
+const char* select_res(int resnr, const char* title, gmx::ArrayRef<const RtpRename> rr)
 {
     printf("Which %s type do you want for residue %d\n", title, resnr + 1);
-    for (int sel = 0; (sel < nr); sel++)
+    for (auto sel : gmx::EnumerationWrapper<EnumType>{})
     {
-        printf("%d. %s (%s)\n", sel, expl[sel], res2bb_notermini(name[sel], rr));
+        printf("%d. %s (%s)\n",
+               static_cast<int>(sel),
+               enumValueToString(sel),
+               res2bb_notermini(enumValueToString(sel), rr));
     }
     printf("\nType a number:");
     fflush(stdout);
@@ -142,86 +259,37 @@ const char* select_res(int                            nr,
         gmx_fatal(FARGS, "Answer me for res %s %d!", title, resnr + 1);
     }
 
-    return name[userSelection];
+    return enumValueToLongString(static_cast<EnumType>(userSelection));
 }
 
 const char* get_asptp(int resnr, gmx::ArrayRef<const RtpRename> rr)
 {
-    enum
-    {
-        easp,
-        easpH,
-        easpNR
-    };
-    const char* lh[easpNR]   = { "ASP", "ASPH" };
-    const char* expl[easpNR] = { "Not protonated (charge -1)", "Protonated (charge 0)" };
-
-    return select_res(easpNR, resnr, lh, expl, "ASPARTIC ACID", rr);
+    return select_res<AspartateStates>(resnr, "ASPARTIC ACID", rr);
 }
 
 const char* get_glutp(int resnr, gmx::ArrayRef<const RtpRename> rr)
 {
-    enum
-    {
-        eglu,
-        egluH,
-        egluNR
-    };
-    const char* lh[egluNR]   = { "GLU", "GLUH" };
-    const char* expl[egluNR] = { "Not protonated (charge -1)", "Protonated (charge 0)" };
-
-    return select_res(egluNR, resnr, lh, expl, "GLUTAMIC ACID", rr);
+    return select_res<GlutamateStates>(resnr, "GLUTAMIC ACID", rr);
 }
 
 const char* get_glntp(int resnr, gmx::ArrayRef<const RtpRename> rr)
 {
-    enum
-    {
-        egln,
-        eglnH,
-        eglnNR
-    };
-    const char* lh[eglnNR]   = { "GLN", "QLN" };
-    const char* expl[eglnNR] = { "Not protonated (charge 0)", "Protonated (charge +1)" };
-
-    return select_res(eglnNR, resnr, lh, expl, "GLUTAMINE", rr);
+    return select_res<GlutamineStates>(resnr, "GLUTAMINE", rr);
 }
 
 const char* get_lystp(int resnr, gmx::ArrayRef<const RtpRename> rr)
 {
-    enum
-    {
-        elys,
-        elysH,
-        elysNR
-    };
-    const char* lh[elysNR]   = { "LYSN", "LYS" };
-    const char* expl[elysNR] = { "Not protonated (charge 0)", "Protonated (charge +1)" };
-
-    return select_res(elysNR, resnr, lh, expl, "LYSINE", rr);
+    return select_res<LysineStates>(resnr, "LYSINE", rr);
 }
 
 const char* get_argtp(int resnr, gmx::ArrayRef<const RtpRename> rr)
 {
-    enum
-    {
-        earg,
-        eargH,
-        eargNR
-    };
-    const char* lh[eargNR]   = { "ARGN", "ARG" };
-    const char* expl[eargNR] = { "Not protonated (charge 0)", "Protonated (charge +1)" };
-
-    return select_res(eargNR, resnr, lh, expl, "ARGININE", rr);
+    return select_res<ArginineStates>(resnr, "ARGININE", rr);
 }
 
 const char* get_histp(int resnr, gmx::ArrayRef<const RtpRename> rr)
 {
-    const char* expl[ehisNR] = {
-        "H on ND1 only", "H on NE2 only", "H on ND1 and NE2", "Coupled to Heme"
-    };
-
-    return select_res(ehisNR, resnr, hh, expl, "HISTIDINE", rr);
+    return select_res<HistidineStates>(resnr, "HISTIDINE", rr);
 }
 
 void read_rtprename(const char* fname, FILE* fp, std::vector<RtpRename>* rtprename)
@@ -1432,7 +1500,7 @@ bool checkChainCyclicity(t_atoms*                               pdba,
 struct t_pdbchain
 {
     char             chainid   = ' ';
-    char             chainnum  = ' ';
+    int              chainnum  = ' '; // char, but stored as int to make clang-tidy happy
     int              start     = -1;
     int              natom     = -1;
     bool             bAllWat   = false;

diff --git a/src/gromacs/gmxpreprocess/pdb2top.cpp b/src/gromacs/gmxpreprocess/pdb2top.cpp
index e31a25c9679b9e7f8a209fe73702552ff03b333f..5a300debef2592d0ce05cad27a18105e194948e5 100644 (file)
--- a/src/gromacs/gmxpreprocess/pdb2top.cpp
+++ b/src/gromacs/gmxpreprocess/pdb2top.cpp
@@ -68,6 +68,7 @@
 #include "gromacs/utility/binaryinformation.h"
 #include "gromacs/utility/cstringutil.h"
 #include "gromacs/utility/dir_separator.h"
+#include "gromacs/utility/enumerationhelpers.h"
 #include "gromacs/utility/exceptions.h"
 #include "gromacs/utility/fatalerror.h"
 #include "gromacs/utility/futil.h"
@@ -84,8 +85,13 @@
 #include "hackblock.h"
 #include "resall.h"
 
-/* this must correspond to enum in pdb2top.h */
-const char* hh[ehisNR] = { "HISD", "HISE", "HISH", "HIS1" };
+const char* enumValueToString(HistidineStates enumValue)
+{
+    constexpr gmx::EnumerationArray<HistidineStates, const char*> histidineStateNames = {
+        "HISD", "HISE", "HISH", "HIS1"
+    };
+    return histidineStateNames[enumValue];
+}
 
 static int missing_atoms(const PreprocessResidue* rp, int resind, t_atoms* at, int i0, int i, const gmx::MDLogger& logger)
 {

diff --git a/src/gromacs/gmxpreprocess/pdb2top.h b/src/gromacs/gmxpreprocess/pdb2top.h
index 45677bfdb3529ac1befe82d6a770d6f23d1dfd66..c7c8ff2b1d87a81bd22acc3806a65f6832f96101 100644 (file)
--- a/src/gromacs/gmxpreprocess/pdb2top.h
+++ b/src/gromacs/gmxpreprocess/pdb2top.h
@@ -44,6 +44,7 @@
 #include <vector>
 
 #include "gromacs/math/vectypes.h"
+#include "gromacs/utility/enumerationhelpers.h"
 
 class PreprocessingAtomTypes;
 
@@ -64,16 +65,16 @@ struct PreprocessResidue;
 struct DisulfideBond;
 struct t_symtab;
 
-/* this *MUST* correspond to array in pdb2top.c */
-enum
+/* this *MUST* correspond to array in pdb2top.cpp */
+enum class HistidineStates : int
 {
-    ehisA,
-    ehisB,
-    ehisH,
-    ehis1,
-    ehisNR
+    A,
+    B,
+    H,
+    One,
+    Count
 };
-extern const char* hh[ehisNR];
+const char* enumValueToString(HistidineStates enumValue);
 
 void choose_ff(const char*          ffsel,
                char*                forcefield,

diff --git a/src/gromacs/gmxpreprocess/readir.cpp b/src/gromacs/gmxpreprocess/readir.cpp
index 24f78964b2c8fe62edac0cb8909c0c2542fb41fe..55756cec59aceafdb68a2bf6d3fc13c5ed0dc791 100644 (file)
--- a/src/gromacs/gmxpreprocess/readir.cpp
+++ b/src/gromacs/gmxpreprocess/readir.cpp
@@ -116,6 +116,7 @@ struct gmx_inputrec_strings
     char anneal[STRLEN], anneal_npoints[STRLEN], anneal_time[STRLEN], anneal_temp[STRLEN];
 };
 
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
 static gmx_inputrec_strings* inputrecStrings = nullptr;
 
 void init_inputrec_strings()
@@ -147,9 +148,11 @@ enum
                         * make a rest group for the remaining particles.    */
 };
 
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
 static const char* constraints[eshNR + 1] = { "none",     "h-bonds",    "all-bonds",
                                               "h-angles", "all-angles", nullptr };
 
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
 static const char* couple_lam[ecouplamNR + 1] = { "vdw-q", "vdw", "q", "none", nullptr };
 
 static void getSimTemps(int ntemps, t_simtemp* simtemp, gmx::ArrayRef<double> temperature_lambdas)

diff --git a/src/gromacs/gmxpreprocess/readpull.cpp b/src/gromacs/gmxpreprocess/readpull.cpp
index dc724a3599bb583d4094b266ea403ced71217484..c5194634ef138e053438857aeba1a52be3c91050 100644 (file)
--- a/src/gromacs/gmxpreprocess/readpull.cpp
+++ b/src/gromacs/gmxpreprocess/readpull.cpp
@@ -567,9 +567,9 @@ pull_t* set_pull_init(t_inputrec*                    ir,
 
     pull_params_t*           pull = ir->pull.get();
     gmx::LocalAtomSetManager atomSets;
-    pull_work    = init_pull(nullptr, pull, ir, mtop, nullptr, &atomSets, lambda);
-    auto mdAtoms = gmx::makeMDAtoms(nullptr, mtop, *ir, false);
-    auto md      = mdAtoms->mdatoms();
+    pull_work     = init_pull(nullptr, pull, ir, mtop, nullptr, &atomSets, lambda);
+    auto  mdAtoms = gmx::makeMDAtoms(nullptr, mtop, *ir, false);
+    auto* md      = mdAtoms->mdatoms();
     atoms2md(mtop, *ir, -1, {}, mtop.natoms, mdAtoms.get());
     if (ir->efep != FreeEnergyPerturbationType::No)
     {

diff --git a/src/gromacs/gmxpreprocess/readrot.cpp b/src/gromacs/gmxpreprocess/readrot.cpp
index 4e725040602a64956316856a5cf6884e09c0088a..262c658b2a950e3a8c8b7cb569e2c6ce7d87865c 100644 (file)
--- a/src/gromacs/gmxpreprocess/readrot.cpp
+++ b/src/gromacs/gmxpreprocess/readrot.cpp
@@ -56,11 +56,7 @@
 #include "gromacs/utility/smalloc.h"
 #include "gromacs/utility/stringutil.h"
 
-static const char* RotStr = "Enforced rotation:";
-
-
-static char s_vec[STRLEN];
-
+static const std::string RotStr = "Enforced rotation:";
 
 static void string2dvec(char buf[], dvec nums)
 {
@@ -100,6 +96,7 @@ extern std::vector<std::string> read_rotparams(std::vector<t_inpfile>* inp, t_ro
     /* Read the rotation groups */
     std::vector<std::string> rotateGroups(rot->ngrp);
     char                     readBuffer[STRLEN];
+    char                     s_vec[STRLEN];
     for (g = 0; g < rot->ngrp; g++)
     {
         rotg = &rot->grp[g];
@@ -133,7 +130,7 @@ extern std::vector<std::string> read_rotparams(std::vector<t_inpfile>* inp, t_ro
         }
         fprintf(stderr,
                 "%s Group %d (%s) normalized rot. vector: %f %f %f\n",
-                RotStr,
+                RotStr.c_str(),
                 g,
                 enumValueToString(rotg->eType),
                 vec[0],
@@ -249,7 +246,7 @@ static void check_box_unchanged(matrix f_box, matrix box, const char fn[], warni
     }
     if (!bSame)
     {
-        sprintf(warn_buf, "%s Box size in reference file %s differs from actual box size!", RotStr, fn);
+        sprintf(warn_buf, "%s Box size in reference file %s differs from actual box size!", RotStr.c_str(), fn);
         warning(wi, warn_buf);
         pr_rvecs(stderr, 0, "Your box is:", box, 3);
         pr_rvecs(stderr, 0, "Box in file:", f_box, 3);
@@ -268,7 +265,7 @@ extern void set_reference_positions(t_rot* rot, rvec* x, matrix box, const char*
     for (g = 0; g < rot->ngrp; g++)
     {
         rotg = &rot->grp[g];
-        fprintf(stderr, "%s group %d has %d reference positions.\n", RotStr, g, rotg->nat);
+        fprintf(stderr, "%s group %d has %d reference positions.\n", RotStr.c_str(), g, rotg->nat);
         snew(rotg->x_ref, rotg->nat);
 
         /* Construct the name for the file containing the reference positions for this group: */
@@ -283,7 +280,7 @@ extern void set_reference_positions(t_rot* rot, rvec* x, matrix box, const char*
             gmx_fatal(FARGS,
                       "%s The file containing the reference positions was not found.\n"
                       "Expected the file '%s' for group %d.\n",
-                      RotStr,
+                      RotStr.c_str(),
                       reffile,
                       g);
         }

diff --git a/src/gromacs/gmxpreprocess/resall.cpp b/src/gromacs/gmxpreprocess/resall.cpp
index 84485f95d72d664a1bf716608f828b94cfb157c7..749630a79e6ba434558f3cac48a2c623fa1be9ec 100644 (file)
--- a/src/gromacs/gmxpreprocess/resall.cpp
+++ b/src/gromacs/gmxpreprocess/resall.cpp
@@ -56,6 +56,7 @@
 #include "gromacs/topology/atoms.h"
 #include "gromacs/topology/symtab.h"
 #include "gromacs/utility/cstringutil.h"
+#include "gromacs/utility/enumerationhelpers.h"
 #include "gromacs/utility/fatalerror.h"
 #include "gromacs/utility/futil.h"
 #include "gromacs/utility/logger.h"

diff --git a/src/gromacs/gmxpreprocess/specbond.cpp b/src/gromacs/gmxpreprocess/specbond.cpp
index 81385114a82ec234ed9b6cce19d9c0b822bd1f60..f9e3c7750ff8815915ef3c0e288da4dae5b09557 100644 (file)
--- a/src/gromacs/gmxpreprocess/specbond.cpp
+++ b/src/gromacs/gmxpreprocess/specbond.cpp
@@ -4,7 +4,7 @@
  * Copyright (c) 1991-2000, University of Groningen, The Netherlands.
  * Copyright (c) 2001-2004, The GROMACS development team.
  * 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.
@@ -121,17 +121,12 @@ std::vector<SpecialBond> generateSpecialBonds()
 
 static bool is_special(gmx::ArrayRef<const SpecialBond> sb, const char* res, const char* atom)
 {
-    for (const auto& bond : sb)
-    {
-        if (((strncmp(bond.firstResidue.c_str(), res, 3) == 0)
-             && (gmx::equalCaseInsensitive(bond.firstAtomName, atom)))
-            || ((strncmp(bond.secondResidue.c_str(), res, 3) == 0)
-                && (gmx::equalCaseInsensitive(bond.secondAtomName, atom))))
-        {
-            return TRUE;
-        }
-    }
-    return FALSE;
+    return std::any_of(sb.begin(), sb.end(), [res, atom](const auto& bond) {
+        return (((strncmp(bond.firstResidue.c_str(), res, 3) == 0)
+                 && (gmx::equalCaseInsensitive(bond.firstAtomName, atom)))
+                || ((strncmp(bond.secondResidue.c_str(), res, 3) == 0)
+                    && (gmx::equalCaseInsensitive(bond.secondAtomName, atom))));
+    });
 }
 
 static bool is_bond(gmx::ArrayRef<const SpecialBond> sb, t_atoms* pdba, int a1, int a2, real d, int* index_sb, bool* bSwap)

diff --git a/src/gromacs/gmxpreprocess/ter_db.cpp b/src/gromacs/gmxpreprocess/ter_db.cpp
index f4f12e34ec0f4e678e7a896baab3752b8750e3e7..b65145dc9057100b81b4f96dbdccbde3789ae832 100644 (file)
--- a/src/gromacs/gmxpreprocess/ter_db.cpp
+++ b/src/gromacs/gmxpreprocess/ter_db.cpp
@@ -39,10 +39,12 @@
 
 #include "ter_db.h"
 
+#include <array>
 #include <cctype>
 #include <cstring>
 
 #include <algorithm>
+#include <optional>
 #include <string>
 #include <vector>
 
@@ -54,6 +56,7 @@
 #include "gromacs/gmxpreprocess/notset.h"
 #include "gromacs/gmxpreprocess/toputil.h"
 #include "gromacs/utility/cstringutil.h"
+#include "gromacs/utility/enumerationhelpers.h"
 #include "gromacs/utility/fatalerror.h"
 #include "gromacs/utility/futil.h"
 #include "gromacs/utility/enumerationhelpers.h"
@@ -66,6 +69,7 @@
 #include "hackblock.h"
 #include "resall.h"
 
+/* use bonded types definitions in hackblock.h */
 enum class ReplaceType : int
 {
     Repl,

diff --git a/src/gromacs/gmxpreprocess/tests/.clang-tidy b/src/gromacs/gmxpreprocess/tests/.clang-tidy
new file mode 100644 (file)
index 0000000..710212e
--- /dev/null
+++ b/src/gromacs/gmxpreprocess/tests/.clang-tidy
@@ -0,0 +1,96 @@
+# 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
+#
+#         -cppcoreguidelines-init-variables
+# These shall be fixed eventually, but clang-tidy's automatic fix of 
+# initializing them with zeroes does not usually improve the code in
+# a meaningful way, and fixing them all manually is cumbersome.
+#
+#         -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/gromacs/gmxpreprocess/tests/editconf.cpp b/src/gromacs/gmxpreprocess/tests/editconf.cpp
index 292e20222223c53041435b242a4c13fbc6011636..af63fc601edb6c7e4e500e90c116909111459c07 100644 (file)
--- a/src/gromacs/gmxpreprocess/tests/editconf.cpp
+++ b/src/gromacs/gmxpreprocess/tests/editconf.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,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.
@@ -94,7 +94,7 @@ public:
         ASSERT_EQ(0, gmx_editconf(cmdline.argc(), cmdline.argv()));
 
         // Check the output
-        auto                 extension = ftp2ext(std::get<1>(GetParam()));
+        const auto*          extension = ftp2ext(std::get<1>(GetParam()));
         TestReferenceChecker rootChecker(this->rootChecker());
         rootChecker.checkString(extension, testName);
         checkOutputFiles();

diff --git a/src/gromacs/gmxpreprocess/tests/topdirs.cpp b/src/gromacs/gmxpreprocess/tests/topdirs.cpp
index cfc7d306b97609815b1e454dc257109e19ab298d..281b3e37661e872929baf42db5ba74b1d800a309 100644 (file)
--- a/src/gromacs/gmxpreprocess/tests/topdirs.cpp
+++ b/src/gromacs/gmxpreprocess/tests/topdirs.cpp
@@ -54,7 +54,7 @@ TEST(TopDirTests, NamesArrayHasCorrectSize)
         // If the enumeration is extended, but there is no matching
         // name, then at least one element will be value initialized,
         // ie. to nullptr, which this test will catch.
-        auto name = enumValueToString(d);
+        const auto* name = enumValueToString(d);
         EXPECT_NE(name, nullptr);
     }
 }

diff --git a/src/gromacs/gmxpreprocess/topdirs.cpp b/src/gromacs/gmxpreprocess/topdirs.cpp
index ea66de5f80e4495c3bf6695161ee7c9b6c4e824c..4d1a3292edc417719b48133be3d7e12914fd27c9 100644 (file)
--- a/src/gromacs/gmxpreprocess/topdirs.cpp
+++ b/src/gromacs/gmxpreprocess/topdirs.cpp
@@ -291,6 +291,7 @@ Directive str2dir(char* dstr)
     return Directive::d_invalid;
 }
 
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
 static gmx::EnumerationArray<Directive, Directive*> necessary = { { nullptr } };
 
 static void set_nec(Directive** n, ...)

diff --git a/src/gromacs/gmxpreprocess/vsite_parm.cpp b/src/gromacs/gmxpreprocess/vsite_parm.cpp
index 53bd6f354c37c77d452edb6980ac0f0668c00f3e..405e3f4ce8f331980f684204b2dee3c02d677116 100644 (file)
--- a/src/gromacs/gmxpreprocess/vsite_parm.cpp
+++ b/src/gromacs/gmxpreprocess/vsite_parm.cpp
@@ -80,7 +80,7 @@ public:
     {
         GMX_RELEASE_ASSERT(atomIndex.size() <= atomIndex_.size(),
                            "Cannot add more atom indices than maximum number");
-        auto atomIndexIt = atomIndex_.begin();
+        auto* atomIndexIt = atomIndex_.begin();
         for (const auto index : atomIndex)
         {
             *atomIndexIt++ = index;
@@ -182,7 +182,7 @@ static AllVsiteBondedInteractions createVsiteBondedInformation(int
     AllVsiteBondedInteractions allVsiteBondeds;
     for (int k = 0; k < nrat; k++)
     {
-        for (auto& vsite : at2vb[atoms[k]].vSiteBondedParameters)
+        for (const auto& vsite : at2vb[atoms[k]].vSiteBondedParameters)
         {
             int                      ftype   = vsite.ftype_;
             const InteractionOfType& type    = vsite.vsiteInteraction_;