- Generated by
1.9.8
|
DNDSR 0.2.1
Distributed Numeric Data Structure for CFV
|
Living document driving the check-by-check cleanup of the DNDSR C++ tree. Each "pass" below is a unit of work resulting in one commit; each bucketed check is addressed to completion before the next starts.
Driver: scripts/run_clang_tidy.py. Config: /.clang-tidy (reporting) and /.clang-tidy-fix (auto-fix profile). See docs/guides/style_guide.md for usage.
clang-diagnostic-error from omp.h inside Eigen's PCH — unrelated to DNDS code; cannot be fixed without installing libomp-<v>-dev matched to the clang-tidy / clangd binary version).build/clang-tidy-logs/baseline-dnds.txt).Summary counts after each pass (DNDS only, all-TU):
| After | Total | Distinct | Delta |
|---|---|---|---|
| Baseline | 24 597 | 51 | — |
| Pass 1 (macro-parentheses NOLINT) | 24 067 | 50 | −530 |
| Pass 2 (nodiscard –fix) | 22 495 | 49 | −1 572 |
| Pass 3 (init-variables –fix) | 21 297 | 49 | −1 198 |
| Pass 4 (missing-std-forward disable + YAML trap) | 20 794 | 47 | −503 |
| Pass 5 (reserved-identifier rename) | 19 390 | 46 | −1 404 |
| KA bucket disables | 7 341 | 35 | −12 049 |
| Pass 6 (special-member-functions) | 5 691 | 34 | −1 650 |
| Pass 7 (macro-usage disable) | 3 581 | 33 | −2 110 |
| Pass 8 (redundant-casting –fix) | 2 805 | 32 | −776 |
| Pass 9 (member-init –fix) | 2 330 | 31 | −475 |
| Pass 10 (use-nullptr –fix) | 2 099 | 30 | −231 |
| Pass 11 (use-emplace –fix) | 1 987 | 29 | −112 |
| Pass 12 (use-equals-default –fix) | 1 928 | 28 | −59 |
| Pass 13 (qualified-auto –fix) | 1 876 | 27 | −52 |
| Pass 14 (named-parameter –fix) | 1 654 | 26 | −222 |
| Pass 15 (simplify-boolean –fix + manual) | 1 499 | 25 | −155 |
| Pass 16 (unnecessary-value-param) | 1 180 | 24 | −319 |
| Pass 17 (loop-convert –fix + NOLINT) | 1 031 | 23 | −149 |
| Pass 18 (prefer-member-initializer –fix) | 971 | 23 | −60 |
| Pass 19 (cstyle-cast manual) | 847 | 22 | −124 |
| Pass 20 (non-const-global disable) | 523 | 21 | −324 |
| Pass 21 (avoid-c-arrays) | 455 | 20 | −68 |
| Pass 22 (unhandled-self-assign) | 402 | 19 | −53 |
| Pass 23 (branch-clone NOLINTBEGIN/END) | 80 | 18 | −322 |
| Pass 24 (implicit-widening NOLINT) | 14 | 14 | −66 |
| Pass 25 (rvalue-ref move) | 13 | 13 | −1 |
| Pass 26 (long-tail sweep) | 1 | 1 | −12 |
// NOLINT(...)), or Re-read later.cmake --build build -j32 must succeed. Relevant ctests (currently ‘ctest -R ’^dnds_') must pass. scripts/run_clang_format.py –check <scope>must pass.**Spot-check every auto-fix.** After any–fixrun, sample at least a handful of changed sites (random spread, not just the first) before committing.**Never skip the pre-commit hook.****No CUDA TUs.**.cufiles are excluded by the driver (nvcc flags incompile_commands.jsonbreak clang's CUDA frontend). CUDA-included headers are still tidied via their.cpp` includers.Captured with:
Top-20 (after f1fb769 / 2d5257c, with the summary regex fixed, 68 TUs, 24 597 total diagnostics across 51 distinct checks):
| # | Check | Hits |
|---|---|---|
| 1 | cppcoreguidelines-non-private-member-variables-in-classes | 3 412 |
| 2 | cppcoreguidelines-avoid-magic-numbers | 2 690 |
| 3 | cppcoreguidelines-pro-bounds-pointer-arithmetic | 2 119 |
| 4 | cppcoreguidelines-macro-usage | 2 110 |
| 5 | cppcoreguidelines-special-member-functions | 1 645 |
| 6 | modernize-use-nodiscard | 1 572 |
| 7 | bugprone-reserved-identifier | 1 404 |
| 8 | cppcoreguidelines-pro-type-vararg | 1 402 |
| 9 | cppcoreguidelines-init-variables | 1 198 |
| 10 | cppcoreguidelines-pro-type-reinterpret-cast | 842 |
| 11 | readability-redundant-casting | 776 |
| 12 | readability-redundant-access-specifiers | 537 |
| 13 | bugprone-macro-parentheses | 530 |
| 14 | cppcoreguidelines-missing-std-forward | 503 |
| 15 | cppcoreguidelines-pro-type-member-init | 475 |
| 16 | cppcoreguidelines-pro-type-const-cast | 371 |
| 17 | cppcoreguidelines-pro-bounds-array-to-pointer-decay | 334 |
| 18 | cppcoreguidelines-avoid-non-const-global-variables | 324 |
| 19 | bugprone-branch-clone | 322 |
| 20 | performance-unnecessary-value-param | 319 |
The summary regex originally matched bracketed tokens like
[[nodiscard]],<loc>,<pos>,<name>inside clang-tidy note lines, inflating themodernize-use-nodiscardrow to 3 621 and adding four phantom "checks". The regex was tightened (require a-or.inside the name) before the table above was captured. Delta between the two runs: 3 740 spurious hits removed, real totals unchanged.
Goal: after all passes in this plan, the top-20 should drop by more than 50 % of total volume, with the "Keep & fix" rows going to near-zero.
Bucket legend:
.clang-tidy disables.// NOLINT(check-name) + a one-line reason.| Check | Bucket | Rationale |
|---|---|---|
cppcoreguidelines-non-private-member-variables-in-classes | KA | Project uses struct with public fields as data bags extensively (Array storage, MPI views, config sections). Enforcing private+getters would be a massive architectural change with no safety gain at our scale. |
cppcoreguidelines-avoid-magic-numbers | KA | Overlaps with already-disabled readability-magic-numbers. Identical hit set, identical reasoning. |
cppcoreguidelines-pro-bounds-pointer-arithmetic | KA | CSR storage and MPI byte buffers are fundamentally pointer-arithmetic. Fixing means migrating to std::span/gsl::span — a design move, not a tidy pass. |
cppcoreguidelines-macro-usage | RL | Project uses macros for device portability, Eigen workarounds, config registration. Load-bearing. Revisit once other noise is gone. |
cppcoreguidelines-special-member-functions | KF | Missing rule-of-five declarations. Can catch real bugs around implicit moves/copies. Pass 6. |
modernize-use-nodiscard | KF | Mechanical, mostly auto-fixable. Spot-check needed (some functions are legitimately called for side effects). Pass 2. |
bugprone-reserved-identifier | KF | _Tp, __DNDS_str, etc. are reserved-name patterns. Mechanical rename, not auto-fixable across TUs. Pass 5. |
cppcoreguidelines-pro-type-vararg | KA | MPI and printf-family APIs require varargs. Silencing globally is correct. |
cppcoreguidelines-init-variables | KF | Mechanical, mostly auto-fixable. Spot-check for cases where default-init is intentional (e.g. int rank; right before MPI_Comm_rank). Pass 3. |
cppcoreguidelines-pro-type-reinterpret-cast | KA | Used for MPI byte buffers and serialization. No idiomatic replacement. |
readability-redundant-casting | RL | Some are style-only, some indicate real type drift. Revisit after pass 7. |
readability-redundant-access-specifiers | KA | Project explicitly repeats public: for readability in large classes. Documented convention. |
bugprone-macro-parentheses | KF | Cheap, mechanical, catches real macro-expansion bugs. Some hits are design-intentional (token-paste operands, argument types) and need NOLINT. Pass 1. |
cppcoreguidelines-missing-std-forward | KF | Mechanical, auto-fixable. Pass 4. |
cppcoreguidelines-pro-type-member-init | RL | Overlaps with cppcoreguidelines-init-variables (pass 3) on members. Revisit delta. |
cppcoreguidelines-pro-type-const-cast | KA | Needed for interop with C APIs (MPI, CGNS, HDF5) that take non-const pointers. |
cppcoreguidelines-pro-bounds-array-to-pointer-decay | KA | Same reasoning as pointer-arithmetic. |
cppcoreguidelines-avoid-non-const-global-variables | RL | Globals for signal handlers, log streams, MPI info are intentional; there may be stragglers worth trimming. Revisit after pass 2. |
bugprone-branch-clone | RL | Sometimes a real duplication, sometimes stylistic (exhaustive if/else cascades for enum handling). Needs eyes. |
performance-unnecessary-value-param | KF candidate later | Would flag things like ssp<T> taken by value where const-ref or move would do. Needs care — some by-value params are deliberate (MPI type, POD). Defer until pass 6 is done. |
Each pass below records: objective, scope, commands, sample reviews, verification (build + any ctests), commit hash, post-pass delta.
Outcome. 530 hits → 0. Total diagnostics 24 597 → 24 067.
All 530 hits collapsed to 17 unique source locations in 3 files:
| File | Lines (col) | Distinct sites | Dupes per site |
|---|---|---|---|
src/DNDS/Defines.hpp | 86:28, 88:39, 93:28, 95:39 | 4 | 66 |
src/DNDS/ArrayDOF.hpp | 68..79:5 | 12 | 22 |
src/DNDS/Config/ConfigParam.hpp | 700:60 | 1 | 2 |
Decision. All 17 are false positives: every flagged token is a C++ type name or storage-class specifier in a context where parenthesization is not valid syntax:
DNDS_DEVICE_TRIVIAL_COPY_DEFINE(T, T_Self) — T and T_Self appear as parameter types in constructor and assignment-operator signatures.DNDS_ARRAY_DOF_OP_FUNC_LIST(..., spec) — spec is always passed static; it's the leading storage-class specifier of a function declaration.DNDS_DECLARE_CONFIG(Type_) — Type_ is used as a parameter type and as a template argument.Fix. Three NOLINTBEGIN / NOLINTEND pairs around the macro definitions, each with a one-sentence rationale comment. 13 lines of comments replace 530 diagnostics.
Verification. cmake --build build -t dnds -j32 succeeds; the summary drop of 530 exactly matches the decrement in the total (no secondary effects).
Commit: see git log -- docs/dev/clang_tidy_plan.md.
Outcome. 1 572 hits → 0. Total diagnostics 24 067 → 22 495.
Method. Single-check override config at /tmp/pass2.clang-tidy:
Driven by scripts/run_clang_tidy.py --fix --config-file /tmp/pass2.clang-tidy DNDS.
Incident & fix. The first --fix attempt ran with 64 parallel workers and produced syntax-error corruption in Vector.hpp, Array.hpp, ArrayPair.hpp, and several other headers because the parallel workers race when the same header is edited from multiple TUs (observed: interleaved [[nodiscard]] insertions in mid-token positions).
Reverted via git checkout HEAD -- src/DNDS/ and patched the driver to force jobs=1 whenever --fix is set (with --unsafe-parallel-fix escape hatch). Serialized run completed cleanly. Committed as a separate fix(tooling) commit so the pass's diff stays pure.
Diff footprint. 11 files, 32 line-replacements (one [[nodiscard]] prefix each). 1 572 repeated hits collapsed to 32 unique decls because each header declaration is reported once per including TU.
Sample review. 4 spot-checks:
Array.hpp:570 at() const — const getter, value return.ArrayBasic.hpp:423 at_compressed(...) const with DNDS_DEVICE_CALLABLE prefix — attribute order is valid.EigenUtil.hpp:278..294 rows()/cols()/size() const — Eigen dimension wrappers, pure reads.SerializerH5.cpp:180 get_indent() const — local helper, string-builder, no side effects.All good; no call sites in DNDS were found that call these functions for side effects.
Verification. cmake --build build -t dnds --clean-first -j32 succeeds; euler target builds. Pre-commit clang-format ran on 11 modified files, no drift.
Commits: 7502b8d (driver serialize-on-fix) + pass 2 commit.
Outcome. 1 198 hits → 0. Total diagnostics 22 495 → 21 297.
Method. Same recipe as Pass 2; single-check config at /tmp/pass3.clang-tidy, --fix, serialized. One full run fixed all 1 198 hits in one shot (no stragglers).
Diff footprint. 8 files, ~30 line-replacements. Most are the classic "declare-then-immediately-MPI-writes" pattern; int x; MPI_*(&x) now int x = 0; MPI_*(&x).
Sample review found two corrections needed:
ArrayTransformer.hpp:838,867 — clang-tidy initialised MPI_Datatype dtype to nullptr. That's only valid on MPI implementations where MPI_Datatype is a pointer typedef (OpenMPI). MPICH defines it as int, so nullptr would not compile. Manually corrected to MPI_DATATYPE_NULL, the canonical sentinel that works on both.ArrayDOF_op.hxx — clang-tidy inserted #include <math.h> and initialised real sqrSumAll = NAN;. The sibling function on line 228 initialises the same variable to 0. Corrected to match the sibling and removed the unnecessary include.Verification. cmake --build build -t dnds -j32 succeeds after both corrections. Pre-commit clang-format re-ran, no drift.
Lesson learned. cppcoreguidelines-init-variables with --fix picks unusual sentinel values (nullptr based on typedef, NAN for floats). Always review auto-fix output for semantic appropriateness, not just build success.
Commit: see git log.
Outcome. 503 hits → 0, by reclassifying as KA (keep & accept) and adding to .clang-tidy disables.
Method. Single-check --fix attempt produced 0 edits: this check does not implement auto-fix. Switched to manual review.
Sample analysis (all 9 unique sites in DNDS):
Array.hpp:477 — Resize(index, TFRowSize&& FRowSize). FRowSize is a functor called in-place. Forwarding-ref was chosen only to accept both lvalue and rvalue callables; std::forward on a functor that the body keeps calling directly would move-from on first call and break subsequent ones.Array.hpp:723 — ResizeRowsAndCompress(TRowSizeFunc&&). Same pattern (functor called inside a loop).ArrayPair.hpp:249 — runFunctionAppendedIndex(index, TF&& F). Same pattern (F(*father, i) inside the body).ArrayEigenUniMatrixBatch.hpp:110 — Resize(..., TFRowSize&& rsf). Functor called twice inside a lambda capturing it by reference.Defines.hpp:834 (×2 columns) — hash functor operator()(TBegin&& begin, TEnd&& end). Iterators used for random access; no forwarding semantics apply.IndexMapping.hpp:215 — OffsetAscendIndexMapping(...) taking TpullSet&& pullingIndexGlobal. The body mutates the collection in place (sort, unique, erase, shrink_to_fit) but never moves it elsewhere. Should be TpullSet&, but the author's commented-out // std::forward<...>(...); // might delete shows they considered it and decided against.IndexMapping.hpp:298-299 — analogous ctor overload taking two collections, used read-only.Decision. All nine sites are either functor-called-in-place (cannot be forwarded without breaking repeated calls) or collection-used-by-reference (should be T&/const T&, not T&&). The latter is an API change that ripples through call sites and is outside the scope of "tidy passes." The check cannot distinguish legitimate forwarding-template usage from these patterns, so it produces a steady stream of false positives.
Action. Bucket moved from KF → KA. Added to .clang-tidy disables; Checks list now has an -cppcoreguidelines-missing-std-forward line.
Secondary fix: YAML folded-scalar trap. During this pass I discovered that rationale comments placed inside the Checks: > folded scalar are parsed as text (YAML # is only a comment at line start, not inside a folded block), which silently concatenated my commentary into a malformed check name and the subsequent disables were ignored. Fixed by moving all rationale comments into the file header as a table; the Checks: block is kept comment-free. A warning note was added to the header explaining this.
Commit: see git log.
Outcome. 1 404 hits → 0. Total diagnostics 20 794 → 19 390.
Method. Manual; this check has no auto-fix. The 1 404 hits collapsed to ~25 unique identifiers, each flagged because it starts with __ or _[A-Z] or contains __ (all reserved-name patterns per [lex.name]/3.2).
Renames applied (leading underscores dropped):
| Before | After | Notes |
|---|---|---|
_Tp | Tp | Template parameter in 2 sites (Defines.hpp). |
__DNDS_str | DNDS_str | Token-stringize helper macro. |
__DNDS__json_to_config | DNDS_json_to_config | Both leading and internal __ removed. |
__DNDSToMPITypeInt, __DNDSToMPITypeFloat | drop leading __. | |
__EndTimerType | EndTimerType | Timer callback type. |
__InSituPackStartPull, __InSituPackStartPush | drop leading __. | |
__OneMatGetRowSize | OneMatGetRowSize | Template helper. |
__ReadSerializerData, __ReadSerializerDataAndPropagateOffset, __ReadSerializerStructuralAndResolveDataOffset, __WriteSerializerData | drop leading __. | Serializer internals. |
__Row_size | Row_size | Template metafunction. |
__p_indices | p_indices | Helper in bind module. |
__start_timer, __stop_timer | start_timer, stop_timer | Timer API. |
__pybind11_callBind*s_rowsizes_sequence (×8) | drop leading __. | Our own template helpers that happened to live next to pybind11 code. |
__EigenPCH, __ExprtkPCH | EigenPCH_tag, ExprtkPCH_tag | Module-tag strings; avoided colliding with the class / filename. |
Collision-handled:
| Before | After | Rationale |
|---|---|---|
__size, __offset (SerializerBase.hpp:40) | sz, ofs | Ctor params; size/offset have 521 / 302 existing uses. |
_GetDataLayout (ArrayBasic.hpp / Array.hpp) | ComputeDataLayout | GetDataLayout already exists as a different member. |
Verification. cmake --build build -t dnds -j32 succeeds; cmake --build build -t euler -j32 succeeds (catches cross-module consumers in Euler/CFV); clang-tidy re-summary shows 0 hits for bugprone-reserved-identifier.
Lesson. A plain leading-underscore strip is not enough when the identifier also has an internal __; re-running the check after each bulk sed pass catches the residue quickly.
Commit: see git log.
Outcome. 1 645 → 0. Completed as one commit (1880d48).
Per-class audit. The 1 645 warnings collapsed to ~20 distinct class declarations, each bucketed into one of four categories with an explicit rule-of-five closure:
shared_ptr, host_device_vector, POD, or std::vector): add = default move ctor, move assign, and destructor alongside the existing custom copy. Default move is a shallow transfer of the shared handles — correct and observably identical to "copy source + reset source" on the moved-from side. Classes: Array, ArrayAdjacency, ArrayDof, ArrayEigenMatrix, ArrayEigenMatrixBatch, ArrayEigenUniMatrixBatch, ArrayEigenVector, ArrayTransformer, ParArray, AdjacencyRow, RowView, EmptyNoDefault, host_device_vector_r0, host_device_vector_r1, and their nested iterator classes.= delete copy and move to prevent slicing and double-close. Classes: SerializerBase, SerializerJSON, SerializerH5, DeviceStorageBase, DeviceHostSingleAllocationBase, DeviceHostSingleAllocationDirect, ExprtkWrapperEvaluator.= delete copy/move + = default dtor. Classes: CommStrategy, MPIBufferHandler, ResourceRecycler, PerformanceTimer.this with ResourceRecycler by raw pointer): = delete copy/move to prevent registering the same this twice. Classes: MPIReqHolder, MPITypePairHolder.Every declaration carries a one-line rationale comment explaining which bucket the class falls in and why the chosen semantics are correct.
Outcome. 2 110 → 0. Config-only disable commit (9c71e4b).
All 41 distinct macros in DNDS are legitimate uses that a constexpr template function cannot express:
__FILE__ / __LINE__ (DNDS_assert*, DNDS_check_throw*, DNDS_HD_assert*).DNDS_DECLARE_CONFIG, DNDS_FIELD, DNDS_json_to_config, DNDS_NLOHMANN_DEFINE_*, pybind11_bind_*, DNDS_DEVICE_TRIVIAL_COPY_DEFINE*).MPICH_SKIP_MPICXX, OMPI_SKIP_MPICXX, EIGEN_DONT_PARALLELIZE).DNDS_likely, DNDS_unlikely, DNDS_FORCEINLINE, DISABLE_WARNING via _Pragma).DNDS_VERSION_STRING).The full list is in the .clang-tidy header disables table.
Outcome. 776 → 0 via single-check --fix run (742119d).
Three patterns auto-fixed:
MPI_Datatype(MPI_FLOAT) etc. (OpenMPI constants are already MPI_Datatype, so the functional cast is a no-op) — 11 sites in MPI.hpp.reinterpret_cast<uint8_t *>(uint8_t *) identity casts in Device/DeviceStorage.cpp — 3 sites.index(nSend) where nSend is already index in ArrayTransformer.hpp.Outcome. 475 → 0 via --fix + 1 manual fix (173ad1a).
Raw class members (index _size, rowsize Row_size, MPI_Aint pushSendSize, ConfigTypeTag typeTag, tStart array) gained {} default initializers. Local std::array scratch buffers in MPI.cpp, SerializerH5.cpp, Defines.cpp, ArrayEigenMatrix.hpp, ArrayTransformer.hpp zero-init'd the same way.
Manual fix: auto-fix emitted struct winsize w { }; on two lines; reformatted inline to struct winsize w{};.
Outcome. 231 → 0 via --fix (6c494f4).
(T *)(NULL) → (T *)nullptr in the past-the-end row inquiry (ArrayBasic.hpp), getenv() comparisons (MPI.hpp, MPI.cpp), and HDF5 handle probes (SerializerH5.cpp).
Outcome. 112 → 0 via --fix (56c43af).
push_back(std::make_pair(r, dtype)) → emplace_back(r, dtype) in MPI type-pair vector appends (2 sites in ArrayTransformer.hpp) and HDF5 dimension vectors (SerializerH5.cpp), plus push_back(std::string(...)) → emplace_back(...) in MPI.cpp / MPI_bind.cpp.
Outcome. 59 → 0 via --fix (9231ae6).
Two sites (duplicated across TUs): DeviceHostSingleAllocationBase::~DeviceHostSingleAllocationBase() {} and DeviceHostSingleAllocationDirect::~DeviceHostSingleAllocationDirect() override {} → = default.
Outcome. 52 → 0 via --fix (7586d2f).
auto ptr = reinterpret_cast<T*>(...) → auto *ptr = ... in pybind11 binding helpers; for (auto &[k, v] : map.items()) → for (const auto &[k, v] : map.items()) in SerializerJSON.cpp.
Outcome. 222 → 0 via --fix (0bf9edd).
Added /*unused*/ on tag-dispatch parameters (std::index_sequence<Is...> /*unused*/) in the pybind11 binding machinery.
Outcome. 155 → 0 via --fix + 1 manual (e07c315).
!(a && b) / !(a || b) DeMorgan expansions in ArrayDOF.hpp (SFINAE enable_if), ArrayDOF_bind.hpp (if constexpr), EigenUtil.hpp (ternary condition), and Defines.hpp::checkedIndexTo32 (manual — auto-fix emitted 66 duplicates into header-include paths).
Outcome. 319 → 0 via --fix + manual sed (b2019a7).
py::buffer row by value → const py::buffer &row in 15 pybind11 setitem / operator overloads across the 5 _bind.hpp headers. Two additional sites in Serializer_bind.hpp (py::object options_in) and MPI_bind.cpp (Allreduce py_sendbuf / py_recvbuf).
Outcome. 149 → 0 via --fix + NOLINTBEGIN/END (9a88b36).
Auto-fix converted two index-based loops over std::vector<index> in ArrayRedistributor.hpp to range-based form. One site in Array_bind.hpp (for (ssize_t i = 0; i < pullIndexGlobal.size(); i++) pullIndexVec.push_back(pullIndexGlobal.at(i));) carries NOLINTBEGIN / NOLINTEND — pybind11 array_t iterators yield pybind11::handle, not long, and the explicit index-based form is required for the numpy-to-long conversion.
A plain NOLINTNEXTLINE is insufficient: --fix rewrites the for line itself, erasing the preceding comment. Block-form NOLINT survives.
Outcome. 60 → 0 via --fix (a3cb4a5).
One unique site: MPIInfo::MPIInfo(MPI_Comm ncomm) — moved comm = ncomm from the body into the member-initializer list.
Outcome. 124 → 0 via 6 manual edits (226ead9).
Four sites in SerializerH5.cpp / SerializerJSON.cpp: (ssp<tValue> *)(pth_2_ssp[refPath]) → explicit reinterpret_cast<ssp<tValue> *>(...) on the type-erased dedup registry (matches the author TODO).
Two sites in MPI.hpp: MPI_IN_PLACE expands to the OpenMPI-defined ((void *)1) sentinel. NOLINTNEXTLINE placed immediately above the offending Allreduce(...) call — multi- line rationale comments between the NOLINT and the offending line break the suppression (same trap as Pass 17).
Outcome. 324 → 0 via config disable (c407cff).
Every global mutable in DNDS is intentional and cannot be made const, thread_local, or class-scoped without wider redesign: logStream, useCout, outputDelim, HDF_mutex, isDebugging, EigenPCH_tag, ExprtkPCH_tag. Full rationale in the .clang-tidy header table.
Outcome. 68 → 0 via 2 manual edits (1d60a57).
Two sites, both stack scratch buffers for printf-family calls:
Errors.hpp::genFatalErrorMessage: char format_buf[1024*512] → std::array<char, 1024*512> format_buf{}.SerializerFactory.hpp: char BUF[512] → std::array<char, 512>.Outcome. 53 → 0 via 1 manual edit (dd31508).
AdjacencyRow::operator=(const AdjacencyRow &r) called std::copy(r.cbegin(), r.cend(), p_indices). On self-assign, source and destination ranges are fully overlapping — UB. Added if (this == &r) return; early-return guard with a comment explaining the UB.
Outcome. 322 → 0 via NOLINTBEGIN / NOLINTEND blocks and rationale comments (10f5305).
All 7 unique sites are intentional — the diagnostic fires where two logically distinct branches happen to produce the same code:
ArrayBasic.hpp, Array.hpp — if constexpr cascades over _dataLayout. TABLE_Fixed and TABLE_Max currently both compute iRow * _row_size_dynamic + iCol; the layouts are conceptually distinct (padded rows may diverge).ArrayEigenUniMatrixBatch.hpp, ArrayEigenUniMatrixBatch_DeviceView.hpp, EigenUtil.hpp::MatrixFMTSafe — ternary for the Eigen options template parameter; both non-row-vector arms intentionally select ColMajor.Config/ConfigParam.hpp — switch over ConfigTypeTag → JSON Schema type strings; several enums collapse to the same built-in (Enum→"string", ArrayOfObjects→"array", MapOfObjects→"object").NOLINTBEGIN/END is required because clang-tidy reports the diagnostic at the first clone of the pair; NOLINTNEXTLINE on one line of the pair was insufficient.
Outcome. 66 → 0 via 1 NOLINT (583ab45).
One source site: std::array<char, 1024 * 512> in Errors.hpp. Compile-time constant 524 288 trivially fits in int32_t; the widening to size_t happens at compile time during template argument deduction. Spurious diagnostic; NOLINTNEXTLINE with rationale.
Outcome. 44 → 0 via 1 edit (6c90a62).
ArrayDofDeviceView(t_base &&base_view) : t_base(base_view) {} and the Const variant: base_view inside the body is an lvalue, so t_base(base_view) silently copied. Added std::move(base_view) on the base-init to perform the intended move.
Outcome. 12 warnings across 10 distinct checks → 0 via 13 edits (0d6d0d9). Drains every remaining check to zero:
performance-move-const-arg (3 sites): removed no-op std::move of const py::buffer & / const py::module_ &.readability-avoid-return-with-void-value (5 sites): changed pybind11 setitem helpers from auto (deduced void) to explicit void, dropped return from the wrapping lambdas.readability-make-member-function-const (2 sites): made SerializerFactory::BuildSerializer / ::ModifyFilePath const.bugprone-empty-catch (4 sites): four env-var parse catches in CommStrategy::CommStrategy guarded with NOLINTBEGIN/END.readability-redundant-member-init: dropped : SerializerBase() in SerializerH5.hpp.modernize-pass-by-value: SerializerFactory(const std::string &) → SerializerFactory(std::string _type) : type(std::move(_type)) {}.modernize-use-auto (2 sites): py::buffer buf = v.cast<...>() → auto buf = ...; TraverseData *data = static_cast<...> → auto *data.readability-container-size-empty: ver.length() → !ver.empty() in Defines.cpp.bugprone-exception-escape: ~SerializerJSON() wraps CloseFileNonVirtual() in try/catch — destructors mustn't throw (NOLINTBEGIN/END guards the empty catch).modernize-return-braced-init-list: kept return std::string(n, ch) in SerializerH5.cpp::get_indent with NOLINTNEXTLINE — brace init is ambiguous with initializer_list<char> and triggers -Wnarrowing.performance-unnecessary-copy-initialization: NOLINT on T vV = v in SerializerH5.cpp (clang-tidy misses that the variable is addressed via &vV in the non-string if constexpr branch).cppcoreguidelines-avoid-const-or-ref-data-members: NOLINT on H5Contents &contents in the TraverseData per-call aggregate.performance-no-int-to-ptr: NOLINT on MPI_Comm(pComm) — Python side passes an opaque uintptr_t.performance-inefficient-vector-operation: added pArgvOut.reserve(*pargc) before the emplace_back loop in MPI_bind.cpp.bugprone-multi-level-implicit-pointer-conversion: NOLINT on H5Aread(attr_id, dtype_id, &attr_value) where attr_value is char * — HDF5 wants void *buf and explicit static_cast<void*> does not silence the check.cppcoreguidelines-owning-memory (7 sites): NOLINT on shared-pointer deleter callback (DeviceStorage.cpp), opaque exprtk pointers (ExprtkWrapper.cpp), and MPI_Init argv allocation (MPI_bind.cpp) with rationale.Every auto-fix pass in this session re-taught the same lesson:
NOLINTNEXTLINE(check) applies to the line immediately following the directive. Rationale comments must come before the directive, not between it and the offending code, otherwise the NOLINT applies to the rationale line.--fix can rewrite the flagged line (e.g. modernize-loop-convert, modernize-return-braced-init-list), use NOLINTBEGIN(check) ... NOLINTEND(check) around the preserved block — the directive line survives the rewrite.switch / ?: / chained if / else if with bugprone-branch-clone, the diagnostic is reported at the first clone of the pair, which may not match the line the edit would change. NOLINTBEGIN/END is the safe form.Added to .clang-tidy during this effort (all motivated by the triage table above):
| Check | Reason |
|---|---|
cppcoreguidelines-missing-std-forward | Functor T&& called in-place (Pass 4). |
cppcoreguidelines-non-private-member-variables-in-classes | Project uses struct-of-fields data bags. |
cppcoreguidelines-avoid-magic-numbers | Duplicate of already-disabled readability-magic-numbers. |
cppcoreguidelines-pro-bounds-pointer-arithmetic | CSR / MPI buffer idioms. |
cppcoreguidelines-pro-bounds-array-to-pointer-decay | Same. |
cppcoreguidelines-pro-bounds-constant-array-index | Same. |
cppcoreguidelines-pro-type-vararg | MPI / printf-family. |
cppcoreguidelines-pro-type-reinterpret-cast | MPI byte buffers, serialization. |
cppcoreguidelines-pro-type-const-cast | C-API interop (MPI, CGNS, HDF5). |
readability-redundant-access-specifiers | Repeated public: is a project convention. |
modernize-use-transparent-functors | Eigen expression templates break with std::less<> etc. |
cppcoreguidelines-c-copy-assignment-signature | Duplicate of misc-unconventional-assign-operator. |
cppcoreguidelines-macro-usage | All 41 DNDS macros require __FILE__/__LINE__ capture, token pasting, code generation, or define-before-include semantics (Pass 7). |
cppcoreguidelines-avoid-non-const-global-variables | Every DNDS global mutable (logStream, useCout, outputDelim, HDF_mutex, isDebugging, PCH tags) is intentional; redesign out of scope for a tidy pass (Pass 20). |
Rationale comments live in the .clang-tidy file header, not inside the Checks: > folded scalar (see Pass 4 / YAML trap).
The tidy session left ~70 targeted NOLINT markers. All of them pair with a rationale comment. Representative breakdown:
| Check | Count | Notable sites |
|---|---|---|
bugprone-branch-clone | 5 | ArrayBasic.hpp, Array.hpp, Eigen options ternaries, ConfigParam.hpp switch |
cppcoreguidelines-owning-memory | 4 | Device/DeviceStorage.cpp deleter, ExprtkWrapper.cpp opaque new, MPI_bind.cpp argv alloc |
bugprone-empty-catch | 5 | 4x CommStrategy::CommStrategy env-var parse, 1x ~SerializerJSON |
modernize-loop-convert | 1 | Array_bind.hpp pybind11 array_t iteration |
cppcoreguidelines-pro-type-cstyle-cast | 2 | MPI_IN_PLACE (OpenMPI (void*)1 sentinel) |
bugprone-implicit-widening-of-multiplication-result | 1 | Errors.hpp compile-time 1024*512 |
modernize-return-braced-init-list | 1 | SerializerH5.cpp std::string(n, ch) vs initializer_list overload |
performance-unnecessary-copy-initialization | 2 | SerializerH5.cpp T vV = v used in non-string if constexpr branch |
cppcoreguidelines-avoid-const-or-ref-data-members | 1 | TraverseData H5Literate callback state |
performance-no-int-to-ptr | 1 | MPI_Comm(pComm) in pybind11 ctor |
bugprone-multi-level-implicit-pointer-conversion | 1 | H5Aread(..., &char_ptr) |
bugprone-macro-parentheses | 3 blocks | Unparenthesizable type-name / storage-class args |
DNDS is now clean. Every check with >= 1 actionable instance has been driven to zero, either by fixing the code or by adding a rationale-commented NOLINT / .clang-tidy disable. The sole remaining diagnostic is a clang-diagnostic-error on omp.h inside Eigen's PCH, which is an Eigen-internal include path issue unrelated to DNDS source.
Repeat the recipe for the other modules in this order:
src/Solver/ — small, limited blast radius; good next target.src/Geom/ — largest module; expect a new set of checks specific to mesh connectivity loops. Start with clang-analyzer-optin.cplusplus.VirtualCall per the historical note in .clang-tidy.src/CFV/ — follows Geom closely.src/Euler/, src/EulerP/ — solver layer; bugprone-branch-clone will matter here (exhaustive if / else cascades for enum handling).The .clang-tidy disables and NOLINT placement lessons carry forward unchanged. Any new module-specific disables should be appended to the header table, not inside the Checks: block (YAML folded scalars eat # as literal text).
The .clang-tidy disables carry forward unchanged. Any new module- specific disables should be appended in the same table at the top of .clang-tidy, not inside the Checks: block.
1.9.8