Merge bitcoin/bitcoin#26749: refactor: Use move semantics instead of custom swap functions

95ad70ab652ddde7de65f633c36c1378b26a313a test: Default initialize `should_freeze` to `true` (Hennadii Stepanov)
cea50521fe810111a8a3c84ad14f944eafb5b658 refactor: Drop no longer used `swap` member functions (Hennadii Stepanov)
a87fb6bee5a7fb0879b3adea9a29997f1331acb0 clang-tidy: Fix modernize-use-default-member-init in `CScriptCheck` (Hennadii Stepanov)
b4bed5c1f98c0eed18f52fdcea11a420c10ed98d refactor: Drop no longer used `CScriptCheck()` default constructor (Hennadii Stepanov)
d8427cc28e3a9ac3319fb452b16661957c812b8f refactor: Use move semantics in `CCheckQueue::Loop` (Hennadii Stepanov)
9a0b5241396efe3b3ceb3931717c30bb94f99bfb clang-tidy, test: Fix bugprone-use-after-move in `Correct_Queue_range()` (Hennadii Stepanov)
04831fee6dca3eb86cd1d6b9ef879b296263fe35 refactor: Make move semantics explicit for callers (Hennadii Stepanov)
6c2d5972f3544c4f3e987828a99e88f27b62cf87 refactor: Use move semantics in `CCheckQueue::Add` (Hennadii Stepanov)
06820032142a75cc3c5b832045058bc6f6f74786 test, refactor: Avoid `CScriptCheck::swap` in `transaction_tests` (Hennadii Stepanov)
15209d97c6aad7d5c199fe007ad39b91c8ee6562 consensus, refactor: Avoid `CScriptCheck::swap` in `CheckInputScripts` (Hennadii Stepanov)

Pull request description:

  This PR makes code more succinct and readable by using move semantics.

ACKs for top commit:
  martinus:
    re-ACK 95ad70ab652ddde7de65f633c36c1378b26a313a
  achow101:
    ACK 95ad70ab652ddde7de65f633c36c1378b26a313a
  TheCharlatan:
    re-ACK https://github.com/bitcoin/bitcoin/commit/95ad70ab652ddde7de65f633c36c1378b26a313a
  MarcoFalke:
    re-ACK 95ad70ab652ddde7de65f633c36c1378b26a313a 🚥

Tree-SHA512: adda760891b12d252dc9b823fe7c41eed660364b6fb1a69f17607d7a31eb0bbb82a80d154a7acfaa241b5de37d42a293c2b6e059f26a8e92d88d3a87c99768fb

3 files changed, 25 insertions, 44 deletions

diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp
index 135f107159..012b0eb321 100644
--- a/src/test/checkqueue_tests.cpp
+++ b/src/test/checkqueue_tests.cpp
@@ -43,7 +43,6 @@ struct FakeCheck {
     {
         return true;
     }
-    void swap(FakeCheck& x) noexcept {};
 };
 
 struct FakeCheckCheckCompletion {
@@ -53,7 +52,6 @@ struct FakeCheckCheckCompletion {
         n_calls.fetch_add(1, std::memory_order_relaxed);
         return true;
     }
-    void swap(FakeCheckCheckCompletion& x) noexcept {};
 };
 
 struct FailingCheck {
@@ -64,10 +62,6 @@ struct FailingCheck {
     {
         return !fails;
     }
-    void swap(FailingCheck& x) noexcept
-    {
-        std::swap(fails, x.fails);
-    };
 };
 
 struct UniqueCheck {
@@ -82,10 +76,6 @@ struct UniqueCheck {
         results.insert(check_id);
         return true;
     }
-    void swap(UniqueCheck& x) noexcept
-    {
-        std::swap(x.check_id, check_id);
-    };
 };
 
 
@@ -113,19 +103,13 @@ struct MemoryCheck {
     {
         fake_allocated_memory.fetch_sub(b, std::memory_order_relaxed);
     };
-    void swap(MemoryCheck& x) noexcept
-    {
-        std::swap(b, x.b);
-    };
 };
 
 struct FrozenCleanupCheck {
     static std::atomic<uint64_t> nFrozen;
     static std::condition_variable cv;
     static std::mutex m;
-    // Freezing can't be the default initialized behavior given how the queue
-    // swaps in default initialized Checks.
-    bool should_freeze {false};
+    bool should_freeze{true};
     bool operator()() const
     {
         return true;
@@ -140,10 +124,17 @@ struct FrozenCleanupCheck {
             cv.wait(l, []{ return nFrozen.load(std::memory_order_relaxed) == 0;});
         }
     }
-    void swap(FrozenCleanupCheck& x) noexcept
+    FrozenCleanupCheck(FrozenCleanupCheck&& other) noexcept
     {
-        std::swap(should_freeze, x.should_freeze);
-    };
+        should_freeze = other.should_freeze;
+        other.should_freeze = false;
+    }
+    FrozenCleanupCheck& operator=(FrozenCleanupCheck&& other) noexcept
+    {
+        should_freeze = other.should_freeze;
+        other.should_freeze = false;
+        return *this;
+    }
 };
 
 // Static Allocations
