clang-tidy gespräche

franzpoeschel · franzpoeschel · commit 370fddf53718 · 2025-09-10T14:58:27.000+02:00
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-*,-performance-avoid-endl,modernize-*,-modernize-use-trailing-return-type,-modernize-use-bool-literals,-modernize-avoid-c-arrays,-modernize-use-auto,-modernize-return-braced-init-list,-bugprone-easily-swappable-parameters
+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-*,-performance-avoid-endl,modernize-*,-modernize-use-trailing-return-type,-modernize-use-bool-literals,-modernize-avoid-c-arrays,-modernize-use-auto,-modernize-return-braced-init-list,-bugprone-easily-swappable-parameters,-bugprone-chained-comparison
 HeaderFilterRegex: '((^(?!\/share\/openPMD\/).*)*include\/openPMD\/.+\.hpp|src\/^(?!binding).+\.cpp$)'
diff --git a/examples/8b_benchmark_read_parallel.cpp b/examples/8b_benchmark_read_parallel.cpp
@@ -135,7 +135,6 @@ class Timer
     ~Timer()
     {
         MPI_Barrier(MPI_COMM_WORLD);
-        std::string tt = "~" + m_Tag;
         // MemoryProfiler (m_Rank, tt.c_str());
         m_End = std::chrono::system_clock::now();
 
diff --git a/include/openPMD/IO/JSON/JSONIOHandlerImpl.hpp b/include/openPMD/IO/JSON/JSONIOHandlerImpl.hpp
@@ -525,9 +525,7 @@ class JSONIOHandlerImpl : public AbstractIOHandlerImpl
     };
 
     template <typename T>
-    struct JsonToCpp<
-        T,
-        typename std::enable_if<std::is_floating_point<T>::value>::type>
+    struct JsonToCpp<T, typename std::enable_if_t<std::is_floating_point_v<T>>>
     {
         T operator()(nlohmann::json const &);
     };
diff --git a/include/openPMD/backend/BaseRecord.hpp b/include/openPMD/backend/BaseRecord.hpp
@@ -317,7 +317,7 @@ class BaseRecord
     template <class InputIt>
     void insert(InputIt first, InputIt last);
     void insert(std::initializer_list<value_type> ilist);
-    void swap(BaseRecord &other);
+    void swap(BaseRecord &other) noexcept;
     bool contains(key_type const &key) const;
     template <class... Args>
     auto emplace(Args &&...args) -> std::pair<iterator, bool>;
