Avoid overflowing remainder for minimum-value GCD with -1

doc/gcd/math-gcd.qbk

@@ -150,6 +150,7 @@ They are also declared `noexcept` when appropriate.
 
 The boost::integer::gcd function template returns the greatest common
 (nonnegative) divisor of the two integers passed to it.
+The nonnegative result must be representable by `IntegerType`.
 `boost::integer::gcd_range` is the iteration of the above gcd algorithm over a
 range, returning the greatest common divisor of all the elements. The algorithm
 terminates when the gcd reaches unity or the end of the range. Thus it also

include/boost/integer/common_factor_rt.hpp

@@ -440,10 +440,16 @@ namespace boost {
 template <typename Integer>
 inline BOOST_CXX14_CONSTEXPR Integer gcd(Integer const &a, Integer const &b) BOOST_GCD_NOEXCEPT(Integer)
 {
+    // gcd(min, -1) is 1, but min % -1 overflows. Only a signed type has a negative unit:
+    // static_cast<Integer>(-1) is the largest value of an unsigned type and must not match.
     if(a == (std::numeric_limits<Integer>::min)())
-       return a == static_cast<Integer>(0) ? gcd_detail::gcd_traits<Integer>::abs(b) : boost::integer::gcd(static_cast<Integer>(a % b), b);
+       return a == static_cast<Integer>(0) ? gcd_detail::gcd_traits<Integer>::abs(b)
+          : (std::numeric_limits<Integer>::is_signed && b == static_cast<Integer>(-1)) ? static_cast<Integer>(1)
+          : boost::integer::gcd(static_cast<Integer>(a % b), b);
     else if (b == (std::numeric_limits<Integer>::min)())
-       return b == static_cast<Integer>(0) ? gcd_detail::gcd_traits<Integer>::abs(a) : boost::integer::gcd(a, static_cast<Integer>(b % a));
+       return b == static_cast<Integer>(0) ? gcd_detail::gcd_traits<Integer>::abs(a)
+          : (std::numeric_limits<Integer>::is_signed && a == static_cast<Integer>(-1)) ? static_cast<Integer>(1)
+          : boost::integer::gcd(a, static_cast<Integer>(b % a));
     return gcd_detail::optimal_gcd_select(static_cast<Integer>(gcd_detail::gcd_traits<Integer>::abs(a)), static_cast<Integer>(gcd_detail::gcd_traits<Integer>::abs(b)));
 }
 

test/common_factor_test.cpp

@@ -301,6 +301,15 @@ template< class T > void gcd_int_test() // signed_test_types
     BOOST_TEST_EQ(gcd_evaluator<T>()(3, 7), static_cast<T>(1));
     BOOST_TEST_EQ(gcd_evaluator<T>()(8, 9), static_cast<T>(1));
     BOOST_TEST_EQ(gcd_evaluator<T>()(7, 49), static_cast<T>(7));
+
+    if (std::numeric_limits<T>::is_bounded && std::numeric_limits<T>::is_signed)
+    {
+        const T minimum = (std::numeric_limits<T>::min)();
+        BOOST_TEST_EQ(boost::integer::gcd(minimum, static_cast<T>(-1)), static_cast<T>(1));
+        BOOST_TEST_EQ(boost::integer::gcd(static_cast<T>(-1), minimum), static_cast<T>(1));
+        BOOST_TEST_EQ(gcd_evaluator<T>()(minimum, static_cast<T>(-1)), static_cast<T>(1));
+        BOOST_TEST_EQ(gcd_evaluator<T>()(static_cast<T>(-1), minimum), static_cast<T>(1));
+    }
 }
 
 // GCD on unmarked signed integer type
@@ -353,6 +362,14 @@ template< class T > void gcd_unsigned_test() // unsigned_test_types
     BOOST_TEST_EQ( boost::integer::gcd(static_cast<T>(3u), static_cast<T>(7u)), static_cast<T>( 1u) );
     BOOST_TEST_EQ( boost::integer::gcd(static_cast<T>(8u), static_cast<T>(9u)), static_cast<T>( 1u) );
     BOOST_TEST_EQ( boost::integer::gcd(static_cast<T>(7u), static_cast<T>(49u)), static_cast<T>( 7u) );
+
+    // An unsigned type has no negative unit: static_cast<T>(-1) is its largest
+    // value, and the GCD of zero and that value is that value, not one.
+    const T largest = static_cast<T>(-1);
+    BOOST_TEST_EQ( boost::integer::gcd(static_cast<T>(0u), largest), largest );
+    BOOST_TEST_EQ( boost::integer::gcd(largest, static_cast<T>(0u)), largest );
+    BOOST_TEST_EQ( boost::integer::gcd_evaluator<T>()(static_cast<T>(0u), largest), largest );
+    BOOST_TEST_EQ( boost::integer::gcd_evaluator<T>()(largest, static_cast<T>(0u)), largest );
 }
 
 // GCD at compile-time

test/gcd_constexpr14_test.cpp

@@ -5,6 +5,7 @@
 //  https://www.boost.org/LICENSE_1_0.txt)
 
 #include <boost/integer/common_factor.hpp>
+#include <limits>
 
 #ifndef BOOST_NO_CXX14_CONSTEXPR
 
@@ -61,6 +62,29 @@ void test_constexpr5()
 
    static_assert(k == 347, "Expected result not integer in constexpr gcd.");
 }
+
+void test_constexpr_minimum_unit()
+{
+   constexpr int minimum = (std::numeric_limits<int>::min)();
+   static_assert(boost::integer::gcd(minimum, -1) == 1, "Expected gcd(min, -1) == 1.");
+   static_assert(boost::integer::gcd(-1, minimum) == 1, "Expected gcd(-1, min) == 1.");
+
+   constexpr boost::int64_t minimum64 = (std::numeric_limits<boost::int64_t>::min)();
+   static_assert(boost::integer::gcd(minimum64, boost::int64_t(-1)) == 1, "Expected gcd(min64, -1) == 1.");
+   static_assert(boost::integer::gcd(boost::int64_t(-1), minimum64) == 1, "Expected gcd(-1, min64) == 1.");
+}
+
+void test_constexpr_unsigned_maximum()
+{
+   // The minimum of an unsigned type is zero and unsigned(-1) is its maximum.
+   constexpr unsigned maximum = (std::numeric_limits<unsigned>::max)();
+   static_assert(boost::integer::gcd(0u, maximum) == maximum, "Expected gcd(0, max) == max.");
+   static_assert(boost::integer::gcd(maximum, 0u) == maximum, "Expected gcd(max, 0) == max.");
+
+   constexpr boost::uint64_t maximum64 = (std::numeric_limits<boost::uint64_t>::max)();
+   static_assert(boost::integer::gcd(boost::uint64_t(0), maximum64) == maximum64, "Expected gcd(0, max64) == max64.");
+   static_assert(boost::integer::gcd(maximum64, boost::uint64_t(0)) == maximum64, "Expected gcd(max64, 0) == max64.");
+}
 #endif