Merge bitcoin/bitcoin#28687: C++20 std::views::reverse

2925bd537c refactor: use c++20 std::views::reverse instead of reverse_iterator.h (stickies-v)

Pull request description:

  C++20 introduces [`std::ranges::views::reverse`](https://en.cppreference.com/w/cpp/ranges/reverse_view), which allows us to drop our own `reverse_iterator.h` implementation and also makes it easier to chain views (even though I think we currently don't use this).

ACKs for top commit:
  achow101:
    ACK 2925bd537c
  maflcko:
    ACK 2925bd537c 🎷

Tree-SHA512: 567666ec44af5d1beb7a271836bcc89c4c577abc77f522fcc18bc6d4de516ae9b0df766d0bfa6dd217569e6878331c2aee1d9815620860375e3510dad7fed476

contrib/devtools/copyright_header.py

@@ -19,7 +19,6 @@ EXCLUDE = [
     'src/qt/bitcoinstrings.cpp',
     'src/chainparamsseeds.h',
     # other external copyrights:
-    'src/reverse_iterator.h',
     'src/test/fuzz/FuzzedDataProvider.h',
     'src/tinyformat.h',
     'src/bench/nanobench.h',

src/Makefile.am

@@ -257,7 +257,6 @@ BITCOIN_CORE_H = \
   random.h \
   randomenv.h \
   rest.h \
-  reverse_iterator.h \
   rpc/blockchain.h \
   rpc/client.h \
   rpc/mempool.h \

src/net_processing.cpp

@@ -32,7 +32,6 @@
 #include <primitives/block.h>
 #include <primitives/transaction.h>
 #include <random.h>
-#include <reverse_iterator.h>
 #include <scheduler.h>
 #include <streams.h>
 #include <sync.h>
@@ -51,6 +50,7 @@
 #include <future>
 #include <memory>
 #include <optional>
+#include <ranges>
 #include <typeinfo>
 #include <utility>
 
@@ -2259,7 +2259,7 @@ void PeerManagerImpl::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlock
         for (auto& it : m_peer_map) {
             Peer& peer = *it.second;
             LOCK(peer.m_block_inv_mutex);
-            for (const uint256& hash : reverse_iterate(vHashes)) {
+            for (const uint256& hash : vHashes | std::views::reverse) {
                 peer.m_blocks_for_headers_relay.push_back(hash);
             }
         }
@@ -2958,7 +2958,7 @@ void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, c
         } else {
             std::vector<CInv> vGetData;
             // Download as much as possible, from earliest to latest.
-            for (const CBlockIndex *pindex : reverse_iterate(vToFetch)) {
+            for (const CBlockIndex* pindex : vToFetch | std::views::reverse) {
                 if (nodestate->vBlocksInFlight.size() >= MAX_BLOCKS_IN_TRANSIT_PER_PEER) {
                     // Can't download any more from this peer
                     break;

src/node/blockstorage.cpp

@@ -20,7 +20,6 @@
 #include <primitives/block.h>
 #include <primitives/transaction.h>
 #include <random.h>
-#include <reverse_iterator.h>
 #include <serialize.h>
 #include <signet.h>
 #include <span.h>
@@ -38,6 +37,7 @@
 #include <validation.h>
 
 #include <map>
+#include <ranges>
 #include <unordered_map>
 
 namespace kernel {
@@ -579,7 +579,7 @@ const CBlockIndex* BlockManager::GetLastCheckpoint(const CCheckpointData& data)
 {
     const MapCheckpoints& checkpoints = data.mapCheckpoints;
 
-    for (const MapCheckpoints::value_type& i : reverse_iterate(checkpoints)) {
+    for (const MapCheckpoints::value_type& i : checkpoints | std::views::reverse) {
         const uint256& hash = i.second;
         const CBlockIndex* pindex = LookupBlockIndex(hash);
         if (pindex) {

src/reverse_iterator.h

@@ -1,39 +0,0 @@
-// Taken from https://gist.github.com/arvidsson/7231973
-
-#ifndef BITCOIN_REVERSE_ITERATOR_H
-#define BITCOIN_REVERSE_ITERATOR_H
-
-/**
- * Template used for reverse iteration in range-based for loops.
- *
- *   std::vector<int> v = {1, 2, 3, 4, 5};
- *   for (auto x : reverse_iterate(v))
- *       std::cout << x << " ";
- */
-
-template <typename T>
-class reverse_range
-{
-    T &m_x;
-
-public:
-    explicit reverse_range(T &x) : m_x(x) {}
-
-    auto begin() const -> decltype(this->m_x.rbegin())
-    {
-        return m_x.rbegin();
-    }
-
-    auto end() const -> decltype(this->m_x.rend())
-    {
-        return m_x.rend();
-    }
-};
-
-template <typename T>
-reverse_range<T> reverse_iterate(T &x)
-{
-    return reverse_range<T>(x);
-}
-
-#endif // BITCOIN_REVERSE_ITERATOR_H

src/test/fuzz/prevector.cpp

@@ -2,16 +2,14 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <prevector.h>
+#include <serialize.h>
+#include <streams.h>
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 
-#include <prevector.h>
+#include <ranges>
 #include <vector>
-
-#include <reverse_iterator.h>
-#include <serialize.h>
-#include <streams.h>
-
 namespace {
 
 template <unsigned int N, typename T>
@@ -47,7 +45,7 @@ public:
             assert(v == real_vector[pos]);
             ++pos;
         }
-        for (const T& v : reverse_iterate(pre_vector)) {
+        for (const T& v : pre_vector | std::views::reverse) {
             --pos;
             assert(v == real_vector[pos]);
         }
@@ -55,7 +53,7 @@ public:
             assert(v == real_vector[pos]);
             ++pos;
         }
-        for (const T& v : reverse_iterate(const_pre_vector)) {
+        for (const T& v : const_pre_vector | std::views::reverse) {
             --pos;
             assert(v == real_vector[pos]);
         }

src/test/prevector_tests.cpp

@@ -3,17 +3,16 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <prevector.h>
-#include <vector>
-
-#include <reverse_iterator.h>
 #include <serialize.h>
 #include <streams.h>
-
 #include <test/util/random.h>
 #include <test/util/setup_common.h>
 
 #include <boost/test/unit_test.hpp>
 
+#include <ranges>
+#include <vector>
+
 BOOST_FIXTURE_TEST_SUITE(prevector_tests, TestingSetup)
 
 template<unsigned int N, typename T>
@@ -58,14 +57,14 @@ class prevector_tester {
         for (const T& v : pre_vector) {
              local_check(v == real_vector[pos++]);
         }
-        for (const T& v : reverse_iterate(pre_vector)) {
-             local_check(v == real_vector[--pos]);
+        for (const T& v : pre_vector | std::views::reverse) {
+            local_check(v == real_vector[--pos]);
         }
         for (const T& v : const_pre_vector) {
              local_check(v == real_vector[pos++]);
         }
-        for (const T& v : reverse_iterate(const_pre_vector)) {
-             local_check(v == real_vector[--pos]);
+        for (const T& v : const_pre_vector | std::views::reverse) {
+            local_check(v == real_vector[--pos]);
         }
         DataStream ss1{};
         DataStream ss2{};

src/txmempool.cpp

@@ -15,7 +15,6 @@
 #include <policy/policy.h>
 #include <policy/settings.h>
 #include <random.h>
-#include <reverse_iterator.h>
 #include <tinyformat.h>
 #include <util/check.h>
 #include <util/feefrac.h>
@@ -31,6 +30,7 @@
 #include <cmath>
 #include <numeric>
 #include <optional>
+#include <ranges>
 #include <string_view>
 #include <utility>
 
@@ -121,7 +121,7 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256>& vHashes
     // This maximizes the benefit of the descendant cache and guarantees that
     // CTxMemPoolEntry::m_children will be updated, an assumption made in
     // UpdateForDescendants.
-    for (const uint256 &hash : reverse_iterate(vHashesToUpdate)) {
+    for (const uint256& hash : vHashesToUpdate | std::views::reverse) {
         // calculate children from mapNextTx
         txiter it = mapTx.find(hash);
         if (it == mapTx.end()) {

src/validation.cpp

@@ -40,7 +40,6 @@
 #include <primitives/block.h>
 #include <primitives/transaction.h>
 #include <random.h>
-#include <reverse_iterator.h>
 #include <script/script.h>
 #include <script/sigcache.h>
 #include <signet.h>
@@ -70,6 +69,7 @@
 #include <deque>
 #include <numeric>
 #include <optional>
+#include <ranges>
 #include <string>
 #include <tuple>
 #include <utility>
@@ -3357,7 +3357,7 @@ bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex*
         nHeight = nTargetHeight;
 
         // Connect new blocks.
-        for (CBlockIndex* pindexConnect : reverse_iterate(vpindexToConnect)) {
+        for (CBlockIndex* pindexConnect : vpindexToConnect | std::views::reverse) {
             if (!ConnectTip(state, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr<const CBlock>(), connectTrace, disconnectpool)) {
                 if (state.IsInvalid()) {
                     // The block violates a consensus rule.