Merge #21404: refactor: Remove MakeUnique<T>()

1a6323bdbe20bdb7b1c907d8fa0333ffa88b21ff doc: update developer notes for removal of MakeUnique (fanquake)
3ba2840e7ee81341b0748c0121aedc2e9305041a scripted-diff: remove MakeUnique<T>() (fanquake)

Pull request description:

  Since requiring C++17, this is just pointless abstraction. I think we should just "tear the band-aid off" and remove it. Similar to the changes happening in #21366.

  Also, having a comment saying this is deprecated doesn't prevent it's usage in new code. i.e : https://github.com/bitcoin/bitcoin/pull/20946#discussion_r561949731.

  The repository is fairly quiet at the moment, so any potential complaints about having to rebase should be minimal. Might as well get this over and done with.

ACKs for top commit:
  jnewbery:
    utACK 1a6323bdbe20bdb7b1c907d8fa0333ffa88b21ff
  practicalswift:
    cr ACK 1a6323bdbe20bdb7b1c907d8fa0333ffa88b21ff: patch looks correct
  ajtowns:
    ACK 1a6323bdbe20bdb7b1c907d8fa0333ffa88b21ff -- code review only
  glozow:
    ACK https://github.com/bitcoin/bitcoin/commit/1a6323bdbe20bdb7b1c907d8fa0333ffa88b21ff looks correct

Tree-SHA512: 4a14b9611b60b9b3026b54d6f5a2dce4c5d9b63a7b93d7de1307512df736503ed84bac66e7b93372c76e3117f49bf9f29cd473d3a47cb41fb2775bc10234736f

18 files changed, 26 insertions, 43 deletions

diff --git a/src/test/allocator_tests.cpp b/src/test/allocator_tests.cpp
index d33d668a04..b523173a45 100644
--- a/src/test/allocator_tests.cpp
+++ b/src/test/allocator_tests.cpp
@@ -2,7 +2,6 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-#include <util/memory.h>
 #include <util/system.h>
 
 #include <test/util/setup_common.h>
