From 8ce14652da643adc88ca4a9be94650565a62f49f Mon Sep 17 00:00:00 2001 From: Joe Jordan Date: Tue, 20 Apr 2021 09:18:07 +0000 Subject: [PATCH] clang-tidy-11 fixes for utils --- src/gromacs/utility/cstringutil.cpp | 2 +- src/gromacs/utility/enumerationhelpers.h | 2 +- src/gromacs/utility/fatalerror.cpp | 11 ++- src/gromacs/utility/futil.cpp | 12 ++- src/gromacs/utility/keyvaluetree.cpp | 1 - .../utility/keyvaluetreeserializer.cpp | 15 ++- src/gromacs/utility/keyvaluetreetransform.cpp | 2 +- src/gromacs/utility/programcontext.cpp | 3 +- src/gromacs/utility/smalloc.cpp | 4 +- src/gromacs/utility/tests/.clang-tidy | 91 +++++++++++++++++++ 10 files changed, 123 insertions(+), 20 deletions(-) create mode 100644 src/gromacs/utility/tests/.clang-tidy diff --git a/src/gromacs/utility/cstringutil.cpp b/src/gromacs/utility/cstringutil.cpp index 161fb7396e..dbefa55d97 100644 --- a/src/gromacs/utility/cstringutil.cpp +++ b/src/gromacs/utility/cstringutil.cpp @@ -298,7 +298,7 @@ const unsigned int gmx_string_hash_init = 5381; unsigned int gmx_string_fullhash_func(const char* s, unsigned int hash_init) { - int c = 0; + char c = 0; while ((c = (*s++)) != '\0') { diff --git a/src/gromacs/utility/enumerationhelpers.h b/src/gromacs/utility/enumerationhelpers.h index e2352b2421..b30c8fee7a 100644 --- a/src/gromacs/utility/enumerationhelpers.h +++ b/src/gromacs/utility/enumerationhelpers.h @@ -200,7 +200,7 @@ public: * \tparam ArraySize Size in entries of the array. */ template -struct EnumerationArray final +struct EnumerationArray final // NOLINT { //! Convenience alias using EnumerationWrapperType = EnumerationWrapper; diff --git a/src/gromacs/utility/fatalerror.cpp b/src/gromacs/utility/fatalerror.cpp index a9d416459e..a8eb96e755 100644 --- a/src/gromacs/utility/fatalerror.cpp +++ b/src/gromacs/utility/fatalerror.cpp @@ -65,13 +65,13 @@ #include "errorformat.h" -static bool bDebug = false; +static bool bDebug = false; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) -FILE* debug = nullptr; -gmx_bool gmx_debug_at = FALSE; +FILE* debug = nullptr; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) +bool gmx_debug_at = false; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) -static FILE* log_file = nullptr; -static std::mutex error_mutex; +static FILE* log_file = nullptr; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) +static std::mutex error_mutex; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) using Lock = std::lock_guard; @@ -112,6 +112,7 @@ static void default_error_handler(const char* title, const std::string& msg, con gmx::internal::printFatalErrorFooter(stderr); } +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) static gmx_error_handler_t gmx_error_handler = default_error_handler; void gmx_set_error_handler(gmx_error_handler_t func) diff --git a/src/gromacs/utility/futil.cpp b/src/gromacs/utility/futil.cpp index 95a0eaa04c..8a54e0d864 100644 --- a/src/gromacs/utility/futil.cpp +++ b/src/gromacs/utility/futil.cpp @@ -82,12 +82,16 @@ typedef struct t_pstack struct t_pstack* prev; } t_pstack; -static t_pstack* pstack = nullptr; -static bool bUnbuffered = false; -static int s_maxBackupCount = 0; +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +static t_pstack* pstack = nullptr; +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +static bool bUnbuffered = false; +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +static int s_maxBackupCount = 0; /* this linked list is an intrinsically globally shared object, so we have to protect it with mutexes */ +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) static std::mutex pstack_mutex; using Lock = std::lock_guard; @@ -97,7 +101,7 @@ namespace gmx namespace { //! Global library file finder; stores the object set with setLibraryFileFinder(). -const DataFileFinder* g_libFileFinder; +const DataFileFinder* g_libFileFinder; //NOLINT(cppcoreguidelines-avoid-non-const-global-variables) //! Default library file finder if nothing is set. const DataFileFinder g_defaultLibFileFinder; } // namespace diff --git a/src/gromacs/utility/keyvaluetree.cpp b/src/gromacs/utility/keyvaluetree.cpp index 774af0a366..028939954f 100644 --- a/src/gromacs/utility/keyvaluetree.cpp +++ b/src/gromacs/utility/keyvaluetree.cpp @@ -174,7 +174,6 @@ public: abstol_(abstol) { } - void compareObjects(const KeyValueTreeObject& obj1, const KeyValueTreeObject& obj2) { for (const auto& prop1 : obj1.properties()) diff --git a/src/gromacs/utility/keyvaluetreeserializer.cpp b/src/gromacs/utility/keyvaluetreeserializer.cpp index c111a78998..3395bb2371 100644 --- a/src/gromacs/utility/keyvaluetreeserializer.cpp +++ b/src/gromacs/utility/keyvaluetreeserializer.cpp @@ -68,14 +68,19 @@ private: SerializerFunction serialize; DeserializerFunction deserialize; }; - - static std::mutex s_initMutex; - static std::map s_serializers; + // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) + static std::mutex s_initMutex; + // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) + static std::map s_serializers; + // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) static std::map s_deserializers; }; -std::mutex ValueSerializer::s_initMutex; -std::map ValueSerializer::s_serializers; +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +std::mutex ValueSerializer::s_initMutex; +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +std::map ValueSerializer::s_serializers; +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) std::map ValueSerializer::s_deserializers; template diff --git a/src/gromacs/utility/keyvaluetreetransform.cpp b/src/gromacs/utility/keyvaluetreetransform.cpp index 45ac7b8723..6c72642c69 100644 --- a/src/gromacs/utility/keyvaluetreetransform.cpp +++ b/src/gromacs/utility/keyvaluetreetransform.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2016,2017,2018,2019,2020, by the GROMACS development team, led by + * Copyright (c) 2016,2017,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. diff --git a/src/gromacs/utility/programcontext.cpp b/src/gromacs/utility/programcontext.cpp index acc70d8d15..020c9c9335 100644 --- a/src/gromacs/utility/programcontext.cpp +++ b/src/gromacs/utility/programcontext.cpp @@ -2,7 +2,7 @@ * This file is part of the GROMACS molecular simulation package. * * 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. @@ -79,6 +79,7 @@ public: }; //! Global program info; stores the object set with setProgramContext(). +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) const IProgramContext* g_programContext; //! Default program context if nothing is set. const DefaultProgramContext g_defaultContext; diff --git a/src/gromacs/utility/smalloc.cpp b/src/gromacs/utility/smalloc.cpp index 190fb2a7f1..ff51280306 100644 --- a/src/gromacs/utility/smalloc.cpp +++ b/src/gromacs/utility/smalloc.cpp @@ -56,7 +56,9 @@ # include "gromacs/utility/gmxmpi.h" #endif -static bool g_bOverAllocDD = false; +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +static bool g_bOverAllocDD = false; +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) static std::mutex g_overAllocMutex; void* save_malloc(const char* name, const char* file, int line, size_t size) diff --git a/src/gromacs/utility/tests/.clang-tidy b/src/gromacs/utility/tests/.clang-tidy new file mode 100644 index 0000000000..0adf51e3ee --- /dev/null +++ b/src/gromacs/utility/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 -- 2.22.0