clang-tidy-11 fixes for topology, gpu_utils, and selection

diff --git a/src/gromacs/gpu_utils/device_context.h b/src/gromacs/gpu_utils/device_context.h
index d9f61507284752859c8b2fec5f652bb717b9d6be..ee201f4771326f1a486615966c5d31facecc3c17 100644 (file)
--- a/src/gromacs/gpu_utils/device_context.h
+++ b/src/gromacs/gpu_utils/device_context.h
@@ -72,6 +72,7 @@ public:
     //! Constructor.
     DeviceContext(const DeviceInformation& deviceInfo);
     //! Destructor
+    // NOLINTNEXTLINE(performance-trivially-destructible)
     ~DeviceContext();
 
     //! Get the associated device information

diff --git a/src/gromacs/gpu_utils/device_stream.h b/src/gromacs/gpu_utils/device_stream.h
index c93ffdce5f88ce767a36be5006517c8fdbd6525c..e9794e087ebcc4350ba84a26660eaaac0edb048a 100644 (file)
--- a/src/gromacs/gpu_utils/device_stream.h
+++ b/src/gromacs/gpu_utils/device_stream.h
@@ -101,6 +101,7 @@ public:
     DeviceStream(const DeviceContext& deviceContext, DeviceStreamPriority priority, bool useTiming);
 
     //! Destructor
