Merge pull request #248 from eseiler/misc/codechecker

[MISC] Resolve codechecker warnings

.github/workflows/ci_codechecker.yml

@@ -64,7 +64,9 @@ jobs:
                             --disable clang-diagnostic-implicit-int-float-conversion \
                             --disable clang-diagnostic-float-conversion \
                             --disable clang-diagnostic-implicit-int-conversion \
+                            --disable bugprone-exception-escape \
                             --disable bugprone-narrowing-conversions \
+                            --disable deadcode.DeadStores \
                             --output ./results 2>/dev/null | grep --line-buffered -E "\[[0-9]+/[0-9]+\]" || true
           echo "-${GITHUB_WORKSPACE}/include/hibf/contrib/*" > skipfile
           echo "+${GITHUB_WORKSPACE}/*" >> skipfile
@@ -76,6 +78,7 @@ jobs:
                             --trim-path-prefix "${GITHUB_WORKSPACE}/" > parse.log || true
           awk '/----====/{y=1;}y' parse.log
           sed -i 's/<option value="100">/<option value="100" selected>/g' html/index.html
+          sed -i 's@<title>Plist HTML Viewer</title>@<title>CodeChecker hibf</title>@g' html/*.html
 
       - name: Upload Report
         if: github.repository_owner != 'seqan'

include/hibf/misc/bit_vector.hpp

@@ -119,11 +119,13 @@ class bit_vector :
         bit_reference(bit_reference && other) = default;
         bit_reference & operator=(bit_reference const & other)
         {
-            return *this = bool(other);
+            *this = bool(other);
+            return *this;
         }
         bit_reference & operator=(bit_reference && other) noexcept
         {
-            return *this = bool(other);
+            *this = bool(other);
+            return *this;
         }
 
         /*!\brief Assigns a bit to the referenced bit.
@@ -139,6 +141,7 @@ class bit_vector :
         //!\overload
         // Needed to model std::output_iterator<bit_iterator, bool>, which requires the assignment to an const && version
         // of the proxy.
+        // NOLINTNEXTLINE(misc-unconventional-assign-operator)
         constexpr bit_reference const & operator=(bool const bit) const noexcept
             requires (!is_const)
         {

include/hibf/misc/insert_iterator.hpp

@@ -50,7 +50,7 @@ class insert_iterator
 
     explicit constexpr insert_iterator(ibf_t & ibf, size_t ibf_bin_index) :
         ptr{std::addressof(ibf)},
-        ibf_bin_index{ibf_bin_index},
+        ibf_bin_idx{ibf_bin_index},
         type{data_type::ibf}
     {}
 
@@ -61,6 +61,7 @@ class insert_iterator
     {
         assert(ptr != nullptr);
 
+        // NOLINTNEXTLINE(clang-diagnostic-switch-enum)
         switch (type)
         {
         case data_type::unordered_set:
@@ -70,7 +71,7 @@ class insert_iterator
             static_cast<sketch_t *>(ptr)->add(value);
             break;
         case data_type::ibf:
-            static_cast<ibf_t *>(ptr)->emplace(value, static_cast<bin_index>(ibf_bin_index));
+            static_cast<ibf_t *>(ptr)->emplace(value, static_cast<bin_index>(ibf_bin_idx));
             break;
         default:
             assert(type == data_type::function);
@@ -105,7 +106,7 @@ class insert_iterator
         function
     };
 
-    size_t ibf_bin_index{};
+    size_t ibf_bin_idx{};
     data_type type{};
 };
 

include/hibf/sketch/hyperloglog.hpp

@@ -39,13 +39,13 @@ class hyperloglog
      * \{
      */
     /*!\brief Default constructor.
-     * \param[in] bits The bit width in [5,32].
+     * \param[in] num_bits The bit width in [5,32].
      *
-     * Allocates 2^`bits` bytes of memory.
+     * Allocates 2^`num_bits` bytes of memory.
      *
-     * \throws std::invalid_argument if bits is not in [5,32].
+     * \throws std::invalid_argument if num_bits is not in [5,32].
      */
