Enable more clang-tidy checks for new code
authorRoland Schulz <roland.schulz@intel.com>
Fri, 3 Aug 2018 01:19:40 +0000 (18:19 -0700)
committerMark Abraham <mark.j.abraham@gmail.com>
Mon, 27 Aug 2018 08:44:20 +0000 (10:44 +0200)
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

29 files changed:
cmake/gmxCFlags.cmake
src/.clang-tidy
src/.clang-tidy.new.code [new file with mode: 0644]
src/external/googletest/googletest/include/gtest/internal/gtest-internal.h
src/gromacs/CMakeLists.txt
src/gromacs/analysisdata/.clang-tidy [new symlink]
src/gromacs/analysisdata/datamodulemanager.cpp
src/gromacs/analysisdata/modules/plot.cpp
src/gromacs/compat/CMakeLists.txt [new file with mode: 0644]
src/gromacs/compat/make_unique.h
src/gromacs/options/.clang-tidy [new symlink]
src/gromacs/options/abstractsection.h
src/gromacs/options/basicoptions.cpp
src/gromacs/options/basicoptions.h
src/gromacs/options/optionsection.cpp
src/gromacs/options/optionsection.h
src/gromacs/options/optionstoragetemplate.h
src/gromacs/options/repeatingsection.h
src/gromacs/options/tests/repeatingsection.cpp
src/gromacs/options/timeunitmanager.cpp
src/gromacs/random/threefry.h
src/gromacs/selection/nbsearch.h
src/gromacs/trajectoryanalysis/.clang-tidy [new symlink]
src/gromacs/trajectoryanalysis/modules/angle.cpp
src/gromacs/trajectoryanalysis/modules/distance.cpp
src/gromacs/trajectoryanalysis/modules/trajectory.cpp
src/gromacs/trajectoryanalysis/tests/surfacearea.cpp
src/gromacs/trajectoryanalysis/topologyinformation.cpp
src/gromacs/utility/variant.h

index 48dac9c7064246cb3dba72c3170def0b96de33f8..752392771a32a741629bc5794e95f46df4951930 100644 (file)
@@ -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)
index c74d0c1ca887cc39aad500a48541e38a07a23705..15261590fc58b49948eb39d6e2ee467bbd65c52b 100644 (file)
@@ -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 (file)
index 0000000..65d42e9
--- /dev/null
@@ -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
index ebd1cf615de07d9b5ab0e60c71008261efe54213..3f8e2dd9a0c80bab22f6869a13932aa5e641c785 100644 (file)
@@ -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<GTEST_TEST_CLASS_NAME_(test_case_name, test_name)>); /*NOLINT(cppcoreguidelines-owning-memory)*/ \
+ void GTEST_TEST_CLASS_NAME_(test_case_name, test_name)::TestBody()
 
 #endif  // GTEST_INCLUDE_GTEST_INTERNAL_GTEST_INTERNAL_H_
 
index a1b441d44c8ac2a84c06e5a159eb7c8d7d9b94f5..ffe58496ba8d0d9051ac60e64a65662436a61906 100644 (file)
@@ -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 (symlink)
index 0000000..6669ade
--- /dev/null
@@ -0,0 +1 @@
+../../.clang-tidy.new.code
\ No newline at end of file
index 173c2a712a3a8c00ceb7cfb02a8949588659cd39..c8e34606da7e74be5b641036155978e241852d43 100644 (file)
@@ -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
index 0741a07f6516217e1a70bee9b200672d8beb2fac..03659c2b144534a2c8e93365c1bc311d3499301d 100644 (file)
@@ -126,17 +126,16 @@ class AbstractPlotModule::Impl
         std::string               xlabel_;
         std::string               ylabel_;
         std::vector<std::string>  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 (file)
index 0000000..5e4c52a
--- /dev/null
@@ -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()
index 72242b048b9bc134cf9135ed649edcd8d78543d3..663b15d8aea9715fc1c8cb9e57e0a2b3f85b0ab3 100644 (file)
@@ -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 (symlink)
index 0000000..6669ade
--- /dev/null
@@ -0,0 +1 @@
+../../.clang-tidy.new.code
\ No newline at end of file
index 8ff6c8d7e1d02428e2f9909db4a9206f5f5e968a..e56bfb4ca14093364b92d4f3c81aed58d3679bd3 100644 (file)
@@ -79,7 +79,7 @@ class AbstractOptionSection
          *
          * Similar to AbstractOption::createStorage().
          */
-        virtual IOptionSectionStorage *createStorage() const = 0;
+        virtual std::unique_ptr<IOptionSectionStorage> createStorage() const = 0;
         //! \endcond
 
     private:
index 6d92b1fef63e697a2a3cdf50f390913a4a2abe55..fa33e4f1e73abc80ce859bd6d278a2bae15e66fc 100644 (file)
@@ -655,11 +655,10 @@ AbstractOptionStorage *
 createEnumOptionStorage(const AbstractOption &option,
                         const char *const *enumValues, int count,
                         int defaultValue, int defaultValueIfSet,
-                        IOptionValueStore<int> *store)
+                        std::unique_ptr<IOptionValueStore<int> > store)
 {
-    std::unique_ptr<IOptionValueStore<int> > storePtr(store);
     return new EnumOptionStorage(option, enumValues, count, defaultValue,
-                                 defaultValueIfSet, move(storePtr));
+                                 defaultValueIfSet, move(store));
 }
 //! \endcond
 
index 9cfd3bd93d2016c0daffbbb560b5601389b0407b..c409787effbe64efe564d516bbd46f2f56efac6e 100644 (file)
@@ -49,6 +49,7 @@
 #include <string>
 #include <vector>
 