@@ -173,14 +164,16 @@ static void Correct_Queue_range(std::vector<size_t> range)
     small_queue->StartWorkerThreads(SCRIPT_CHECK_THREADS);
     // Make vChecks here to save on malloc (this test can be slow...)
     std::vector<FakeCheckCheckCompletion> vChecks;
+    vChecks.reserve(9);
     for (const size_t i : range) {
         size_t total = i;
         FakeCheckCheckCompletion::n_calls = 0;
         CCheckQueueControl<FakeCheckCheckCompletion> control(small_queue.get());
         while (total) {
-            vChecks.resize(std::min(total, (size_t) InsecureRandRange(10)));
+            vChecks.clear();
+            vChecks.resize(std::min<size_t>(total, InsecureRandRange(10)));
             total -= vChecks.size();
-            control.Add(vChecks);
+            control.Add(std::move(vChecks));
         }
         BOOST_REQUIRE(control.Wait());
         if (FakeCheckCheckCompletion::n_calls != i) {
@@ -242,7 +235,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Catches_Failure)
             vChecks.reserve(r);
             for (size_t k = 0; k < r && remaining; k++, remaining--)
                 vChecks.emplace_back(remaining == 1);
-            control.Add(vChecks);
+            control.Add(std::move(vChecks));
         }
         bool success = control.Wait();
         if (i > 0) {
@@ -267,7 +260,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Recovers_From_Failure)
                 std::vector<FailingCheck> vChecks;
                 vChecks.resize(100, false);
                 vChecks[99] = end_fails;
-                control.Add(vChecks);
+                control.Add(std::move(vChecks));
             }
             bool r =control.Wait();
             BOOST_REQUIRE(r != end_fails);
@@ -293,7 +286,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_UniqueCheck)
             std::vector<UniqueCheck> vChecks;
             for (size_t k = 0; k < r && total; k++)
                 vChecks.emplace_back(--total);
-            control.Add(vChecks);
+            control.Add(std::move(vChecks));
         }
     }
     {
@@ -331,7 +324,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Memory)
                     // to catch any sort of deallocation failure
                     vChecks.emplace_back(total == 0 || total == i || total == i/2);
                 }
-                control.Add(vChecks);
+                control.Add(std::move(vChecks));
             }
         }
         BOOST_REQUIRE_EQUAL(MemoryCheck::fake_allocated_memory, 0U);
@@ -349,11 +342,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_FrozenCleanup)
     std::thread t0([&]() {
         CCheckQueueControl<FrozenCleanupCheck> control(queue.get());
         std::vector<FrozenCleanupCheck> vChecks(1);
-        // Freezing can't be the default initialized behavior given how the queue
-        // swaps in default initialized Checks (otherwise freezing destructor
-        // would get called twice).
-        vChecks[0].should_freeze = true;
-        control.Add(vChecks);
+        control.Add(std::move(vChecks));
         bool waitResult = control.Wait(); // Hangs here
         assert(waitResult);
     });
diff --git a/src/test/fuzz/checkqueue.cpp b/src/test/fuzz/checkqueue.cpp
index 6256aefaa3..429570526f 100644
--- a/src/test/fuzz/checkqueue.cpp
+++ b/src/test/fuzz/checkqueue.cpp
@@ -13,9 +13,7 @@
 
 namespace {
 struct DumbCheck {
-    const bool result = false;
-
-    DumbCheck() = default;
+    bool result = false;
 
     explicit DumbCheck(const bool _result) : result(_result)
     {
@@ -25,10 +23,6 @@ struct DumbCheck {
     {
         return result;
     }
-
-    void swap(DumbCheck& x) noexcept
-    {
-    }
 };
 } // namespace
 
@@ -48,7 +42,7 @@ FUZZ_TARGET(checkqueue)
         checks_2.emplace_back(result);
     }
     if (fuzzed_data_provider.ConsumeBool()) {
-        check_queue_1.Add(checks_1);
+        check_queue_1.Add(std::move(checks_1));
     }
     if (fuzzed_data_provider.ConsumeBool()) {
         (void)check_queue_1.Wait();
@@ -56,7 +50,7 @@ FUZZ_TARGET(checkqueue)
 
     CCheckQueueControl<DumbCheck> check_queue_control{&check_queue_2};
     if (fuzzed_data_provider.ConsumeBool()) {
-        check_queue_control.Add(checks_2);
+        check_queue_control.Add(std::move(checks_2));
     }
     if (fuzzed_data_provider.ConsumeBool()) {
         (void)check_queue_control.Wait();
diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp
index 11efb6a5c3..7511dff9e8 100644
--- a/src/test/transaction_tests.cpp
+++ b/src/test/transaction_tests.cpp
@@ -547,10 +547,8 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction)
 
     for(uint32_t i = 0; i < mtx.vin.size(); i++) {
         std::vector<CScriptCheck> vChecks;
-        CScriptCheck check(coins[tx.vin[i].prevout.n].out, tx, i, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false, &txdata);
-        vChecks.push_back(CScriptCheck());
-        check.swap(vChecks.back());
-        control.Add(vChecks);
+        vChecks.emplace_back(coins[tx.vin[i].prevout.n].out, tx, i, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false, &txdata);
+        control.Add(std::move(vChecks));
     }
 
     bool controlCheck = control.Wait();