From a1a451c37ab8d152432f68631f5e3ecd44f7fa1d Mon Sep 17 00:00:00 2001
From: Chip Hogg <chogg@aurora.tech>
Date: Sat, 23 Nov 2024 08:47:40 -0500
Subject: [PATCH] Use a simple indexed loop (#329)

Apparently, some platforms struggle with range-based `for` loops on
C-style arrays.  Other platforms don't seem to like the C-style array at
all, so we're trying a `std::array`.  You'd think we'd be able to use
range-for with this... but, no, `begin` isn't `constexpr` until C++17,
so we're using an indexed loop anyways.

Tested on the Aurora-internal code that revealed the problem.

Follow-up for #217.
---
 au/code/au/utility/factoring.hh           | 16 ++++++++--------
 au/code/au/utility/test/factoring_test.cc |  5 ++---
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/au/code/au/utility/factoring.hh b/au/code/au/utility/factoring.hh
index e8bf7cf..d8af10f 100644
--- a/au/code/au/utility/factoring.hh
+++ b/au/code/au/utility/factoring.hh
@@ -14,6 +14,7 @@
 
 #pragma once
 
+#include <array>
 #include <cstdint>
 
 #include "au/utility/probable_primes.hh"
@@ -75,29 +76,28 @@ constexpr std::uintmax_t find_pollard_rho_factor(std::uintmax_t n) {
 
 template <typename T = void>
 struct FirstPrimesImpl {
-    static constexpr uint16_t values[] = {
+    static constexpr std::array<uint16_t, 100u> values = {
         2,   3,   5,   7,   11,  13,  17,  19,  23,  29,  31,  37,  41,  43,  47,  53,  59,
         61,  67,  71,  73,  79,  83,  89,  97,  101, 103, 107, 109, 113, 127, 131, 137, 139,
         149, 151, 157, 163, 167, 173, 179, 181, 191, 193, 197, 199, 211, 223, 227, 229, 233,
         239, 241, 251, 257, 263, 269, 271, 277, 281, 283, 293, 307, 311, 313, 317, 331, 337,
         347, 349, 353, 359, 367, 373, 379, 383, 389, 397, 401, 409, 419, 421, 431, 433, 439,
         443, 449, 457, 461, 463, 467, 479, 487, 491, 499, 503, 509, 521, 523, 541};
-    static constexpr std::size_t N = sizeof(values) / sizeof(values[0]);
 };
 template <typename T>
-constexpr uint16_t FirstPrimesImpl<T>::values[];
-template <typename T>
-constexpr std::size_t FirstPrimesImpl<T>::N;
+constexpr std::array<uint16_t, 100u> FirstPrimesImpl<T>::values;
 using FirstPrimes = FirstPrimesImpl<>;
 
 // Find the smallest factor which divides n.
 //
 // Undefined unless (n > 1).
 constexpr std::uintmax_t find_prime_factor(std::uintmax_t n) {
-    const auto &first_primes = FirstPrimes::values;
-
     // First, do trial division against the first N primes.
-    for (const auto &p : first_primes) {
+    //
+    // Note that range-for isn't supported until C++17, so we need to use an index.
+    for (auto i = 0u; i < FirstPrimes::values.size(); ++i) {
+        const auto &p = FirstPrimes::values[i];
+
         if (n % p == 0u) {
             return p;
         }
diff --git a/au/code/au/utility/test/factoring_test.cc b/au/code/au/utility/test/factoring_test.cc
index 54e7693..7f2ea95 100644
--- a/au/code/au/utility/test/factoring_test.cc
+++ b/au/code/au/utility/test/factoring_test.cc
@@ -30,9 +30,8 @@ std::uintmax_t cube(std::uintmax_t n) { return n * n * n; }
 
 TEST(FirstPrimes, HasOnlyPrimesInOrderAndDoesntSkipAny) {
     const auto &first_primes = FirstPrimes::values;
-    const auto &n_primes = FirstPrimes::N;
     auto i_prime = 0u;
-    for (auto i = 2u; i <= first_primes[n_primes - 1u]; ++i) {
+    for (auto i = 2u; i <= first_primes.back(); ++i) {
         if (i == first_primes[i_prime]) {
             EXPECT_TRUE(is_prime(i)) << i;
             ++i_prime;
@@ -64,7 +63,7 @@ TEST(FindFactor, CanFactorNumbersWithLargePrimeFactor) {
     EXPECT_THAT(find_prime_factor(3u * 9'007'199'254'740'881u),
                 AnyOf(Eq(3u), Eq(9'007'199'254'740'881u)));
 
-    constexpr auto LAST_TRIAL_PRIME = FirstPrimes::values[FirstPrimes::N - 1u];
+    constexpr auto LAST_TRIAL_PRIME = FirstPrimes::values.back();
 
     // Large prime factor, with a number that trial division would find.
     ASSERT_THAT(541u, Le(LAST_TRIAL_PRIME));