+#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<int> *store);
+                        std::unique_ptr<IOptionValueStore<int> > store);
 //! \endcond
 
 }   // namespace internal
@@ -576,7 +577,7 @@ class EnumOption : public OptionTemplate<EnumType, EnumOption<EnumType> >
                     *this, enumValues_, enumValuesCount_,
                     convertToInt(MyBase::defaultValue()),
                     convertToInt(MyBase::defaultValueIfSet()),
-                    new internal::EnumIndexStore<EnumType>(
+                    compat::make_unique<internal::EnumIndexStore<EnumType> >(
                             MyBase::store(), MyBase::storeVector()));
         }
 
index 5578d36135ebf3d0691fc67ed65c7acfbd267f4d..bfe7d107ea82a8725b06b6e86f824c1daf6e2380 100644 (file)
@@ -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<IOptionSectionStorage> OptionSection::createStorage() const
 {
-    return new OptionSectionStorage();
+    return compat::make_unique<OptionSectionStorage>();
 }
 
 } // namespace gmx
index b5f5479b595c8bcdc49e79f34099778a2d7a9429..781829649af3aa7742d8d3749c744febc7298990 100644 (file)
@@ -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<IOptionSectionStorage> createStorage() const;
 };
 
 /*! \brief
index ed10c95e26c19eb059091fbeca8cf2a24da8eb2a..43a745f1c43098ff05763cafbed79cf2da7b26c6 100644 (file)
@@ -601,7 +601,7 @@ void OptionStorageTemplate<T>::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<T>(value);
 }
 
 } // namespace gmx
index 683a41a18941f2f24599250b3c0a831f2ae9b39f..6d2a97384d34a1db764c4590589f34477b5674fb 100644 (file)
@@ -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 <memory>
 #include <vector>
 
+#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<IOptionSectionStorage> createStorage() const;
 
         std::vector<T> *values_;
 
@@ -150,9 +151,10 @@ class RepeatingOptionSectionStorage : public IOptionSectionStorage
 };
 
 template <class T>
-IOptionSectionStorage *RepeatingOptionSection<T>::createStorage() const
+std::unique_ptr<IOptionSectionStorage> RepeatingOptionSection<T>::createStorage() const
 {
-    return new RepeatingOptionSectionStorage<T>(*this);
+
+    return compat::make_unique<RepeatingOptionSectionStorage<T> >(*this);
 }
 
 /*! \brief
index faf16c2e7282af58b9bd53dfa757355daf2393cd..04060766d631043449762152727a719454d8feec 100644 (file)
@@ -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<SectionData> subsec;
 };
 
index e03245e742af9f3e59a5ebce565c87ea00ad385b..c5557c920ae1a1e08a19c527112665466ae409e1 100644 (file)
@@ -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;
     }
 }
 
index 44e9a38d8cce69fd3fc1feb18f2cba6a78589d8b..faadc66b9405162af4a6354985285fb1d64a15fa 100644 (file)
@@ -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);
index 86bafacaf47a2d8b6f0b9fe5424c5cdc07aebd5a..47d681d076ed4e2097573d2524c00b8539a0d38d 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,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 (symlink)
index 0000000..6669ade
--- /dev/null
@@ -0,0 +1 @@
+../../.clang-tidy.new.code
\ No newline at end of file
index 4d3f83bcfa22e06d4de1eb398d9d893a22ee0aef..6d2ebb181cff58411fbb3aac4f836af2eac030cd 100644 (file)
@@ -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<AnalysisDataFrameAverageModule>();
     angles_.addModule(averageModule_);
-    histogramModule_.reset(new AnalysisDataSimpleHistogramModule());
+    histogramModule_ = compat::make_unique<AnalysisDataSimpleHistogramModule>();
     angles_.addModule(histogramModule_);
 
     registerAnalysisDataset(&angles_, "angle");
index c766073734ed8481b65175c85e04ef7be3b46467..aab79943c2721c6f0711500e7a7bba32367d66c9 100644 (file)
@@ -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<AnalysisDataAverageModule>()),
+      allStatsModule_(compat::make_unique<AnalysisDataAverageModule>()),
+      averageModule_(compat::make_unique<AnalysisDataFrameAverageModule>()),
+      histogramModule_(compat::make_unique<AnalysisDataSimpleHistogramModule>())
 {
-    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");
index 0d6b02e00602203d52f92ef195134478c95af757..3b4ba9f465e77674d6fb0460a2f00107400c497e 100644 (file)
@@ -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<bool, 4>                 dimMask_;
+        std::array<bool, 4>                 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();
index 73a874e620366bd5d281f48c68be691384686052..80e421b04afb8799cde5c1989a0e911260762f15 100644 (file)
@@ -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()
         {
index 482cda74ca4826dfa46bd45f1dcfd908a7b6c85a..7331e4eefb39195196b776dfb5ffaaeac5b62587 100644 (file)
@@ -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_);
 }
 
 
index 088f80a9ef72861a73af27cc4d98be1cc7917771..38e1d828d922b908456acb9e5af3e0928f8b3cff 100644 (file)
@@ -50,6 +50,7 @@
 #include <typeinfo>
 #include <utility>
 
+#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<IContent> clone() const = 0;
         };
 
         template <typename T>
@@ -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<IContent> clone() const { return compat::make_unique<Content>(value_); }
 
                 T value_;
         };
 
         //! Creates a deep copy of the content.
-        IContent *cloneContent() const
+        std::unique_ptr<IContent> cloneContent() const
         {
             return content_ != nullptr ? content_->clone() : nullptr;
         }