-    hyperloglog(uint8_t const bits = 5u);
+    hyperloglog(uint8_t const num_bits = 5u);
     hyperloglog(hyperloglog const &) = default;             //!< Defaulted.
     hyperloglog & operator=(hyperloglog const &) = default; //!< Defaulted.
     hyperloglog(hyperloglog &&) = default;                  //!< Defaulted.

src/hierarchical_interleaved_bloom_filter.cpp

@@ -123,16 +123,16 @@ size_t hierarchical_build(hierarchical_interleaved_bloom_filter & hibf,
         {
             auto & child = children[index];
 
-            robin_hood::unordered_flat_set<uint64_t> kmers{};
-            size_t const ibf_pos = hierarchical_build(hibf, kmers, child, data, false);
+            robin_hood::unordered_flat_set<uint64_t> local_kmers{};
+            size_t const local_ibf_pos = hierarchical_build(hibf, local_kmers, child, data, false);
             auto parent_bin_index = child.parent_bin_index;
             {
                 size_t const mutex_id{parent_bin_index / 64};
                 std::lock_guard<std::mutex> guard{local_ibf_mutex[mutex_id]};
-                technical_bin_to_ibf_id[parent_bin_index] = ibf_pos;
-                build::insert_into_ibf(kmers, 1, parent_bin_index, ibf, data.fill_ibf_timer);
+                technical_bin_to_ibf_id[parent_bin_index] = local_ibf_pos;
+                build::insert_into_ibf(local_kmers, 1, parent_bin_index, ibf, data.fill_ibf_timer);
                 if (!is_root)
-                    build::update_parent_kmers(parent_kmers, kmers, data.merge_kmers_timer);
+                    build::update_parent_kmers(parent_kmers, local_kmers, data.merge_kmers_timer);
             }
         }
     };

src/interleaved_bloom_filter.cpp