diff --git a/src/IO/AbstractIOHandlerHelper.cpp b/src/IO/AbstractIOHandlerHelper.cpp
@@ -81,7 +81,7 @@ std::unique_ptr<AbstractIOHandler> createIOHandler<json::TracingJSON>(
         return constructIOHandler<ParallelHDF5IOHandler, openPMD_HAVE_HDF5>(
             "HDF5",
             std::move(initialize_from),
-            path,
+            std::move(path),
             access,
             comm,
             std::move(options));
@@ -139,7 +139,7 @@ std::unique_ptr<AbstractIOHandler> createIOHandler<json::TracingJSON>(
         return constructIOHandler<JSONIOHandler, openPMD_HAVE_JSON>(
             "JSON",
             std::move(initialize_from),
-            path,
+            std::move(path),
             access,
             comm,
             std::move(options),
@@ -149,7 +149,7 @@ std::unique_ptr<AbstractIOHandler> createIOHandler<json::TracingJSON>(
         return constructIOHandler<JSONIOHandler, openPMD_HAVE_JSON>(
             "JSON",
             std::move(initialize_from),
-            path,
+            std::move(path),
             access,
             comm,
             std::move(options),
@@ -181,7 +181,7 @@ std::unique_ptr<AbstractIOHandler> createIOHandler<json::TracingJSON>(
         return constructIOHandler<HDF5IOHandler, openPMD_HAVE_HDF5>(
             "HDF5",
             std::move(initialize_from),
-            path,
+            std::move(path),
             access,
             std::move(options));
     case Format::ADIOS2_BP:
diff --git a/src/IO/JSON/JSONIOHandlerImpl.cpp b/src/IO/JSON/JSONIOHandlerImpl.cpp
@@ -1203,13 +1203,13 @@ void JSONIOHandlerImpl::writeAttribute(
     switch (m_attributeMode.m_mode)
     {
     case AttributeMode::Long:
-        (*jsonVal)[filePosition->id]["attributes"][parameter.name] = {
+        (*jsonVal)[filePosition->id]["attributes"][name] = {
             {"datatype", jsonDatatypeToString(parameter.dtype)},
             {"value", value}};
         break;
     case AttributeMode::Short:
         // short form
-        (*jsonVal)[filePosition->id]["attributes"][parameter.name] = value;
+        (*jsonVal)[filePosition->id]["attributes"][name] = value;
         break;
     }
     writable->written = true;
@@ -1540,7 +1540,6 @@ void JSONIOHandlerImpl::readAttribute(
     auto const &jsonContents = obtainJsonContents(writable);
     auto const &jsonLoc = jsonContents["attributes"];
     setAndGetFilePosition(writable);
-    std::string error_msg("[JSON] No such attribute '");
     if (!hasKey(jsonLoc, name))
     {
         throw error::ReadError(
@@ -2365,9 +2364,9 @@ nlohmann::json JSONIOHandlerImpl::platformSpecifics()
         Datatype::CDOUBLE,
         Datatype::CLONG_DOUBLE,
         Datatype::BOOL};
-    for (auto it = std::begin(datatypes); it != std::end(datatypes); it++)
+    for (auto &datatype : datatypes)
     {
-        res[jsonDatatypeToString(*it)] = toBytes(*it);
+        res[jsonDatatypeToString(datatype)] = toBytes(datatype);
     }
     return res;
 }
@@ -2483,10 +2482,9 @@ std::array<T, n> JSONIOHandlerImpl::JsonToCpp<std::array<T, n>>::operator()(
 }
 
 template <typename T>
-T JSONIOHandlerImpl::JsonToCpp<
-    T,
-    typename std::enable_if<std::is_floating_point<T>::value>::type>::
-operator()(nlohmann::json const &j)
+T JSONIOHandlerImpl::
+    JsonToCpp<T, std::enable_if_t<std::is_floating_point_v<T>>>::operator()(
+        nlohmann::json const &j)
 {
     try
     {
diff --git a/src/RecordComponent.cpp b/src/RecordComponent.cpp
@@ -132,6 +132,12 @@ RecordComponent &RecordComponent::resetDataset(Dataset d)
     rc.m_isEmpty = false;
     if (written())
     {
+        if (!rc.m_dataset.has_value())
+        {
+            throw error::Internal(
+                "Internal control flow error: Written record component must "
+                "have defined datatype and extent.");
+        }
         rc.m_dataset.value().extend(std::move(d.extent));
     }
     else
diff --git a/src/Series.cpp b/src/Series.cpp
@@ -1092,6 +1092,12 @@ void Series::initSeries(
     series.m_filenamePrefix = input->filenamePrefix;
     series.m_filenamePostfix = input->filenamePostfix;
     series.m_filenamePadding = input->filenamePadding;
+    if (!input->filenameExtension)
+    {
+        throw error::Internal(
+            "Control flow error: filename extension has not been resolved.");
+    }
+    // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
     series.m_filenameExtension = input->filenameExtension.value();
 
     if (series.m_iterationEncoding == IterationEncoding::fileBased &&
@@ -1704,7 +1710,8 @@ void Series::readFileBased(
         {
             if (forwardFirstError.has_value())
             {
-                auto &firstError = forwardFirstError.value();
+                // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
+                auto &firstError = *forwardFirstError;
                 firstError.description.append(
                     "\n[Note] Not a single iteration can be successfully "
                     "parsed (see above errors). Returning the first observed "
@@ -3406,7 +3413,7 @@ auto Series::currentSnapshot() -> std::optional<std::vector<IterationIndex_t>>
         auto res = attribute.getOptional<vec_t>();
         if (res.has_value())
         {
-            return res.value();
+            return res;
         }
         else
         {
@@ -3429,6 +3436,7 @@ AbstractIOHandler *Series::runDeferredInitialization()
     auto &series = get();
     if (series.m_deferred_initialization.has_value())
     {
+        // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
         auto functor = std::move(*m_series->m_deferred_initialization);
         m_series->m_deferred_initialization = std::nullopt;
         return functor(*this);
diff --git a/src/auxiliary/Memory.cpp b/src/auxiliary/Memory.cpp
@@ -40,7 +40,8 @@ allocatePtr(Datatype dtype, uint64_t numPoints)
     {
         using DT = Datatype;
     case DT::VEC_STRING:
-        data = new char *[numPoints];
+        // NOLINTNEXTLINE(bugprone-multi-level-implicit-pointer-conversion)
+        data = static_cast<void *>(new char *[numPoints]);
         del = [](void *p) { delete[] static_cast<char **>(p); };
         break;
     case DT::VEC_LONG_DOUBLE:
diff --git a/src/backend/Attribute.cpp b/src/backend/Attribute.cpp
@@ -73,6 +73,8 @@ auto Attribute::get_impl() const -> std::variant<U, std::runtime_error>
     switch (index)
     {
         OPENPMD_FOREACH_DATATYPE(OPENPMD_ENUMERATE_TYPES)
+    default:
+        return {std::runtime_error("Unreachable!")};
     }
 #undef OPENPMD_ENUMERATE_TYPES
 
diff --git a/src/backend/BaseRecord.cpp b/src/backend/BaseRecord.cpp
@@ -744,7 +744,7 @@ auto BaseRecord<T_elem>::insert(std::initializer_list<value_type> ilist) -> void
 }
 
 template <typename T_elem>
-auto BaseRecord<T_elem>::swap(BaseRecord &other) -> void
+auto BaseRecord<T_elem>::swap(BaseRecord &other) noexcept -> void
 {
     detail::verifyNonscalar(this);
     detail::verifyNonscalar(&other);
diff --git a/src/backend/Writable.cpp b/src/backend/Writable.cpp
@@ -33,7 +33,12 @@ Writable::Writable(internal::AttributableData *a) : attributable{a}
 
 Writable::~Writable()
 {
-    if (!IOHandler || !IOHandler->has_value())
+    if (!IOHandler)
+    {
+        return;
+    }
+    auto &optional = *IOHandler;
+    if (!optional.has_value())
     {
         return;
     }
@@ -42,7 +47,7 @@ Writable::~Writable()
      * The DEREGISTER task must not dereference the pointer, but only use it to
      * remove references to this object from internal data structures.
      */
-    IOHandler->value()->enqueue(
+    (*optional)->enqueue(
         IOTask(this, Parameter<Operation::DEREGISTER>(parent)));
 }
 
diff --git a/src/binding/python/RecordComponent.cpp b/src/binding/python/RecordComponent.cpp
@@ -202,6 +202,7 @@ inline std::tuple<Offset, Extent, std::vector<bool>> parseTupleSlices(
 
             continue;
         }
+        // NOLINTNEXTLINE(bugprone-empty-catch)
         catch (const py::cast_error &e)
         {
             // not an index
diff --git a/src/cli/convert-toml-json.cpp b/src/cli/convert-toml-json.cpp
@@ -13,6 +13,7 @@ void parsed_main(std::string jsonOrToml)
     auto [config, originallySpecifiedAs] = json::parseOptions(
         jsonOrToml, /* considerFiles = */ true, /* convertLowercase = */ false);
     {
+        // NOLINTNEXTLINE(bugprone-unused-local-non-trivial-variable)
         [[maybe_unused]] auto _ = std::move(jsonOrToml);
     }
     switch (originallySpecifiedAs)
diff --git a/src/snapshots/ContainerImpls.cpp b/src/snapshots/ContainerImpls.cpp
@@ -195,12 +195,18 @@ auto StatefulSnapshotsContainer::operator[](key_type const &key)
 {
     auto base_iterator = get();
     auto &shared = base_iterator->m_data;
-    if (!shared || !shared->has_value())
+    if (!shared)
     {
         throw error::WrongAPIUsage(
             "[WriteIterations] Trying to access after closing Series.");
     }
-    auto &s = shared->value();
+    auto &optional = *shared;
+    if (!optional.has_value())
+    {
+        throw error::WrongAPIUsage(
+            "[WriteIterations] Trying to access after closing Series.");
+    }
+    auto &s = optional.value();
     auto access = s.series.IOHandler()->m_frontendAccess;
 
     if (access == Access::READ_WRITE)
diff --git a/src/snapshots/StatefulIterator.cpp b/src/snapshots/StatefulIterator.cpp
@@ -172,10 +172,14 @@ StatefulIterator::operator=(StatefulIterator &&other) noexcept = default;
 
 auto StatefulIterator::get() -> SharedData &
 {
+    // forward the std::optional exception in doubt
+    // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
     return m_data->value();
 }
 auto StatefulIterator::get() const -> SharedData const &
 {
+    // forward the std::optional exception in doubt
+    // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
     return m_data->value();
 }
 
@@ -346,11 +350,16 @@ auto StatefulIterator::resetCurrentIterationToBegin(
 auto StatefulIterator::peekCurrentlyOpenIteration() const
     -> std::optional<value_type const *>
 {
-    if (!m_data || !m_data->has_value())
+    if (!m_data)
     {
         return std::nullopt;
     }
-    auto &s = m_data->value();
+    auto &optional = *m_data;
+    if (!optional.has_value())
+    {
+        return std::nullopt;
+    }
+    auto &s = optional.value();
     auto const &maybeCurrentIteration = s.currentIteration();
     if (!maybeCurrentIteration.has_value())
     {
@@ -938,15 +947,20 @@ StatefulIterator StatefulIterator::end()
 
 auto StatefulIterator::is_end() const -> bool
 {
-    return !m_data || !m_data->has_value() ||
+    if (!m_data)
+    {
+        return true;
+    }
+    auto &optional = *m_data;
+    return !optional.has_value() ||
         std::visit(
             auxiliary::overloaded{
                 [](CurrentStep::Before_t const &) { return false; },
                 [](CurrentStep::During_t const &during) {
                     return !during.iteration_idx.has_value();
                 },
                 [](CurrentStep::After_t const &) { return true; }},
-            (**m_data).currentStep.as_base());
+            optional->currentStep.as_base());
 }
 
 auto StatefulIterator::assert_end_iterator() const -> void

Original file line number	Diff line number	Diff line change
`@@ -135,7 +135,6 @@ class Timer`
`135`	`135`	`~Timer()`
`136`	`136`	`{`
`137`	`137`	`MPI_Barrier(MPI_COMM_WORLD);`
`138`		`- std::string tt = "~" + m_Tag;`
`139`	`138`	`// MemoryProfiler (m_Rank, tt.c_str());`
`140`	`139`	`m_End = std::chrono::system_clock::now();`
`141`	`140`
Original file line number	Diff line number	Diff line change
`@@ -1203,13 +1203,13 @@ void JSONIOHandlerImpl::writeAttribute(`
`1203`	`1203`	`switch (m_attributeMode.m_mode)`
`1204`	`1204`	`{`
`1205`	`1205`	`case AttributeMode::Long:`
`1206`		`- (*jsonVal)[filePosition->id]["attributes"][parameter.name] = {`
	`1206`	`+ (*jsonVal)[filePosition->id]["attributes"][name] = {`
`1207`	`1207`	`{"datatype", jsonDatatypeToString(parameter.dtype)},`
`1208`	`1208`	`{"value", value}};`
`1209`	`1209`	`break;`
`1210`	`1210`	`case AttributeMode::Short:`
`1211`	`1211`	`// short form`
`1212`		`- (*jsonVal)[filePosition->id]["attributes"][parameter.name] = value;`
	`1212`	`+ (*jsonVal)[filePosition->id]["attributes"][name] = value;`
`1213`	`1213`	`break;`
`1214`	`1214`	`}`
`1215`	`1215`	`writable->written = true;`
`@@ -1540,7 +1540,6 @@ void JSONIOHandlerImpl::readAttribute(`
`1540`	`1540`	`auto const &jsonContents = obtainJsonContents(writable);`
`1541`	`1541`	`auto const &jsonLoc = jsonContents["attributes"];`
`1542`	`1542`	`setAndGetFilePosition(writable);`
`1543`		`- std::string error_msg("[JSON] No such attribute '");`
`1544`	`1543`	`if (!hasKey(jsonLoc, name))`
`1545`	`1544`	`{`
`1546`	`1545`	`throw error::ReadError(`
`@@ -2365,9 +2364,9 @@ nlohmann::json JSONIOHandlerImpl::platformSpecifics()`
`2365`	`2364`	`Datatype::CDOUBLE,`
`2366`	`2365`	`Datatype::CLONG_DOUBLE,`
`2367`	`2366`	`Datatype::BOOL};`
`2368`		`- for (auto it = std::begin(datatypes); it != std::end(datatypes); it++)`
	`2367`	`+ for (auto &datatype : datatypes)`
`2369`	`2368`	`{`
`2370`		`- res[jsonDatatypeToString(it)] = toBytes(it);`
	`2369`	`+ res[jsonDatatypeToString(datatype)] = toBytes(datatype);`
`2371`	`2370`	`}`
`2372`	`2371`	`return res;`
`2373`	`2372`	`}`
`@@ -2483,10 +2482,9 @@ std::array<T, n> JSONIOHandlerImpl::JsonToCpp<std::array<T, n>>::operator()(`
`2483`	`2482`	`}`
`2484`	`2483`
`2485`	`2484`	`template <typename T>`
`2486`		`-T JSONIOHandlerImpl::JsonToCpp<`
`2487`		`- T,`
`2488`		`- typename std::enable_if<std::is_floating_point<T>::value>::type>::`
`2489`		`-operator()(nlohmann::json const &j)`
	`2485`	`+T JSONIOHandlerImpl::`
	`2486`	`+ JsonToCpp<T, std::enable_if_t<std::is_floating_point_v<T>>>::operator()(`
	`2487`	`+ nlohmann::json const &j)`
`2490`	`2488`	`{`
`2491`	`2489`	`try`
`2492`	`2490`	`{`
Original file line number	Diff line number	Diff line change
`@@ -132,6 +132,12 @@ RecordComponent &RecordComponent::resetDataset(Dataset d)`
`132`	`132`	`rc.m_isEmpty = false;`
`133`	`133`	`if (written())`
`134`	`134`	`{`
	`135`	`+ if (!rc.m_dataset.has_value())`
	`136`	`+ {`
	`137`	`+ throw error::Internal(`
	`138`	`+ "Internal control flow error: Written record component must "`
	`139`	`+ "have defined datatype and extent.");`
	`140`	`+ }`
`135`	`141`	`rc.m_dataset.value().extend(std::move(d.extent));`
`136`	`142`	`}`
`137`	`143`	`else`
Original file line number	Diff line number	Diff line change
`@@ -73,6 +73,8 @@ auto Attribute::get_impl() const -> std::variant<U, std::runtime_error>`
`73`	`73`	`switch (index)`
`74`	`74`	`{`
`75`	`75`	`OPENPMD_FOREACH_DATATYPE(OPENPMD_ENUMERATE_TYPES)`
	`76`	`+ default:`
	`77`	`+ return {std::runtime_error("Unreachable!")};`
`76`	`78`	`}`
`77`	`79`	`#undef OPENPMD_ENUMERATE_TYPES`
`78`	`80`