From: Joe Jordan Date: Wed, 5 May 2021 08:30:45 +0000 (+0000) Subject: Apply clang-tidy11 to tools X-Git-Url: http://biod.pnpi.spb.ru/gitweb/?a=commitdiff_plain;h=8c1ad27d66673e0d27ca1d3d484595391a0ffd26;p=alexxy%2Fgromacs.git Apply clang-tidy11 to tools --- diff --git a/src/gromacs/tools/check.cpp b/src/gromacs/tools/check.cpp index d44370c0c6..c660dc8483 100644 --- a/src/gromacs/tools/check.cpp +++ b/src/gromacs/tools/check.cpp @@ -799,14 +799,14 @@ int gmx_check(int argc, char* argv[]) const char *fn1 = nullptr, *fn2 = nullptr, *tex = nullptr; gmx_output_env_t* oenv; - static real vdw_fac = 0.8; - static real bon_lo = 0.4; - static real bon_hi = 0.7; - static gmx_bool bRMSD = FALSE; - static real ftol = 0.001; - static real abstol = 0.001; - static gmx_bool bCompAB = FALSE; - static char* lastener = nullptr; + real vdw_fac = 0.8; + real bon_lo = 0.4; + real bon_hi = 0.7; + gmx_bool bRMSD = FALSE; + real ftol = 0.001; + real abstol = 0.001; + gmx_bool bCompAB = FALSE; + char* lastener = nullptr; static t_pargs pa[] = { { "-vdwfac", FALSE, diff --git a/src/gromacs/tools/make_ndx.cpp b/src/gromacs/tools/make_ndx.cpp index 3f8dbe2aad..f344e0d4b9 100644 --- a/src/gromacs/tools/make_ndx.cpp +++ b/src/gromacs/tools/make_ndx.cpp @@ -43,6 +43,8 @@ #include #include +#include +#include #include "gromacs/commandline/pargs.h" #include "gromacs/fileio/confio.h" @@ -51,6 +53,7 @@ #include "gromacs/topology/index.h" #include "gromacs/topology/mtop_util.h" #include "gromacs/topology/topology.h" +#include "gromacs/utility/arrayref.h" #include "gromacs/utility/arraysize.h" #include "gromacs/utility/cstringutil.h" #include "gromacs/utility/fatalerror.h" @@ -64,6 +67,7 @@ #define NAME_LEN 1024 static const int NOTSET = -92637; +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) static gmx_bool bCase = FALSE; static int or_groups(int nr1, const int* at1, int nr2, const int* at2, int* nr, int* at) @@ -157,7 +161,7 @@ static gmx_bool is_name_char(char c) return (c != '\0' && std::strchr(spec, c) == nullptr); } -static int parse_names(char** string, int* n_names, char** names) +static int parse_names(char** string, int* n_names, gmx::ArrayRef names) { int i; @@ -198,7 +202,7 @@ static int parse_names(char** string, int* n_names, char** names) return *n_names; } -static gmx_bool parse_int_char(char** string, int* nr, char* c) +static gmx_bool parse_int_char(char** string, int* nr, unsigned char* c) { char* orig; gmx_bool bRet; @@ -248,8 +252,9 @@ static gmx_bool parse_int_char(char** string, int* nr, char* c) static gmx_bool parse_int(char** string, int* nr) { - char * orig, c; - gmx_bool bRet; + char* orig; + unsigned char c; + gmx_bool bRet; orig = *string; bRet = parse_int_char(string, nr, &c); @@ -356,7 +361,8 @@ static int select_atomnumbers(char** string, const t_atoms* atoms, int n1, int* return *nr; } -static int select_residuenumbers(char** string, const t_atoms* atoms, int n1, char c, int* nr, int* index, char* gname) +static int +select_residuenumbers(char** string, const t_atoms* atoms, int n1, unsigned char c, int* nr, int* index, char* gname) { char buf[STRLEN]; int i, j, up; @@ -425,7 +431,8 @@ static int select_residuenumbers(char** string, const t_atoms* atoms, int n1, ch return *nr; } -static int select_residueindices(char** string, const t_atoms* atoms, int n1, char c, int* nr, int* index, char* gname) +static int +select_residueindices(char** string, const t_atoms* atoms, int n1, unsigned char c, int* nr, int* index, char* gname) { /*this should be similar to select_residuenumbers except select by index (sequential numbering in file)*/ /*resind+1 for 1-indexing*/ @@ -569,7 +576,7 @@ static gmx_bool comp_name(const char* name, const char* search) return matches; } -static int select_chainnames(const t_atoms* atoms, int n_names, char** names, int* nr, int* index) +static int select_chainnames(const t_atoms* atoms, int n_names, gmx::ArrayRef names, int* nr, int* index) { char name[2]; int j; @@ -601,7 +608,7 @@ static int select_chainnames(const t_atoms* atoms, int n_names, char** names, in return *nr; } -static int select_atomnames(const t_atoms* atoms, int n_names, char** names, int* nr, int* index, gmx_bool bType) +static int select_atomnames(const t_atoms* atoms, int n_names, gmx::ArrayRef names, int* nr, int* index, gmx_bool bType) { char* name; int j; @@ -639,7 +646,7 @@ static int select_atomnames(const t_atoms* atoms, int n_names, char** names, int return *nr; } -static int select_residuenames(const t_atoms* atoms, int n_names, char** names, int* nr, int* index) +static int select_residuenames(const t_atoms* atoms, int n_names, gmx::ArrayRef names, int* nr, int* index) { char* name; int j; @@ -686,7 +693,7 @@ static void copy2block(int n, const int* index, t_blocka* block) } } -static void make_gname(int n, char** names, char* gname) +static void make_gname(int n, gmx::ArrayRef names, char* gname) { int i; @@ -907,31 +914,21 @@ static gmx_bool check_have_atoms(const t_atoms* atoms, char* string) } } -static gmx_bool parse_entry(char** string, - int natoms, - const t_atoms* atoms, - t_blocka* block, - char*** gn, - int* nr, - int* index, - char* gname) +static gmx_bool parse_entry(char** string, + int natoms, + const t_atoms* atoms, + t_blocka* block, + char*** gn, + int* nr, + int* index, + char* gname, + gmx::ArrayRef entryNames) { - static char ** names, *ostring; - static gmx_bool bFirst = TRUE; - int j, n_names, sel_nr1; - int i, nr1, *index1; - char c; - gmx_bool bRet, bCompl; - - if (bFirst) - { - bFirst = FALSE; - snew(names, MAXNAMES); - for (i = 0; i < MAXNAMES; i++) - { - snew(names[i], NAME_LEN + 1); - } - } + char* ostring; + int j, n_names, sel_nr1; + int i, nr1, *index1; + unsigned char c; + gmx_bool bRet, bCompl; bRet = FALSE; sel_nr1 = NOTSET; @@ -980,17 +977,17 @@ static gmx_bool parse_entry(char** string, { bRet = (select_atomnumbers(string, atoms, sel_nr1, nr, index, gname) != 0); } - else if (parse_names(string, &n_names, names)) + else if (parse_names(string, &n_names, entryNames)) { - bRet = (select_atomnames(atoms, n_names, names, nr, index, FALSE) != 0); - make_gname(n_names, names, gname); + bRet = (select_atomnames(atoms, n_names, entryNames, nr, index, FALSE) != 0); + make_gname(n_names, entryNames, gname); } } } else if ((*string)[0] == 't') { (*string)++; - if (check_have_atoms(atoms, ostring) && parse_names(string, &n_names, names)) + if (check_have_atoms(atoms, ostring) && parse_names(string, &n_names, entryNames)) { if (!(atoms->haveType)) { @@ -998,8 +995,8 @@ static gmx_bool parse_entry(char** string, } else { - bRet = (select_atomnames(atoms, n_names, names, nr, index, TRUE) != 0); - make_gname(n_names, names, gname); + bRet = (select_atomnames(atoms, n_names, entryNames, nr, index, TRUE) != 0); + make_gname(n_names, entryNames, gname); } } } @@ -1030,23 +1027,23 @@ static gmx_bool parse_entry(char** string, { bRet = (select_residuenumbers(string, atoms, sel_nr1, c, nr, index, gname) != 0); } - else if (parse_names(string, &n_names, names)) + else if (parse_names(string, &n_names, entryNames)) { - bRet = (select_residuenames(atoms, n_names, names, nr, index) != 0); - make_gname(n_names, names, gname); + bRet = (select_residuenames(atoms, n_names, entryNames, nr, index) != 0); + make_gname(n_names, entryNames, gname); } } } else if (std::strncmp(*string, "chain", 5) == 0) { (*string) += 5; - if (check_have_atoms(atoms, ostring) && parse_names(string, &n_names, names)) + if (check_have_atoms(atoms, ostring) && parse_names(string, &n_names, entryNames)) { - bRet = (select_chainnames(atoms, n_names, names, nr, index) != 0); - sprintf(gname, "ch%s", names[0]); + bRet = (select_chainnames(atoms, n_names, entryNames, nr, index) != 0); + sprintf(gname, "ch%s", entryNames[0]); for (i = 1; i < n_names; i++) { - std::strcat(gname, names[i]); + std::strcat(gname, entryNames[i]); } } } @@ -1135,23 +1132,12 @@ static void list_residues(const t_atoms* atoms) static void edit_index(int natoms, const t_atoms* atoms, const rvec* x, t_blocka* block, char*** gn, gmx_bool bVerbose) { - static char ** atnames, *ostring; - static gmx_bool bFirst = TRUE; - char inp_string[STRLEN], *string; - char gname[STRLEN * 3], gname1[STRLEN], gname2[STRLEN]; - int i, i0, i1, sel_nr, sel_nr2, newgroup; - int nr, nr1, nr2, *index, *index1, *index2; - gmx_bool bAnd, bOr, bPrintOnce; - - if (bFirst) - { - bFirst = FALSE; - snew(atnames, MAXNAMES); - for (i = 0; i < MAXNAMES; i++) - { - snew(atnames[i], NAME_LEN + 1); - } - } + char* ostring; + char inp_string[STRLEN], *string; + char gname[STRLEN * 3], gname1[STRLEN], gname2[STRLEN]; + int i, i0, i1, sel_nr, sel_nr2, newgroup; + int nr, nr1, nr2, *index, *index1, *index2; + gmx_bool bAnd, bOr, bPrintOnce; string = nullptr; @@ -1161,6 +1147,12 @@ static void edit_index(int natoms, const t_atoms* atoms, const rvec* x, t_blocka newgroup = NOTSET; bPrintOnce = TRUE; + std::array entryNames; + for (auto& name : entryNames) + { + snew(name, NAME_LEN + 1); + } + do { gname1[0] = '\0'; @@ -1373,7 +1365,7 @@ static void edit_index(int natoms, const t_atoms* atoms, const rvec* x, t_blocka else if (string[0] != 'q') { nr2 = -1; - if (parse_entry(&string, natoms, atoms, block, gn, &nr, index, gname)) + if (parse_entry(&string, natoms, atoms, block, gn, &nr, index, gname, entryNames)) { do { @@ -1402,7 +1394,7 @@ static void edit_index(int natoms, const t_atoms* atoms, const rvec* x, t_blocka index1[i] = index[i]; } std::strcpy(gname1, gname); - if (parse_entry(&string, natoms, atoms, block, gn, &nr2, index2, gname2)) + if (parse_entry(&string, natoms, atoms, block, gn, &nr2, index2, gname2, entryNames)) { if (bOr) { @@ -1440,6 +1432,10 @@ static void edit_index(int natoms, const t_atoms* atoms, const rvec* x, t_blocka } } while (string[0] != 'q'); + for (auto& name : entryNames) + { + sfree(name); + } sfree(index); sfree(index1); sfree(index2); @@ -1611,7 +1607,6 @@ int gmx_make_ndx(int argc, char* argv[]) natoms = block2natoms(block); printf("Counted atom numbers up to %d in index file\n", natoms); } - edit_index(natoms, &atoms, x, block, &gnames, bVerbose); write_index(ndxoutfile, block, gnames, bDuplicate, natoms); diff --git a/src/gromacs/tools/tests/.clang-tidy b/src/gromacs/tools/tests/.clang-tidy new file mode 100644 index 0000000000..0adf51e3ee --- /dev/null +++ b/src/gromacs/tools/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/gromacs/tools/trjconv.cpp b/src/gromacs/tools/trjconv.cpp index 67eb02938a..95975b6809 100644 --- a/src/gromacs/tools/trjconv.cpp +++ b/src/gromacs/tools/trjconv.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. @@ -402,14 +402,14 @@ int gmx_trjconv(int argc, char* argv[]) const char* fit[efNR + 1] = { nullptr, "none", "rot+trans", "rotxy+transxy", "translation", "transxy", "progressive", nullptr }; - static gmx_bool bSeparate = FALSE, bVels = TRUE, bForce = FALSE, bCONECT = FALSE; - static gmx_bool bCenter = FALSE; - static int skip_nr = 1, ndec = 3, nzero = 0; - static real tzero = 0, delta_t = 0, timestep = 0, ttrunc = -1, tdump = -1, split_t = 0; - static rvec newbox = { 0, 0, 0 }, shift = { 0, 0, 0 }, trans = { 0, 0, 0 }; - static char* exec_command = nullptr; - static real dropunder = 0, dropover = 0; - static gmx_bool bRound = FALSE; + gmx_bool bSeparate = FALSE, bVels = TRUE, bForce = FALSE, bCONECT = FALSE; + gmx_bool bCenter = FALSE; + int skip_nr = 1, ndec = 3, nzero = 0; + real tzero = 0, delta_t = 0, timestep = 0, ttrunc = -1, tdump = -1, split_t = 0; + rvec newbox = { 0, 0, 0 }, shift = { 0, 0, 0 }, trans = { 0, 0, 0 }; + char* exec_command = nullptr; + real dropunder = 0, dropover = 0; + gmx_bool bRound = FALSE; t_pargs pa[] = { { "-skip", FALSE, etINT, { &skip_nr }, "Only write every nr-th frame" },