Enable targeted clang-tidy readability checks

.clang-tidy

@@ -23,6 +23,23 @@ Checks:
   # Enum representation is an ABI and data-layout decision, not a blanket
   # optimization; smaller underlying types can also add conversion overhead.
   - '-performance-enum-size'
+  - readability-ambiguous-smartptr-reset-call
+  - readability-avoid-return-with-void-value
+  - readability-avoid-unconditional-preprocessor-if
+  - readability-braces-around-statements
+  - readability-const-return-type
+  - readability-container-contains
+  - readability-container-data-pointer
+  - readability-container-size-empty
+  - readability-delete-null-pointer
+  - readability-duplicate-include
+  - readability-inconsistent-ifelse-braces
+  - readability-misleading-indentation
+  - readability-misplaced-array-index
+  - readability-qualified-auto
+  - readability-redundant-casting
+  - readability-reference-to-constructed-temporary
+  - readability-uniqueptr-delete-release
   - cppcoreguidelines-*
   # Required for CUDA, C, and RDKit interoperability.
   - '-cppcoreguidelines-avoid-c-arrays'
@@ -73,7 +90,7 @@ Checks:
   - '-misc-use-anonymous-namespace'
   - '-misc-use-internal-linkage'
 WarningsAsErrors: '*'
-HeaderFilterRegex: '.*/(src|rdkit_extensions)/.*'
+HeaderFilterRegex: '.*/src/.*'
 FormatStyle: none
 CheckOptions:
   # int8_t is used as a small integer (including negative sentinels), not as a

admin/run_clang_tidy.sh

@@ -82,8 +82,9 @@ cmake "${cmake_args[@]}"
   -config-file "${ROOT}/.clang-tidy" \
   -p "${BUILD_DIR}" \
   -j "${JOBS}" \
+  -header-filter "^${ROOT}/src/" \
   "${FIX_ARGS[@]}" \
-  '.*\.cpp$'
+  "^${ROOT}/src/.*\.cpp$"
 
 PYTHON="${PYTHON_BINARY:-$(find_tool python3 python)}"
 if [[ -z "${PYTHON}" ]]; then
@@ -109,7 +110,7 @@ CUDA_TIDY_BUILD_DIR="${BUILD_DIR}/clang_tidy_cuda"
   -p "${CUDA_TIDY_BUILD_DIR}" \
   -j "${JOBS}" \
   -extra-arg=--cuda-host-only \
-  -header-filter "^${ROOT}/(src|rdkit_extensions)/" \
-  -line-filter "[{\"name\":\"^${ROOT}/(src|rdkit_extensions)/\"}]" \
+  -header-filter "^${ROOT}/src/" \
+  -line-filter "[{\"name\":\"^${ROOT}/src/\"}]" \
   "${FIX_ARGS[@]}" \
   '.*\.cu$'

src/conformer_rmsd_mol.cpp

