Clang-tidy: enable further tests
authorRoland Schulz <roland.schulz@intel.com>
Sun, 20 May 2018 08:22:36 +0000 (01:22 -0700)
committerRoland Schulz <roland.schulz@intel.com>
Thu, 28 Jun 2018 02:03:22 +0000 (19:03 -0700)
Those out of misc, performance, readiability, mpi with managable
number of required fixes.

Remaining checks:
      4 readability-redundant-smartptr-get
      4 readability-redundant-string-cstr
      4 readability-simplify-boolean-expr
      5 misc-misplaced-widening-cast
      5 readability-named-parameter
      6 performance-noexcept-move-constructor
      8 readability-misleading-indentation
     10 readability-container-size-empty
     13 misc-suspicious-string-compare
     13 readability-redundant-control-flow
     17 performance-unnecessary-value-param
     17 readability-static-definition-in-anonymous-namespace
     18 misc-suspicious-missing-comma
     20 readability-redundant-member-init
     40 misc-misplaced-const
     75 performance-type-promotion-in-math-fn
     88 misc-incorrect-roundings
    105 misc-macro-parentheses
    151 readability-function-size
    201 readability-else-after-return
    202 readability-inconsistent-declaration-parameter-name
    316 misc-throw-by-value-catch-by-reference
    383 readability-non-const-parameter
  10284 readability-implicit-bool-conversion

Change-Id: I5b35ce33e723349fa583f527fec55bbf29a57508

18 files changed:
src/gromacs/CMakeLists.txt
src/gromacs/awh/biasstate.cpp
src/gromacs/awh/grid.cpp
src/gromacs/commandline/cmdlinehelpmodule.cpp
src/gromacs/domdec/domdec.cpp
src/gromacs/gmxana/gmx_awh.cpp
src/gromacs/gmxana/gmx_clustsize.cpp
src/gromacs/gmxana/gmx_nmeig.cpp
src/gromacs/gmxlib/nonbonded/nonbonded.cpp
src/gromacs/gmxpreprocess/vsite_parm.cpp
src/gromacs/hardware/printhardware.cpp
src/gromacs/linearalgebra/CMakeLists.txt
src/gromacs/mdlib/lincs.cpp
src/gromacs/options/optionsvisitor.cpp
src/gromacs/selection/CMakeLists.txt
src/gromacs/selection/nbsearch.cpp
src/gromacs/selection/scanner_internal.cpp
src/gromacs/utility/stringstream.cpp

index 3c5fa7d627b2f7d2317070f183034e32e3180ccf..7685806035618853b92771a89b3bb0c810fbfac6 100644 (file)
@@ -44,8 +44,10 @@ set_property(GLOBAL PROPERTY GMX_INSTALLED_HEADERS)
 set_property(GLOBAL PROPERTY GMX_AVX_512_SOURCE)
 
 add_library(libgromacs_external OBJECT "")
+add_library(libgromacs_generated OBJECT "")
 if (BUILD_SHARED_LIBS)
     set_target_properties(libgromacs_external PROPERTIES POSITION_INDEPENDENT_CODE true)
+    set_target_properties(libgromacs_generated PROPERTIES POSITION_INDEPENDENT_CODE true)
 endif()
 
 function (_gmx_add_files_to_property PROPERTY)
@@ -165,7 +167,8 @@ gmx_configure_version_file(
     utility/baseversion-gen.c.cmakein ${GENERATED_VERSION_FILE}
     REMOTE_HASH)
 list(APPEND LIBGROMACS_SOURCES ${GENERATED_VERSION_FILE}
-     $<TARGET_OBJECTS:libgromacs_external>)
+     $<TARGET_OBJECTS:libgromacs_external>
+     $<TARGET_OBJECTS:libgromacs_generated>)
 
 # Mark some shared GPU implementation files to compile with CUDA if needed
 if (GMX_USE_CUDA)
@@ -273,7 +276,18 @@ if (GMX_CLANG_TIDY)
    if(CMAKE_VERSION VERSION_LESS "3.6")
        message(ERROR "clang-tidy support requires cmake 3.6.")
    endif()
