From 12aa23e6def460cdf9852fcc4c930ea4ae8c849f Mon Sep 17 00:00:00 2001
From: Chip Hogg <chogg@aurora.tech>
Date: Tue, 12 Nov 2024 16:06:20 -0500
Subject: [PATCH] Address review feedback

---
 au/code/au/utility/probable_primes.hh           | 11 +++++++----
 au/code/au/utility/test/probable_primes_test.cc | 10 ++++++++++
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/au/code/au/utility/probable_primes.hh b/au/code/au/utility/probable_primes.hh
index 744a9946..0b7c917a 100644
--- a/au/code/au/utility/probable_primes.hh
+++ b/au/code/au/utility/probable_primes.hh
@@ -14,6 +14,8 @@
 
 #pragma once
 
+#include <cmath>
+
 #include "au/utility/mod.hh"
 
 namespace au {
@@ -91,9 +93,10 @@ constexpr uint64_t gcd(uint64_t a, uint64_t b) {
     return a;
 }
 
-// The conversions `true` -> `1` and `false` -> `0` are guaranteed by the standard.
+// Map `true` onto `1`, and `false` onto `0`.
 //
-// This is a branchless implementation, which should generally be faster.
+// The conversions `true` -> `1` and `false` -> `0` are guaranteed by the standard.  This is a
+// branchless implementation, which should generally be faster.
 constexpr int bool_sign(bool x) { return x - (!x); }
 
 //
@@ -113,7 +116,7 @@ constexpr int bool_sign(bool x) { return x - (!x); }
 //     either a or n is congruent to 1 (mod 4), and 1 otherwise.
 //
 constexpr int jacobi_symbol_positive_numerator(uint64_t a, uint64_t n, int start) {
-    int &result = start;
+    int result = start;
 
     while (a != 0u) {
         // Handle even numbers in the "numerator".
@@ -155,7 +158,7 @@ constexpr int jacobi_symbol(int64_t raw_a, uint64_t n) {
     // Starting conditions: transform `a` to strictly non-negative values, setting `result` to the
     // sign we pick up from this operation (if any).
     int result = bool_sign((raw_a >= 0) || (n % 4u == 1u));
-    auto a = static_cast<uint64_t>(raw_a * bool_sign(raw_a >= 0)) % n;
+    auto a = static_cast<uint64_t>(std::abs(raw_a)) % n;
 
     // Delegate to an implementation which can only handle positive numbers.
     return jacobi_symbol_positive_numerator(a, n, result);
diff --git a/au/code/au/utility/test/probable_primes_test.cc b/au/code/au/utility/test/probable_primes_test.cc
index 8c9d9450..41285c47 100644
--- a/au/code/au/utility/test/probable_primes_test.cc
+++ b/au/code/au/utility/test/probable_primes_test.cc
@@ -241,6 +241,16 @@ TEST(JacobiSymbol, AlwaysOneWhenFirstInputIsOne) {
     }
 }
 
+TEST(JacobiSymbol, ReproducesExamplesFromWikipedia) {
+    // https://en.wikipedia.org/wiki/Jacobi_symbol#Example_of_calculations
+    EXPECT_EQ(jacobi_symbol(1001, 9907), -1);
+
+    // https://en.wikipedia.org/wiki/Jacobi_symbol#Primality_testing
+    EXPECT_EQ(jacobi_symbol(19, 45), 1);
+    EXPECT_EQ(jacobi_symbol(8, 21), -1);
+    EXPECT_EQ(jacobi_symbol(5, 21), 1);
+}
+
 TEST(BoolSign, ReturnsCorrectValues) {
     EXPECT_EQ(bool_sign(true), 1);
     EXPECT_EQ(bool_sign(false), -1);