@@ -163,7 +162,7 @@ private:
 BOOST_AUTO_TEST_CASE(lockedpool_tests_mock)
 {
     // Test over three virtual arenas, of which one will succeed being locked
-    std::unique_ptr<LockedPageAllocator> x = MakeUnique<TestLockedPageAllocator>(3, 1);
+    std::unique_ptr<LockedPageAllocator> x = std::make_unique<TestLockedPageAllocator>(3, 1);
     LockedPool pool(std::move(x));
     BOOST_CHECK(pool.stats().total == 0);
     BOOST_CHECK(pool.stats().locked == 0);
diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp
index 21921375b3..64c6d7f634 100644
--- a/src/test/checkqueue_tests.cpp
+++ b/src/test/checkqueue_tests.cpp
@@ -5,7 +5,6 @@
 #include <checkqueue.h>
 #include <sync.h>
 #include <test/util/setup_common.h>
-#include <util/memory.h>
 #include <util/system.h>
 #include <util/time.h>
 
@@ -146,7 +145,7 @@ typedef CCheckQueue<FrozenCleanupCheck> FrozenCleanup_Queue;
  */
 static void Correct_Queue_range(std::vector<size_t> range)
 {
-    auto small_queue = MakeUnique<Correct_Queue>(QUEUE_BATCH_SIZE);
+    auto small_queue = std::make_unique<Correct_Queue>(QUEUE_BATCH_SIZE);
     small_queue->StartWorkerThreads(SCRIPT_CHECK_THREADS);
     // Make vChecks here to save on malloc (this test can be slow...)
     std::vector<FakeCheckCheckCompletion> vChecks;
@@ -206,7 +205,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Correct_Random)
 /** Test that failing checks are caught */
 BOOST_AUTO_TEST_CASE(test_CheckQueue_Catches_Failure)
 {
-    auto fail_queue = MakeUnique<Failing_Queue>(QUEUE_BATCH_SIZE);
+    auto fail_queue = std::make_unique<Failing_Queue>(QUEUE_BATCH_SIZE);
     fail_queue->StartWorkerThreads(SCRIPT_CHECK_THREADS);
 
     for (size_t i = 0; i < 1001; ++i) {
@@ -234,7 +233,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Catches_Failure)
 // future blocks, ie, the bad state is cleared.
 BOOST_AUTO_TEST_CASE(test_CheckQueue_Recovers_From_Failure)
 {
-    auto fail_queue = MakeUnique<Failing_Queue>(QUEUE_BATCH_SIZE);
+    auto fail_queue = std::make_unique<Failing_Queue>(QUEUE_BATCH_SIZE);
     fail_queue->StartWorkerThreads(SCRIPT_CHECK_THREADS);
 
     for (auto times = 0; times < 10; ++times) {
@@ -258,7 +257,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Recovers_From_Failure)
 // more than once as well
 BOOST_AUTO_TEST_CASE(test_CheckQueue_UniqueCheck)
 {
-    auto queue = MakeUnique<Unique_Queue>(QUEUE_BATCH_SIZE);
+    auto queue = std::make_unique<Unique_Queue>(QUEUE_BATCH_SIZE);
     queue->StartWorkerThreads(SCRIPT_CHECK_THREADS);
 
     size_t COUNT = 100000;
@@ -293,7 +292,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_UniqueCheck)
 // time could leave the data hanging across a sequence of blocks.
 BOOST_AUTO_TEST_CASE(test_CheckQueue_Memory)
 {
-    auto queue = MakeUnique<Memory_Queue>(QUEUE_BATCH_SIZE);
+    auto queue = std::make_unique<Memory_Queue>(QUEUE_BATCH_SIZE);
     queue->StartWorkerThreads(SCRIPT_CHECK_THREADS);
     for (size_t i = 0; i < 1000; ++i) {
         size_t total = i;
@@ -320,7 +319,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Memory)
 // have been destructed
 BOOST_AUTO_TEST_CASE(test_CheckQueue_FrozenCleanup)
 {
-    auto queue = MakeUnique<FrozenCleanup_Queue>(QUEUE_BATCH_SIZE);
+    auto queue = std::make_unique<FrozenCleanup_Queue>(QUEUE_BATCH_SIZE);
     bool fails = false;
     queue->StartWorkerThreads(SCRIPT_CHECK_THREADS);
     std::thread t0([&]() {
@@ -360,7 +359,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_FrozenCleanup)
 /** Test that CCheckQueueControl is threadsafe */
 BOOST_AUTO_TEST_CASE(test_CheckQueueControl_Locks)
 {
-    auto queue = MakeUnique<Standard_Queue>(QUEUE_BATCH_SIZE);
+    auto queue = std::make_unique<Standard_Queue>(QUEUE_BATCH_SIZE);
     {
         std::vector<std::thread> tg;
         std::atomic<int> nThreads {0};
diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp
index 3d802cbeb3..a6080ad3dd 100644
--- a/src/test/dbwrapper_tests.cpp
+++ b/src/test/dbwrapper_tests.cpp
@@ -5,7 +5,6 @@
 #include <dbwrapper.h>
 #include <test/util/setup_common.h>
 #include <uint256.h>
-#include <util/memory.h>
 
 #include <memory>
 
@@ -207,7 +206,7 @@ BOOST_AUTO_TEST_CASE(existing_data_no_obfuscate)
     create_directories(ph);
 
     // Set up a non-obfuscated wrapper to write some initial data.
-    std::unique_ptr<CDBWrapper> dbw = MakeUnique<CDBWrapper>(ph, (1 << 10), false, false, false);
+    std::unique_ptr<CDBWrapper> dbw = std::make_unique<CDBWrapper>(ph, (1 << 10), false, false, false);
     char key = 'k';
     uint256 in = InsecureRand256();
     uint256 res;
@@ -248,7 +247,7 @@ BOOST_AUTO_TEST_CASE(existing_data_reindex)
     create_directories(ph);
 
     // Set up a non-obfuscated wrapper to write some initial data.
-    std::unique_ptr<CDBWrapper> dbw = MakeUnique<CDBWrapper>(ph, (1 << 10), false, false, false);
+    std::unique_ptr<CDBWrapper> dbw = std::make_unique<CDBWrapper>(ph, (1 << 10), false, false, false);
     char key = 'k';
     uint256 in = InsecureRand256();
     uint256 res;
diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp
index 5906913b58..e75982bc6f 100644
--- a/src/test/denialofservice_tests.cpp
+++ b/src/test/denialofservice_tests.cpp
@@ -15,7 +15,6 @@
 #include <script/standard.h>
 #include <serialize.h>
 #include <txorphanage.h>
-#include <util/memory.h>
 #include <util/string.h>
 #include <util/system.h>
 #include <util/time.h>
@@ -68,7 +67,7 @@ BOOST_FIXTURE_TEST_SUITE(denialofservice_tests, TestingSetup)
 BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
 {
     const CChainParams& chainparams = Params();
-    auto connman = MakeUnique<CConnman>(0x1337, 0x1337);
+    auto connman = std::make_unique<CConnman>(0x1337, 0x1337);
     auto peerLogic = PeerManager::make(chainparams, *connman, nullptr, *m_node.scheduler,
                                        *m_node.chainman, *m_node.mempool, false);
 
@@ -138,7 +137,7 @@ static void AddRandomOutboundPeer(std::vector<CNode *> &vNodes, PeerManager &pee
 BOOST_AUTO_TEST_CASE(stale_tip_peer_management)
 {
     const CChainParams& chainparams = Params();
-    auto connman = MakeUnique<CConnmanTest>(0x1337, 0x1337);
+    auto connman = std::make_unique<CConnmanTest>(0x1337, 0x1337);
     auto peerLogic = PeerManager::make(chainparams, *connman, nullptr, *m_node.scheduler,
                                        *m_node.chainman, *m_node.mempool, false);
 
@@ -211,8 +210,8 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management)
 BOOST_AUTO_TEST_CASE(peer_discouragement)
 {
     const CChainParams& chainparams = Params();
-    auto banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME);
-    auto connman = MakeUnique<CConnman>(0x1337, 0x1337);
+    auto banman = std::make_unique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME);
+    auto connman = std::make_unique<CConnman>(0x1337, 0x1337);
     auto peerLogic = PeerManager::make(chainparams, *connman, banman.get(), *m_node.scheduler,
                                        *m_node.chainman, *m_node.mempool, false);
 
@@ -258,8 +257,8 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
 BOOST_AUTO_TEST_CASE(DoS_bantime)
 {
     const CChainParams& chainparams = Params();
-    auto banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME);
-    auto connman = MakeUnique<CConnman>(0x1337, 0x1337);
+    auto banman = std::make_unique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME);
+    auto connman = std::make_unique<CConnman>(0x1337, 0x1337);
     auto peerLogic = PeerManager::make(chainparams, *connman, banman.get(), *m_node.scheduler,
                                        *m_node.chainman, *m_node.mempool, false);
 
diff --git a/src/test/fuzz/descriptor_parse.cpp b/src/test/fuzz/descriptor_parse.cpp
index 0d1921f285..ffe4855662 100644
--- a/src/test/fuzz/descriptor_parse.cpp
+++ b/src/test/fuzz/descriptor_parse.cpp
@@ -6,7 +6,6 @@
 #include <pubkey.h>
 #include <script/descriptor.h>
 #include <test/fuzz/fuzz.h>
-#include <util/memory.h>
 
 void initialize_descriptor_parse()
 {
diff --git a/src/test/fuzz/eval_script.cpp b/src/test/fuzz/eval_script.cpp
index 635288fc36..77ed798923 100644
--- a/src/test/fuzz/eval_script.cpp
+++ b/src/test/fuzz/eval_script.cpp
@@ -6,7 +6,6 @@
 #include <script/interpreter.h>
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
-#include <util/memory.h>
 
 #include <limits>
 
diff --git a/src/test/fuzz/key.cpp b/src/test/fuzz/key.cpp
index aa8f826e4a..32077b1fe2 100644
--- a/src/test/fuzz/key.cpp
+++ b/src/test/fuzz/key.cpp
@@ -17,7 +17,6 @@
 #include <script/standard.h>
 #include <streams.h>
 #include <test/fuzz/fuzz.h>
-#include <util/memory.h>
 #include <util/strencodings.h>
 
 #include <cassert>
diff --git a/src/test/fuzz/parse_univalue.cpp b/src/test/fuzz/parse_univalue.cpp
index afe382ba21..3fffaac8d0 100644
--- a/src/test/fuzz/parse_univalue.cpp
+++ b/src/test/fuzz/parse_univalue.cpp
@@ -7,7 +7,6 @@
 #include <rpc/client.h>
 #include <rpc/util.h>
 #include <test/fuzz/fuzz.h>
-#include <util/memory.h>
 
 #include <limits>
 #include <string>
diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp
index 0289d49ccc..04fc6da9b1 100644
--- a/src/test/fuzz/process_message.cpp
+++ b/src/test/fuzz/process_message.cpp
@@ -19,7 +19,6 @@
 #include <test/util/setup_common.h>
 #include <test/util/validation.h>
 #include <txorphanage.h>
-#include <util/memory.h>
 #include <validationinterface.h>
 #include <version.h>
 
diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp
index 617a71ea60..ee402dba38 100644
--- a/src/test/fuzz/process_messages.cpp
+++ b/src/test/fuzz/process_messages.cpp
@@ -14,7 +14,6 @@
 #include <test/util/setup_common.h>
 #include <test/util/validation.h>
 #include <txorphanage.h>
-#include <util/memory.h>
 #include <validation.h>
 #include <validationinterface.h>
 
diff --git a/src/test/fuzz/psbt.cpp b/src/test/fuzz/psbt.cpp
index 0b4588c4ce..eb2754054a 100644
--- a/src/test/fuzz/psbt.cpp
+++ b/src/test/fuzz/psbt.cpp
@@ -10,7 +10,6 @@
 #include <pubkey.h>
 #include <script/script.h>
 #include <streams.h>
-#include <util/memory.h>
 #include <version.h>
 
 #include <cstdint>
diff --git a/src/test/fuzz/script.cpp b/src/test/fuzz/script.cpp
index 193862e847..8219a04e49 100644
--- a/src/test/fuzz/script.cpp
+++ b/src/test/fuzz/script.cpp
@@ -20,7 +20,6 @@
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
 #include <univalue.h>
-#include <util/memory.h>
 
 #include <algorithm>
 #include <cassert>
diff --git a/src/test/fuzz/script_flags.cpp b/src/test/fuzz/script_flags.cpp
index ce8915ca2c..561230707c 100644
--- a/src/test/fuzz/script_flags.cpp
+++ b/src/test/fuzz/script_flags.cpp
@@ -5,7 +5,6 @@
 #include <pubkey.h>
 #include <script/interpreter.h>
 #include <streams.h>
-#include <util/memory.h>
 #include <version.h>
 
 #include <test/fuzz/fuzz.h>
diff --git a/src/test/fuzz/signature_checker.cpp b/src/test/fuzz/signature_checker.cpp
index 3e7b72805e..7b57c5dfd8 100644
--- a/src/test/fuzz/signature_checker.cpp
+++ b/src/test/fuzz/signature_checker.cpp
@@ -6,7 +6,6 @@
 #include <script/interpreter.h>
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
-#include <util/memory.h>
 
 #include <cstdint>
 #include <limits>
@@ -15,7 +14,7 @@
 
 void initialize_signature_checker()
 {
-    static const auto verify_handle = MakeUnique<ECCVerifyHandle>();
+    static const auto verify_handle = std::make_unique<ECCVerifyHandle>();
 }
 
 namespace {
diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp
index a29c6a2665..167242a971 100644
--- a/src/test/net_tests.cpp
+++ b/src/test/net_tests.cpp
@@ -14,7 +14,6 @@
 #include <span.h>
 #include <streams.h>
 #include <test/util/setup_common.h>
-#include <util/memory.h>
 #include <util/strencodings.h>
 #include <util/string.h>
 #include <util/system.h>
@@ -188,7 +187,7 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
     CAddress addr = CAddress(CService(ipv4Addr, 7777), NODE_NETWORK);
     std::string pszDest;
 
-    std::unique_ptr<CNode> pnode1 = MakeUnique<CNode>(
+    std::unique_ptr<CNode> pnode1 = std::make_unique<CNode>(
         id++, NODE_NETWORK, hSocket, addr,
         /* nKeyedNetGroupIn = */ 0,
         /* nLocalHostNonceIn = */ 0,
@@ -203,7 +202,7 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
     BOOST_CHECK(pnode1->m_inbound_onion == false);
     BOOST_CHECK_EQUAL(pnode1->ConnectedThroughNetwork(), Network::NET_IPV4);
 
-    std::unique_ptr<CNode> pnode2 = MakeUnique<CNode>(
+    std::unique_ptr<CNode> pnode2 = std::make_unique<CNode>(
         id++, NODE_NETWORK, hSocket, addr,
         /* nKeyedNetGroupIn = */ 1,
         /* nLocalHostNonceIn = */ 1,
@@ -218,7 +217,7 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
     BOOST_CHECK(pnode2->m_inbound_onion == false);
     BOOST_CHECK_EQUAL(pnode2->ConnectedThroughNetwork(), Network::NET_IPV4);
 
-    std::unique_ptr<CNode> pnode3 = MakeUnique<CNode>(
+    std::unique_ptr<CNode> pnode3 = std::make_unique<CNode>(
         id++, NODE_NETWORK, hSocket, addr,
         /* nKeyedNetGroupIn = */ 0,
         /* nLocalHostNonceIn = */ 0,
@@ -233,7 +232,7 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
     BOOST_CHECK(pnode3->m_inbound_onion == false);
     BOOST_CHECK_EQUAL(pnode3->ConnectedThroughNetwork(), Network::NET_IPV4);
 
-    std::unique_ptr<CNode> pnode4 = MakeUnique<CNode>(
+    std::unique_ptr<CNode> pnode4 = std::make_unique<CNode>(
         id++, NODE_NETWORK, hSocket, addr,
         /* nKeyedNetGroupIn = */ 1,
         /* nLocalHostNonceIn = */ 1,
@@ -711,7 +710,7 @@ BOOST_AUTO_TEST_CASE(ipv4_peer_with_ipv6_addrMe_test)
     in_addr ipv4AddrPeer;
     ipv4AddrPeer.s_addr = 0xa0b0c001;
     CAddress addr = CAddress(CService(ipv4AddrPeer, 7777), NODE_NETWORK);
-    std::unique_ptr<CNode> pnode = MakeUnique<CNode>(0, NODE_NETWORK, INVALID_SOCKET, addr, /* nKeyedNetGroupIn */ 0, /* nLocalHostNonceIn */ 0, CAddress{}, /* pszDest */ std::string{}, ConnectionType::OUTBOUND_FULL_RELAY, /* inbound_onion */ false);
+    std::unique_ptr<CNode> pnode = std::make_unique<CNode>(0, NODE_NETWORK, INVALID_SOCKET, addr, /* nKeyedNetGroupIn */ 0, /* nLocalHostNonceIn */ 0, CAddress{}, /* pszDest */ std::string{}, ConnectionType::OUTBOUND_FULL_RELAY, /* inbound_onion */ false);
     pnode->fSuccessfullyConnected.store(true);
 
     // the peer claims to be reaching us via IPv6
diff --git a/src/test/util/logging.cpp b/src/test/util/logging.cpp
index c1dd47f06c..66f4efc139 100644
--- a/src/test/util/logging.cpp
+++ b/src/test/util/logging.cpp
@@ -7,7 +7,6 @@
 #include <logging.h>
 #include <noui.h>
 #include <tinyformat.h>
-#include <util/memory.h>
 
 #include <stdexcept>
 
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index f2174cd084..f866c2a1f9 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -25,7 +25,6 @@
 #include <script/sigcache.h>
 #include <streams.h>
 #include <txdb.h>
-#include <util/memory.h>
 #include <util/strencodings.h>
 #include <util/string.h>
 #include <util/time.h>
@@ -131,7 +130,7 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve
 {
     // We have to run a scheduler thread to prevent ActivateBestChain
     // from blocking due to queue overrun.
-    m_node.scheduler = MakeUnique<CScheduler>();
+    m_node.scheduler = std::make_unique<CScheduler>();
     m_node.scheduler->m_service_thread = std::thread([&] { TraceThread("scheduler", [&] { m_node.scheduler->serviceQueue(); }); });
     GetMainSignals().RegisterBackgroundSignalScheduler(*m_node.scheduler);
 
@@ -188,8 +187,8 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const
         throw std::runtime_error(strprintf("ActivateBestChain failed. (%s)", state.ToString()));
     }
 
-    m_node.banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME);
-    m_node.connman = MakeUnique<CConnman>(0x1337, 0x1337); // Deterministic randomness for tests.
+    m_node.banman = std::make_unique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME);
+    m_node.connman = std::make_unique<CConnman>(0x1337, 0x1337); // Deterministic randomness for tests.
     m_node.peerman = PeerManager::make(chainparams, *m_node.connman, m_node.banman.get(),
                                        *m_node.scheduler, *m_node.chainman, *m_node.mempool,
                                        false);
diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
index 5a46002a79..b742fcaf0b 100644
--- a/src/test/util_tests.cpp
+++ b/src/test/util_tests.cpp
@@ -71,7 +71,7 @@ BOOST_AUTO_TEST_CASE(util_datadir)
 BOOST_AUTO_TEST_CASE(util_check)
 {
     // Check that Assert can forward
-    const std::unique_ptr<int> p_two = Assert(MakeUnique<int>(2));
+    const std::unique_ptr<int> p_two = Assert(std::make_unique<int>(2));
     // Check that Assert works on lvalues and rvalues
     const int two = *Assert(p_two);
     Assert(two == 2);