Add overflow helpers for applying rational to int (#212)

C++'s integer promotion rules for small types make this specific use
case --- applying a rational magnitude to an integral type --- really
interesting!  The product type can be bigger than the type of the two
inputs, so the overflow might not happen when you think it should.
_Moreover,_ the fact that we subsequently divide by a denominator means
that we can _re-enter_ the valid range and still produce a correct
result!

The only way I know how to deal with something this complicated is to
make a separate target that does only that one thing.  Happily, it's
still the case that there is some maximum and minimum value for each
type that will not overflow for a particular rational magnitude.  The
new helper target exists to figure out what those limits are.

I exposed this limitation by adding a conversion between meters and
yards in `uint16_t`.  This is a great test case because the unit ratio
between meters and yards is very close to 1 (it's 1143 to 1250), meaning
the denominator division "rescues" the great majority of cases.
Moreover, the multiplicative factor (either 1143 or 1250 depending on
direction) is far less than the ratio between `uint16_t` and its
promoted type, assuming the latter is equivalent to `int32_t`.  So,
despite the fact that we're multiplying by a number which is pretty
large relative to the type, we almost never actually overflow!

I fixed up the test case so that if it _does_ fail (say, we add some new
unit conversion that exposes a new logic error), it will be very easy to
understand why.

au/BUILD.bazel

@@ -113,7 +113,10 @@ cc_test(
 cc_library(
     name = "apply_magnitude",
     hdrs = ["apply_magnitude.hh"],
-    deps = [":magnitude"],
+    deps = [
+        ":apply_rational_magnitude_to_integral",
+        ":magnitude",
+    ],
 )
 
 cc_test(
@@ -128,6 +131,23 @@ cc_test(
     ],
 )
 
+cc_library(
+    name = "apply_rational_magnitude_to_integral",
+    hdrs = ["apply_rational_magnitude_to_integral.hh"],
+    deps = [":magnitude"],
+)
+
+cc_test(
+    name = "apply_rational_magnitude_to_integral_test",
+    size = "small",
+    srcs = ["apply_rational_magnitude_to_integral_test.cc"],
+    deps = [
+        ":apply_rational_magnitude_to_integral",
+        ":testing",
+        "@com_google_googletest//:gtest_main",
+    ],
+)
+
 cc_library(
     name = "chrono_interop",
     hdrs = ["chrono_interop.hh"],

au/apply_magnitude.hh

@@ -14,6 +14,7 @@
 
 #pragma once
 
+#include "au/apply_rational_magnitude_to_integral.hh"
 #include "au/magnitude.hh"
 
 namespace au {
@@ -132,6 +133,28 @@ struct ApplyMagnitudeImpl<Mag, ApplyAs::INTEGER_DIVIDE, T, is_T_integral> {
     }
 };
 
+template <typename T, typename Mag, bool is_T_signed>
+struct RationalOverflowChecker;
+template <typename T, typename Mag>
+struct RationalOverflowChecker<T, Mag, true> {
+    static constexpr bool would_overflow(const T &x) {
+        static_assert(std::is_signed<T>::value,
+                      "Mismatched instantiation (should never be done manually)");
+        const bool safe = (x <= MaxNonOverflowingValue<T, Mag>::value()) &&
+                          (x >= MinNonOverflowingValue<T, Mag>::value());
+        return !safe;
+    }
+};
+template <typename T, typename Mag>
+struct RationalOverflowChecker<T, Mag, false> {
+    static constexpr bool would_overflow(const T &x) {
+        static_assert(!std::is_signed<T>::value,
+                      "Mismatched instantiation (should never be done manually)");
+        const bool safe = (x <= MaxNonOverflowingValue<T, Mag>::value());
+        return !safe;
+    }
+};
+
 // Applying a (non-integer, non-inverse-integer) rational, for any integral type T.
 template <typename Mag, typename T>
 struct ApplyMagnitudeImpl<Mag, ApplyAs::RATIONAL_MULTIPLY, T, true> {
@@ -141,13 +164,13 @@ struct ApplyMagnitudeImpl<Mag, ApplyAs::RATIONAL_MULTIPLY, T, true> {
                   "Mismatched instantiation (should never be done manually)");
 
     constexpr T operator()(const T &x) {
-        return x * get_value<T>(numerator(Mag{})) / get_value<T>(denominator(Mag{}));
+        using P = PromotedType<T>;
+        return static_cast<T>(x * get_value<P>(numerator(Mag{})) /
+                              get_value<P>(denominator(Mag{})));
     }
 
     static constexpr bool would_overflow(const T &x) {
-        constexpr auto mag_value_result = get_value_result<T>(numerator(Mag{}));
-        return OverflowChecker<T, mag_value_result.outcome == MagRepresentationOutcome::OK>::
-            would_product_overflow(x, mag_value_result.value);
+        return RationalOverflowChecker<T, Mag, std::is_signed<T>::value>::would_overflow(x);
     }
 
     static constexpr bool would_truncate(const T &x) {

au/apply_magnitude_test.cc

@@ -90,6 +90,28 @@ TEST(ApplyMagnitude, MultipliesThenDividesForRationalMagnitudeOnInteger) {
     EXPECT_THAT(apply_magnitude(5, three_halves), SameTypeAndValue(7));
 }
 
+TEST(ApplyMagnitude, SupportsNumeratorThatFitsInPromotedTypeButNotOriginalType) {
+    using T = uint16_t;
+    using P = PromotedType<T>;
+    ASSERT_TRUE((std::is_same<P, int32_t>::value))
+        << "This test fails on architectures where `uint16_t` doesn't get promoted to `int32_t`";
+
+    // Choose a magnitude whose effect will basically be to divide by 2.  (We make the denominator
+    // slightly _smaller_ than twice the numerator, rather than slightly _larger_, so that the
+    // division will end up on the "high" side of the target, and truncation will bring it down very
+    // slightly instead of going down a full integer.)
+    auto roughly_one_half = mag<100'000'000>() / mag<199'999'999>();
+
+    // The whole point of this test case is to apply a magnitude whose numerator fits in the
+    // promoted type, but does not fit in the target type itself.
+    ASSERT_EQ(get_value_result<P>(numerator(roughly_one_half)).outcome,
+              MagRepresentationOutcome::OK);
+    ASSERT_EQ(get_value_result<T>(numerator(roughly_one_half)).outcome,
+              MagRepresentationOutcome::ERR_CANNOT_FIT);
+
+    EXPECT_THAT(apply_magnitude(T{18}, roughly_one_half), SameTypeAndValue(T{9}));
+}
+
 TEST(ApplyMagnitude, MultipliesSingleNumberForRationalMagnitudeOnFloatingPoint) {
     // Helper similar to `std::transform`, but with more convenient interfaces.
     auto apply = [](std::vector<float> vals, auto fun) {
@@ -196,10 +218,8 @@ TEST(WouldOverflow, AlwaysFalseForIntegerDivide) {
 }
 
 TEST(WouldOverflow, UsesNumeratorWhenApplyingRationalMagnitudeToIntegralType) {
-    auto TWO_THIRDS = mag<2>() / mag<3>();
-
     {
-        using ApplyTwoThirdsToI32 = ApplyMagnitudeT<int32_t, decltype(TWO_THIRDS)>;
+        using ApplyTwoThirdsToI32 = ApplyMagnitudeT<int32_t, decltype(mag<2>() / mag<3>())>;
 
         EXPECT_TRUE(ApplyTwoThirdsToI32::would_overflow(2'147'483'647));
         EXPECT_TRUE(ApplyTwoThirdsToI32::would_overflow(1'073'741'824));
@@ -215,14 +235,18 @@ TEST(WouldOverflow, UsesNumeratorWhenApplyingRationalMagnitudeToIntegralType) {
     }
 
     {
-        using ApplyTwoThirdsToU8 = ApplyMagnitudeT<uint8_t, decltype(TWO_THIRDS)>;
+        using ApplyRoughlyOneThirdToU8 =
+            ApplyMagnitudeT<uint8_t, decltype(mag<100'000'000>() / mag<300'000'001>())>;
+
+        ASSERT_TRUE((std::is_same<decltype(uint8_t{} * uint8_t{}), int32_t>::value))
+            << "This test fails on architectures where `uint8_t` doesn't get promoted to `int32_t`";
 
-        EXPECT_TRUE(ApplyTwoThirdsToU8::would_overflow(255));
-        EXPECT_TRUE(ApplyTwoThirdsToU8::would_overflow(128));
+        EXPECT_TRUE(ApplyRoughlyOneThirdToU8::would_overflow(255));
+        EXPECT_TRUE(ApplyRoughlyOneThirdToU8::would_overflow(22));
 
-        EXPECT_FALSE(ApplyTwoThirdsToU8::would_overflow(127));
-        EXPECT_FALSE(ApplyTwoThirdsToU8::would_overflow(1));
-        EXPECT_FALSE(ApplyTwoThirdsToU8::would_overflow(0));
+        EXPECT_FALSE(ApplyRoughlyOneThirdToU8::would_overflow(21));
+        EXPECT_FALSE(ApplyRoughlyOneThirdToU8::would_overflow(1));
+        EXPECT_FALSE(ApplyRoughlyOneThirdToU8::would_overflow(0));
     }
 }