From 71aa0e98d7a957a5b050577ad9c563e72e8a77f8 Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Mon, 20 Nov 2023 16:23:52 +0100 Subject: [PATCH] Add all the performance-* clang-tidy checks (#1532) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- .clang-tidy | 2 +- examples/10_streaming_read.cpp | 4 +- examples/8_benchmark_parallel.cpp | 5 +- include/openPMD/Datatype.hpp | 2 +- include/openPMD/Error.hpp | 5 +- include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp | 11 ++-- include/openPMD/IO/AbstractIOHandlerImpl.hpp | 4 +- include/openPMD/IO/HDF5/HDF5Auxiliary.hpp | 2 +- .../openPMD/IO/HDF5/ParallelHDF5IOHandler.hpp | 3 +- include/openPMD/IO/JSON/JSONIOHandler.hpp | 2 +- include/openPMD/IO/JSON/JSONIOHandlerImpl.hpp | 18 +++---- include/openPMD/Iteration.hpp | 4 +- include/openPMD/ReadIterations.hpp | 3 +- include/openPMD/Series.hpp | 14 ++++-- include/openPMD/ThrowError.hpp | 4 +- include/openPMD/backend/Attributable.hpp | 2 +- include/openPMD/backend/Writable.hpp | 2 +- include/openPMD/cli/ls.hpp | 4 +- src/Dataset.cpp | 10 ++-- src/Datatype.cpp | 2 +- src/Error.cpp | 11 ++-- src/IO/ADIOS/ADIOS2IOHandler.cpp | 50 +++++++++++-------- src/IO/AbstractIOHandlerHelper.cpp | 25 +++++----- src/IO/AbstractIOHandlerImpl.cpp | 2 +- src/IO/HDF5/HDF5Auxiliary.cpp | 2 +- src/IO/HDF5/HDF5IOHandler.cpp | 6 ++- src/IO/HDF5/ParallelHDF5IOHandler.cpp | 14 ++++-- src/IO/InvalidatableFile.cpp | 6 +-- src/IO/JSON/JSONIOHandler.cpp | 2 +- src/IO/JSON/JSONIOHandlerImpl.cpp | 30 +++++------ src/Iteration.cpp | 2 +- src/ReadIterations.cpp | 7 +-- src/Series.cpp | 20 ++++---- src/WriteIterations.cpp | 4 +- src/auxiliary/Filesystem.cpp | 3 +- src/auxiliary/JSON.cpp | 4 +- src/backend/Attributable.cpp | 4 +- src/backend/Writable.cpp | 2 +- src/binding/python/Attributable.cpp | 31 ++++++------ src/binding/python/ChunkInfo.cpp | 2 +- src/binding/python/Dataset.cpp | 8 +-- src/binding/python/Datatype.cpp | 2 +- src/binding/python/PatchRecordComponent.cpp | 2 +- src/binding/python/RecordComponent.cpp | 5 +- src/cli/ls.cpp | 1 + src/version.cpp | 9 ++-- test/AuxiliaryTest.cpp | 7 +-- test/CoreTest.cpp | 1 + test/ParallelIOTest.cpp | 21 ++++---- test/SerialIOTest.cpp | 49 ++++++++++-------- 50 files changed, 240 insertions(+), 195 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 7998697f29..034a863660 100644 --- 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 index 6e6aba1fd9..99da2c5a5c 100644 --- 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( 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 index 5adf2512ff..2dc63632ff 100644 --- a/examples/8_benchmark_parallel.cpp +++ b/examples/8_benchmark_parallel.cpp @@ -12,7 +12,7 @@ #include #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 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 index 05d0ddefbb..c83c865e59 100644 --- 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 index c50e2918b6..3e516e16ec 100644 --- 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 index 86afffbbdf..e63c6a493b 100644 --- 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 resource); + Attribute::resource &resource); template static Datatype call(Params &&...); @@ -488,8 +489,8 @@ namespace detail template static void call( ADIOS2IOHandlerImpl *impl, - InvalidatableFile, - const std::string &varName, + InvalidatableFile const &, + std::string const &varName, Parameter ¶meters); static constexpr char const *errorMsg = "ADIOS2: openDataset()"; diff --git a/include/openPMD/IO/AbstractIOHandlerImpl.hpp b/include/openPMD/IO/AbstractIOHandlerImpl.hpp index 8d13f3feb8..dafbc24111 100644 --- 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 param); + void keepSynchronous( + Writable *, Parameter const ¶m); /** 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 index da7ff2f68f..dc040f7053 100644 --- 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 -getOptimalChunkDims(std::vector const dims, size_t const typeSize); +getOptimalChunkDims(std::vector const &dims, size_t const typeSize); } // namespace openPMD diff --git a/include/openPMD/IO/HDF5/ParallelHDF5IOHandler.hpp b/include/openPMD/IO/HDF5/ParallelHDF5IOHandler.hpp index 18d43c93ab..cd951be5d2 100644 --- 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 index 452098137e..7fdea5b6f0 100644 --- 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 index c935647665..5ce9d057c3 100644 --- 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::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,7 +304,7 @@ 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 @@ -312,24 +312,24 @@ class JSONIOHandlerImpl : public AbstractIOHandlerImpl // The bool is true iff the pointer has been newly-created. // The iterator is an iterator for m_files std::tuple::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 obtainJsonContents(File); + std::shared_ptr 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 - 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 index fd1f14cd00..49e8c3296c 100644 --- 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 index a7cf0d18b5..35f2f740ce 100644 --- a/include/openPMD/ReadIterations.hpp +++ b/include/openPMD/ReadIterations.hpp @@ -77,7 +77,8 @@ class SeriesIterator explicit SeriesIterator(); SeriesIterator( - Series, std::optional parsePreference); + Series const &, + std::optional const &parsePreference); SeriesIterator &operator++(); diff --git a/include/openPMD/Series.hpp b/include/openPMD/Series.hpp index 1d99b54a84..094ed24fbf 100644 --- 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 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 index f2695f7ae0..3888c40b12 100644 --- a/include/openPMD/ThrowError.hpp +++ b/include/openPMD/ThrowError.hpp @@ -50,8 +50,8 @@ enum class Reason [[noreturn]] OPENPMDAPI_EXPORT void throwBackendConfigSchema(std::vector 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 index db7aec37de..2b45ac0e45 100644 --- 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 index 7aeead3dfe..5e1272317e 100644 --- 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 index 1d2313e250..b61e34e13d 100644 --- 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 index 587598db63..662bd2d29f 100644 --- 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(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(extent.size()); +} Dataset::Dataset(Extent e) : Dataset(Datatype::UNDEFINED, std::move(e)) {} diff --git a/src/Datatype.cpp b/src/Datatype.cpp index f0f26f7ae9..298bcc8790 100644 --- 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 m{ {"CHAR", Datatype::CHAR}, diff --git a/src/Error.cpp b/src/Error.cpp index a8e83338ed..f2e27a0213 100644 --- 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) {} diff --git a/src/IO/ADIOS/ADIOS2IOHandler.cpp b/src/IO/ADIOS/ADIOS2IOHandler.cpp index a9af92e4a0..8cddd578a0 100644 --- a/src/IO/ADIOS/ADIOS2IOHandler.cpp +++ b/src/IO/ADIOS/ADIOS2IOHandler.cpp @@ -1111,7 +1111,7 @@ void ADIOS2IOHandlerImpl::readAttribute( } Datatype ret = switchType( - type, *this, ba.m_IO, name, parameters.resource); + type, *this, ba.m_IO, name, *parameters.resource); *parameters.dtype = ret; } @@ -1241,7 +1241,7 @@ void ADIOS2IOHandlerImpl::listPaths( } for (auto &path : subdirs) { - parameters.paths->emplace_back(std::move(path)); + parameters.paths->emplace_back(path); } } @@ -1284,7 +1284,7 @@ void ADIOS2IOHandlerImpl::listDatasets( } for (auto &dataset : subdirs) { - parameters.datasets->emplace_back(std::move(dataset)); + parameters.datasets->emplace_back(dataset); } } @@ -1345,7 +1345,7 @@ void ADIOS2IOHandlerImpl::closePath( return; } auto position = setAndGetFilePosition(writable); - auto const positionString = filePositionToString(position); + auto positionString = filePositionToString(position); VERIFY( !auxiliary::ends_with(positionString, '/'), "[ADIOS2] Position string has unexpected format. This is a bug " @@ -1354,7 +1354,8 @@ void ADIOS2IOHandlerImpl::closePath( for (auto const &attr : fileData.availableAttributesPrefixed(positionString)) { - fileData.m_IO.RemoveAttribute(positionString + '/' + attr); + fileData.m_IO.RemoveAttribute( + std::string(positionString).append("/").append(attr)); } } @@ -1491,8 +1492,8 @@ std::string ADIOS2IOHandlerImpl::nameOfAttribute(Writable *writable, std::string attribute) { auto pos = setAndGetFilePosition(writable); - return filePositionToString( - extendFilePosition(pos, auxiliary::removeSlashes(attribute))); + return filePositionToString(extendFilePosition( + pos, auxiliary::removeSlashes(std::move(attribute)))); } GroupOrDataset ADIOS2IOHandlerImpl::groupOrDataset(Writable *writable) @@ -1500,8 +1501,8 @@ GroupOrDataset ADIOS2IOHandlerImpl::groupOrDataset(Writable *writable) return setAndGetFilePosition(writable)->gd; } -detail::BufferedActions & -ADIOS2IOHandlerImpl::getFileData(InvalidatableFile file, IfFileNotOpen flag) +detail::BufferedActions &ADIOS2IOHandlerImpl::getFileData( + InvalidatableFile const &file, IfFileNotOpen flag) { VERIFY_ALWAYS( file.valid(), @@ -1515,8 +1516,7 @@ ADIOS2IOHandlerImpl::getFileData(InvalidatableFile file, IfFileNotOpen flag) case IfFileNotOpen::OpenImplicitly: { auto res = m_fileData.emplace( - std::move(file), - std::make_unique(*this, file)); + file, std::make_unique(*this, file)); return *res.first->second; } case IfFileNotOpen::ThrowError: @@ -1531,7 +1531,7 @@ ADIOS2IOHandlerImpl::getFileData(InvalidatableFile file, IfFileNotOpen flag) } } -void ADIOS2IOHandlerImpl::dropFileData(InvalidatableFile file) +void ADIOS2IOHandlerImpl::dropFileData(InvalidatableFile const &file) { auto it = m_fileData.find(file); if (it != m_fileData.end()) @@ -1615,7 +1615,7 @@ namespace detail ADIOS2IOHandlerImpl &impl, adios2::IO &IO, std::string name, - std::shared_ptr resource) + Attribute::resource &resource) { (void)impl; /* @@ -1653,11 +1653,11 @@ namespace detail auto meta = IO.InquireAttribute(metaAttr); if (meta.Data().size() == 1 && meta.Data()[0] == 1) { - *resource = bool_repr::fromRep(attr.Data()[0]); + resource = bool_repr::fromRep(attr.Data()[0]); return determineDatatype(); } } - *resource = attr.Data()[0]; + resource = attr.Data()[0]; } else if constexpr (IsUnsupportedComplex_v) { @@ -1674,7 +1674,7 @@ namespace detail "[ADIOS2] Internal error: Failed reading attribute '" + name + "'."); } - *resource = attr.Data(); + resource = attr.Data(); } else if constexpr (auxiliary::IsArray_v) { @@ -1691,7 +1691,7 @@ namespace detail { res[i] = data[i]; } - *resource = res; + resource = res; } else if constexpr (std::is_same_v) { @@ -1707,7 +1707,7 @@ namespace detail "[ADIOS2] Internal error: Failed reading attribute '" + name + "'."); } - *resource = attr.Data()[0]; + resource = attr.Data()[0]; } return determineDatatype(); @@ -1892,7 +1892,7 @@ namespace detail template void DatasetOpener::call( ADIOS2IOHandlerImpl *impl, - InvalidatableFile file, + InvalidatableFile const &file, const std::string &varName, Parameter ¶meters) { @@ -3438,8 +3438,11 @@ ADIOS2IOHandler::ADIOS2IOHandler( std::string path, Access at, MPI_Comm comm, + // NOLINTNEXTLINE(performance-unnecessary-value-param) json::TracingJSON, + // NOLINTNEXTLINE(performance-unnecessary-value-param) std::string, + // NOLINTNEXTLINE(performance-unnecessary-value-param) std::string) : AbstractIOHandler(std::move(path), at, comm) {} @@ -3447,7 +3450,14 @@ ADIOS2IOHandler::ADIOS2IOHandler( #endif // openPMD_HAVE_MPI ADIOS2IOHandler::ADIOS2IOHandler( - std::string path, Access at, json::TracingJSON, std::string, std::string) + std::string path, + Access at, + // NOLINTNEXTLINE(performance-unnecessary-value-param) + json::TracingJSON, + // NOLINTNEXTLINE(performance-unnecessary-value-param) + std::string, + // NOLINTNEXTLINE(performance-unnecessary-value-param) + std::string) : AbstractIOHandler(std::move(path), at) {} diff --git a/src/IO/AbstractIOHandlerHelper.cpp b/src/IO/AbstractIOHandlerHelper.cpp index c6cd69f8a5..699dfd3619 100644 --- a/src/IO/AbstractIOHandlerHelper.cpp +++ b/src/IO/AbstractIOHandlerHelper.cpp @@ -69,7 +69,6 @@ std::unique_ptr createIOHandler( Access access, Format format, std::string originalExtension, - MPI_Comm comm, json::TracingJSON options, std::string const &pathAsItWasSpecifiedInTheConstructor) @@ -83,7 +82,7 @@ std::unique_ptr createIOHandler( case Format::ADIOS2_BP: return constructIOHandler( "ADIOS2", - path, + std::move(path), access, comm, std::move(options), @@ -92,7 +91,7 @@ std::unique_ptr createIOHandler( case Format::ADIOS2_BP4: return constructIOHandler( "ADIOS2", - path, + std::move(path), access, comm, std::move(options), @@ -101,7 +100,7 @@ std::unique_ptr createIOHandler( case Format::ADIOS2_BP5: return constructIOHandler( "ADIOS2", - path, + std::move(path), access, comm, std::move(options), @@ -110,7 +109,7 @@ std::unique_ptr createIOHandler( case Format::ADIOS2_SST: return constructIOHandler( "ADIOS2", - path, + std::move(path), access, comm, std::move(options), @@ -119,7 +118,7 @@ std::unique_ptr createIOHandler( case Format::ADIOS2_SSC: return constructIOHandler( "ADIOS2", - path, + std::move(path), access, comm, std::move(options), @@ -155,7 +154,7 @@ std::unique_ptr createIOHandler( case Format::ADIOS2_BP: return constructIOHandler( "ADIOS2", - path, + std::move(path), access, std::move(options), "file", @@ -163,7 +162,7 @@ std::unique_ptr createIOHandler( case Format::ADIOS2_BP4: return constructIOHandler( "ADIOS2", - path, + std::move(path), access, std::move(options), "bp4", @@ -171,7 +170,7 @@ std::unique_ptr createIOHandler( case Format::ADIOS2_BP5: return constructIOHandler( "ADIOS2", - path, + std::move(path), access, std::move(options), "bp5", @@ -179,7 +178,7 @@ std::unique_ptr createIOHandler( case Format::ADIOS2_SST: return constructIOHandler( "ADIOS2", - path, + std::move(path), access, std::move(options), "sst", @@ -187,7 +186,7 @@ std::unique_ptr createIOHandler( case Format::ADIOS2_SSC: return constructIOHandler( "ADIOS2", - path, + std::move(path), access, std::move(options), "ssc", @@ -195,7 +194,7 @@ std::unique_ptr createIOHandler( case Format::JSON: return constructIOHandler( "JSON", - path, + std::move(path), access, std::move(options), JSONIOHandlerImpl::FileFormat::Json, @@ -203,7 +202,7 @@ std::unique_ptr createIOHandler( case Format::TOML: return constructIOHandler( "JSON", - path, + std::move(path), access, std::move(options), JSONIOHandlerImpl::FileFormat::Toml, diff --git a/src/IO/AbstractIOHandlerImpl.cpp b/src/IO/AbstractIOHandlerImpl.cpp index af827704a1..01b489f4dd 100644 --- a/src/IO/AbstractIOHandlerImpl.cpp +++ b/src/IO/AbstractIOHandlerImpl.cpp @@ -38,7 +38,7 @@ AbstractIOHandlerImpl::AbstractIOHandlerImpl(AbstractIOHandler *handler) } void AbstractIOHandlerImpl::keepSynchronous( - Writable *writable, Parameter param) + Writable *writable, Parameter const ¶m) { writable->abstractFilePosition = param.otherWritable->abstractFilePosition; writable->written = true; diff --git a/src/IO/HDF5/HDF5Auxiliary.cpp b/src/IO/HDF5/HDF5Auxiliary.cpp index 53fb1fb390..75207720cd 100644 --- a/src/IO/HDF5/HDF5Auxiliary.cpp +++ b/src/IO/HDF5/HDF5Auxiliary.cpp @@ -314,7 +314,7 @@ std::string openPMD::concrete_h5_file_position(Writable *w) } std::vector openPMD::getOptimalChunkDims( - std::vector const dims, size_t const typeSize) + std::vector const &dims, size_t const typeSize) { auto const ndims = dims.size(); std::vector chunk_dims(dims.size()); diff --git a/src/IO/HDF5/HDF5IOHandler.cpp b/src/IO/HDF5/HDF5IOHandler.cpp index a0faac5fe8..e2fa63d9b3 100644 --- a/src/IO/HDF5/HDF5IOHandler.cpp +++ b/src/IO/HDF5/HDF5IOHandler.cpp @@ -2814,8 +2814,12 @@ std::future HDF5IOHandler::flush(internal::ParsedFlushParams &) return m_impl->flush(); } #else + HDF5IOHandler::HDF5IOHandler( - std::string path, Access at, json::TracingJSON /* config */) + std::string path, + Access at, + // NOLINTNEXTLINE(performance-unnecessary-value-param) + [[maybe_unused]] json::TracingJSON config) : AbstractIOHandler(std::move(path), at) { throw std::runtime_error("openPMD-api built without HDF5 support"); diff --git a/src/IO/HDF5/ParallelHDF5IOHandler.cpp b/src/IO/HDF5/ParallelHDF5IOHandler.cpp index f7a6dc1a1c..47a7764480 100644 --- a/src/IO/HDF5/ParallelHDF5IOHandler.cpp +++ b/src/IO/HDF5/ParallelHDF5IOHandler.cpp @@ -180,17 +180,25 @@ ParallelHDF5IOHandlerImpl::~ParallelHDF5IOHandlerImpl() } } #else + #if openPMD_HAVE_MPI ParallelHDF5IOHandler::ParallelHDF5IOHandler( - std::string path, Access at, MPI_Comm comm, json::TracingJSON /* config */) + std::string path, + Access at, + MPI_Comm comm, + // NOLINTNEXTLINE(performance-unnecessary-value-param) + [[maybe_unused]] json::TracingJSON config) : AbstractIOHandler(std::move(path), at, comm) { throw std::runtime_error("openPMD-api built without HDF5 support"); } #else ParallelHDF5IOHandler::ParallelHDF5IOHandler( - std::string path, Access at, json::TracingJSON /* config */) - : AbstractIOHandler(std::move(path), at) + std::string const &path, + Access at, + // NOLINTNEXTLINE(performance-unnecessary-value-param) + [[maybe_unused]] json::TracingJSON config) + : AbstractIOHandler(path, at) { throw std::runtime_error( "openPMD-api built without parallel support and without HDF5 support"); diff --git a/src/IO/InvalidatableFile.cpp b/src/IO/InvalidatableFile.cpp index 8d4b688673..4d512f8064 100644 --- a/src/IO/InvalidatableFile.cpp +++ b/src/IO/InvalidatableFile.cpp @@ -22,7 +22,7 @@ #include "openPMD/IO/InvalidatableFile.hpp" openPMD::InvalidatableFile::InvalidatableFile(std::string s) - : fileState{std::make_shared(s)} + : fileState{std::make_shared(std::move(s))} {} void openPMD::InvalidatableFile::invalidate() @@ -39,11 +39,11 @@ openPMD::InvalidatableFile &openPMD::InvalidatableFile::operator=(std::string s) { if (fileState) { - fileState->name = s; + fileState->name = std::move(s); } else { - fileState = std::make_shared(s); + fileState = std::make_shared(std::move(s)); } return *this; } diff --git a/src/IO/JSON/JSONIOHandler.cpp b/src/IO/JSON/JSONIOHandler.cpp index 7eb8a57278..041b236340 100644 --- a/src/IO/JSON/JSONIOHandler.cpp +++ b/src/IO/JSON/JSONIOHandler.cpp @@ -26,7 +26,7 @@ namespace openPMD JSONIOHandler::~JSONIOHandler() = default; JSONIOHandler::JSONIOHandler( - std::string path, + std::string const &path, Access at, openPMD::json::TracingJSON jsonCfg, JSONIOHandlerImpl::FileFormat format, diff --git a/src/IO/JSON/JSONIOHandlerImpl.cpp b/src/IO/JSON/JSONIOHandlerImpl.cpp index 73d50366f7..a4e1bb39ab 100644 --- a/src/IO/JSON/JSONIOHandlerImpl.cpp +++ b/src/IO/JSON/JSONIOHandlerImpl.cpp @@ -124,16 +124,14 @@ namespace JSONIOHandlerImpl::JSONIOHandlerImpl( AbstractIOHandler *handler, - openPMD::json::TracingJSON config, + // NOLINTNEXTLINE(performance-unnecessary-value-param) + [[maybe_unused]] openPMD::json::TracingJSON config, FileFormat format, std::string originalExtension) : AbstractIOHandlerImpl(handler) , m_fileFormat{format} , m_originalExtension{std::move(originalExtension)} -{ - // Currently unused - (void)config; -} +{} JSONIOHandlerImpl::~JSONIOHandlerImpl() = default; @@ -1011,13 +1009,13 @@ void JSONIOHandlerImpl::deregister( m_files.erase(writable); } -auto JSONIOHandlerImpl::getFilehandle(File fileName, Access access) +auto JSONIOHandlerImpl::getFilehandle(File const &fileName, Access access) -> std::tuple, std::istream *, std::ostream *> { VERIFY_ALWAYS( fileName.valid(), "[JSON] Tried opening a file that has been overwritten or deleted.") - auto path = fullPath(std::move(fileName)); + auto path = fullPath(fileName); auto fs = std::make_unique(); std::istream *istream = nullptr; std::ostream *ostream = nullptr; @@ -1057,7 +1055,7 @@ auto JSONIOHandlerImpl::getFilehandle(File fileName, Access access) return std::make_tuple(std::move(fs), istream, ostream); } -std::string JSONIOHandlerImpl::fullPath(File fileName) +std::string JSONIOHandlerImpl::fullPath(File const &fileName) { return fullPath(*fileName); } @@ -1189,7 +1187,8 @@ bool JSONIOHandlerImpl::hasKey(nlohmann::json const &j, KeyT &&key) return j.find(std::forward(key)) != j.end(); } -void JSONIOHandlerImpl::ensurePath(nlohmann::json *jsonp, std::string path) +void JSONIOHandlerImpl::ensurePath( + nlohmann::json *jsonp, std::string const &path) { auto groups = auxiliary::split(path, "/"); for (std::string &group : groups) @@ -1206,7 +1205,7 @@ void JSONIOHandlerImpl::ensurePath(nlohmann::json *jsonp, std::string path) } std::tuple::iterator, bool> -JSONIOHandlerImpl::getPossiblyExisting(std::string file) +JSONIOHandlerImpl::getPossiblyExisting(std::string const &file) { auto it = std::find_if( @@ -1233,7 +1232,8 @@ JSONIOHandlerImpl::getPossiblyExisting(std::string file) std::move(name), it, newlyCreated); } -std::shared_ptr JSONIOHandlerImpl::obtainJsonContents(File file) +std::shared_ptr +JSONIOHandlerImpl::obtainJsonContents(File const &file) { VERIFY_ALWAYS( file.valid(), @@ -1270,7 +1270,7 @@ nlohmann::json &JSONIOHandlerImpl::obtainJsonContents(Writable *writable) } void JSONIOHandlerImpl::putJsonContents( - File filename, + File const &filename, bool unsetDirty // = true ) { @@ -1305,8 +1305,8 @@ void JSONIOHandlerImpl::putJsonContents( } } -std::shared_ptr -JSONIOHandlerImpl::setAndGetFilePosition(Writable *writable, std::string extend) +std::shared_ptr JSONIOHandlerImpl::setAndGetFilePosition( + Writable *writable, std::string const &extend) { std::string path; if (writable->abstractFilePosition) @@ -1388,7 +1388,7 @@ bool JSONIOHandlerImpl::isDataset(nlohmann::json const &j) return i != j.end() && i.value().is_array(); } -bool JSONIOHandlerImpl::isGroup(nlohmann::json::const_iterator it) +bool JSONIOHandlerImpl::isGroup(nlohmann::json::const_iterator const &it) { auto &j = it.value(); if (it.key() == "attributes" || it.key() == "platform_byte_widths" || diff --git a/src/Iteration.cpp b/src/Iteration.cpp index ce35aee652..9e402f419d 100644 --- a/src/Iteration.cpp +++ b/src/Iteration.cpp @@ -372,7 +372,7 @@ void Iteration::reread(std::string const &path) } void Iteration::readFileBased( - std::string filePath, std::string const &groupPath, bool doBeginStep) + std::string const &filePath, std::string const &groupPath, bool doBeginStep) { if (doBeginStep) { diff --git a/src/ReadIterations.cpp b/src/ReadIterations.cpp index 41edd4ed05..9457a22421 100644 --- a/src/ReadIterations.cpp +++ b/src/ReadIterations.cpp @@ -123,11 +123,12 @@ void SeriesIterator::close() } SeriesIterator::SeriesIterator( - Series series_in, std::optional parsePreference) + Series const &series_in, + std::optional const &parsePreference) : m_data{std::make_shared>(std::in_place)} { auto &data = get(); - data.parsePreference = std::move(parsePreference); + data.parsePreference = parsePreference; /* * Since the iterator is stored in * internal::SeriesData::m_sharedStatefulIterator, @@ -628,7 +629,7 @@ ReadIterations::ReadIterations( Series series, Access access, std::optional parsePreference) - : m_series(std::move(series)), m_parsePreference(std::move(parsePreference)) + : m_series(std::move(series)), m_parsePreference(parsePreference) { auto &data = m_series.get(); if (access == Access::READ_LINEAR && !data.m_sharedStatefulIterator) diff --git a/src/Series.cpp b/src/Series.cpp index 7d9a8413fb..b7c807eb4c 100644 --- a/src/Series.cpp +++ b/src/Series.cpp @@ -508,7 +508,7 @@ namespace */ template int autoDetectPadding( - std::function isPartOfSeries, + std::function const &isPartOfSeries, std::string const &directory, MappingFunction &&mappingFunction) { @@ -539,11 +539,11 @@ namespace } int autoDetectPadding( - std::function isPartOfSeries, + std::function const &isPartOfSeries, std::string const &directory) { return autoDetectPadding( - std::move(isPartOfSeries), + isPartOfSeries, directory, [](Series::IterationIndex_t index, std::string const &filename) { (void)index; @@ -707,7 +707,7 @@ void Series::initDefaults(IterationEncoding ie, bool initAll) std::future Series::flush_impl( iterations_iterator begin, iterations_iterator end, - internal::FlushParams flushParams, + internal::FlushParams const &flushParams, bool flushIOHandler) { IOHandler()->m_lastFlushSuccessful = true; @@ -745,7 +745,7 @@ std::future Series::flush_impl( void Series::flushFileBased( iterations_iterator begin, iterations_iterator end, - internal::FlushParams flushParams, + internal::FlushParams const &flushParams, bool flushIOHandler) { auto &series = get(); @@ -862,7 +862,7 @@ void Series::flushFileBased( void Series::flushGorVBased( iterations_iterator begin, iterations_iterator end, - internal::FlushParams flushParams, + internal::FlushParams const &flushParams, bool flushIOHandler) { auto &series = get(); @@ -1020,7 +1020,7 @@ void Series::readFileBased() series.m_filenameExtension); int padding = autoDetectPadding( - std::move(isPartOfSeries), + isPartOfSeries, IOHandler()->directory, // foreach found file with `filename` and `index`: [&series](IterationIndex_t index, std::string const &filename) { @@ -1457,7 +1457,7 @@ creating new iterations. auto readSingleIteration = [&series, &pOpen, this]( IterationIndex_t index, - std::string path, + std::string const &path, bool guardAgainstRereading, bool beginStep) -> std::optional { if (series.iterations.contains(index)) @@ -2386,7 +2386,9 @@ ReadIterations Series::readIterations() // Use private constructor instead of copy constructor to avoid // object slicing return { - this->m_series, IOHandler()->m_frontendAccess, get().m_parsePreference}; + Series(this->m_series), + IOHandler()->m_frontendAccess, + get().m_parsePreference}; } void Series::parseBase() diff --git a/src/WriteIterations.cpp b/src/WriteIterations.cpp index 334df0ec3a..0ae7246ae0 100644 --- a/src/WriteIterations.cpp +++ b/src/WriteIterations.cpp @@ -75,7 +75,7 @@ WriteIterations::mapped_type &WriteIterations::operator[](key_type &&key) auto lastIteration_v = lastIteration.value(); if (lastIteration_v.iterationIndex == key) { - return s.iterations.at(std::move(key)); + return s.iterations.at(std::forward(key)); } else { @@ -83,7 +83,7 @@ WriteIterations::mapped_type &WriteIterations::operator[](key_type &&key) } } s.currentlyOpen = key; - auto &res = s.iterations[std::move(key)]; + auto &res = s.iterations[std::forward(key)]; if (res.getStepStatus() == StepStatus::NoStep) { try diff --git a/src/auxiliary/Filesystem.cpp b/src/auxiliary/Filesystem.cpp index 38d8e209f8..cce80b9d17 100644 --- a/src/auxiliary/Filesystem.cpp +++ b/src/auxiliary/Filesystem.cpp @@ -152,7 +152,8 @@ bool remove_directory(std::string const &path) #endif for (auto const &entry : list_directory(path)) { - std::string partialPath = path + directory_separator + entry; + auto partialPath = path; + partialPath.append(std::string(1, directory_separator)).append(entry); if (directory_exists(partialPath)) success &= remove_directory(partialPath); else if (file_exists(partialPath)) diff --git a/src/auxiliary/JSON.cpp b/src/auxiliary/JSON.cpp index 168cab7bf6..dd0825c33c 100644 --- a/src/auxiliary/JSON.cpp +++ b/src/auxiliary/JSON.cpp @@ -226,7 +226,7 @@ namespace res[pair.key()] = jsonToToml(pair.value(), currentPath); currentPath.pop_back(); } - return toml::value(std::move(res)); + return toml::value(res); } case nlohmann::json::value_t::array: { toml::value::array_type res; @@ -238,7 +238,7 @@ namespace res.emplace_back(jsonToToml(entry, currentPath)); currentPath.pop_back(); } - return toml::value(std::move(res)); + return toml::value(res); } case nlohmann::json::value_t::string: return val.get(); diff --git a/src/backend/Attributable.cpp b/src/backend/Attributable.cpp index 2d97dd0ff3..52ddda28d3 100644 --- a/src/backend/Attributable.cpp +++ b/src/backend/Attributable.cpp @@ -207,7 +207,7 @@ auto Attributable::myPath() const -> MyPath return res; } -void Attributable::seriesFlush(internal::FlushParams flushParams) +void Attributable::seriesFlush(internal::FlushParams const &flushParams) { writable().seriesFlush(flushParams); } @@ -311,7 +311,7 @@ void Attributable::readAttributes(ReadMode mode) } std::array arr; std::copy_n(vector.begin(), 7, arr.begin()); - setAttribute(key, std::move(arr)); + setAttribute(key, arr); } else { diff --git a/src/backend/Writable.cpp b/src/backend/Writable.cpp index f886e94046..97d13ce5f0 100644 --- a/src/backend/Writable.cpp +++ b/src/backend/Writable.cpp @@ -47,7 +47,7 @@ void Writable::seriesFlush(std::string backendConfig) seriesFlush({FlushLevel::UserFlush, std::move(backendConfig)}); } -void Writable::seriesFlush(internal::FlushParams flushParams) +void Writable::seriesFlush(internal::FlushParams const &flushParams) { auto series = Attributable({attributable, [](auto const *) {}}).retrieveSeries(); diff --git a/src/binding/python/Attributable.cpp b/src/binding/python/Attributable.cpp index 88dbb95cbc..a3fa63cdca 100644 --- a/src/binding/python/Attributable.cpp +++ b/src/binding/python/Attributable.cpp @@ -155,61 +155,61 @@ bool setAttributeFromBufferInfo( ) ); else */ // std::cout << "+++++++++++ BUFFER: " << buf.format << std::endl; - if (buf.format.find("b") != std::string::npos) + if (buf.format.find('b') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("h") != std::string::npos) + else if (buf.format.find('h') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("i") != std::string::npos) + else if (buf.format.find('i') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("l") != std::string::npos) + else if (buf.format.find('l') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("q") != std::string::npos) + else if (buf.format.find('q') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("B") != std::string::npos) + else if (buf.format.find('B') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("H") != std::string::npos) + else if (buf.format.find('H') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("I") != std::string::npos) + else if (buf.format.find('I') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("L") != std::string::npos) + else if (buf.format.find('L') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("Q") != std::string::npos) + else if (buf.format.find('Q') != std::string::npos) return attr.setAttribute( key, std::vector( @@ -234,19 +234,19 @@ bool setAttributeFromBufferInfo( static_cast *>(buf.ptr), static_cast *>(buf.ptr) + buf.size)); - else if (buf.format.find("f") != std::string::npos) + else if (buf.format.find('f') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("d") != std::string::npos) + else if (buf.format.find('d') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("g") != std::string::npos) + else if (buf.format.find('g') != std::string::npos) return attr.setAttribute( key, std::vector( @@ -349,7 +349,7 @@ bool setAttributeFromObject( py::object &obj, pybind11::dtype datatype) { - Datatype requestedDatatype = dtype_from_numpy(datatype); + Datatype requestedDatatype = dtype_from_numpy(std::move(datatype)); return switchNonVectorType( requestedDatatype, attr, key, obj); } @@ -401,7 +401,8 @@ void init_Attributable(py::module &m) std::string const &key, py::object &obj, pybind11::dtype datatype) { - return setAttributeFromObject(attr, key, obj, datatype); + return setAttributeFromObject( + attr, key, obj, std::move(datatype)); }, py::arg("key"), py::arg("value"), diff --git a/src/binding/python/ChunkInfo.cpp b/src/binding/python/ChunkInfo.cpp index d86a579905..86bcb0128a 100644 --- a/src/binding/python/ChunkInfo.cpp +++ b/src/binding/python/ChunkInfo.cpp @@ -61,7 +61,7 @@ void init_Chunk(py::module &m) }, // __setstate__ - [](py::tuple t) { + [](py::tuple const &t) { // our state tuple has exactly three values if (t.size() != 3) throw std::runtime_error("Invalid state!"); diff --git a/src/binding/python/Dataset.cpp b/src/binding/python/Dataset.cpp index a67d7f1221..656cd59ea8 100644 --- a/src/binding/python/Dataset.cpp +++ b/src/binding/python/Dataset.cpp @@ -32,8 +32,8 @@ void init_Dataset(py::module &m) .def(py::init(), py::arg("dtype"), py::arg("extent")) .def(py::init(), py::arg("extent")) .def( - py::init([](py::dtype dt, Extent e) { - auto const d = dtype_from_numpy(dt); + py::init([](py::dtype dt, Extent const &e) { + auto const d = dtype_from_numpy(std::move(dt)); return new Dataset{d, e}; }), py::arg("dtype"), @@ -44,8 +44,8 @@ void init_Dataset(py::module &m) py::arg("extent"), py::arg("options")) .def( - py::init([](py::dtype dt, Extent e, std::string options) { - auto const d = dtype_from_numpy(dt); + py::init([](py::dtype dt, Extent const &e, std::string options) { + auto const d = dtype_from_numpy(std::move(dt)); return new Dataset{d, e, std::move(options)}; }), py::arg("dtype"), diff --git a/src/binding/python/Datatype.cpp b/src/binding/python/Datatype.cpp index 9d53fba5ca..1a603855b0 100644 --- a/src/binding/python/Datatype.cpp +++ b/src/binding/python/Datatype.cpp @@ -58,7 +58,7 @@ void init_Datatype(py::module &m) .value("BOOL", Datatype::BOOL) .value("UNDEFINED", Datatype::UNDEFINED); - m.def("determine_datatype", [](py::dtype const dt) { + m.def("determine_datatype", [](py::dtype const &dt) { return dtype_from_numpy(dt); }); m.def("determine_datatype", [](py::array const &a) { diff --git a/src/binding/python/PatchRecordComponent.cpp b/src/binding/python/PatchRecordComponent.cpp index 08059826e7..33b04e733c 100644 --- a/src/binding/python/PatchRecordComponent.cpp +++ b/src/binding/python/PatchRecordComponent.cpp @@ -95,7 +95,7 @@ void init_PatchRecordComponent(py::module &m) // all buffer types .def( "store", - [](PatchRecordComponent &prc, uint64_t idx, py::buffer a) { + [](PatchRecordComponent &prc, uint64_t idx, py::buffer const &a) { py::buffer_info buf = a.request(); auto const dtype = dtype_from_bufferformat(buf.format); diff --git a/src/binding/python/RecordComponent.cpp b/src/binding/python/RecordComponent.cpp index f5ef98f6f6..9b8154e906 100644 --- a/src/binding/python/RecordComponent.cpp +++ b/src/binding/python/RecordComponent.cpp @@ -932,7 +932,7 @@ void init_RecordComponent(py::module &m) .def( "make_empty", [](RecordComponent &rc, - pybind11::dtype const dt, + pybind11::dtype const &dt, uint8_t dimensionality) { return rc.makeEmpty(dtype_from_numpy(dt), dimensionality); }) @@ -1114,7 +1114,8 @@ void init_RecordComponent(py::module &m) py::arg_v("extent", Extent(1, -1u), "array.shape")) .def_property_readonly_static( - "SCALAR", [](py::object) { return RecordComponent::SCALAR; }) + "SCALAR", + [](py::object const &) { return RecordComponent::SCALAR; }) // TODO remove in future versions (deprecated) .def("set_unit_SI", &RecordComponent::setUnitSI) // deprecated diff --git a/src/cli/ls.cpp b/src/cli/ls.cpp index 65bb226602..3f13770f56 100644 --- a/src/cli/ls.cpp +++ b/src/cli/ls.cpp @@ -27,6 +27,7 @@ int main(int argc, char *argv[]) { std::vector str_argv; + str_argv.reserve(argc); for (int i = 0; i < argc; ++i) str_argv.emplace_back(argv[i]); diff --git a/src/version.cpp b/src/version.cpp index 6fa0a9ecfa..c2e8809a32 100644 --- a/src/version.cpp +++ b/src/version.cpp @@ -30,8 +30,7 @@ std::string openPMD::getVersion() << OPENPMDAPI_VERSION_PATCH; if (std::string(OPENPMDAPI_VERSION_LABEL).size() > 0) api << "-" << OPENPMDAPI_VERSION_LABEL; - std::string const apistr = api.str(); - return apistr; + return api.str(); } std::string openPMD::getStandard() @@ -39,8 +38,7 @@ std::string openPMD::getStandard() std::stringstream standard; standard << OPENPMD_STANDARD_MAJOR << "." << OPENPMD_STANDARD_MINOR << "." << OPENPMD_STANDARD_PATCH; - std::string const standardstr = standard.str(); - return standardstr; + return standard.str(); } std::string openPMD::getStandardMinimum() @@ -49,6 +47,5 @@ std::string openPMD::getStandardMinimum() standardMin << OPENPMD_STANDARD_MIN_MAJOR << "." << OPENPMD_STANDARD_MIN_MINOR << "." << OPENPMD_STANDARD_MIN_PATCH; - std::string const standardMinstr = standardMin.str(); - return standardMinstr; + return standardMin.str(); } diff --git a/test/AuxiliaryTest.cpp b/test/AuxiliaryTest.cpp index 491bb37794..7cc88f3b65 100644 --- a/test/AuxiliaryTest.cpp +++ b/test/AuxiliaryTest.cpp @@ -64,6 +64,7 @@ TEST_CASE("deref_cast_test", "[auxiliary]") B const value = {123.45}; B const *const ptr = &value; + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) auto const a = deref_dynamic_cast(ptr); auto const &ra = deref_dynamic_cast(ptr); (void)a; @@ -175,7 +176,7 @@ struct structure : public TestHelper } structure &setText(std::string newText) { - setAttribute("text", newText); + setAttribute("text", std::move(newText)); return *this; } }; @@ -308,7 +309,7 @@ struct AttributedWidget : public TestHelper AttributedWidget() : TestHelper() {} - Attribute::resource get(std::string key) + Attribute::resource get(std::string const &key) { return getAttribute(key).getResource(); } @@ -380,7 +381,7 @@ struct Dotty : public TestHelper } Dotty &setAtt3(std::string s) { - setAttribute("att3", s); + setAttribute("att3", std::move(s)); return *this; } }; diff --git a/test/CoreTest.cpp b/test/CoreTest.cpp index f6f93e065c..58505ec85f 100644 --- a/test/CoreTest.cpp +++ b/test/CoreTest.cpp @@ -49,6 +49,7 @@ TEST_CASE("attribute_dtype_test", "[core]") REQUIRE(Datatype::CHAR == a.dtype); { // check that copy constructor works + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) [[maybe_unused]] Attribute b = a; } { diff --git a/test/ParallelIOTest.cpp b/test/ParallelIOTest.cpp index d884766f15..eb9765375a 100644 --- a/test/ParallelIOTest.cpp +++ b/test/ParallelIOTest.cpp @@ -75,9 +75,8 @@ TEST_CASE("parallel_multi_series_test", "[parallel]") // have multiple serial series alive at the same time for (auto const sn : {1, 2, 3}) { - for (auto const &t : myBackends) + for (auto const &file_ending : myBackends) { - auto const file_ending = t; std::cout << file_ending << std::endl; allSeries.emplace_back( std::string("../samples/parallel_multi_open_test_") @@ -115,7 +114,7 @@ TEST_CASE("parallel_multi_series_test", "[parallel]") void write_test_zero_extent( bool fileBased, - std::string file_ending, + std::string const &file_ending, bool writeAllChunks, bool declareFromAll) { @@ -379,7 +378,7 @@ TEST_CASE("no_parallel_hdf5", "[parallel][hdf5]") #endif #if openPMD_HAVE_ADIOS2 && openPMD_HAVE_MPI -void available_chunks_test(std::string file_ending) +void available_chunks_test(std::string const &file_ending) { int r_mpi_rank{-1}, r_mpi_size{-1}; MPI_Comm_rank(MPI_COMM_WORLD, &r_mpi_rank); @@ -628,7 +627,7 @@ TEST_CASE("hzdr_adios_sample_content_test", "[parallel][adios2][bp3]") #endif #if openPMD_HAVE_MPI -void write_4D_test(std::string file_ending) +void write_4D_test(std::string const &file_ending) { int mpi_s{-1}; int mpi_r{-1}; @@ -662,7 +661,7 @@ TEST_CASE("write_4D_test", "[parallel]") } } -void write_makeconst_some(std::string file_ending) +void write_makeconst_some(std::string const &file_ending) { int mpi_s{-1}; int mpi_r{-1}; @@ -695,7 +694,7 @@ TEST_CASE("write_makeconst_some", "[parallel]") } } -void close_iteration_test(std::string file_ending) +void close_iteration_test(std::string const &file_ending) { int i_mpi_rank{-1}, i_mpi_size{-1}; MPI_Comm_rank(MPI_COMM_WORLD, &i_mpi_rank); @@ -765,7 +764,7 @@ TEST_CASE("close_iteration_test", "[parallel]") } } -void file_based_write_read(std::string file_ending) +void file_based_write_read(std::string const &file_ending) { namespace io = openPMD; @@ -872,7 +871,7 @@ TEST_CASE("file_based_write_read", "[parallel]") } } -void hipace_like_write(std::string file_ending) +void hipace_like_write(std::string const &file_ending) { namespace io = openPMD; @@ -1397,7 +1396,7 @@ void append_mode( std::string const &extension, bool variableBased, ParseMode parseMode, - std::string jsonConfig = "{}") + std::string const &jsonConfig = "{}") { std::string filename = (variableBased ? "../samples/append/append_variablebased." @@ -1415,7 +1414,7 @@ void append_mode( std::vector data(10, 999); auto writeSomeIterations = [&data, mpi_size, mpi_rank]( WriteIterations &&writeIterations, - std::vector indices) { + std::vector const &indices) { for (auto index : indices) { auto it = writeIterations[index]; diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp index 2ab428c83f..52ee3c387a 100644 --- a/test/SerialIOTest.cpp +++ b/test/SerialIOTest.cpp @@ -67,7 +67,7 @@ std::vector testedBackends() if (lookup != extensions.end()) { std::string extension = lookup->second; - res.push_back({std::move(pair.first), std::move(extension)}); + res.push_back({pair.first, std::move(extension)}); } } } @@ -237,9 +237,8 @@ TEST_CASE("multi_series_test", "[serial]") // have multiple serial series alive at the same time for (auto const sn : {1, 2, 3}) { - for (auto const &t : myfileExtensions) + for (auto const &file_ending : myfileExtensions) { - auto const file_ending = t; std::cout << file_ending << std::endl; allSeries.emplace_back( std::string("../samples/multi_open_test_") @@ -414,7 +413,7 @@ TEST_CASE("multiple_series_handles_test", "[serial]") #endif } -void close_iteration_test(std::string file_ending) +void close_iteration_test(std::string const &file_ending) { std::string name = "../samples/close_iterations_%T." + file_ending; @@ -500,7 +499,7 @@ TEST_CASE("close_iteration_test", "[serial]") } void close_iteration_interleaved_test( - std::string const file_ending, IterationEncoding const it_encoding) + std::string const &file_ending, IterationEncoding const it_encoding) { std::string name = "../samples/close_iterations_interleaved_"; if (it_encoding == IterationEncoding::fileBased) @@ -597,12 +596,18 @@ struct NonCopyableDeleter : std::function {} NonCopyableDeleter(NonCopyableDeleter const &) = delete; NonCopyableDeleter &operator=(NonCopyableDeleter const &) = delete; + /* + * MSVC does not recognize these when declaring noexcept and this is + * for a test only anyway. + */ + // NOLINTNEXTLINE(performance-noexcept-move-constructor) NonCopyableDeleter(NonCopyableDeleter &&) = default; + // NOLINTNEXTLINE(performance-noexcept-move-constructor) NonCopyableDeleter &operator=(NonCopyableDeleter &&) = default; }; } // namespace detail -void close_and_copy_attributable_test(std::string file_ending) +void close_and_copy_attributable_test(std::string const &file_ending) { using position_t = int; @@ -794,7 +799,7 @@ TEST_CASE("close_iteration_throws_test", "[serial]") } #endif -inline void empty_dataset_test(std::string file_ending) +inline void empty_dataset_test(std::string const &file_ending) { { Series series( @@ -898,7 +903,7 @@ TEST_CASE("empty_dataset_test", "[serial]") } } -inline void constant_scalar(std::string file_ending) +inline void constant_scalar(std::string const &file_ending) { Mesh::Geometry const geometry = Mesh::Geometry::spherical; std::string const geometryParameters = "dummyGeometryParameters"; @@ -1170,7 +1175,7 @@ TEST_CASE("flush_without_position_positionOffset", "[serial]") } } -inline void particle_patches(std::string file_ending) +inline void particle_patches(std::string const &file_ending) { constexpr auto SCALAR = openPMD::RecordComponent::SCALAR; @@ -2176,7 +2181,7 @@ TEST_CASE("fileBased_write_test", "[serial]") } } -inline void sample_write_thetaMode(std::string file_ending) +inline void sample_write_thetaMode(std::string const &file_ending) { Series o = Series( std::string("../samples/thetaMode_%05T.").append(file_ending), @@ -2512,8 +2517,8 @@ inline void optional_paths_110_test(const std::string &backend) } void git_early_chunk_query( - std::string const filename, - std::string const species, + std::string const &filename, + std::string const &species, int const step, std::string const &jsonConfig = "{}") { @@ -5839,7 +5844,7 @@ void variableBasedParticleData() 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( Offset(rc.getDimensionality(), 0), rc.getExtent()); @@ -5850,7 +5855,6 @@ void variableBasedParticleData() for (size_t i = 0; i < 3; ++i) { - std::string dim = dimensions[i]; Extent const &extent = extents[i]; auto chunk = loadedChunks[i]; for (size_t j = 0; j < extent[0]; ++j) @@ -6041,7 +6045,9 @@ TEST_CASE("automatically_deactivate_span", "[serial][adios2]") // @todo Upon switching to ADIOS2 2.7.0, test this the other way around also void iterate_nonstreaming_series( - std::string const &file, bool variableBasedLayout, std::string jsonConfig) + std::string const &file, + bool variableBasedLayout, + std::string const &jsonConfig) { constexpr size_t extent = 100; { @@ -6461,9 +6467,10 @@ void deferred_parsing(std::string const &extension) { padding += "0"; } - infix = padding + infix; + infix = padding.append(infix); std::ofstream file; - file.open(basename + infix + "." + extension); + file.open(std::string(basename).append(infix).append(".").append( + extension)); file.close(); } } @@ -6850,7 +6857,7 @@ TEST_CASE("late_setting_of_iterationencoding", "[serial]") auxiliary::file_exists("../samples/change_name_and_encoding_10.json")); } -void varying_pattern(std::string const file_ending) +void varying_pattern(std::string const &file_ending) { { std::string filename = "../samples/varying_pattern_%06T." + file_ending; @@ -6938,7 +6945,7 @@ void append_mode( std::string const &filename, bool variableBased, ParseMode parseMode, - std::string jsonConfig = "{}") + std::string const &jsonConfig = "{}") { if (auxiliary::directory_exists("../samples/append")) { @@ -6947,7 +6954,7 @@ void append_mode( std::vector data(10, 999); auto writeSomeIterations = [&data]( WriteIterations &&writeIterations, - std::vector indices) { + std::vector const &indices) { for (auto index : indices) { auto it = writeIterations[index]; @@ -7314,7 +7321,7 @@ void append_mode_filebased(std::string const &extension) } })END"; auto writeSomeIterations = [](WriteIterations &&writeIterations, - std::vector indices) { + std::vector const &indices) { for (auto index : indices) { auto it = writeIterations[index];