Merge #20429: refactor: replace (sizeof(a)/sizeof(a[0])) with C++17 std::size

e829c9afbf75e930db6c3fe77a269b0af5e7a3ad refactor: replace sizeof(a)/sizeof(a[0]) by std::size (C++17) (Sebastian Falbesoner)
365539c84691d470b44d35df374d8c049f8c1192 refactor: init vectors via std::{begin,end} to avoid pointer arithmetic (Sebastian Falbesoner)
63d4ee1968144cc3d115f92baef95785abf813ac refactor: iterate arrays via C++11 range-based for loops if idx is not needed (Sebastian Falbesoner)

Pull request description:

  This refactoring PR picks up the idea of #19626 and replaces all occurences of `sizeof(x)/sizeof(x[0])` (or `sizeof(x)/sizeof(*x)`, respectively) with the now-available C++17 [`std::size`](https://en.cppreference.com/w/cpp/iterator/size)  (as [suggested by sipa](https://github.com/bitcoin/bitcoin/pull/19626#issuecomment-666487228)), making the macro `ARRAYLEN` obsolete.

  As preparation for this, two other changes are done to eliminate `sizeof(x)/sizeof(x[0])` usage:
  * all places where arrays are iterated via an index are changed to use C++11 range-based for loops If the index' only purpose is to access the array element (as [suggested by MarcoFalke](https://github.com/bitcoin/bitcoin/pull/19626#discussion_r463404541)).
  * `std::vector` initializations are done via `std::begin` and `std::end` rather than using pointer arithmetic to calculate the end (also [suggested by MarcoFalke](https://github.com/bitcoin/bitcoin/pull/20429#discussion_r567418821)).

ACKs for top commit:
  practicalswift:
    cr ACK e829c9afbf75e930db6c3fe77a269b0af5e7a3ad: patch looks correct
  fanquake:
    ACK e829c9afbf75e930db6c3fe77a269b0af5e7a3ad
  MarcoFalke:
    review ACK e829c9afbf75e930db6c3fe77a269b0af5e7a3ad 🌩

Tree-SHA512: b01d32c04b9e04d562b7717cae00a651ec9a718645047a90761be6959e0cc2adbd67494e058fe894641076711bb09c3b47a047d0275c736f0b2218e1ce0d193d

6 files changed, 13 insertions, 14 deletions

diff --git a/src/test/base32_tests.cpp b/src/test/base32_tests.cpp
index 3f7c5e99ee..3b44564ddb 100644
--- a/src/test/base32_tests.cpp
+++ b/src/test/base32_tests.cpp
@@ -17,7 +17,7 @@ BOOST_AUTO_TEST_CASE(base32_testvectors)
     static const std::string vstrIn[]  = {"","f","fo","foo","foob","fooba","foobar"};
     static const std::string vstrOut[] = {"","my======","mzxq====","mzxw6===","mzxw6yq=","mzxw6ytb","mzxw6ytboi======"};
     static const std::string vstrOutNoPadding[] = {"","my","mzxq","mzxw6","mzxw6yq","mzxw6ytb","mzxw6ytboi"};
-    for (unsigned int i=0; i<sizeof(vstrIn)/sizeof(vstrIn[0]); i++)
+    for (unsigned int i=0; i<std::size(vstrIn); i++)
     {
         std::string strEnc = EncodeBase32(vstrIn[i]);
         BOOST_CHECK_EQUAL(strEnc, vstrOut[i]);
diff --git a/src/test/base64_tests.cpp b/src/test/base64_tests.cpp
index bb8d102bd0..714fccffaa 100644
--- a/src/test/base64_tests.cpp
+++ b/src/test/base64_tests.cpp
@@ -16,7 +16,7 @@ BOOST_AUTO_TEST_CASE(base64_testvectors)
 {
     static const std::string vstrIn[]  = {"","f","fo","foo","foob","fooba","foobar"};
     static const std::string vstrOut[] = {"","Zg==","Zm8=","Zm9v","Zm9vYg==","Zm9vYmE=","Zm9vYmFy"};
-    for (unsigned int i=0; i<sizeof(vstrIn)/sizeof(vstrIn[0]); i++)
+    for (unsigned int i=0; i<std::size(vstrIn); i++)
     {
         std::string strEnc = EncodeBase64(vstrIn[i]);
         BOOST_CHECK_EQUAL(strEnc, vstrOut[i]);
diff --git a/src/test/hash_tests.cpp b/src/test/hash_tests.cpp
index 87f6470afa..41a626c0ea 100644
--- a/src/test/hash_tests.cpp
+++ b/src/test/hash_tests.cpp
@@ -107,14 +107,14 @@ BOOST_AUTO_TEST_CASE(siphash)
 
     // Check test vectors from spec, one byte at a time
     CSipHasher hasher2(0x0706050403020100ULL, 0x0F0E0D0C0B0A0908ULL);
-    for (uint8_t x=0; x<ARRAYLEN(siphash_4_2_testvec); ++x)
+    for (uint8_t x=0; x<std::size(siphash_4_2_testvec); ++x)
     {
         BOOST_CHECK_EQUAL(hasher2.Finalize(), siphash_4_2_testvec[x]);
         hasher2.Write(&x, 1);
     }
     // Check test vectors from spec, eight bytes at a time
     CSipHasher hasher3(0x0706050403020100ULL, 0x0F0E0D0C0B0A0908ULL);
-    for (uint8_t x=0; x<ARRAYLEN(siphash_4_2_testvec); x+=8)
+    for (uint8_t x=0; x<std::size(siphash_4_2_testvec); x+=8)
     {
         BOOST_CHECK_EQUAL(hasher3.Finalize(), siphash_4_2_testvec[x]);
         hasher3.Write(uint64_t(x)|(uint64_t(x+1)<<8)|(uint64_t(x+2)<<16)|(uint64_t(x+3)<<24)|
diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp
index 9c06283a58..9073a5050c 100644
--- a/src/test/miner_tests.cpp
+++ b/src/test/miner_tests.cpp
@@ -219,11 +219,10 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
 
     // We can't make transactions until we have inputs
     // Therefore, load 110 blocks :)
-    static_assert(sizeof(blockinfo) / sizeof(*blockinfo) == 110, "Should have 110 blocks to import");
+    static_assert(std::size(blockinfo) == 110, "Should have 110 blocks to import");
     int baseheight = 0;
     std::vector<CTransactionRef> txFirst;
-    for (unsigned int i = 0; i < sizeof(blockinfo)/sizeof(*blockinfo); ++i)
-    {
+    for (const auto& bi : blockinfo) {
         CBlock *pblock = &pblocktemplate->block; // pointer for convenience
         {
             LOCK(cs_main);
@@ -232,7 +231,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
             CMutableTransaction txCoinbase(*pblock->vtx[0]);
             txCoinbase.nVersion = 1;
             txCoinbase.vin[0].scriptSig = CScript();
-            txCoinbase.vin[0].scriptSig.push_back(blockinfo[i].extranonce);
+            txCoinbase.vin[0].scriptSig.push_back(bi.extranonce);
             txCoinbase.vin[0].scriptSig.push_back(::ChainActive().Height());
             txCoinbase.vout.resize(1); // Ignore the (optional) segwit commitment added by CreateNewBlock (as the hardcoded nonces don't account for this)
             txCoinbase.vout[0].scriptPubKey = CScript();
@@ -242,7 +241,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
             if (txFirst.size() < 4)
                 txFirst.push_back(pblock->vtx[0]);
             pblock->hashMerkleRoot = BlockMerkleRoot(*pblock);
-            pblock->nNonce = blockinfo[i].nonce;
+            pblock->nNonce = bi.nonce;
         }
         std::shared_ptr<const CBlock> shared_pblock = std::make_shared<const CBlock>(*pblock);
         BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlock(chainparams, shared_pblock, true, nullptr));
diff --git a/src/test/scriptnum_tests.cpp b/src/test/scriptnum_tests.cpp
index 281018be9f..746d4d3c6b 100644
--- a/src/test/scriptnum_tests.cpp
+++ b/src/test/scriptnum_tests.cpp
@@ -164,9 +164,9 @@ static void RunOperators(const int64_t& num1, const int64_t& num2)
 
 BOOST_AUTO_TEST_CASE(creation)
 {
-    for(size_t i = 0; i < sizeof(values) / sizeof(values[0]); ++i)
+    for(size_t i = 0; i < std::size(values); ++i)
     {
-        for(size_t j = 0; j < sizeof(offsets) / sizeof(offsets[0]); ++j)
+        for(size_t j = 0; j < std::size(offsets); ++j)
         {
             RunCreate(values[i]);
             RunCreate(values[i] + offsets[j]);
@@ -177,9 +177,9 @@ BOOST_AUTO_TEST_CASE(creation)
 
 BOOST_AUTO_TEST_CASE(operators)
 {
-    for(size_t i = 0; i < sizeof(values) / sizeof(values[0]); ++i)
+    for(size_t i = 0; i < std::size(values); ++i)
     {
-        for(size_t j = 0; j < sizeof(offsets) / sizeof(offsets[0]); ++j)
+        for(size_t j = 0; j < std::size(offsets); ++j)
         {
             RunOperators(values[i], values[i]);
             RunOperators(values[i], -values[i]);
diff --git a/src/test/sighash_tests.cpp b/src/test/sighash_tests.cpp
index bc862de78a..2eb980e8cd 100644
--- a/src/test/sighash_tests.cpp
+++ b/src/test/sighash_tests.cpp
@@ -88,7 +88,7 @@ void static RandomScript(CScript &script) {
     script = CScript();
     int ops = (InsecureRandRange(10));
     for (int i=0; i<ops; i++)
-        script << oplist[InsecureRandRange(sizeof(oplist)/sizeof(oplist[0]))];
+        script << oplist[InsecureRandRange(std::size(oplist))];
 }
 
 void static RandomTransaction(CMutableTransaction &tx, bool fSingle) {