-   set_target_properties(libgromacs PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_EXE};--checks=-clang-analyzer-security.insecureAPI.strcpy,-clang-analyzer-core.NullDereference,bugprone-*,-bugprone-integer-division;-warnings-as-errors=*")
+   set_target_properties(libgromacs PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_EXE};--checks=\
+       -clang-analyzer-security.insecureAPI.strcpy,-clang-analyzer-core.NullDereference,\
+       bugprone-*,misc-*,readability-*,performance-*,mpi-*,-bugprone-integer-division,\
+       -readability-redundant-smartptr-get,-readability-redundant-string-cstr,-readability-simplify-boolean-expr,\
+       -misc-misplaced-widening-cast,-readability-named-parameter,-performance-noexcept-move-constructor,\
+       -readability-misleading-indentation,-readability-container-size-empty,-misc-suspicious-string-compare,\
+       -readability-redundant-control-flow,-performance-unnecessary-value-param,\
+       -readability-static-definition-in-anonymous-namespace,-misc-suspicious-missing-comma,\
+       -readability-redundant-member-init,-misc-misplaced-const,-performance-type-promotion-in-math-fn,\
+       -misc-incorrect-roundings,-misc-macro-parentheses,-readability-function-size,-readability-else-after-return,\
+       -readability-inconsistent-declaration-parameter-name,-misc-throw-by-value-catch-by-reference,\
+       -readability-non-const-parameter,-readability-implicit-bool-conversion;-warnings-as-errors=*")
 endif()
 
 gmx_write_installed_header_list()