+    // NOLINTNEXTLINE(performance-trivially-destructible)
     ~DeviceStream();
 
     /*! \brief Check if the underlying stream is valid.

diff --git a/src/gromacs/gpu_utils/tests/.clang-tidy b/src/gromacs/gpu_utils/tests/.clang-tidy
new file mode 100644 (file)
index 0000000..0adf51e
--- /dev/null
+++ b/src/gromacs/gpu_utils/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/selection/.clang-tidy b/src/gromacs/selection/.clang-tidy
new file mode 100644 (file)
index 0000000..beb02dc
--- /dev/null
+++ b/src/gromacs/selection/.clang-tidy
@@ -0,0 +1,90 @@
+# 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
+#
+#         -bugprone-reserved-identifier
+# Quite a large number of functions use the _FName style for naming
+#
+#         -cppcoreguidelines-avoid-non-const-global-variables
+# The selection code has a large amount of static or exported non-const
+# variables that are out of scope for quick rework
+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, -bugprone-reserved-identifier,
+         -cppcoreguidelines-avoid-non-const-global-variables
+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/selection/params.cpp b/src/gromacs/selection/params.cpp
index a5d23a9c480f670b13cba002f05edfe06998ac0b..c6b42e81b5d0a8991a34ece1ed02776ee1568109 100644 (file)
--- a/src/gromacs/selection/params.cpp
+++ b/src/gromacs/selection/params.cpp
@@ -328,7 +328,7 @@ static void parse_values_range(const SelectionParserValueList& values, gmx_ana_s
     /* Sort the ranges and merge consequent ones */
     if (param->val.type == INT_VALUE)
     {
-        const auto range_data = reinterpret_cast<std::array<int, 2>*>(idata);
+        auto* range_data = reinterpret_cast<std::array<int, 2>*>(idata);
         sort(range_data, range_data + n, cmp_range<int>);
         for (i = j = 2; i < 2 * n; i += 2)
         {
@@ -349,7 +349,7 @@ static void parse_values_range(const SelectionParserValueList& values, gmx_ana_s
     }
     else
     {
-        const auto range_data = reinterpret_cast<std::array<real, 2>*>(rdata);
+        auto* range_data = reinterpret_cast<std::array<real, 2>*>(rdata);
         sort(range_data, range_data + n, cmp_range<real>);
         for (i = j = 2; i < 2 * n; i += 2)
         {

diff --git a/src/gromacs/selection/selectionoptionbehavior.cpp b/src/gromacs/selection/selectionoptionbehavior.cpp
index 962355c9d0d89239fbf2f3e8d1678487ae3b4ba2..3da677e2b12ae2488989730bd18f7091a66be3f8 100644 (file)
--- a/src/gromacs/selection/selectionoptionbehavior.cpp
+++ b/src/gromacs/selection/selectionoptionbehavior.cpp
@@ -148,7 +148,7 @@ public:
         getMassesIfRequired(top);
     }
 
-    void getMassesIfRequired(gmx_mtop_t* top)
+    void getMassesIfRequired(gmx_mtop_t* top) const
     {
         const bool massRequired = selections_.requiredTopologyProperties().needsMasses;
         if (!massRequired)

diff --git a/src/gromacs/topology/mtop_util.cpp b/src/gromacs/topology/mtop_util.cpp
index 42eb320ddd3a4150c68cfbde2c6770b11b9fd28a..900dcc7e5845224a7259a9e1e6576765b5fa87b1 100644 (file)
--- a/src/gromacs/topology/mtop_util.cpp
+++ b/src/gromacs/topology/mtop_util.cpp
@@ -53,6 +53,7 @@
 #include "gromacs/topology/topology.h"
 #include "gromacs/topology/topsort.h"
 #include "gromacs/utility/arrayref.h"
+#include "gromacs/utility/enumerationhelpers.h"
 #include "gromacs/utility/fatalerror.h"
 #include "gromacs/utility/real.h"
 #include "gromacs/utility/smalloc.h"

diff --git a/src/gromacs/topology/tests/.clang-tidy b/src/gromacs/topology/tests/.clang-tidy
new file mode 100644 (file)
index 0000000..0adf51e
--- /dev/null
+++ b/src/gromacs/topology/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/topology/topology.cpp b/src/gromacs/topology/topology.cpp
index fc859513fd4e43705efa4048ad5ebdc4368eab10..791bd8bc7373e7e3d98e700d90cd4ef2b4d65930 100644 (file)
--- a/src/gromacs/topology/topology.cpp
+++ b/src/gromacs/topology/topology.cpp
@@ -37,6 +37,7 @@
  */
 #include "gmxpre.h"
 
+#include "gromacs/utility/enumerationhelpers.h"
 #include "topology.h"
 
 #include <cstdio>
@@ -55,22 +56,22 @@
 #include "gromacs/utility/strconvert.h"
 #include "gromacs/utility/txtdump.h"
 
-static gmx::EnumerationArray<SimulationAtomGroupType, const char*> c_simulationAtomGroupTypeShortNames = {
-    { "T-Coupling",
-      "Energy Mon.",
-      "Acc. not used",
-      "Freeze",
-      "User1",
-      "User2",
-      "VCM",
-      "Compressed X",
-      "Or. Res. Fit",
-      "QMMM" }
-};
-
 const char* shortName(SimulationAtomGroupType type)
 {
-    return c_simulationAtomGroupTypeShortNames[type];
+    constexpr gmx::EnumerationArray<SimulationAtomGroupType, const char*> sc_simulationAtomGroupTypeShortNames = {
+        { "T-Coupling",
+          "Energy Mon.",
+          "Acc. not used",
+          "Freeze",
+          "User1",
+          "User2",
+          "VCM",
+          "Compressed X",
+          "Or. Res. Fit",
+          "QMMM" }
+    };
+
+    return sc_simulationAtomGroupTypeShortNames[type];
 }
 
 void init_top(t_topology* top)
@@ -287,7 +288,11 @@ static void pr_grps(FILE* fp, const char* title, gmx::ArrayRef<const AtomGroupIn
     int index = 0;
     for (const auto& group : grps)
     {
-        fprintf(fp, "%s[%-12s] nr=%zu, name=[", title, c_simulationAtomGroupTypeShortNames[index], group.size());
+        fprintf(fp,
+                "%s[%-12s] nr=%zu, name=[",
+                title,
+                shortName(static_cast<SimulationAtomGroupType>(index)),
+                group.size());
         for (const auto& entry : group)
         {
             fprintf(fp, " %s", *(grpname[entry]));
@@ -309,9 +314,9 @@ static void pr_groups(FILE* fp, int indent, const SimulationGroups& groups, gmx_
 
     pr_indent(fp, indent);
     fprintf(fp, "groups          ");
-    for (const auto& group : c_simulationAtomGroupTypeShortNames)
+    for (const auto group : gmx::EnumerationWrapper<SimulationAtomGroupType>{})
     {
-        printf(" %5.5s", group);
+        printf(" %5.5s", shortName(group));
     }
     printf("\n");
 
@@ -744,11 +749,7 @@ void compareAtomGroups(FILE* fp, const SimulationGroups& g0, const SimulationGro
         {
             for (int j = 0; j < natoms0; j++)
             {
-                cmp_int(fp,
-                        c_simulationAtomGroupTypeShortNames[group],
-                        j,
-                        getGroupType(g0, group, j),
-                        getGroupType(g1, group, j));
+                cmp_int(fp, shortName(group), j, getGroupType(g0, group, j), getGroupType(g1, group, j));
             }
         }
     }