From f68840d4a84928102000705ed55d776e9a093552 Mon Sep 17 00:00:00 2001 From: ejjordan Date: Mon, 19 Apr 2021 20:17:38 +0200 Subject: [PATCH] clang-tidy-11 fixes for analysisdata --- .../analysisdata/datamodulemanager.cpp | 6 +- src/gromacs/analysisdata/datastorage.cpp | 2 +- .../analysisdata/modules/displacement.cpp | 6 +- .../analysisdata/modules/histogram.cpp | 35 ++----- src/gromacs/analysisdata/tests/.clang-tidy | 91 +++++++++++++++++++ 5 files changed, 104 insertions(+), 36 deletions(-) create mode 100644 src/gromacs/analysisdata/tests/.clang-tidy diff --git a/src/gromacs/analysisdata/datamodulemanager.cpp b/src/gromacs/analysisdata/datamodulemanager.cpp index bd51891b02..c5e982f579 100644 --- a/src/gromacs/analysisdata/datamodulemanager.cpp +++ b/src/gromacs/analysisdata/datamodulemanager.cpp @@ -2,7 +2,7 @@ * This file is part of the GROMACS molecular simulation package. * * Copyright (c) 2010-2018, 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. @@ -134,7 +134,7 @@ public: * calls the notification functions in \p module as if the module had * been registered to the data object when the data was added. */ - void presentData(AbstractAnalysisData* data, IAnalysisDataModule* module); + void presentData(AbstractAnalysisData* data, IAnalysisDataModule* module) const; //! List of modules added to the data. ModuleList modules_; @@ -216,7 +216,7 @@ void AnalysisDataModuleManager::Impl::checkModuleProperties(const IAnalysisDataM } } -void AnalysisDataModuleManager::Impl::presentData(AbstractAnalysisData* data, IAnalysisDataModule* module) +void AnalysisDataModuleManager::Impl::presentData(AbstractAnalysisData* data, IAnalysisDataModule* module) const { if (state_ == eNotStarted) { diff --git a/src/gromacs/analysisdata/datastorage.cpp b/src/gromacs/analysisdata/datastorage.cpp index 7f80c082a8..2e701f2ecf 100644 --- a/src/gromacs/analysisdata/datastorage.cpp +++ b/src/gromacs/analysisdata/datastorage.cpp @@ -810,7 +810,7 @@ void AnalysisDataStorage::startParallelDataStorage(AbstractAnalysisData* AnalysisDataStorageFrame& AnalysisDataStorage::startFrame(const AnalysisDataFrameHeader& header) { GMX_ASSERT(header.isValid(), "Invalid header"); - internal::AnalysisDataStorageFrameData* storedFrame; + internal::AnalysisDataStorageFrameData* storedFrame = nullptr; if (impl_->storeAll()) { size_t size = header.index() + 1; diff --git a/src/gromacs/analysisdata/modules/displacement.cpp b/src/gromacs/analysisdata/modules/displacement.cpp index 1468268f66..6e5ffb9b54 100644 --- a/src/gromacs/analysisdata/modules/displacement.cpp +++ b/src/gromacs/analysisdata/modules/displacement.cpp @@ -2,7 +2,7 @@ * This file is part of the GROMACS molecular simulation package. * * Copyright (c) 2010-2018, The GROMACS development team. - * 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. @@ -252,8 +252,6 @@ void AnalysisDataDisplacementModule::frameFinished(const AnalysisDataFrameHeader return; } - int step, i; - if (_impl->nstored == 2) { if (_impl->histm) @@ -266,7 +264,7 @@ void AnalysisDataDisplacementModule::frameFinished(const AnalysisDataFrameHeader AnalysisDataFrameHeader header(_impl->nstored - 2, _impl->t, 0); moduleManager().notifyFrameStart(header); - for (i = _impl->ci - _impl->nmax, step = 1; step < _impl->nstored && i != _impl->ci; + for (int i = _impl->ci - _impl->nmax, step = 1; step < _impl->nstored && i != _impl->ci; i -= _impl->nmax, ++step) { if (i < 0) diff --git a/src/gromacs/analysisdata/modules/histogram.cpp b/src/gromacs/analysisdata/modules/histogram.cpp index 72fb6e5e75..59d5ac6ac7 100644 --- a/src/gromacs/analysisdata/modules/histogram.cpp +++ b/src/gromacs/analysisdata/modules/histogram.cpp @@ -2,7 +2,7 @@ * This file is part of the GROMACS molecular simulation package. * * Copyright (c) 2010,2011,2012,2013,2014 by the GROMACS development team. - * Copyright (c) 2015,2018,2019,2020, by the GROMACS development team, led by + * Copyright (c) 2015,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. @@ -267,43 +267,22 @@ void AbstractAverageHistogram::init(const AnalysisHistogramSettings& settings) AverageHistogramPointer AbstractAverageHistogram::resampleDoubleBinWidth(bool bIntegerBins) const { - int nbins; - if (bIntegerBins) - { - nbins = (rowCount() + 1) / 2; - } - else - { - nbins = rowCount() / 2; - } + const int nbins = bIntegerBins ? (rowCount() + 1) / 2 : rowCount() / 2; AverageHistogramPointer dest(new StaticAverageHistogram( histogramFromBins(settings().firstEdge(), nbins, 2 * xstep()).integerBins(bIntegerBins))); dest->setColumnCount(columnCount()); dest->allocateValues(); - int i, j; - for (i = j = 0; i < nbins; ++i) + for (int i = 0, j = 0; i < nbins; ++i) { const bool bFirstHalfBin = (bIntegerBins && i == 0); for (int c = 0; c < columnCount(); ++c) { - real v1, v2; - real e1, e2; - if (bFirstHalfBin) - { - v1 = value(0, c).value(); - e1 = value(0, c).error(); - v2 = 0; - e2 = 0; - } - else - { - v1 = value(j, c).value(); - e1 = value(j, c).error(); - v2 = value(j + 1, c).value(); - e2 = value(j + 1, c).error(); - } + const real v1 = bFirstHalfBin ? value(0, c).value() : value(j, c).value(); + const real v2 = bFirstHalfBin ? 0 : value(j + 1, c).value(); + const real e1 = bFirstHalfBin ? value(0, c).error() : value(j, c).error(); + const real e2 = bFirstHalfBin ? 0 : value(j + 1, c).error(); dest->value(i, c).setValue(v1 + v2, std::sqrt(e1 * e1 + e2 * e2)); } if (bFirstHalfBin) diff --git a/src/gromacs/analysisdata/tests/.clang-tidy b/src/gromacs/analysisdata/tests/.clang-tidy new file mode 100644 index 0000000000..0adf51e3ee --- /dev/null +++ b/src/gromacs/analysisdata/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