Skip to content

Commit

Permalink
Use a simple indexed loop (#329)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
chiphogg authored Nov 23, 2024
1 parent 122af52 commit a1a451c
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 11 deletions.
16 changes: 8 additions & 8 deletions au/code/au/utility/factoring.hh
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

#pragma once

#include <array>
#include <cstdint>

#include "au/utility/probable_primes.hh"
Expand Down Expand Up @@ -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;
}
Expand Down
5 changes: 2 additions & 3 deletions au/code/au/utility/test/factoring_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
Expand Down

0 comments on commit a1a451c

Please sign in to comment.