index 650662c52a5ee1f80bc9568776ddbdf3a5b8d6b2..b689e72dd1c6293795635ef81d439162fa161ee8 100644 (file)
@@ -1459,7 +1459,7 @@ static void readUserPmfAndTargetDistribution(const std::vector<DimParams> &dimPa
     std::string filenameModified(filename);
     if (numBias > 1)
     {
-        size_t n = filenameModified.rfind(".");
+        size_t n = filenameModified.rfind('.');
         GMX_RELEASE_ASSERT(n != std::string::npos, "The filename should contain an extension starting with .");
         filenameModified.insert(n, formatString("%d", biasIndex));
     }
index 9285f6c3402a987e7436857c3859d610b2bf1934..44eef27c966cac32d51d2699b8e71ab17ef2b15f 100644 (file)
@@ -768,7 +768,6 @@ void mapGridToDataGrid(std::vector<int>    *gridpointToDatapoint,
 {
     /* Transform the data into a grid in order to map each grid point to a data point
        using the grid functions. */
-    std::vector<GridAxis> axis_;
 
     /* Count the number of points for each dimension. Each dimension
        has its own stride. */
@@ -805,6 +804,8 @@ void mapGridToDataGrid(std::vector<int>    *gridpointToDatapoint,
         GMX_THROW(InvalidInputError(mesg));
     }
 
+    std::vector<GridAxis> axis_;
+    axis_.reserve(grid.numDimensions());
     /* The data grid has the data that was read and the properties of the AWH grid */
     for (int d = 0; d < grid.numDimensions(); d++)
     {
index ccbde0f2535bf865df78481cc67c9bd7b840064f..da8043a3ca27e19187ef7589d52ba40b6837a980 100644 (file)
@@ -588,7 +588,7 @@ void HelpExportReStructuredText::exportModuleHelp(
         = outputRedirector_->openTextOutputFile("onlinehelp/" + tag + ".rst");
     TextWriter              writer(file);
     writer.writeLine(formatString(".. _%s:", displayName.c_str()));
-    if (0 == displayName.compare(binaryName_ + " mdrun"))
+    if (displayName == binaryName_ + " mdrun")
     {
         // Make an extra link target for the convenience of
         // MPI-specific documentation
index 7d8df77ad30fdd262b35ffee95979b9b7ae3a2f1..b80b0a522763c4a2e13910585dd416cec08ed389 100644 (file)
@@ -1428,11 +1428,15 @@ NumPmeDomains getNumPmeDomains(const gmx_domdec_t *dd)
 {
     if (dd != nullptr)
     {
-        return { dd->comm->npmenodes_x, dd->comm->npmenodes_y };
+        return {
+                   dd->comm->npmenodes_x, dd->comm->npmenodes_y
+        };
     }
     else
     {
-        return { 1, 1 };
+        return {
+                   1, 1
+        };
     }
 }
 
@@ -2384,7 +2388,7 @@ static void print_dd_load_av(FILE *fplog, gmx_domdec_t *dd)
         lossFraction    = dd_force_imb_perf_loss(dd);
 
         std::string msg         = "\n Dynamic load balancing report:\n";
-        std::string dlbStateStr = "";
+        std::string dlbStateStr;
 
         switch (dd->comm->dlbState)
         {
index 36f3aa3a13b969c0ab0a33d4076758b667aedb99..60138e6109e00f4344ec783facfdcef574afa800 100644 (file)
@@ -109,8 +109,8 @@ class OutputFile
          * \param[in] numBias    The total number of AWH biases in the system.
          * \param[in] biasIndex  The index of this bias.
          */
-        OutputFile(const std::string  filename,
-                   const std::string  baseTitle,
+        OutputFile(std::string        filename,
+                   std::string        baseTitle,
                    int                numBias,
                    int                biasIndex);
 
index 3a81b9b7362b7c5cc2b9e672ea9e3c17666aafe8..5cfacb743e556563aa67d941963558b239279ab9 100644 (file)
@@ -442,10 +442,7 @@ static void clust_size(const char *ndx, const char *trx, const char *xpm,
                "Size", n_x, max_size, t_x, t_y, cs_dist, 0, cmid, cmax,
                rlo, rmid, rhi, &nlevels);
     gmx_ffclose(fp);
-    if (mtop)
-    {
-        delete mtop;
-    }
+    delete mtop;
     sfree(t_x);
     sfree(t_y);
     for (i = 0; (i < n_x); i++)
index a0ba7e9d925328358800c143fb1f536ae2afba0c..bd5f4ca40275b8818463d373b2a5b3751b044d7a 100644 (file)
@@ -341,11 +341,7 @@ static void analyzeThermochemistry(FILE                      *fp,
                                    real                       scale_factor,
                                    real                       linear_toler)
 {
-    std::vector<int> index;
-    for (auto &ai : atom_index)
-    {
-        index.push_back(static_cast<int>(ai));
-    }
+    std::vector<int>       index(atom_index.begin(), atom_index.end());
 
     rvec                   xcm;
     double                 tmass = calc_xcm(top_x, index.size(),
index 68f5f9a723e85e38be0103796093a2ea3a050097..417c8c4c9cd4ea14a4f909d160bbef2e6bef0a37 100644 (file)
@@ -380,7 +380,7 @@ void do_nonbonded(const t_forcerec  *fr,
          * someone assumes that extending the group-scheme table-type
          * enumeration is something that GROMACS supports. */
         /* cppcheck-suppress duplicateExpression */
-        assert(etiNR == 3);
+        static_assert(etiNR == 3, "");
 
         kernel_data.table_elec              = nblists->table_elec;
         kernel_data.table_vdw               = nblists->table_vdw;
index 1b9ea7770a401f6383b43b0ab989c5b179f3ff11..f10501ab0634b005f71d32ee355026d5720ff9a7 100644 (file)
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 1991-2000, University of Groningen, The Netherlands.
  * Copyright (c) 2001-2004, The GROMACS development team.
- * Copyright (c) 2013,2014,2015,2017, by the GROMACS development team, led by
+ * Copyright (c) 2013,2014,2015,2017,2018, 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.
@@ -1083,7 +1083,6 @@ static void clean_vsite_bonds(t_params *plist, t_pindex pindex[],
                     }
                     oatom = ps->param[i].a[1-k]; /* the other atom */
                     if (vsite_type[oatom] == NOTSET &&
-                        vsite_type[oatom] != F_VSITEN &&
                         oatom == plist[pindex[atom].ftype].param[pindex[atom].parnr].aj())
                     {
                         /* if the other atom isn't a vsite, and it is AI */
@@ -1181,7 +1180,7 @@ static void clean_vsite_bonds(t_params *plist, t_pindex pindex[],
             for (k = 0; (k < 2) && !bKeep; k++) /* for all atoms in the bond */
             {
                 atom = ps->param[i].a[k];
-                if (vsite_type[atom] == NOTSET && vsite_type[atom] != F_VSITEN)
+                if (vsite_type[atom] == NOTSET)
                 {
                     bUsed = FALSE;
                     for (m = 0; (m < vsnral) && !bUsed; m++)
@@ -1372,7 +1371,7 @@ static void clean_vsite_angles(t_params *plist, t_pindex pindex[],
         for (k = 0; (k < 3) && !bKeep; k++) /* for all atoms in the angle */
         {
             atom = ps->param[i].a[k];
-            if (vsite_type[atom] == NOTSET && vsite_type[atom] != F_VSITEN)
+            if (vsite_type[atom] == NOTSET)
             {
                 bUsed = FALSE;
                 for (m = 0; (m < vsnral) && !bUsed; m++)
@@ -1517,7 +1516,7 @@ static void clean_vsite_dihs(t_params *plist, t_pindex pindex[],
             GMX_ASSERT(vsnral != 0, "If we've seen a vsite before, we know how many constructing atoms it had");
             GMX_ASSERT(first_atoms != NULL, "If we've seen a vsite before, we know what its first atom index was");
             atom = ps->param[i].a[k];
-            if (vsite_type[atom] == NOTSET && vsite_type[atom] != F_VSITEN)
+            if (vsite_type[atom] == NOTSET)
             {
                 /* vsnral will be set here, we don't get here with nvsite==0 */
                 bUsed = FALSE;
index f6f1eb6c69b60a9525b765f93b9ed594fe98a060..c8662b5d2face584addf58da6d159bbe8a3a0020 100644 (file)
@@ -223,7 +223,7 @@ static std::string detected_hardware_string(const gmx_hw_info_t *hwinfo,
         s += gmx::formatString("    Features:");
         for (auto &f : cpuInfo.featureSet())
         {
-            s += gmx::formatString(" %s", cpuInfo.featureString(f).c_str());;
+            s += gmx::formatString(" %s", gmx::CpuInfo::featureString(f).c_str());
         }
         s += gmx::formatString("\n");
     }
index 2afb058033fa3e4aedc34a95d619d7c15a501379..3bd2fd1570e594199d33cf24c89b4ce1802318a4 100644 (file)
@@ -1,7 +1,7 @@
 #
 # This file is part of the GROMACS molecular simulation package.
 #
-# Copyright (c) 2012,2013,2014,2015, by the GROMACS development team, led by
+# Copyright (c) 2012,2013,2014,2015,2018, 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.
@@ -45,8 +45,7 @@ endif()
 set(LINEARALGEBRA_SOURCES
     ${LINEARALGEBRA_SOURCES} ${BLAS_SOURCES} ${LAPACK_SOURCES})
 
-set(LIBGROMACS_SOURCES
-    ${LIBGROMACS_SOURCES} ${LINEARALGEBRA_SOURCES} PARENT_SCOPE)
+target_sources(libgromacs_external PRIVATE ${LINEARALGEBRA_SOURCES})
 
 gmx_install_headers(
     eigensolver.h
index 5772b2514009dc091724190e433e6c36145c7982..7fcd40c418fbc26873335a809f26f6ed8b2e2598 100644 (file)
@@ -1608,10 +1608,7 @@ Lincs *init_lincs(FILE *fplog, const gmx_mtop_t &mtop,
 
 void done_lincs(Lincs *li)
 {
-    if (li)
-    {
-        delete li;
-    }
+    delete li;
 }
 
 /*! \brief Sets up the work division over the threads. */
index 07397c888638019dde71300a9ae36326c9706dbb..a665803f9f6961820bdec463433c5f3f35260e25 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2010,2012,2014,2015,2016, by the GROMACS development team, led by
+ * Copyright (c) 2010,2012,2014,2015,2016,2018, 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.
@@ -52,8 +52,6 @@
 namespace gmx
 {
 
-using internal::OptionsImpl;
-
 namespace
 {
 
index afc18431bb3ee982359aa9e1d520fcbe1c6b557a..d902a5b91e0102f0d77af78b839c3c2867b3d1ce 100644 (file)
@@ -1,7 +1,7 @@
 #
 # This file is part of the GROMACS molecular simulation package.
 #
-# Copyright (c) 2010,2012,2013,2014,2015, by the GROMACS development team, led by
+# Copyright (c) 2010,2012,2013,2014,2015,2018, 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.
 # the research papers on the package. Check out http://www.gromacs.org.
 
 file(GLOB SELECTION_SOURCES *.cpp)
+set(LIBGROMACS_SOURCES_GENERATED
+    "${CMAKE_CURRENT_LIST_DIR}/scanner.cpp"
+    "${CMAKE_CURRENT_LIST_DIR}/parser.cpp")
+list(REMOVE_ITEM SELECTION_SOURCES ${LIBGROMACS_SOURCES_GENERATED})
 set(LIBGROMACS_SOURCES ${LIBGROMACS_SOURCES} ${SELECTION_SOURCES} PARENT_SCOPE)
+target_sources(libgromacs_generated PRIVATE ${LIBGROMACS_SOURCES_GENERATED})
 
 gmx_install_headers(
     nbsearch.h
index 55a5010ce4071c1e0f783a1f89e1e27a67c707e2..4e3defa990014f72a7071ef1c1226abc6979e7cc 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2009,2010,2011,2012,2013,2014,2015,2016,2017, by the GROMACS development team, led by
+ * Copyright (c) 2009,2010,2011,2012,2013,2014,2015,2016,2017,2018, 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.
@@ -244,7 +244,7 @@ class AnalysisNeighborhoodSearchImpl
          * is over a periodic boundary from `centerCell`, triclinic shifts
          * should have been applied to `centerCell` X/Y components.
          */
-        real computeCutoffExtent(const RVec centerCell, const ivec cell, int dim) const;
+        real computeCutoffExtent(RVec centerCell, const ivec cell, int dim) const;
         /*! \brief
          * Advances cell pair loop to the next cell.
          *
index 5ff51acdf41dbeab4e08c7455c37571f71646553..5dbeef8b419d283ebb8e5360ae21ace027ac6173 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2009,2010,2011,2012,2014,2015,2016,2017, by the GROMACS development team, led by
+ * Copyright (c) 2009,2010,2011,2012,2014,2015,2016,2017,2018, 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.
@@ -301,7 +301,7 @@ _gmx_sel_lexer_process_identifier(YYSTYPE *yylval, YYLTYPE *yylloc,
     /* For variable symbols, return the type of the variable value */
     if (symtype == gmx::SelectionParserSymbol::VariableSymbol)
     {
-        gmx::SelectionTreeElementPointer var = symbol->variableValue();
+        const gmx::SelectionTreeElementPointer &var = symbol->variableValue();
         /* Return simple tokens for constant variables */
         if (var->type == SEL_CONST)
         {
index ff127d5976ba497c90453f2aff60fa5425d74057..51518b4166a094766cc640cf65b9ca647ae74fcd 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2015,2017, by the GROMACS development team, led by
+ * Copyright (c) 2015,2017,2018, 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.
@@ -86,7 +86,7 @@ bool StringInputStream::readLine(std::string *line)
     }
     else
     {
-        size_t newpos = input_.find("\n", pos_);
+        size_t newpos = input_.find('\n', pos_);
         if (newpos == std::string::npos)
         {
             newpos = input_.size();