@@ -103,7 +103,6 @@ interleaved_bloom_filter::interleaved_bloom_filter(config & configuration, size_
                              seqan::hibf::bin_size{max_bin_size(configuration, max_bin_elements)},
                              seqan::hibf::hash_function_count{configuration.number_of_hash_functions}}
 {
-    // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores)
     size_t const chunk_size = std::clamp<size_t>(std::bit_ceil(bin_count() / configuration.threads), 8u, 64u);
 
 #pragma omp parallel for schedule(dynamic, chunk_size) num_threads(configuration.threads)
@@ -205,57 +204,59 @@ interleaved_bloom_filter::membership_agent_type::bulk_contains(size_t const valu
     assert(result_buffer.size() == ibf_ptr->bin_count());
 
     // Needed for auto-vectorization of loop. ibf_ptr->bin_words could change bewtween loops.
-    size_t const bin_words = ibf_ptr->bin_words;
-    size_t const hash_funs = ibf_ptr->hash_funs;
+    size_t const bin_words_ = ibf_ptr->bin_words;
+    size_t const hash_funs_ = ibf_ptr->hash_funs;
 
 #ifndef NDEBUG
-    assert(bin_words != 0u);
-    assert(hash_funs != 0u);
+    assert(bin_words_ != 0u);
+    assert(hash_funs_ != 0u);
 #else
-    // Removes case for bin_words == 0u. The same statment inside the switch-case wouldn't have that effect.
-    if (bin_words == 0u)
+    // Removes case for bin_words_ == 0u. The same statment inside the switch-case wouldn't have that effect.
+    if (bin_words_ == 0u)
         __builtin_unreachable();
-    if (hash_funs == 0u)
+    if (hash_funs_ == 0u)
         __builtin_unreachable();
 #endif
 
-    for (size_t i = 0; i < hash_funs; ++i)
+    for (size_t i = 0; i < hash_funs_; ++i)
         bloom_filter_indices[i] = ibf_ptr->hash_and_fit(value, ibf_ptr->hash_seeds[i]) / 64u;
 
     uint64_t * const raw = result_buffer.data();
     uint64_t const * const ibf_data = ibf_ptr->data();
-    std::memcpy(raw, ibf_data + bloom_filter_indices[0], sizeof(uint64_t) * bin_words);
+    std::memcpy(raw, ibf_data + bloom_filter_indices[0], sizeof(uint64_t) * bin_words_);
 
     // GCOVR_EXCL_START
+    // NOLINTBEGIN(bugprone-branch-clone)
     auto impl = [&]<size_t extent = 0u>()
     {
-        for (size_t i = 1; i < hash_funs; ++i)
+        for (size_t i = 1; i < hash_funs_; ++i)
         {
             uint64_t const * const ibf_raw = ibf_data + bloom_filter_indices[i];
 
             if constexpr (extent == 0u)
             {
 #pragma omp simd
-                for (size_t i = 0; i < bin_words; ++i)
-                    raw[i] &= ibf_raw[i];
+                for (size_t j = 0; j < bin_words_; ++j)
+                    raw[j] &= ibf_raw[j];
             }
             else if constexpr (extent == 2u || extent == 4u || extent == 8u)
             {
 #pragma omp simd
-                for (size_t i = 0; i < extent; ++i)
-                    raw[i] &= ibf_raw[i];
+                for (size_t j = 0; j < extent; ++j)
+                    raw[j] &= ibf_raw[j];
             }
             else
             {
-                for (size_t i = 0; i < extent; ++i)
-                    raw[i] &= ibf_raw[i];
+                for (size_t j = 0; j < extent; ++j)
+                    raw[j] &= ibf_raw[j];
             }
         }
     };
+    // NOLINTEND(bugprone-branch-clone)
 
     // https://godbolt.org/z/rqaeWGGer
     // Having the loop inside impl instead of around the switch/case is faster.
-    switch (bin_words)
+    switch (bin_words_)
     {
     case 1u: // 1 AND (64 bit)
         impl.operator()<1u>();

src/layout/compute_layout.cpp

@@ -54,6 +54,8 @@ layout compute_layout(config const & config,
     union_estimation_timer = store.union_estimation_timer;
     rearrangement_timer = store.rearrangement_timer;
 
+// Silence clangsa cplusplus.Move warning.
+#ifndef __clang_analyzer__
     // sort records ascending by the number of bin indices (corresponds to the IBF levels)
     // GCOVR_EXCL_START
     std::ranges::sort(store.hibf_layout->max_bins,
@@ -62,6 +64,7 @@ layout compute_layout(config const & config,
                           return r.previous_TB_indices.size() < l.previous_TB_indices.size();
                       });
     // GCOVR_EXCL_STOP
+#endif
 
     return *store.hibf_layout;
 }

src/layout/hierarchical_binning.cpp

@@ -301,7 +301,6 @@ void hierarchical_binning::backtrack_split_bin(size_t trace_j,
     // update max bin
     size_t const cardinality = (*data->kmer_counts)[data->positions[trace_j]];
     size_t const corrected_cardinality = static_cast<size_t>(cardinality * data->fpr_correction[number_of_bins]);
-    // NOLINTNEXTLINE(clang-analyzer-core.DivideZero)
     size_t const cardinality_per_bin = divide_and_ceil(corrected_cardinality, number_of_bins);
 
     max_tracker.update_max(bin_id, cardinality_per_bin);

src/layout/layout.cpp

@@ -64,7 +64,7 @@ void seqan::hibf::layout::layout::read_from(std::istream & stream)
     {
         std::vector<size_t> result;
 
-        auto buffer_start = &buffer[0];
+        auto buffer_start = buffer.data();
         auto const buffer_end = buffer_start + buffer.size();
 
         size_t tmp{};
@@ -82,7 +82,7 @@ void seqan::hibf::layout::layout::read_from(std::istream & stream)
     auto parse_first_bin = [](std::string_view const & buffer)
     {
         size_t tmp{};
-        std::from_chars(&buffer[0], &buffer[0] + buffer.size(), tmp);
+        std::from_chars(buffer.data(), buffer.data() + buffer.size(), tmp);
         return tmp;
     };
 

src/layout/simple_binning.cpp

@@ -34,10 +34,10 @@ size_t simple_binning::execute()
     size_t const extra_bins = num_technical_bins - num_user_bins + 1;
 
     // initialize first column (first row is initialized with inf)
-    double const ub_cardinality = static_cast<double>((*data->kmer_counts)[data->positions[0]]);
+    double const init_ub_cardinality = static_cast<double>((*data->kmer_counts)[data->positions[0]]);
     for (size_t i = 0; i < extra_bins; ++i)
     {
-        size_t const corrected_ub_cardinality = static_cast<size_t>(ub_cardinality * data->fpr_correction[i + 1]);
+        size_t const corrected_ub_cardinality = static_cast<size_t>(init_ub_cardinality * data->fpr_correction[i + 1]);
         matrix[i][0] = divide_and_ceil(corrected_ub_cardinality, i + 1u);
     }
 
@@ -105,7 +105,6 @@ size_t simple_binning::execute()
     ++trace_i; // because we want the length not the index. Now trace_i == number_of_bins
     size_t const cardinality = (*data->kmer_counts)[data->positions[0]];
     size_t const corrected_cardinality = static_cast<size_t>(cardinality * data->fpr_correction[trace_i]);
-    // NOLINTNEXTLINE(clang-analyzer-core.DivideZero)
     size_t const cardinality_per_bin = divide_and_ceil(corrected_cardinality, trace_i);
 
     data->hibf_layout->user_bins.emplace_back(data->previous.bin_indices, bin_id, trace_i, data->positions[0]);

src/sketch/compute_sketches.cpp

@@ -59,13 +59,13 @@ struct too_few_kmers_handler
         return flag.test();
     }
 
-    inline void set(size_t const available) noexcept
+    inline void set(size_t const new_available) noexcept
     {
         // Sets the flag to true and returns previous value.
         // If the flag was already set, another thread encountered this block at the same time.
         // This basically acts as a mutex for setting available.
         if (!flag.test_and_set())
-            this->available = available;
+            this->available = new_available;
     }
 
     inline void check_and_throw()

src/sketch/hyperloglog.cpp

@@ -25,7 +25,7 @@
 namespace seqan::hibf::sketch
 {
 
-hyperloglog::hyperloglog(uint8_t const bits) : bits{bits}, size{1ULL << bits}, data(size, 0u)
+hyperloglog::hyperloglog(uint8_t const num_bits) : bits{num_bits}, size{1ULL << bits}, data(size, 0u)
 {
     if (bits < 5u || bits > 32u)
         throw std::invalid_argument("[HyperLogLog] bit width must be in the range [5,32].");
@@ -110,7 +110,7 @@ double hyperloglog::estimate() const
         sum += *(sum_it + i);
     }
 
-    double estimate = normalization_factor / sum;
+    double estimate = normalization_factor / static_cast<double>(sum);
 
     // Small value correction: linear counting of zeros
     if (estimate <= 2.5 * size)

src/sketch/toolbox.cpp

@@ -257,7 +257,7 @@ void cluster_bins(std::vector<hyperloglog> const & sketches,
 
                 // compute the final min_id from the min_ids of the worker threads
                 size_t min_id = min_ids[0];
-                double min_dist = std::numeric_limits<double>::max();
+                double min_id_dist = std::numeric_limits<double>::max();
                 for (auto candidate_id : min_ids)
                 {
                     // check if the thread saw any id
@@ -266,9 +266,9 @@ void cluster_bins(std::vector<hyperloglog> const & sketches,
 
                     size_t const dist_index = remaining_ids.at(candidate_id);
                     neighbor const & curr = dist[dist_index].pq.top();
-                    if (curr.dist < min_dist)
+                    if (curr.dist < min_id_dist)
                     {
-                        min_dist = curr.dist;
+                        min_id_dist = curr.dist;
                         min_id = candidate_id;
                     }
                 }