ci: fix clang tidy after PR 2829 (#2799)

Author: canepat

silkworm/capi/instance.hpp

@@ -51,7 +51,7 @@ struct SilkwormInstance {
 
     // TODO: This has to be changed and encapsulated by a proper block caching state
     struct ExecutionResult {
-        silkworm::TxnId tx_id = 0;
+        silkworm::TxnId txn_id = 0;
         uint64_t blob_gas_used = 0;
         silkworm::Receipt receipt;
         uint64_t log_index = 0;

silkworm/capi/silkworm.cpp

@@ -612,14 +612,14 @@ SILKWORM_EXPORT int silkworm_execute_txn(SilkwormHandle handle, MDBX_txn* mdbx_t
     silkworm::Hash block_header_hash{};
     memcpy(block_header_hash.bytes, block_hash.bytes, sizeof(block_hash.bytes));
     BlockNum block_number{block_num};
-    TxnId txn_id_{txn_num};
+    TxnId txn_id{txn_num};
 
     auto unmanaged_tx = datastore::kvdb::RWTxnUnmanaged{mdbx_tx};
     auto unmanaged_env = silkworm::datastore::kvdb::EnvUnmanaged{::mdbx_txn_env(mdbx_tx)};
     auto chain_db = db::DataStore::make_chaindata_database(std::move(unmanaged_env));
     auto db_ref = chain_db.ref();
     silkworm::execution::DomainState state{
-        txn_id_,
+        txn_id,
         unmanaged_tx,
         db_ref,
         *handle->blocks_repository,
@@ -662,13 +662,13 @@ SILKWORM_EXPORT int silkworm_execute_txn(SilkwormHandle handle, MDBX_txn* mdbx_t
                << " TxnHash " << silkworm::to_hex(transaction.hash().bytes, true)
                << " Sender " << silkworm::to_hex(transaction.sender().value_or(evmc::address{}).bytes, true);
 
-    auto protocol_rule_set_{protocol::rule_set_factory(*handle->chain_config)};
-    if (!protocol_rule_set_) {
+    auto protocol_rule_set{protocol::rule_set_factory(*handle->chain_config)};
+    if (!protocol_rule_set) {
         SILK_ERROR << "Protocol rule set not created";
         return SILKWORM_INTERNAL_ERROR;
     }
 
-    ExecutionProcessor processor{block, *protocol_rule_set_, state, *handle->chain_config, false};
+    ExecutionProcessor processor{block, *protocol_rule_set, state, *handle->chain_config, false};
     // TODO: add analysis cache, check block exec for more
 
     silkworm::Receipt receipt{};
@@ -685,7 +685,7 @@ SILKWORM_EXPORT int silkworm_execute_txn(SilkwormHandle handle, MDBX_txn* mdbx_t
     try {
         processor.flush_state();
         SilkwormInstance::ExecutionResult exec_result{
-            .tx_id = txn_id_,
+            .txn_id = txn_id,
             .blob_gas_used = transaction.total_blob_gas(),
             .receipt = receipt,
             .log_index = 0};
@@ -696,7 +696,7 @@ SILKWORM_EXPORT int silkworm_execute_txn(SilkwormHandle handle, MDBX_txn* mdbx_t
             exec_result.receipt.cumulative_gas_used += prev.receipt.cumulative_gas_used;
             exec_result.log_index += std::size(prev.receipt.logs);
         }
-        handle->executions_in_block.push_back(std::move(exec_result));
+        handle->executions_in_block.emplace_back(std::move(exec_result));
     } catch (const std::exception& ex) {
         SILK_ERROR << "transaction post-processing failed: " << ex.what();
         return SILKWORM_INTERNAL_ERROR;
@@ -762,10 +762,10 @@ SILKWORM_EXPORT int silkworm_block_exec_end(SilkwormHandle handle, MDBX_txn* mdb
 
         rw_in_mem_cursor->insert(datastore::kvdb::Slice(key), datastore::kvdb::Slice(rlp_encoded));
 
-        const auto& tx_id = handle->executions_in_block[index].tx_id;
+        const auto& txn_id = handle->executions_in_block[index].txn_id;
 
         execution::DomainState state{
-            tx_id,
+            txn_id,
             unmanaged_tx,
             db_ref,
             *handle->blocks_repository,

silkworm/db/chain/remote_chain_storage_test.cpp

@@ -7,7 +7,6 @@
 
 #include <catch2/catch_test_macros.hpp>
 #include <catch2/matchers/catch_matchers_exception.hpp>
-#include <gmock/gmock.h>
 #include <nlohmann/json.hpp>
 
 #include <silkworm/core/common/util.hpp>
@@ -55,16 +54,15 @@ struct RemoteChainStorageTest : public silkworm::test_util::ContextTestBase {
     chain::Providers& providers{storage.providers()};
 };
 
+// Exclude on MSVC due to error LNK2001: unresolved external symbol testing::Matcher<class std::basic_string_view...
+// See also https://github.com/google/googletest/issues/4357
+#ifndef _WIN32
 TEST_CASE_METHOD(RemoteChainStorageTest, "read_chain_config") {
     SECTION("empty chain data") {
         EXPECT_CALL(transaction, get_one(table::kConfigName, _)).WillOnce(InvokeWithoutArgs([]() -> Task<Bytes> {
             co_return Bytes{};
         }));
-#ifdef SILKWORM_SANITIZE  // Avoid comparison against exception message: it triggers a TSAN data race seemingly related to libstdc++ string implementation
         CHECK_THROWS_AS(spawn_and_wait(storage.read_chain_config()), std::invalid_argument);
-#else
-        CHECK_THROWS_MATCHES(spawn_and_wait(storage.read_chain_config()), std::invalid_argument, Message("empty chain config data in read_chain_config"));
-#endif  // SILKWORM_SANITIZE
     }
 
     SECTION("invalid JSON chain data") {
@@ -98,6 +96,7 @@ TEST_CASE_METHOD(RemoteChainStorageTest, "read_chain_config") {
             })"_json);
     }
 }
+#endif  // _WIN32
 
 TEST_CASE_METHOD(RemoteChainStorageTest, "read_transaction_by_idx_in_block") {
     SECTION("not found") {
@@ -107,9 +106,9 @@ TEST_CASE_METHOD(RemoteChainStorageTest, "read_transaction_by_idx_in_block") {
     SECTION("found") {
         Block block = silkworm::test_util::sample_block();
         providers.block = [&](BlockNum, HashAsSpan, bool, Block& b) -> Task<bool> { b = block; co_return true; };
-        const auto txn0 = spawn_and_wait(storage.read_transaction_by_idx_in_block(block.header.number, /*txn_id=*/0));
+        const auto txn0 = spawn_and_wait(storage.read_transaction_by_idx_in_block(block.header.number, /*txn_idx=*/0));
         REQUIRE(txn0 == silkworm::test_util::kSampleTx0);
-        const auto txn1 = spawn_and_wait(storage.read_transaction_by_idx_in_block(block.header.number, /*txn_id=*/1));
+        const auto txn1 = spawn_and_wait(storage.read_transaction_by_idx_in_block(block.header.number, /*txn_idx=*/1));
         REQUIRE(txn1 == silkworm::test_util::kSampleTx1);
     }
 }

silkworm/db/cli/db_toolbox.cpp

@@ -2,11 +2,10 @@
 // SPDX-License-Identifier: Apache-2.0
 
 #include <algorithm>
-#include <bit>
-#include <bitset>
 #include <filesystem>
 #include <fstream>
 #include <iostream>
+#include <ranges>
 #include <regex>
 #include <stdexcept>
 #include <string>
@@ -251,7 +250,7 @@ void table_get(EnvConfig& config, const std::string& table, const std::optional<
     ensure(k.has_value() || block_num.has_value(), "You must specify either --key or --block");
     auto env = open_env(config);
     auto txn = env.start_read();
-    ensure(has_map(txn, table.c_str()), [&table]() { return "Table " + table + " not found"; });
+    ensure(has_map(txn, table), [&table]() { return "Table " + table + " not found"; });
     ::mdbx::map_handle table_map = txn.open_map(table);
     const std::string_view key_identifier = block_num ? "block_key" : "key";
     const Bytes key = k.value_or(block_key(*block_num));
@@ -277,8 +276,7 @@ void table_get(EnvConfig& config, const std::string& table, const std::optional<
     std::cout << "\n";
 }
 
-void do_clear(EnvConfig& config, bool dry, bool always_yes, const std::vector<std::string>& table_names,
-              bool drop) {
+void do_clear(EnvConfig& config, bool dry, bool always_yes, const std::vector<std::string>& table_names, bool drop) {
     config.readonly = false;
 
     if (!config.exclusive) {
@@ -288,22 +286,22 @@ void do_clear(EnvConfig& config, bool dry, bool always_yes, const std::vector<st
     auto env{open_env(config)};
     auto txn{env.start_write()};
 
-    for (const auto& tablename : table_names) {
-        if (!has_map(txn, tablename.c_str())) {
-            std::cout << "Table " << tablename << " not found\n";
+    for (const auto& table : table_names) {
+        if (!has_map(txn, table)) {
+            std::cout << "Table " << table << " not found\n";
             continue;
         }
 
-        mdbx::map_handle table_map{txn.open_map(tablename)};
+        mdbx::map_handle table_map{txn.open_map(table)};
         size_t rcount{txn.get_map_stat(table_map).ms_entries};
 
         if (!rcount && !drop) {
-            std::cout << " Table " << tablename << " is already empty. Skipping\n";
+            std::cout << " Table " << table << " is already empty. Skipping\n";
             continue;
         }
 
         std::cout << "\n"
-                  << (drop ? "Dropping" : "Emptying") << " table " << tablename << " (" << rcount << " records) "
+                  << (drop ? "Dropping" : "Emptying") << " table " << table << " (" << rcount << " records) "
                   << std::flush;
 
         if (!always_yes) {
@@ -1003,12 +1001,12 @@ static void print_table_diff(ROTxn& txn1, ROTxn& txn2, const DbTableInfo& table1
            [&]() { return "value_mode mismatch: " + std::to_string(static_cast<int>(table1.info.value_mode())) + " vs " + std::to_string(static_cast<int>(table2.info.value_mode())); });
 
     MapConfig table1_config{
-        .name = table1.name.c_str(),
+        .name = table1.name,
         .key_mode = table1.info.key_mode(),
         .value_mode = table1.info.value_mode(),
     };
     MapConfig table2_config{
-        .name = table2.name.c_str(),
+        .name = table2.name,
         .key_mode = table2.info.key_mode(),
         .value_mode = table2.info.value_mode(),
     };
@@ -1024,7 +1022,7 @@ static void print_table_diff(ROTxn& txn1, ROTxn& txn2, const DbTableInfo& table1
             "MAIN_DBI"sv,
             "DbInfo"sv,
         };
-        std::any_of(kIrrelevantTables.begin(), kIrrelevantTables.end(), [&table1](const std::string_view table_name) { return table_name == table1.name; })) {
+        std::ranges::any_of(kIrrelevantTables, [&table1](const std::string_view table_name) { return table_name == table1.name; })) {
         std::cout << "Skipping irrelevant table: " << table1.name << "\n";
         return;
     }
@@ -1050,7 +1048,7 @@ static void print_table_diff(ROTxn& txn1, ROTxn& txn2, const DbTableInfo& table1
 
 static std::optional<DbTableInfo> find_table(const DbInfo& db_info, std::string_view table) {
     const auto& db_tables{db_info.tables};
-    const auto it{std::find_if(db_tables.begin(), db_tables.end(), [=](const auto& t) { return t.name == table; })};
+    const auto it{std::ranges::find_if(db_tables, [=](const auto& t) { return t.name == table; })};
     return it != db_tables.end() ? std::make_optional<DbTableInfo>(*it) : std::nullopt;
 }
 
@@ -1066,12 +1064,12 @@ static DbComparisonResult compare_db_schema(const DbInfo& db1_info, const DbInfo
 
     // Check both databases have the same table names
     for (auto& db1_table : db1_tables) {
-        if (std::find(db2_tables.begin(), db2_tables.end(), db1_table) == db2_tables.end()) {
+        if (std::ranges::find(db2_tables, db1_table) == db2_tables.end()) {
             return tl::make_unexpected("db1 table " + db1_table.name + " not present in db2\n");
         }
     }
     for (auto& db2_table : db2_tables) {
-        if (std::find(db1_tables.begin(), db1_tables.end(), db2_table) == db1_tables.end()) {
+        if (std::ranges::find(db1_tables, db2_table) == db1_tables.end()) {
             return tl::make_unexpected("db2 table " + db2_table.name + " not present in db1\n");
         }
     }
@@ -1374,12 +1372,10 @@ void do_first_byte_analysis(EnvConfig& config) {
 
     // Sort histogram by usage (from most used to less used)
     std::vector<std::pair<uint8_t, size_t>> histogram_sorted;
-    std::copy(histogram.begin(), histogram.end(),
-              std::back_inserter<std::vector<std::pair<uint8_t, size_t>>>(histogram_sorted));
-    std::sort(histogram_sorted.begin(), histogram_sorted.end(),
-              [](std::pair<uint8_t, size_t>& a, std::pair<uint8_t, size_t>& b) -> bool {
-                  return a.second == b.second ? a.first < b.first : a.second > b.second;
-              });
+    std::ranges::copy(histogram, std::back_inserter<std::vector<std::pair<uint8_t, size_t>>>(histogram_sorted));
+    std::ranges::sort(histogram_sorted, [](std::pair<uint8_t, size_t>& a, std::pair<uint8_t, size_t>& b) -> bool {
+        return a.second == b.second ? a.first < b.first : a.second > b.second;
+    });
 
     if (!histogram_sorted.empty()) {
         std::cout << (boost::format(" %-4s %8s") % "Byte" % "Count") << "\n"

silkworm/db/datastore/common/ranges/caching_view.hpp

@@ -105,6 +105,6 @@ struct CachingViewFactory {
     }
 };
 
-inline constexpr CachingViewFactory caching;
+inline constexpr CachingViewFactory caching;  // NOLINT(*-identifier-naming)
 
 }  // namespace silkworm::views

silkworm/db/datastore/common/ranges/if_view.hpp

@@ -98,14 +98,13 @@ class IfView : public std::ranges::view_interface<IfView<Range1, Range2>> {
                 std::nullopt,
                 std::nullopt,
             };
-        } else {
-            return Iterator{
-                std::nullopt,
-                std::nullopt,
-                std::ranges::begin(range2_),
-                std::ranges::end(range2_),
-            };
         }
+        return Iterator{
+            std::nullopt,
+            std::nullopt,
+            std::ranges::begin(range2_),
+            std::ranges::end(range2_),
+        };
     }
 
     std::default_sentinel_t end() const { return std::default_sentinel; }

silkworm/db/datastore/common/ranges/lazy_view.hpp

@@ -21,7 +21,7 @@ class LazyView : public std::ranges::view_interface<LazyView<TRangeFactory, TRan
     LazyView& operator=(LazyView&& other) noexcept {
         range_factory_ = std::exchange(std::move(other.range_factory_), std::nullopt);
         range_ = std::exchange(std::move(other.range_), std::nullopt);
-        return this;
+        return *this;
     };
 
     std::ranges::iterator_t<TRange> begin() { return std::ranges::begin(range()); }

silkworm/db/datastore/common/ranges/merge_unique_view.hpp

@@ -135,10 +135,10 @@ class MergeUniqueView : public std::ranges::view_interface<MergeUniqueView<Range
             }
         }
 
-        Range1Iterator it1_;
-        Range1Sentinel sentinel1_;
-        Range2Iterator it2_;
-        Range2Sentinel sentinel2_;
+        Range1Iterator it1_{};
+        Range1Sentinel sentinel1_{};
+        Range2Iterator it2_{};
+        Range2Sentinel sentinel2_{};
         const Comp* comp_{nullptr};
         Proj1 proj1_;
         Proj2 proj2_;

silkworm/db/datastore/common/ranges/merge_view.hpp

@@ -46,9 +46,8 @@ class MergeView : public std::ranges::view_interface<MergeView<Range1, Range2, C
               sentinel2_{std::ranges::end(range2)},
               comp_{std::move(comp)},
               proj1_{std::move(proj1)},
-              proj2_{std::move(proj2)} {
-            selector_ = select(it1_ended(), it2_ended());
-        }
+              proj2_{std::move(proj2)},
+              selector_{select(it1_ended(), it2_ended())} {}
 
         reference operator*() const {
             switch (selector_) {

silkworm/db/datastore/common/ranges/unique_view.hpp

@@ -29,6 +29,6 @@ struct UniqueViewFactory {
 };
 
 template <class Comp = MergeCompareFunc, class Proj = std::identity>
-inline constexpr UniqueViewFactory<Comp, Proj> unique;
+inline constexpr UniqueViewFactory<Comp, Proj> unique;  // NOLINT(*-identifier-naming)
 
 }  // namespace silkworm::views