From c1ea59c785f57e38b6740697d6882f42e451007f Mon Sep 17 00:00:00 2001 From: Joe Jordan Date: Wed, 21 Apr 2021 14:07:45 +0000 Subject: [PATCH] clang-tidy-11 fixes for gmxpreprocess --- src/gromacs/gmxpreprocess/convparm.cpp | 2 +- src/gromacs/gmxpreprocess/gen_ad.cpp | 1 + src/gromacs/gmxpreprocess/gen_vsite.cpp | 2 +- src/gromacs/gmxpreprocess/genhydro.cpp | 2 +- src/gromacs/gmxpreprocess/gmxcpp.cpp | 4 +- src/gromacs/gmxpreprocess/grompp.cpp | 2 +- src/gromacs/gmxpreprocess/hackblock.cpp | 1 + src/gromacs/gmxpreprocess/hizzie.cpp | 32 +-- src/gromacs/gmxpreprocess/pdb2gmx.cpp | 198 +++++++++++++------ src/gromacs/gmxpreprocess/pdb2top.cpp | 10 +- src/gromacs/gmxpreprocess/pdb2top.h | 17 +- src/gromacs/gmxpreprocess/readir.cpp | 3 + src/gromacs/gmxpreprocess/readpull.cpp | 6 +- src/gromacs/gmxpreprocess/readrot.cpp | 15 +- src/gromacs/gmxpreprocess/resall.cpp | 1 + src/gromacs/gmxpreprocess/specbond.cpp | 19 +- src/gromacs/gmxpreprocess/ter_db.cpp | 4 + src/gromacs/gmxpreprocess/tests/.clang-tidy | 96 +++++++++ src/gromacs/gmxpreprocess/tests/editconf.cpp | 4 +- src/gromacs/gmxpreprocess/tests/topdirs.cpp | 2 +- src/gromacs/gmxpreprocess/topdirs.cpp | 1 + src/gromacs/gmxpreprocess/vsite_parm.cpp | 4 +- 22 files changed, 302 insertions(+), 124 deletions(-) create mode 100644 src/gromacs/gmxpreprocess/tests/.clang-tidy diff --git a/src/gromacs/gmxpreprocess/convparm.cpp b/src/gromacs/gmxpreprocess/convparm.cpp index 1712ec3c16..37b59f1eb5 100644 --- 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 643963f73d..a39d5c8810 100644 --- 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 eec1e75386..d94a14e582 100644 --- 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 rtpFFDB, PreprocessingAtomTypes* atype, diff --git a/src/gromacs/gmxpreprocess/genhydro.cpp b/src/gromacs/gmxpreprocess/genhydro.cpp index 5ee31602a3..b9c8680c2e 100644 --- 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 1a597a4101..bde57d2443 100644 --- 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 c8faf6d05b..6e9b8d4764 100644 --- a/src/gromacs/gmxpreprocess/grompp.cpp +++ b/src/gromacs/gmxpreprocess/grompp.cpp @@ -122,7 +122,7 @@ InteractionOfType::InteractionOfType(gmx::ArrayRef 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 5cbc9e3d9b..956e0d72e1 100644 --- 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 67f17ada1f..946ada2f39 100644 --- 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 5346f3804e..d562476a1e 100644 --- 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 @@ -121,17 +122,133 @@ const char* res2bb_notermini(const std::string& name, gmx::ArrayRefmain.c_str() : name.c_str(); } -const char* select_res(int nr, - int resnr, - const char* name[], - const char* expl[], - const char* title, - gmx::ArrayRef rr) +const char* enumValueToLongString(HistidineStates enumValue) +{ + constexpr gmx::EnumerationArray 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 aspartateStateNames = { "ASP", + "ASPH" }; + return aspartateStateNames[enumValue]; +} + +const char* enumValueToLongString(AspartateStates enumValue) +{ + constexpr gmx::EnumerationArray 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 glutamateStateNames = { "GLU", + "GLUH" }; + return glutamateStateNames[enumValue]; +} + +const char* enumValueToLongString(GlutamateStates enumValue) +{ + constexpr gmx::EnumerationArray 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 glutamineStateNames = { "GLN", + "QLN" }; + return glutamineStateNames[enumValue]; +} + +const char* enumValueToLongString(GlutamineStates enumValue) +{ + constexpr gmx::EnumerationArray 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 lysineStateNames = { "LYSN", "LYS" }; + return lysineStateNames[enumValue]; +} + +const char* enumValueToLongString(LysineStates enumValue) +{ + constexpr gmx::EnumerationArray 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 arginineStatesNames = { "ARGN", + "ARG" }; + return arginineStatesNames[enumValue]; +} + +const char* enumValueToLongString(ArginineStates enumValue) +{ + constexpr gmx::EnumerationArray arginineStatesLongNames = { + "Not protonated (charge 0)", "Protonated (charge +1)" + }; + return arginineStatesLongNames[enumValue]; +} + +template +const char* select_res(int resnr, const char* title, gmx::ArrayRef 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{}) { - printf("%d. %s (%s)\n", sel, expl[sel], res2bb_notermini(name[sel], rr)); + printf("%d. %s (%s)\n", + static_cast(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(userSelection)); } const char* get_asptp(int resnr, gmx::ArrayRef 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(resnr, "ASPARTIC ACID", rr); } const char* get_glutp(int resnr, gmx::ArrayRef 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(resnr, "GLUTAMIC ACID", rr); } const char* get_glntp(int resnr, gmx::ArrayRef 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(resnr, "GLUTAMINE", rr); } const char* get_lystp(int resnr, gmx::ArrayRef 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(resnr, "LYSINE", rr); } const char* get_argtp(int resnr, gmx::ArrayRef 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(resnr, "ARGININE", rr); } const char* get_histp(int resnr, gmx::ArrayRef 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(resnr, "HISTIDINE", rr); } void read_rtprename(const char* fname, FILE* fp, std::vector* 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 e31a25c967..5a300debef 100644 --- 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 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 45677bfdb3..c7c8ff2b1d 100644 --- a/src/gromacs/gmxpreprocess/pdb2top.h +++ b/src/gromacs/gmxpreprocess/pdb2top.h @@ -44,6 +44,7 @@ #include #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 24f78964b2..55756cec59 100644 --- 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 temperature_lambdas) diff --git a/src/gromacs/gmxpreprocess/readpull.cpp b/src/gromacs/gmxpreprocess/readpull.cpp index dc724a3599..c5194634ef 100644 --- 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 4e72504060..262c658b2a 100644 --- 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 read_rotparams(std::vector* inp, t_ro /* Read the rotation groups */ std::vector 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 read_rotparams(std::vector* 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 84485f95d7..749630a79e 100644 --- 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 81385114a8..f9e3c7750f 100644 --- 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 generateSpecialBonds() static bool is_special(gmx::ArrayRef 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 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 f4f12e34ec..b65145dc90 100644 --- a/src/gromacs/gmxpreprocess/ter_db.cpp +++ b/src/gromacs/gmxpreprocess/ter_db.cpp @@ -39,10 +39,12 @@ #include "ter_db.h" +#include #include #include #include +#include #include #include @@ -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 index 0000000000..710212e09d --- /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 292e202222..af63fc601e 100644 --- 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 cfc7d306b9..281b3e3766 100644 --- 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 ea66de5f80..4d1a3292ed 100644 --- 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 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 53bd6f354c..405e3f4ce8 100644 --- 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_; -- 2.22.0