Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add all the performance-* clang-tidy checks #1532

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
ff828a8
add all the performance-* clang-tidy checks
lucafedeli88 Oct 4, 2023
05b956e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 4, 2023
495cef0
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 4, 2023
e400d5e
address residual issues found with clang-tidy
lucafedeli88 Oct 4, 2023
8e2c236
enforce east const
lucafedeli88 Oct 4, 2023
bbd8f26
Merge branch 'clang_tidy_all_performance_checks' of github.com:/lucaf…
lucafedeli88 Oct 4, 2023
36e49d1
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 4, 2023
dd8dabe
fix bug
lucafedeli88 Oct 5, 2023
3633203
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 5, 2023
0d9f2f4
cleaning
lucafedeli88 Oct 5, 2023
7bdd53b
Merge branch 'clang_tidy_all_performance_checks' of github.com:lucafe…
lucafedeli88 Oct 5, 2023
99a98f7
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 5, 2023
def0ed9
remove unused variable
lucafedeli88 Oct 5, 2023
80f3bbc
Merge branch 'clang_tidy_all_performance_checks' of github.com:lucafe…
lucafedeli88 Oct 5, 2023
d216aad
Fix some corrections that were too ambitious
franzpoeschel Oct 20, 2023
d92d69a
Some little forgotten clang-tidy checks
franzpoeschel Nov 9, 2023
55976fc
Merge branch 'dev' into clang_tidy_all_performance_checks
franzpoeschel Nov 9, 2023
6aba6b8
Fixes after merging the dev
franzpoeschel Nov 9, 2023
21ecbc9
Replace auxiliary::forget() with NOLINT comments
franzpoeschel Nov 10, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -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$)'
4 changes: 2 additions & 2 deletions examples/10_streaming_read.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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];
Expand Down
5 changes: 3 additions & 2 deletions examples/8_benchmark_parallel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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";
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion include/openPMD/Datatype.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
5 changes: 3 additions & 2 deletions include/openPMD/Error.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};

/**
Expand All @@ -62,7 +63,7 @@ namespace error
class WrongAPIUsage : public Error
{
public:
WrongAPIUsage(std::string what);
WrongAPIUsage(std::string const &what);
};

class BackendConfigSchema : public Error
Expand Down
11 changes: 6 additions & 5 deletions include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 &&...);
Expand All @@ -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()";
Expand Down
4 changes: 2 additions & 2 deletions include/openPMD/IO/AbstractIOHandlerImpl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
2 changes: 1 addition & 1 deletion include/openPMD/IO/HDF5/HDF5Auxiliary.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 2 additions & 1 deletion include/openPMD/IO/HDF5/ParallelHDF5IOHandler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion include/openPMD/IO/JSON/JSONIOHandler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
18 changes: 9 additions & 9 deletions include/openPMD/IO/JSON/JSONIOHandlerImpl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ struct File
return fileState->valid;
}

File &operator=(std::string s)
File &operator=(std::string const &s)
{
if (fileState)
{
Expand Down Expand Up @@ -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 &);

Expand Down Expand Up @@ -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)
Expand All @@ -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);

Expand Down
4 changes: 3 additions & 1 deletion include/openPMD/Iteration.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion include/openPMD/ReadIterations.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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++();

Expand Down
14 changes: 10 additions & 4 deletions include/openPMD/Series.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions include/openPMD/ThrowError.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion include/openPMD/backend/Attributable.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ OPENPMD_protected
Iteration &containingIteration();
/** @} */

void seriesFlush(internal::FlushParams);
void seriesFlush(internal::FlushParams const &);

void flushAttributes(internal::FlushParams const &);

Expand Down
2 changes: 1 addition & 1 deletion include/openPMD/backend/Writable.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions include/openPMD/cli/ls.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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";
Expand Down
10 changes: 5 additions & 5 deletions src/Dataset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{}
Expand Down
2 changes: 1 addition & 1 deletion src/Datatype.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
11 changes: 5 additions & 6 deletions src/Error.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{}

Expand Down
Loading