From: Roland Schulz Date: Fri, 3 Aug 2018 01:19:40 +0000 (-0700) Subject: Enable more clang-tidy checks for new code X-Git-Url: http://biod.pnpi.spb.ru/gitweb/?a=commitdiff_plain;h=88754544f483f03d0d939da8a973274acd17da62;p=alexxy%2Fgromacs.git Enable more clang-tidy checks for new code Some of the clang-tidy checks would require too many code changes when enabled globally. To get the benefits from the checks without changing all legacy code add a second configuration. Which configuration is used is selected by directory: Any new module should use the new configuration. Additionally it is recommended to use the new configuration on a per file or change basis (clang-tidy-diff.py) manually. This change contains the proposed configuration for new code and and enables it for three folders which contain new code. Checks enabled relative to base configuration: clang-analyzer-security.insecureAPI.strcpy readability-inconsistent-declaration-parameter-name readability-function-size cppcoreguidelines-owning-memory cppcoreguidelines-no-malloc cppcoreguidelines-pro-type-member-init cppcoreguidelines-pro-type-union-access The reasons that the remaining checks are still disabled (don't know a good place to document because json doesn't allow comments): misc-incorrect-roundings: #2562 readability-else-after-return: I don't think it is something we want to follow cppcoreguidelines-pro-type-*cast: While it is best to not do those cast there are valid reasons for them. And because they are already easy to spot, mandating a NOLINT for those is probably not helpful. cppcoreguidelines-special-member-functions: conflicts with Wunused-member cppcoreguidelines-pro-type-vararg: #2570 cppcoreguidelines-pro-bounds-constant-array-index: While I think it would be nice to have a compile mode with bound checking enabled. It is so ugly to have to write at(v, n) instead of v[n]. cppcoreguidelines-pro-bounds-array-to-pointer-decay: This would be particular nice to have But it makes it very hard to write warning free code with legacy APIs which uses e.g. rvec. Those have to be modernized first. cppcoreguidelines-pro-bounds-pointer-arithmetic: This would also be very nice. But it requires that depending APIs use ArrayRef rather than pointers for non-owning array passing. Change-Id: I891a576d2c185ef6587224a1a19324f1a8967237 --- diff --git a/cmake/gmxCFlags.cmake b/cmake/gmxCFlags.cmake index 48dac9c706..752392771a 100644 --- a/cmake/gmxCFlags.cmake +++ b/cmake/gmxCFlags.cmake @@ -315,6 +315,9 @@ GMX_TEST_CFLAG(CFLAGS_WARN "/W3 /wd177 /wd411 /wd593 /wd981 /wd1418 /wd1419 /wd1 GMX_TEST_CXXFLAG(CXXFLAGS_PRAGMA "-Wno-unknown-pragmas" GMXC_CXXFLAGS) endif() if (GMX_COMPILER_WARNINGS) + if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS "6.0") #LLVM BUG #21629 + GMX_TEST_CXXFLAG(CXXFLAGS_WARN_NO_BRACES "-Wno-missing-braces" GMXC_CXXFLAGS) + endif() GMX_TEST_CXXFLAG(CXXFLAGS_WARN "-Wall" GMXC_CXXFLAGS) GMX_TEST_CXXFLAG(CXXFLAGS_WARN_EXTRA "-Wextra -Wno-missing-field-initializers -Wpointer-arith -Wmissing-prototypes" GMXC_CXXFLAGS) GMX_TEST_CXXFLAG(CXXFLAGS_DEPRECATED "-Wdeprecated" GMXC_CXXFLAGS) diff --git a/src/.clang-tidy b/src/.clang-tidy index c74d0c1ca8..15261590fc 100644 --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -10,7 +10,7 @@ Checks: clang-diagnostic-*,clang-analyzer-*,-clang-analyzer-security.insecureAP -cppcoreguidelines-no-malloc,-cppcoreguidelines-special-member-functions, google-*,-google-build-using-namespace,-google-explicit-constructor, -google-readability-function-size,-google-readability-todo,-google-runtime-int - +HeaderFilterRegex: .* CheckOptions: - key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor value: 1 diff --git a/src/.clang-tidy.new.code b/src/.clang-tidy.new.code new file mode 100644 index 0000000000..65d42e9273 --- /dev/null +++ b/src/.clang-tidy.new.code @@ -0,0 +1,32 @@ +Checks: clang-diagnostic-*,clang-analyzer-*, + bugprone-*,misc-*,readability-*,performance-*,mpi-*,-misc-incorrect-roundings, + -readability-else-after-return, + modernize-use-nullptr,modernize-use-emplace, + modernize-make-unique,modernize-make-shared, + modernize-avoid-bind, + modernize-redundant-void-arg,modernize-use-bool-literals, + cppcoreguidelines-*,-cppcoreguidelines-pro-type-reinterpret-cast,-cppcoreguidelines-pro-type-const-cast, + -cppcoreguidelines-pro-type-static-cast-downcast, + -cppcoreguidelines-special-member-functions,-cppcoreguidelines-pro-type-vararg, + -cppcoreguidelines-pro-bounds-constant-array-index, + -cppcoreguidelines-pro-bounds-array-to-pointer-decay, + -cppcoreguidelines-pro-bounds-pointer-arithmetic, + google-*,-google-build-using-namespace,-google-explicit-constructor, + -google-readability-todo, + -google-runtime-int +HeaderFilterRegex: .* +CheckOptions: + - key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor + value: 1 + - key: modernize-make-unique.MakeSmartPtrFunction + value: gmx::compat::make_unique + - key: modernize-make-unique.MakeSmartPtrFunctionHeader + value: gromacs/compat/make_unique.h + - 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 diff --git a/src/external/googletest/googletest/include/gtest/internal/gtest-internal.h b/src/external/googletest/googletest/include/gtest/internal/gtest-internal.h index ebd1cf615d..3f8e2dd9a0 100644 --- a/src/external/googletest/googletest/include/gtest/internal/gtest-internal.h +++ b/src/external/googletest/googletest/include/gtest/internal/gtest-internal.h @@ -1230,9 +1230,8 @@ class GTEST_TEST_CLASS_NAME_(test_case_name, test_name) : public parent_class {\ (parent_id), \ parent_class::SetUpTestCase, \ parent_class::TearDownTestCase, \ - new ::testing::internal::TestFactoryImpl<\ - GTEST_TEST_CLASS_NAME_(test_case_name, test_name)>);\ -void GTEST_TEST_CLASS_NAME_(test_case_name, test_name)::TestBody() + new ::testing::internal::TestFactoryImpl); /*NOLINT(cppcoreguidelines-owning-memory)*/ \ + void GTEST_TEST_CLASS_NAME_(test_case_name, test_name)::TestBody() #endif // GTEST_INCLUDE_GTEST_INTERNAL_GTEST_INTERNAL_H_ diff --git a/src/gromacs/CMakeLists.txt b/src/gromacs/CMakeLists.txt index a1b441d44c..ffe58496ba 100644 --- a/src/gromacs/CMakeLists.txt +++ b/src/gromacs/CMakeLists.txt @@ -120,6 +120,7 @@ add_subdirectory(pulling) add_subdirectory(awh) add_subdirectory(simd) add_subdirectory(imd) +add_subdirectory(compat) if (NOT GMX_BUILD_MDRUN_ONLY) add_subdirectory(gmxana) add_subdirectory(gmxpreprocess) @@ -302,7 +303,7 @@ endif() if (GMX_CLANG_TIDY) set_target_properties(libgromacs PROPERTIES CXX_CLANG_TIDY - "${CLANG_TIDY_EXE};-warnings-as-errors=*;-header-filter=.*") + "${CLANG_TIDY_EXE};-warnings-as-errors=*") endif() gmx_write_installed_header_list() @@ -347,7 +348,3 @@ if(GMX_USE_OPENCL) install(FILES ${OPENCL_KERNELS} DESTINATION ${GMX_INSTALL_OCLDIR} COMPONENT libraries) endif() - -if (BUILD_TESTING) - add_subdirectory(compat/tests) -endif() diff --git a/src/gromacs/analysisdata/.clang-tidy b/src/gromacs/analysisdata/.clang-tidy new file mode 120000 index 0000000000..6669ade155 --- /dev/null +++ b/src/gromacs/analysisdata/.clang-tidy @@ -0,0 +1 @@ +../../.clang-tidy.new.code \ No newline at end of file diff --git a/src/gromacs/analysisdata/datamodulemanager.cpp b/src/gromacs/analysisdata/datamodulemanager.cpp index 173c2a712a..c8e34606da 100644 --- a/src/gromacs/analysisdata/datamodulemanager.cpp +++ b/src/gromacs/analysisdata/datamodulemanager.cpp @@ -163,15 +163,11 @@ class AnalysisDataModuleManager::Impl }; AnalysisDataModuleManager::Impl::Impl() - : bAllowMissing_(true), bSerialModules_(false), bParallelModules_(false), + : bDataProperty_(), // This must be in sync with how AbstractAnalysisData + // is actually initialized. + bAllowMissing_(true), bSerialModules_(false), bParallelModules_(false), state_(eNotStarted), currIndex_(0) { - // This must be in sync with how AbstractAnalysisData is actually - // initialized. - for (int i = 0; i < eDataPropertyNR; ++i) - { - bDataProperty_[i] = false; - } } void diff --git a/src/gromacs/analysisdata/modules/plot.cpp b/src/gromacs/analysisdata/modules/plot.cpp index 0741a07f65..03659c2b14 100644 --- a/src/gromacs/analysisdata/modules/plot.cpp +++ b/src/gromacs/analysisdata/modules/plot.cpp @@ -126,17 +126,16 @@ class AbstractPlotModule::Impl std::string xlabel_; std::string ylabel_; std::vector legend_; - char xformat_[15]; - char yformat_[15]; + std::string xformat_; + std::string yformat_; real xscale_; }; AbstractPlotModule::Impl::Impl(const AnalysisDataPlotSettings &settings) : settings_(settings), fp_(nullptr), bPlain_(false), bOmitX_(false), - bErrorsAsSeparateColumn_(false), xscale_(1.0) + bErrorsAsSeparateColumn_(false), + xformat_("%11.3f"), yformat_(" %8.3f"), xscale_(1.0) { - strcpy(xformat_, "%11.3f"); - strcpy(yformat_, " %8.3f"); } AbstractPlotModule::Impl::~Impl() @@ -301,7 +300,7 @@ AbstractPlotModule::setXFormat(int width, int precision, char format) "Invalid width or precision"); GMX_RELEASE_ASSERT(strchr("eEfFgG", format) != nullptr, "Invalid format specifier"); - sprintf(impl_->xformat_, "%%%d.%d%c", width, precision, format); + impl_->xformat_ = formatString("%%%d.%d%c", width, precision, format); } @@ -313,7 +312,7 @@ AbstractPlotModule::setYFormat(int width, int precision, char format) "Invalid width or precision"); GMX_RELEASE_ASSERT(strchr("eEfFgG", format) != nullptr, "Invalid format specifier"); - sprintf(impl_->yformat_, " %%%d.%d%c", width, precision, format); + impl_->yformat_ = formatString(" %%%d.%d%c", width, precision, format); } @@ -375,7 +374,7 @@ AbstractPlotModule::dataStarted(AbstractAnalysisData * /* data */) void -AbstractPlotModule::frameStarted(const AnalysisDataFrameHeader &frame) +AbstractPlotModule::frameStarted(const AnalysisDataFrameHeader &header) { if (!isFileOpen()) { @@ -383,7 +382,7 @@ AbstractPlotModule::frameStarted(const AnalysisDataFrameHeader &frame) } if (!impl_->bOmitX_) { - std::fprintf(impl_->fp_, impl_->xformat_, frame.x() * impl_->xscale_); + std::fprintf(impl_->fp_, impl_->xformat_.c_str(), header.x() * impl_->xscale_); } } @@ -418,11 +417,11 @@ AbstractPlotModule::writeValue(const AnalysisDataValue &value) const { GMX_ASSERT(isFileOpen(), "File not opened, but write attempted"); const real y = value.isSet() ? value.value() : 0.0; - std::fprintf(impl_->fp_, impl_->yformat_, y); + std::fprintf(impl_->fp_, impl_->yformat_.c_str(), y); if (impl_->bErrorsAsSeparateColumn_) { const real dy = value.isSet() ? value.error() : 0.0; - std::fprintf(impl_->fp_, impl_->yformat_, dy); + std::fprintf(impl_->fp_, impl_->yformat_.c_str(), dy); } } //! \endcond @@ -461,24 +460,16 @@ AnalysisDataPlotModule::pointsAdded(const AnalysisDataPointSetRef &points) */ AnalysisDataVectorPlotModule::AnalysisDataVectorPlotModule() + : bWrite_ {true, true, true, false} { - for (int i = 0; i < DIM; ++i) - { - bWrite_[i] = true; - } - bWrite_[DIM] = false; } AnalysisDataVectorPlotModule::AnalysisDataVectorPlotModule( const AnalysisDataPlotSettings &settings) - : AbstractPlotModule(settings) + : AbstractPlotModule(settings), + bWrite_ {true, true, true, false} { - for (int i = 0; i < DIM; ++i) - { - bWrite_[i] = true; - } - bWrite_[DIM] = false; } diff --git a/src/gromacs/compat/CMakeLists.txt b/src/gromacs/compat/CMakeLists.txt new file mode 100644 index 0000000000..5e4c52a0cd --- /dev/null +++ b/src/gromacs/compat/CMakeLists.txt @@ -0,0 +1,38 @@ +# +# This file is part of the GROMACS molecular simulation package. +# +# Copyright (c) 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. +# +# GROMACS is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public License +# as published by the Free Software Foundation; either version 2.1 +# of the License, or (at your option) any later version. +# +# GROMACS is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with GROMACS; if not, see +# http://www.gnu.org/licenses, or write to the Free Software Foundation, +# Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# +# If you want to redistribute modifications to GROMACS, please +# consider that scientific software is very special. Version +# control is crucial - bugs must be traceable. We will be happy to +# consider code for inclusion in the official distribution, but +# derived work must not be called official GROMACS. Details are found +# in the README & COPYING files - if they are missing, get the +# official version at http://www.gromacs.org. +# +# To help us fund GROMACS development, we humbly ask that you cite +# the research papers on the package. Check out http://www.gromacs.org. + +gmx_install_headers(make_unique.h) +if (BUILD_TESTING) + add_subdirectory(tests) +endif() diff --git a/src/gromacs/compat/make_unique.h b/src/gromacs/compat/make_unique.h index 72242b048b..663b15d8ae 100644 --- a/src/gromacs/compat/make_unique.h +++ b/src/gromacs/compat/make_unique.h @@ -43,7 +43,7 @@ * library implementations as appropriate. */ -/*! \libinternal +/*! * \file * \brief Provides template gmx::compat::make_unique * diff --git a/src/gromacs/options/.clang-tidy b/src/gromacs/options/.clang-tidy new file mode 120000 index 0000000000..6669ade155 --- /dev/null +++ b/src/gromacs/options/.clang-tidy @@ -0,0 +1 @@ +../../.clang-tidy.new.code \ No newline at end of file diff --git a/src/gromacs/options/abstractsection.h b/src/gromacs/options/abstractsection.h index 8ff6c8d7e1..e56bfb4ca1 100644 --- a/src/gromacs/options/abstractsection.h +++ b/src/gromacs/options/abstractsection.h @@ -79,7 +79,7 @@ class AbstractOptionSection * * Similar to AbstractOption::createStorage(). */ - virtual IOptionSectionStorage *createStorage() const = 0; + virtual std::unique_ptr createStorage() const = 0; //! \endcond private: diff --git a/src/gromacs/options/basicoptions.cpp b/src/gromacs/options/basicoptions.cpp index 6d92b1fef6..fa33e4f1e7 100644 --- a/src/gromacs/options/basicoptions.cpp +++ b/src/gromacs/options/basicoptions.cpp @@ -655,11 +655,10 @@ AbstractOptionStorage * createEnumOptionStorage(const AbstractOption &option, const char *const *enumValues, int count, int defaultValue, int defaultValueIfSet, - IOptionValueStore *store) + std::unique_ptr > store) { - std::unique_ptr > storePtr(store); return new EnumOptionStorage(option, enumValues, count, defaultValue, - defaultValueIfSet, move(storePtr)); + defaultValueIfSet, move(store)); } //! \endcond diff --git a/src/gromacs/options/basicoptions.h b/src/gromacs/options/basicoptions.h index 9cfd3bd93d..c409787eff 100644 --- a/src/gromacs/options/basicoptions.h +++ b/src/gromacs/options/basicoptions.h @@ -49,6 +49,7 @@ #include #include +#include "gromacs/compat/make_unique.h" #include "gromacs/options/abstractoption.h" #include "gromacs/options/ivaluestore.h" #include "gromacs/utility/arrayref.h" @@ -463,7 +464,7 @@ AbstractOptionStorage * createEnumOptionStorage(const AbstractOption &option, const char *const *enumValues, int count, int defaultValue, int defaultValueIfSet, - IOptionValueStore *store); + std::unique_ptr > store); //! \endcond } // namespace internal @@ -576,7 +577,7 @@ class EnumOption : public OptionTemplate > *this, enumValues_, enumValuesCount_, convertToInt(MyBase::defaultValue()), convertToInt(MyBase::defaultValueIfSet()), - new internal::EnumIndexStore( + compat::make_unique >( MyBase::store(), MyBase::storeVector())); } diff --git a/src/gromacs/options/optionsection.cpp b/src/gromacs/options/optionsection.cpp index 5578d36135..bfe7d107ea 100644 --- a/src/gromacs/options/optionsection.cpp +++ b/src/gromacs/options/optionsection.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2016, by the GROMACS development team, led by + * Copyright (c) 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. @@ -36,6 +36,7 @@ #include "optionsection.h" +#include "gromacs/compat/make_unique.h" #include "gromacs/options/isectionstorage.h" namespace gmx @@ -54,9 +55,9 @@ class OptionSectionStorage : public IOptionSectionStorage } // namespace -IOptionSectionStorage *OptionSection::createStorage() const +std::unique_ptr OptionSection::createStorage() const { - return new OptionSectionStorage(); + return compat::make_unique(); } } // namespace gmx diff --git a/src/gromacs/options/optionsection.h b/src/gromacs/options/optionsection.h index b5f5479b59..781829649a 100644 --- a/src/gromacs/options/optionsection.h +++ b/src/gromacs/options/optionsection.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2016, by the GROMACS development team, led by + * Copyright (c) 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. @@ -71,7 +71,7 @@ class OptionSection : public AbstractOptionSection explicit OptionSection(const char *name) : AbstractOptionSection(name) {} private: - virtual IOptionSectionStorage *createStorage() const; + virtual std::unique_ptr createStorage() const; }; /*! \brief diff --git a/src/gromacs/options/optionstoragetemplate.h b/src/gromacs/options/optionstoragetemplate.h index ed10c95e26..43a745f1c4 100644 --- a/src/gromacs/options/optionstoragetemplate.h +++ b/src/gromacs/options/optionstoragetemplate.h @@ -601,7 +601,7 @@ void OptionStorageTemplate::setDefaultValueIfSet(const T &value) GMX_THROW(APIError("defaultValueIfSet() is not supported with allowMultiple()")); } setFlag(efOption_DefaultValueIfSetExists); - defaultValueIfSet_.reset(new T(value)); + defaultValueIfSet_ = compat::make_unique(value); } } // namespace gmx diff --git a/src/gromacs/options/repeatingsection.h b/src/gromacs/options/repeatingsection.h index 683a41a189..6d2a97384d 100644 --- a/src/gromacs/options/repeatingsection.h +++ b/src/gromacs/options/repeatingsection.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2016, by the GROMACS development team, led by + * Copyright (c) 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. @@ -46,6 +46,7 @@ #include #include +#include "gromacs/compat/make_unique.h" #include "gromacs/options/abstractsection.h" #include "gromacs/options/ioptionscontainerwithsections.h" #include "gromacs/options/isectionstorage.h" @@ -90,7 +91,7 @@ class RepeatingOptionSection : public AbstractOptionSection } private: - virtual IOptionSectionStorage *createStorage() const; + virtual std::unique_ptr createStorage() const; std::vector *values_; @@ -150,9 +151,10 @@ class RepeatingOptionSectionStorage : public IOptionSectionStorage }; template -IOptionSectionStorage *RepeatingOptionSection::createStorage() const +std::unique_ptr RepeatingOptionSection::createStorage() const { - return new RepeatingOptionSectionStorage(*this); + + return compat::make_unique >(*this); } /*! \brief diff --git a/src/gromacs/options/tests/repeatingsection.cpp b/src/gromacs/options/tests/repeatingsection.cpp index faf16c2e72..04060766d6 100644 --- a/src/gromacs/options/tests/repeatingsection.cpp +++ b/src/gromacs/options/tests/repeatingsection.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2016, by the GROMACS development team, led by + * Copyright (c) 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. @@ -233,7 +233,7 @@ TEST(RepeatingOptionSectionTest, HandlesUnsetOptionWithExplicitDefault) struct NestedSectionData { - int value; + int value {}; std::vector subsec; }; diff --git a/src/gromacs/options/timeunitmanager.cpp b/src/gromacs/options/timeunitmanager.cpp index e03245e742..c5557c920a 100644 --- a/src/gromacs/options/timeunitmanager.cpp +++ b/src/gromacs/options/timeunitmanager.cpp @@ -87,8 +87,10 @@ TimeUnitManager::TimeUnitManager() } TimeUnitManager::TimeUnitManager(TimeUnit unit) + : timeUnit_(unit) { - setTimeUnit(unit); + GMX_RELEASE_ASSERT(unit >= 0 && unit <= TimeUnit_s, + "Invalid time unit"); } void TimeUnitManager::setTimeUnit(TimeUnit unit) @@ -127,14 +129,14 @@ TimeUnitBehavior::TimeUnitBehavior() { } -void TimeUnitBehavior::setTimeUnit(TimeUnit timeUnit) +void TimeUnitBehavior::setTimeUnit(TimeUnit unit) { - GMX_RELEASE_ASSERT(timeUnit >= 0 && timeUnit <= TimeUnit_s, + GMX_RELEASE_ASSERT(unit >= 0 && unit <= TimeUnit_s, "Invalid time unit"); - timeUnit_ = timeUnit; + timeUnit_ = unit; if (timeUnitStore_ != nullptr) { - *timeUnitStore_ = timeUnit; + *timeUnitStore_ = unit; } } diff --git a/src/gromacs/random/threefry.h b/src/gromacs/random/threefry.h index 44e9a38d8c..faadc66b94 100644 --- a/src/gromacs/random/threefry.h +++ b/src/gromacs/random/threefry.h @@ -455,6 +455,7 @@ class ThreeFry2x64General * \throws InternalError if the high bits needed to encode the number of counter * bits are nonzero. */ + //NOLINTNEXTLINE(cppcoreguidelines-pro-type-member-init) ThreeFry2x64General(uint64_t key0 = 0, RandomDomain domain = RandomDomain::Other) { seed(key0, domain); @@ -473,6 +474,7 @@ class ThreeFry2x64General * \throws InternalError if the high bits needed to encode the number of counter * bits are nonzero. To test arbitrary values, use 0 internal counter bits. */ + //NOLINTNEXTLINE(cppcoreguidelines-pro-type-member-init) ThreeFry2x64General(uint64_t key0, uint64_t key1) { seed(key0, key1); diff --git a/src/gromacs/selection/nbsearch.h b/src/gromacs/selection/nbsearch.h index 86bafacaf4..47d681d076 100644 --- a/src/gromacs/selection/nbsearch.h +++ b/src/gromacs/selection/nbsearch.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2009,2010,2011,2012,2013,2014,2015,2017, by the GROMACS development team, led by + * Copyright (c) 2009,2010,2011,2012,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. @@ -324,14 +324,13 @@ class AnalysisNeighborhoodPair { public: //! Initializes an invalid pair. - AnalysisNeighborhoodPair() : refIndex_(-1), testIndex_(0), distance2_(0.0) + AnalysisNeighborhoodPair() : refIndex_(-1), testIndex_(0), distance2_(0.0), dx_() { - clear_rvec(dx_); } //! Initializes a pair object with the given data. AnalysisNeighborhoodPair(int refIndex, int testIndex, real distance2, const rvec dx) - : refIndex_(refIndex), testIndex_(testIndex), distance2_(distance2) + : refIndex_(refIndex), testIndex_(testIndex), distance2_(distance2), dx_() { copy_rvec(dx, dx_); } diff --git a/src/gromacs/trajectoryanalysis/.clang-tidy b/src/gromacs/trajectoryanalysis/.clang-tidy new file mode 120000 index 0000000000..6669ade155 --- /dev/null +++ b/src/gromacs/trajectoryanalysis/.clang-tidy @@ -0,0 +1 @@ +../../.clang-tidy.new.code \ No newline at end of file diff --git a/src/gromacs/trajectoryanalysis/modules/angle.cpp b/src/gromacs/trajectoryanalysis/modules/angle.cpp index 4d3f83bcfa..6d2ebb181c 100644 --- a/src/gromacs/trajectoryanalysis/modules/angle.cpp +++ b/src/gromacs/trajectoryanalysis/modules/angle.cpp @@ -51,6 +51,7 @@ #include "gromacs/analysisdata/modules/average.h" #include "gromacs/analysisdata/modules/histogram.h" #include "gromacs/analysisdata/modules/plot.h" +#include "gromacs/compat/make_unique.h" #include "gromacs/math/units.h" #include "gromacs/math/vec.h" #include "gromacs/options/basicoptions.h" @@ -318,9 +319,9 @@ Angle::Angle() g1type_(Group1Type_Angle), g2type_(Group2Type_None), binWidth_(1.0), natoms1_(0), natoms2_(0) { - averageModule_.reset(new AnalysisDataFrameAverageModule()); + averageModule_ = compat::make_unique(); angles_.addModule(averageModule_); - histogramModule_.reset(new AnalysisDataSimpleHistogramModule()); + histogramModule_ = compat::make_unique(); angles_.addModule(histogramModule_); registerAnalysisDataset(&angles_, "angle"); diff --git a/src/gromacs/trajectoryanalysis/modules/distance.cpp b/src/gromacs/trajectoryanalysis/modules/distance.cpp index c766073734..aab79943c2 100644 --- a/src/gromacs/trajectoryanalysis/modules/distance.cpp +++ b/src/gromacs/trajectoryanalysis/modules/distance.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2010,2011,2012,2013,2014,2015,2016,2017, by the GROMACS development team, led by + * Copyright (c) 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. @@ -49,6 +49,7 @@ #include "gromacs/analysisdata/modules/average.h" #include "gromacs/analysisdata/modules/histogram.h" #include "gromacs/analysisdata/modules/plot.h" +#include "gromacs/compat/make_unique.h" #include "gromacs/math/vec.h" #include "gromacs/options/basicoptions.h" #include "gromacs/options/filenameoption.h" @@ -109,16 +110,16 @@ class Distance : public TrajectoryAnalysisModule }; Distance::Distance() - : meanLength_(0.1), lengthDev_(1.0), binWidth_(0.001) + : meanLength_(0.1), lengthDev_(1.0), binWidth_(0.001), + summaryStatsModule_(compat::make_unique()), + allStatsModule_(compat::make_unique()), + averageModule_(compat::make_unique()), + histogramModule_(compat::make_unique()) { - summaryStatsModule_.reset(new AnalysisDataAverageModule()); summaryStatsModule_->setAverageDataSets(true); distances_.addModule(summaryStatsModule_); - allStatsModule_.reset(new AnalysisDataAverageModule()); distances_.addModule(allStatsModule_); - averageModule_.reset(new AnalysisDataFrameAverageModule()); distances_.addModule(averageModule_); - histogramModule_.reset(new AnalysisDataSimpleHistogramModule()); distances_.addModule(histogramModule_); registerAnalysisDataset(&distances_, "dist"); diff --git a/src/gromacs/trajectoryanalysis/modules/trajectory.cpp b/src/gromacs/trajectoryanalysis/modules/trajectory.cpp index 0d6b02e006..3b4ba9f465 100644 --- a/src/gromacs/trajectoryanalysis/modules/trajectory.cpp +++ b/src/gromacs/trajectoryanalysis/modules/trajectory.cpp @@ -92,19 +92,17 @@ class Trajectory : public TrajectoryAnalysisModule std::string fnX_; std::string fnV_; std::string fnF_; - bool dimMask_[4]; - bool maskSet_[4]; + std::array dimMask_; + std::array maskSet_; AnalysisData xdata_; AnalysisData vdata_; AnalysisData fdata_; }; -Trajectory::Trajectory() +Trajectory::Trajectory() : + dimMask_ {true, true, true, false}, maskSet_ {} { - std::fill(std::begin(dimMask_), std::end(dimMask_), true); - dimMask_[DIM] = false; - std::fill(std::begin(maskSet_), std::end(maskSet_), false); registerAnalysisDataset(&xdata_, "x"); registerAnalysisDataset(&vdata_, "v"); registerAnalysisDataset(&fdata_, "f"); @@ -194,7 +192,7 @@ Trajectory::initAnalysis(const TrajectoryAnalysisSettings &settings, } AnalysisDataVectorPlotModulePointer plot( new AnalysisDataVectorPlotModule(settings.plotSettings())); - plot->setWriteMask(dimMask_); + plot->setWriteMask(dimMask_.data()); plot->setFileName(fnX_); plot->setTitle("Coordinates"); plot->setXAxisIsTime(); @@ -211,7 +209,7 @@ Trajectory::initAnalysis(const TrajectoryAnalysisSettings &settings, } AnalysisDataVectorPlotModulePointer plot( new AnalysisDataVectorPlotModule(settings.plotSettings())); - plot->setWriteMask(dimMask_); + plot->setWriteMask(dimMask_.data()); plot->setFileName(fnV_); plot->setTitle("Velocities"); plot->setXAxisIsTime(); @@ -228,7 +226,7 @@ Trajectory::initAnalysis(const TrajectoryAnalysisSettings &settings, } AnalysisDataVectorPlotModulePointer plot( new AnalysisDataVectorPlotModule(settings.plotSettings())); - plot->setWriteMask(dimMask_); + plot->setWriteMask(dimMask_.data()); plot->setFileName(fnF_); plot->setTitle("Forces"); plot->setXAxisIsTime(); diff --git a/src/gromacs/trajectoryanalysis/tests/surfacearea.cpp b/src/gromacs/trajectoryanalysis/tests/surfacearea.cpp index 73a874e620..80e421b04a 100644 --- a/src/gromacs/trajectoryanalysis/tests/surfacearea.cpp +++ b/src/gromacs/trajectoryanalysis/tests/surfacearea.cpp @@ -70,10 +70,9 @@ class SurfaceAreaTest : public ::testing::Test { public: SurfaceAreaTest() - : rng_(12345), area_(0.0), volume_(0.0), + : box_(), rng_(12345), area_(0.0), volume_(0.0), atomArea_(nullptr), dotCount_(0), dots_(nullptr) { - clear_mat(box_); } ~SurfaceAreaTest() { diff --git a/src/gromacs/trajectoryanalysis/topologyinformation.cpp b/src/gromacs/trajectoryanalysis/topologyinformation.cpp index 482cda74ca..7331e4eefb 100644 --- a/src/gromacs/trajectoryanalysis/topologyinformation.cpp +++ b/src/gromacs/trajectoryanalysis/topologyinformation.cpp @@ -57,9 +57,8 @@ namespace gmx { TopologyInformation::TopologyInformation() - : mtop_(nullptr), top_(nullptr), bTop_(false), xtop_(nullptr), ePBC_(-1) + : mtop_(nullptr), top_(nullptr), bTop_(false), xtop_(nullptr), boxtop_(), ePBC_(-1) { - clear_mat(boxtop_); } diff --git a/src/gromacs/utility/variant.h b/src/gromacs/utility/variant.h index 088f80a9ef..38e1d828d9 100644 --- a/src/gromacs/utility/variant.h +++ b/src/gromacs/utility/variant.h @@ -50,6 +50,7 @@ #include #include +#include "gromacs/compat/make_unique.h" #include "gromacs/utility/gmxassert.h" namespace gmx @@ -126,7 +127,7 @@ class Variant */ Variant &operator=(const Variant &other) { - content_.reset(other.cloneContent()); + content_ = other.cloneContent(); return *this; } //! Move-assigns the variant. @@ -214,7 +215,7 @@ class Variant public: virtual ~IContent() {} virtual const std::type_info &typeInfo() const = 0; - virtual IContent *clone() const = 0; + virtual std::unique_ptr clone() const = 0; }; template @@ -225,13 +226,13 @@ class Variant explicit Content(T &&value) : value_(std::move(value)) {} virtual const std::type_info &typeInfo() const { return typeid(T); } - virtual IContent *clone() const { return new Content(value_); } + virtual std::unique_ptr clone() const { return compat::make_unique(value_); } T value_; }; //! Creates a deep copy of the content. - IContent *cloneContent() const + std::unique_ptr cloneContent() const { return content_ != nullptr ? content_->clone() : nullptr; }