Add all the performance-* clang-tidy checks (openPMD#1532)

* add all the performance-* clang-tidy checks * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * address residual issues found with clang-tidy * enforce east const * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix bug * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * cleaning * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * remove unused variable * Fix some corrections that were too ambitious * Some little forgotten clang-tidy checks * Fixes after merging the dev * Replace auxiliary::forget() with NOLINT comments --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Franz Pöschel <[email protected]>
franzpoeschel · Nov 20, 2023 · 71aa0e9 · 71aa0e9
1 parent 991d0d9
commit 71aa0e9
Show file tree

Hide file tree

Showing 50 changed files with 240 additions and 195 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -2,5 +2,5 @@
 # FIXME: all performance-* reports
 # FIXME: all cert-* reports
 # FIXME: all bugprone-* reports
-Checks: -*,bugprone-*,-bugprone-unhandled-self-assignment,-bugprone-parent-virtual-call,-bugprone-narrowing-conversions,-bugprone-exception-escape,-bugprone-string-literal-with-embedded-nul,cppcoreguidelines-slicing,mpi-*,readability-non-const-parameter,performance-for-range-copy,modernize-*,-modernize-use-trailing-return-type,-modernize-use-bool-literals,-modernize-avoid-c-arrays,-modernize-use-auto,-modernize-return-braced-init-list
+Checks: -*,bugprone-*,-bugprone-unhandled-self-assignment,-bugprone-parent-virtual-call,-bugprone-narrowing-conversions,-bugprone-exception-escape,-bugprone-string-literal-with-embedded-nul,cppcoreguidelines-slicing,mpi-*,readability-non-const-parameter,performance-*,modernize-*,-modernize-use-trailing-return-type,-modernize-use-bool-literals,-modernize-avoid-c-arrays,-modernize-use-auto,-modernize-return-braced-init-list
 HeaderFilterRegex: '((^(?!\/share\/openPMD\/).*)*include\/openPMD\/.+\.hpp|src\/^(?!binding).+\.cpp$)'
diff --git a/examples/10_streaming_read.cpp b/examples/10_streaming_read.cpp
@@ -46,7 +46,7 @@ int main()
 
         for (size_t i = 0; i < 3; ++i)
         {
-            std::string dim = dimensions[i];
+            std::string const &dim = dimensions[i];
             RecordComponent rc = electronPositions[dim];
             loadedChunks[i] = rc.loadChunk<position_t>(
                 Offset(rc.getDimensionality(), 0), rc.getExtent());
@@ -60,7 +60,7 @@ int main()
 
         for (size_t i = 0; i < 3; ++i)
         {
-            std::string dim = dimensions[i];
+            std::string const &dim = dimensions[i];
             Extent const &extent = extents[i];
             std::cout << "\ndim: " << dim << "\n" << std::endl;
             auto chunk = loadedChunks[i];

diff --git a/examples/8_benchmark_parallel.cpp b/examples/8_benchmark_parallel.cpp
@@ -12,7 +12,7 @@
 #include <vector>
 
 #if openPMD_HAVE_MPI
-inline void print_help(std::string const program_name)
+inline void print_help(std::string const &program_name)
 {
     std::cout << "Usage: " << program_name << "\n";
     std::cout << "Run a simple parallel write and read benchmark.\n\n";
@@ -27,7 +27,7 @@ inline void print_help(std::string const program_name)
     std::cout << "    " << program_name << "  # for a strong scaling\n";
 }
 
-inline void print_version(std::string const program_name)
+inline void print_version(std::string const &program_name)
 {
     std::cout << program_name << " (openPMD-api) " << openPMD::getVersion()
               << "\n";
@@ -46,6 +46,7 @@ int main(int argc, char *argv[])
 
     // CLI parsing
     std::vector<std::string> str_argv;
+    str_argv.reserve(argc);
     for (int i = 0; i < argc; ++i)
         str_argv.emplace_back(argv[i]);
     bool weak_scaling = false;

diff --git a/include/openPMD/Datatype.hpp b/include/openPMD/Datatype.hpp
@@ -739,7 +739,7 @@ Datatype toVectorType(Datatype dt);
 
 std::string datatypeToString(Datatype dt);
 
-Datatype stringToDatatype(std::string s);
+Datatype stringToDatatype(const std::string &s);
 
 void warnWrongDtype(std::string const &key, Datatype store, Datatype request);
 

diff --git a/include/openPMD/Error.hpp b/include/openPMD/Error.hpp
@@ -50,7 +50,8 @@ namespace error
     {
     public:
         std::string backend;
-        OperationUnsupportedInBackend(std::string backend_in, std::string what);
+        OperationUnsupportedInBackend(
+            std::string backend_in, std::string const &what);
     };
 
     /**
@@ -62,7 +63,7 @@ namespace error
     class WrongAPIUsage : public Error
     {
     public:
-        WrongAPIUsage(std::string what);
+        WrongAPIUsage(std::string const &what);
     };
 
     class BackendConfigSchema : public Error

diff --git a/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp b/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp
@@ -391,9 +391,10 @@ class ADIOS2IOHandlerImpl
         ThrowError
     };
 
-    detail::BufferedActions &getFileData(InvalidatableFile file, IfFileNotOpen);
+    detail::BufferedActions &
+    getFileData(InvalidatableFile const &file, IfFileNotOpen);
 
-    void dropFileData(InvalidatableFile file);
+    void dropFileData(InvalidatableFile const &file);
 
     /*
      * Prepare a variable that already exists for an IO
@@ -465,7 +466,7 @@ namespace detail
             ADIOS2IOHandlerImpl &,
             adios2::IO &IO,
             std::string name,
-            std::shared_ptr<Attribute::resource> resource);
+            Attribute::resource &resource);
 
         template <int n, typename... Params>
         static Datatype call(Params &&...);
@@ -488,8 +489,8 @@ namespace detail
         template <typename T>
         static void call(
             ADIOS2IOHandlerImpl *impl,
-            InvalidatableFile,
-            const std::string &varName,
+            InvalidatableFile const &,
+            std::string const &varName,
             Parameter<Operation::OPEN_DATASET> &parameters);
 
         static constexpr char const *errorMsg = "ADIOS2: openDataset()";

diff --git a/include/openPMD/IO/AbstractIOHandlerImpl.hpp b/include/openPMD/IO/AbstractIOHandlerImpl.hpp
@@ -390,8 +390,8 @@ class AbstractIOHandlerImpl
      * Using the default implementation (which copies the abstractFilePath
      * into the current writable) should be enough for all backends.
      */
-    void
-    keepSynchronous(Writable *, Parameter<Operation::KEEP_SYNCHRONOUS> param);
+    void keepSynchronous(
+        Writable *, Parameter<Operation::KEEP_SYNCHRONOUS> const &param);
 
     /** Notify the backend that the Writable has been / will be deallocated.
      *

diff --git a/include/openPMD/IO/HDF5/HDF5Auxiliary.hpp b/include/openPMD/IO/HDF5/HDF5Auxiliary.hpp
@@ -62,5 +62,5 @@ std::string concrete_h5_file_position(Writable *w);
  * @return array for resulting chunk dimensions
  */
 std::vector<hsize_t>
-getOptimalChunkDims(std::vector<hsize_t> const dims, size_t const typeSize);
+getOptimalChunkDims(std::vector<hsize_t> const &dims, size_t const typeSize);
 } // namespace openPMD
diff --git a/include/openPMD/IO/HDF5/ParallelHDF5IOHandler.hpp b/include/openPMD/IO/HDF5/ParallelHDF5IOHandler.hpp
@@ -39,7 +39,8 @@ class ParallelHDF5IOHandler : public AbstractIOHandler
     ParallelHDF5IOHandler(
         std::string path, Access, MPI_Comm, json::TracingJSON config);
 #else
-    ParallelHDF5IOHandler(std::string path, Access, json::TracingJSON config);
+    ParallelHDF5IOHandler(
+        std::string const &path, Access, json::TracingJSON config);
 #endif
     ~ParallelHDF5IOHandler() override;
 

diff --git a/include/openPMD/IO/JSON/JSONIOHandler.hpp b/include/openPMD/IO/JSON/JSONIOHandler.hpp
@@ -30,7 +30,7 @@ class JSONIOHandler : public AbstractIOHandler
 {
 public:
     JSONIOHandler(
-        std::string path,
+        std::string const &path,
         Access at,
         openPMD::json::TracingJSON config,
         JSONIOHandlerImpl::FileFormat,

diff --git a/include/openPMD/IO/JSON/JSONIOHandlerImpl.hpp b/include/openPMD/IO/JSON/JSONIOHandlerImpl.hpp
@@ -84,7 +84,7 @@ struct File
         return fileState->valid;
     }
 
-    File &operator=(std::string s)
+    File &operator=(std::string const &s)
     {
         if (fileState)
         {
@@ -259,10 +259,10 @@ class JSONIOHandlerImpl : public AbstractIOHandlerImpl
     // else null. first tuple element needs to be a pointer, since the casted
     // streams are references only.
     std::tuple<std::unique_ptr<FILEHANDLE>, std::istream *, std::ostream *>
-    getFilehandle(File, Access access);
+    getFilehandle(File const &, Access access);
 
     // full operating system path of the given file
-    std::string fullPath(File);
+    std::string fullPath(File const &);
 
     std::string fullPath(std::string const &);
 
@@ -304,32 +304,32 @@ class JSONIOHandlerImpl : public AbstractIOHandlerImpl
 
     // make sure that the given path exists in proper form in
     // the passed json value
-    static void ensurePath(nlohmann::json *json, std::string path);
+    static void ensurePath(nlohmann::json *json, std::string const &path);
 
     // In order not to insert the same file name into the data structures
     // with a new pointer (e.g. when reopening), search for a possibly
     // existing old pointer. Construct a new pointer only upon failure.
     // The bool is true iff the pointer has been newly-created.
     // The iterator is an iterator for m_files
     std::tuple<File, std::unordered_map<Writable *, File>::iterator, bool>
-    getPossiblyExisting(std::string file);
+    getPossiblyExisting(std::string const &file);
 
     // get the json value representing the whole file, possibly reading
     // from disk
-    std::shared_ptr<nlohmann::json> obtainJsonContents(File);
+    std::shared_ptr<nlohmann::json> obtainJsonContents(File const &);
 
     // get the json value at the writable's fileposition
     nlohmann::json &obtainJsonContents(Writable *writable);
 
     // write to disk the json contents associated with the file
     // remove from m_dirty if unsetDirty == true
-    void putJsonContents(File, bool unsetDirty = true);
+    void putJsonContents(File const &, bool unsetDirty = true);
 
     // figure out the file position of the writable
     // (preferring the parent's file position) and extend it
     // by extend. return the modified file position.
     std::shared_ptr<JSONFilePosition>
-    setAndGetFilePosition(Writable *, std::string extend);
+    setAndGetFilePosition(Writable *, std::string const &extend);
 
     // figure out the file position of the writable
     // (preferring the parent's file position)
@@ -345,7 +345,7 @@ class JSONIOHandlerImpl : public AbstractIOHandlerImpl
     void associateWithFile(Writable *writable, File);
 
     // need to check the name too in order to exclude "attributes" key
-    static bool isGroup(nlohmann::json::const_iterator it);
+    static bool isGroup(nlohmann::json::const_iterator const &it);
 
     static bool isDataset(nlohmann::json const &j);
 

diff --git a/include/openPMD/Iteration.hpp b/include/openPMD/Iteration.hpp
@@ -284,7 +284,9 @@ class Iteration : public Attributable
      */
     void reread(std::string const &path);
     void readFileBased(
-        std::string filePath, std::string const &groupPath, bool beginStep);
+        std::string const &filePath,
+        std::string const &groupPath,
+        bool beginStep);
     void readGorVBased(std::string const &groupPath, bool beginStep);
     void read_impl(std::string const &groupPath);
     void readMeshes(std::string const &meshesPath);

diff --git a/include/openPMD/ReadIterations.hpp b/include/openPMD/ReadIterations.hpp
@@ -77,7 +77,8 @@ class SeriesIterator
     explicit SeriesIterator();
 
     SeriesIterator(
-        Series, std::optional<internal::ParsePreference> parsePreference);
+        Series const &,
+        std::optional<internal::ParsePreference> const &parsePreference);
 
     SeriesIterator &operator++();
 

diff --git a/include/openPMD/Series.hpp b/include/openPMD/Series.hpp
@@ -241,7 +241,13 @@ class Series : public Attributable
         Access at,
         std::string const &options = "{}");
 
-    virtual ~Series() = default;
+    Series(Series const &) = default;
+    Series(Series &&) = default;
+
+    Series &operator=(Series const &) = default;
+    Series &operator=(Series &&) = default;
+
+    ~Series() override = default;
 
     /**
      * An unsigned integer type, used to identify Iterations in a Series.
@@ -637,12 +643,12 @@ OPENPMD_private
     std::future<void> flush_impl(
         iterations_iterator begin,
         iterations_iterator end,
-        internal::FlushParams flushParams,
+        internal::FlushParams const &flushParams,
         bool flushIOHandler = true);
     void flushFileBased(
         iterations_iterator begin,
         iterations_iterator end,
-        internal::FlushParams flushParams,
+        internal::FlushParams const &flushParams,
         bool flushIOHandler = true);
     /*
      * Group-based and variable-based iteration layouts share a lot of logic
@@ -654,7 +660,7 @@ OPENPMD_private
     void flushGorVBased(
         iterations_iterator begin,
         iterations_iterator end,
-        internal::FlushParams flushParams,
+        internal::FlushParams const &flushParams,
         bool flushIOHandler = true);
     void flushMeshesPath();
     void flushParticlesPath();

diff --git a/include/openPMD/ThrowError.hpp b/include/openPMD/ThrowError.hpp
@@ -50,8 +50,8 @@ enum class Reason
 [[noreturn]] OPENPMDAPI_EXPORT void
 throwBackendConfigSchema(std::vector<std::string> jsonPath, std::string what);
 
-[[noreturn]] OPENPMDAPI_EXPORT void
-throwOperationUnsupportedInBackend(std::string backend, std::string what);
+[[noreturn]] OPENPMDAPI_EXPORT void throwOperationUnsupportedInBackend(
+    std::string backend, std::string const &what);
 
 [[noreturn]] OPENPMDAPI_EXPORT void throwReadError(
     AffectedObject affectedObject,

diff --git a/include/openPMD/backend/Attributable.hpp b/include/openPMD/backend/Attributable.hpp
@@ -253,7 +253,7 @@ OPENPMD_protected
     Iteration &containingIteration();
     /** @} */
 
-    void seriesFlush(internal::FlushParams);
+    void seriesFlush(internal::FlushParams const &);
 
     void flushAttributes(internal::FlushParams const &);
 

diff --git a/include/openPMD/backend/Writable.hpp b/include/openPMD/backend/Writable.hpp
@@ -119,7 +119,7 @@ class Writable final
 OPENPMD_private
     // clang-format on
 
-    void seriesFlush(internal::FlushParams);
+    void seriesFlush(internal::FlushParams const &);
     /*
      * These members need to be shared pointers since distinct instances of
      * Writable may share them.

diff --git a/include/openPMD/cli/ls.hpp b/include/openPMD/cli/ls.hpp
@@ -34,7 +34,7 @@ namespace cli
 {
     namespace ls
     {
-        inline void print_help(std::string const program_name)
+        inline void print_help(std::string const &program_name)
         {
             std::cout << "Usage: " << program_name << " openPMD-series\n";
             std::cout << "List information about an openPMD data series.\n\n";
@@ -54,7 +54,7 @@ namespace cli
                       << " ./samples/serial_patch.bp\n";
         }
 
-        inline void print_version(std::string const program_name)
+        inline void print_version(std::string const &program_name)
         {
             std::cout << program_name << " (openPMD-api) " << getVersion()
                       << "\n";

diff --git a/src/Dataset.cpp b/src/Dataset.cpp
@@ -26,11 +26,11 @@
 namespace openPMD
 {
 Dataset::Dataset(Datatype d, Extent e, std::string options_in)
-    : extent{e}
-    , dtype{d}
-    , rank{static_cast<uint8_t>(e.size())}
-    , options{std::move(options_in)}
-{}
+    : extent{std::move(e)}, dtype{d}, options{std::move(options_in)}
+{
+    // avoid initialization order issues
+    rank = static_cast<uint8_t>(extent.size());
+}
 
 Dataset::Dataset(Extent e) : Dataset(Datatype::UNDEFINED, std::move(e))
 {}

diff --git a/src/Datatype.cpp b/src/Datatype.cpp
@@ -163,7 +163,7 @@ std::ostream &operator<<(std::ostream &os, openPMD::Datatype const &d)
     return os;
 }
 
-Datatype stringToDatatype(std::string s)
+Datatype stringToDatatype(const std::string &s)
 {
     static std::unordered_map<std::string, Datatype> m{
         {"CHAR", Datatype::CHAR},

diff --git a/src/Error.cpp b/src/Error.cpp
@@ -12,19 +12,18 @@ const char *Error::what() const noexcept
 namespace error
 {
     OperationUnsupportedInBackend::OperationUnsupportedInBackend(
-        std::string backend_in, std::string what)
+        std::string backend_in, std::string const &what)
         : Error("Operation unsupported in " + backend_in + ": " + what)
         , backend{std::move(backend_in)}
     {}
 
-    void
-    throwOperationUnsupportedInBackend(std::string backend, std::string what)
+    void throwOperationUnsupportedInBackend(
+        std::string backend, std::string const &what)
     {
-        throw OperationUnsupportedInBackend(
-            std::move(backend), std::move(what));
+        throw OperationUnsupportedInBackend(std::move(backend), what);
     }
 
-    WrongAPIUsage::WrongAPIUsage(std::string what)
+    WrongAPIUsage::WrongAPIUsage(std::string const &what)
         : Error("Wrong API usage: " + what)
     {}