This is the playbook followed for the consistency-audit work in PRs #559–#564, #566 and #569 (May 2026). It is intended for anyone who wants to redo or extend that audit, ideally driven by an LLM with a human steering and verifying.
master, working build under your normal compiler.gcc 16+ or recent clang. Many bugs that the first round found compile silently under older toolchains. Keep one build directory configured against a strict one.A pass is one trip through the codebase looking for one specific kind of problem. Not "find bugs" — "find this kind of bug." Mixing kinds makes the diff impossible to review.
Useful lenses, roughly in order from least invasive to most:
| # | Lens | What to look for |
|---|---|---|
| 1 | Build-system errors | undefined CMake vars (e.g. ${PROJECT_TEST}), loop-variable typos, options referenced but not declared, missing PUBLIC_HEADER installs, files in CMakeLists.txt that don't exist, duplicate SOURCES / PUBLIC_HEADER keywords |
| 2 | Doc / README rot | .h filenames in docs that no longer exist, API examples that won't compile against current headers, broken badge / link refs |
| 3 | Dead code | files not referenced from any CMakeLists.txt, stub functions never called, members never read or written, extern declarations with no definition, commented-out targets |
| 4 | Header hygiene | using declarations at file scope in public headers (leak into every consumer), duplicate extern template, missing includes that compile only because of a transitive |
| 5 | Python / Cython binding bugs | rebound parameters that should have been mutated in-place, np.array([n]) vs np.zeros(n), missing __del__ that leaks C-side state, unreachable pass |
| 6 | Fortran correctness | implicit-typed loop variables, dropped-letter typos that fall back to IMPLICIT INTEGER, locals shadowing module functions, dead module-private routines that never bind to a type |
| 7 | Subspace / linear-algebra core | inverted-bound comparisons (oX >= i vs i >= oX), dereffing max_element on a possibly empty container, popping the wrong end of a deque, off-by-one between > and >= on past-the-end iterators |
| 8 | DistrArray / std::filesystem family | size_t underflow (current - offset when current < offset), stray ; after if making the body unconditional, file-scope globals without static |
| 9 | HDF5 / PHDF5 ownership | rvalue-reference parameters used as lvalues inside the body (named rvalues are lvalues), copy/move assignment asymmetry, leaked HDF5 IDs |
| 10 | extern "C" and C wrapper layer | declarations in headers as extern "C" but definitions in .cpp without it, unchecked dynamic_cast followed by ->, top-of-stack assumptions when stack might be empty |
| 11 | Destructors that can throw | ~T() is implicitly noexcept. Any call inside that can throw (future::wait, file I/O, profiler->dotgraph, options->parameter) terminates the program. Wrap in try { } catch (...) {} or guard the call. |
A later round added two more lenses that are worth keeping in mind, since both turned up bugs that had survived every earlier pass:
| # | Lens | What to look for |
|---|---|---|
| 12 | Hardcoded scalar types | double where the surrounding code is templated — working buffers, thresholds, Matrix<double> in a generic container, LAPACK calls bound to one precision. See the series under #587. |
| 13 | Row- versus column-major confusion | an Eigen::Map whose storage order disagrees with how the caller filled the buffer. Invisible for a symmetric matrix and wrong for anything else, so it hides until the data stops being symmetric. |
"Scan <tt>src/molpro/linalg/array/</tt> for headers that declare <tt>using ...</tt> at file scope (outside any namespace). For each one, identify the symbols leaked and which downstream files use those symbols unqualified. List the fixes needed (both header-side and consumer-side). Don't change any code yet."
<Component> <kind of fix>: <short summary>. Body lists each commit and explains the audit method.A few traps you'll hit that aren't obvious from the lens list:
using cleanup is not mechanical. ArrayHandler.h declared using subspace::Matrix; at file scope while a downstream OptimizeBFGS.h referenced Matrix<double> unqualified, so moving the using into the right namespace also broke the consumer — which under GCC 16 fails compilation and under older GCCs is a warning. The PR has to fix both sides at once. (This is what initially broke compilation in #556 and was caught by #562 plus the follow-up b359deb3. Both sides are fixed now: the declarations sit inside namespace molpro::linalg::array, and the consumer qualifies the name.)-Wtemplate-body only surface in templated code instantiated by the test suite — not by configure.1e-17 is really asserting that long double is wider than double, which is false on arm64 macOS, where both are IEEE binary64. Express such bounds in units of std::numeric_limits<T>::epsilon(), and skip explicitly where the comparison is vacuous.-O3 and roughly a factor of two at -O0. Branch on the type with if constexpr so the common path is the code that was there before.-mlong-double-64 makes x86-64 GCC use the same long double as arm64 macOS, which turned a round of guesses about a reviewer's failing tests into a local reproduction.Aim for PRs in the range 30–200 changed lines, one lens per PR, each rebased on current master. PRs over ~300 lines tend to attract "I can't review this" comments. The split from the first series (#559–#564, #566 and #569) is the upper end of what's still reviewable — finer-grained is fine.
Note that GitHub will not accept a base branch that lives on a fork, so a stack of interdependent PRs from a fork all have to target master. Each diff then also shows the commits below it until those merge, which is worth saying in the PR body so reviewers are not surprised.
It will:
git ls-files).extern template or a Fortran ISO binding.A reasonable session length is one pass per session. Trying to do all the passes in one session leads to context exhaustion and dropped findings.