@@ -87,8 +87,9 @@ std::vector<AsyncDeviceVector<double>> conformerRmsdBatchMatrixMol(const std::ve
                                                                    cudaStream_t                            stream,
                                                                    const bool alignToFirstConformer) {
   const int numMols = static_cast<int>(mols.size());
-  if (numMols == 0)
+  if (numMols == 0) {
     return {};
+  }
   const bool needsFirstConformerAlignment = !prealigned && alignToFirstConformer;
 
   // --- Validate inputs and compute per-molecule metadata ---
@@ -103,8 +104,9 @@ std::vector<AsyncDeviceVector<double>> conformerRmsdBatchMatrixMol(const std::ve
   std::vector<size_t> coordOffsetsVec(numMols);
 
   pairOffsetsVec[0] = 0;
-  if (needsFirstConformerAlignment)
+  if (needsFirstConformerAlignment) {
     conformerOffsetsVec[0] = 0;
+  }
   size_t totalCoords = 0;
   for (int m = 0; m < numMols; ++m) {
     if (!mols[m]) {
@@ -145,8 +147,9 @@ std::vector<AsyncDeviceVector<double>> conformerRmsdBatchMatrixMol(const std::ve
   AsyncDeviceVector<int>    devPairOffsets(numMols + 1, stream);
   AsyncDeviceVector<int>    devConformerOffsets;
   AsyncDeviceVector<size_t> devCoordOffsets(numMols, stream);
-  if (needsFirstConformerAlignment)
+  if (needsFirstConformerAlignment) {
     devConformerOffsets = AsyncDeviceVector<int>(numMols + 1, stream);
+  }
 
   // Per-molecule output buffers.  Always allocate at least 1 element so that
   // devRmsdPtrs never contains a null — zero-pair molecules dispatch 0 blocks
@@ -172,8 +175,9 @@ std::vector<AsyncDeviceVector<double>> conformerRmsdBatchMatrixMol(const std::ve
     numConfsArr[m]    = numConfsVec[m];
     numAtomsArr[m]    = numAtomsVec[m];
     pairOffsetsArr[m] = pairOffsetsVec[m];
-    if (needsFirstConformerAlignment)
+    if (needsFirstConformerAlignment) {
       conformerOffsetsArr[m] = conformerOffsetsVec[m];
+    }
     coordOffsetsArr[m] = coordOffsetsVec[m];
     hostRmsdPtrs[m]    = devRmsdVecs[m].data();
 
@@ -191,17 +195,20 @@ std::vector<AsyncDeviceVector<double>> conformerRmsdBatchMatrixMol(const std::ve
     }
   }
   pairOffsetsArr[numMols] = pairOffsetsVec[numMols];
-  if (needsFirstConformerAlignment)
+  if (needsFirstConformerAlignment) {
     conformerOffsetsArr[numMols] = conformerOffsetsVec[numMols];
+  }
 
   // --- Transfer to device and launch ---
-  if (totalCoords > 0)
+  if (totalCoords > 0) {
     hostCoords.copyToDevice(devCoords, stream);
+  }
   numConfsArr.copyToDevice(devNumConfs, stream);
   numAtomsArr.copyToDevice(devNumAtoms, stream);
   pairOffsetsArr.copyToDevice(devPairOffsets, stream);
-  if (needsFirstConformerAlignment)
+  if (needsFirstConformerAlignment) {
     conformerOffsetsArr.copyToDevice(devConformerOffsets, stream);
+  }
   coordOffsetsArr.copyToDevice(devCoordOffsets, stream);
   hostRmsdPtrs.copyToDevice(devRmsdPtrs, stream);
 

src/morgan_fingerprint_common.cpp

@@ -151,8 +151,8 @@ void MorganInvariantsGenerator::ComputeInvariantsInto(const std::vector<const RD
         deltaMass = static_cast<int>(tAtom->getMass() - periodicTable->getAtomicWeight(tAtom->getAtomicNum()));
       }
 
-      const auto explicitImplicitHs  = static_cast<unsigned int>(tAtom->getNumExplicitHs() + tAtom->getNumImplicitHs());
-      const unsigned int totalDegree = explicitImplicitHs + bondCounts[atomIdx];
+      const auto         explicitImplicitHs        = tAtom->getNumExplicitHs() + tAtom->getNumImplicitHs();
+      const unsigned int totalDegree               = explicitImplicitHs + bondCounts[atomIdx];
       const unsigned int totalHsIncludingNeighbors = explicitImplicitHs + neighboringHydrogenCounts[atomIdx];
 
       const bool isInRing = ringInfo->numAtomRings(tAtom->getIdx()) > 0;

src/morgan_fingerprint_cpu.cpp

@@ -227,7 +227,7 @@ std::vector<MorganAtomEnv> getEnvironments(const RDKit::ROMol&               mol
     for (const auto& iter : allNeighborhoodsThisRound) {
       // if we haven't seen this exact environment before, add it to the
       // result
-      if (neighborhoods.count(std::get<0>(iter)) == 0) {
+      if (!neighborhoods.contains(std::get<0>(iter))) {
         if (!onlyNonzeroInvariants || (*atomInvariants)[std::get<2>(iter)] != 0) {
           if (includeAtoms[std::get<2>(iter)]) {
             result.emplace_back(std::get<1>(iter), std::get<2>(iter), layer + 1);

src/substruct/molecules.cpp

@@ -230,12 +230,13 @@ int getQueryBondEffectiveType(const RDKit::Bond* bond) {
           if (childDesc == "BondOrder") {
             const auto* eqQuery   = static_cast<const RDKit::BOND_EQUALS_QUERY*>((*it).get());
             int         childType = eqQuery->getVal();
-            if (childType == 1)
+            if (childType == 1) {
               hasSingle = true;
-            else if (childType == 2)
+            } else if (childType == 2) {
               hasDouble = true;
-            else if (childType == 7 || childType == 12)
+            } else if (childType == 7 || childType == 12) {
               hasAromatic = true;
+            }
           } else if (childDesc == "BondIsAromatic") {
             hasAromatic = true;
           }
@@ -1159,10 +1160,12 @@ uint8_t processQueryTree(const RDKit::Atom::QUERYATOM_QUERY* query,
       const auto* rangeQuery = static_cast<const RDKit::ATOM_RANGE_QUERY*>(query);
       int         minVal     = rangeQuery->getLower();
       int         maxVal     = rangeQuery->getUpper();
-      if (minVal < 0)
+      if (minVal < 0) {
         minVal = 0;
-      if (maxVal > 255)
+      }
+      if (maxVal > 255) {
         maxVal = 255;
+      }
       result = builder.addCompare(BoolOp::Range, field, static_cast<uint8_t>(minVal), static_cast<uint8_t>(maxVal));
     }
 
@@ -1766,7 +1769,7 @@ int collectRecursivePatterns(const RDKit::Atom::QUERYATOM_QUERY* query,
     }
 
     const auto* recursiveQuery = static_cast<const RDKit::RecursiveStructureQuery*>(query);
-    auto        queryMol       = recursiveQuery->getQueryMol();
+    const auto* queryMol       = recursiveQuery->getQueryMol();
 
     if (queryMol != nullptr) {
       const int thisPatternIdx = static_cast<int>(patterns.size());
@@ -1951,8 +1954,9 @@ RecursivePatternInfo extractRecursivePatterns(const RDKit::ROMol* mol) {
  */
 void mergeBatch(MoleculesHost& dest, const MoleculesHost& src) {
   ScopedNvtxRange range("mergeBatch");
-  if (src.numMolecules() == 0)
+  if (src.numMolecules() == 0) {
     return;
+  }
 
   const int atomOffset     = static_cast<int>(dest.atomDataPacked.size());
   const int instrOffset    = static_cast<int>(dest.queryInstructions.size());

src/substruct/substruct_launch_config.cpp

@@ -115,56 +115,72 @@ SubstructTemplateConfig selectTemplateConfig(int maxTargetAtoms, int maxQueryAto
   // Map to config enum
   if (targetTier == 32) {
     if (queryTier == 16) {
-      if (bondConfig == 4)
+      if (bondConfig == 4) {
         return SubstructTemplateConfig::Config_T32_Q16_B4;
-      if (bondConfig == 6)
+      }
+      if (bondConfig == 6) {
         return SubstructTemplateConfig::Config_T32_Q16_B6;
+      }
       return SubstructTemplateConfig::Config_T32_Q16_B8;
     } else {  // queryTier == 32
-      if (bondConfig == 4)
+      if (bondConfig == 4) {
         return SubstructTemplateConfig::Config_T32_Q32_B4;
-      if (bondConfig == 6)
+      }
+      if (bondConfig == 6) {
         return SubstructTemplateConfig::Config_T32_Q32_B6;
+      }
       return SubstructTemplateConfig::Config_T32_Q32_B8;
     }
   } else if (targetTier == 64) {
     if (queryTier == 16) {
-      if (bondConfig == 4)
+      if (bondConfig == 4) {
         return SubstructTemplateConfig::Config_T64_Q16_B4;
-      if (bondConfig == 6)
+      }
+      if (bondConfig == 6) {
         return SubstructTemplateConfig::Config_T64_Q16_B6;
+      }
       return SubstructTemplateConfig::Config_T64_Q16_B8;
     } else if (queryTier == 32) {
-      if (bondConfig == 4)
+      if (bondConfig == 4) {
         return SubstructTemplateConfig::Config_T64_Q32_B4;
-      if (bondConfig == 6)
+      }
+      if (bondConfig == 6) {
         return SubstructTemplateConfig::Config_T64_Q32_B6;
+      }
       return SubstructTemplateConfig::Config_T64_Q32_B8;
     } else {  // queryTier == 64
-      if (bondConfig == 4)
+      if (bondConfig == 4) {
         return SubstructTemplateConfig::Config_T64_Q64_B4;
-      if (bondConfig == 6)
+      }
+      if (bondConfig == 6) {
         return SubstructTemplateConfig::Config_T64_Q64_B6;
+      }
       return SubstructTemplateConfig::Config_T64_Q64_B8;
     }
   } else {  // targetTier == 128
     if (queryTier == 16) {
-      if (bondConfig == 4)
+      if (bondConfig == 4) {
         return SubstructTemplateConfig::Config_T128_Q16_B4;
-      if (bondConfig == 6)
+      }
+      if (bondConfig == 6) {
         return SubstructTemplateConfig::Config_T128_Q16_B6;
+      }
       return SubstructTemplateConfig::Config_T128_Q16_B8;
     } else if (queryTier == 32) {
-      if (bondConfig == 4)
+      if (bondConfig == 4) {
         return SubstructTemplateConfig::Config_T128_Q32_B4;
-      if (bondConfig == 6)
+      }
+      if (bondConfig == 6) {
         return SubstructTemplateConfig::Config_T128_Q32_B6;
+      }
       return SubstructTemplateConfig::Config_T128_Q32_B8;
     } else {  // queryTier == 64
-      if (bondConfig == 4)
+      if (bondConfig == 4) {
         return SubstructTemplateConfig::Config_T128_Q64_B4;
-      if (bondConfig == 6)
+      }
+      if (bondConfig == 6) {
         return SubstructTemplateConfig::Config_T128_Q64_B6;
+      }
       return SubstructTemplateConfig::Config_T128_Q64_B8;
     }
   }

src/substruct/substruct_search_internal.cpp

@@ -191,8 +191,9 @@ RDKitFallbackQueue::RDKitFallbackQueue(const std::vector<const RDKit::ROMol*>* t
       maxMatches_(maxMatches) {}
 
 void RDKitFallbackQueue::enqueue(const std::vector<RDKitFallbackEntry>& entries) {
-  if (entries.empty())
+  if (entries.empty()) {
     return;
+  }
   queue_.pushBatch(entries);
 }
 

src/substruct/substruct_search_internal.h

@@ -259,12 +259,14 @@ class RDKitFallbackQueue {
 class FallbackQueueProducerGuard {
  public:
   explicit FallbackQueueProducerGuard(RDKitFallbackQueue* queue) : queue_(queue) {
-    if (queue_)
+    if (queue_) {
       queue_->registerProducer();
+    }
   }
   ~FallbackQueueProducerGuard() {
-    if (queue_)
+    if (queue_) {
       queue_->unregisterProducer();
+    }
   }
   FallbackQueueProducerGuard(const FallbackQueueProducerGuard&)            = delete;
   FallbackQueueProducerGuard& operator=(const FallbackQueueProducerGuard&) = delete;